From f59421334e9448c46a11195c93e8c139430ed321 Mon Sep 17 00:00:00 2001 From: Georgii Kruglov Date: Tue, 21 Mar 2023 23:37:15 +0300 Subject: [PATCH 01/57] mmc: sdhci-of-esdhc: fix quirk to ignore command inhibit for data [ Upstream commit 0dd8316037a2a6d85b2be208bef9991de7b42170 ] If spec_reg is equal to 'SDHCI_PRESENT_STATE', esdhc_readl_fixup() fixes up register value and returns it immediately. As a result, the further block (spec_reg == SDHCI_PRESENT_STATE) &&(esdhc->quirk_ignore_data_inhibit == true), is never executed. The patch merges the second block into the first one. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 1f1929f3f2fa ("mmc: sdhci-of-esdhc: add quirk to ignore command inhibit for data") Signed-off-by: Georgii Kruglov Acked-by: Adrian Hunter Link: https://lore.kernel.org/r/20230321203715.3975-1-georgy.kruglov@yandex.ru Signed-off-by: Ulf Hansson Signed-off-by: Sasha Levin --- drivers/mmc/host/sdhci-of-esdhc.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index d53374991e13..5b853f651b38 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -126,6 +126,7 @@ static u32 esdhc_readl_fixup(struct sdhci_host *host, return ret; } } + /* * The DAT[3:0] line signal levels and the CMD line signal level are * not compatible with standard SDHC register. The line signal levels @@ -137,6 +138,16 @@ static u32 esdhc_readl_fixup(struct sdhci_host *host, ret = value & 0x000fffff; ret |= (value >> 4) & SDHCI_DATA_LVL_MASK; ret |= (value << 1) & SDHCI_CMD_LVL; + + /* + * Some controllers have unreliable Data Line Active + * bit for commands with busy signal. This affects + * Command Inhibit (data) bit. Just ignore it since + * MMC core driver has already polled card status + * with CMD13 after any command with busy siganl. + */ + if (esdhc->quirk_ignore_data_inhibit) + ret &= ~SDHCI_DATA_INHIBIT; return ret; } @@ -151,19 +162,6 @@ static u32 esdhc_readl_fixup(struct sdhci_host *host, return ret; } - /* - * Some controllers have unreliable Data Line Active - * bit for commands with busy signal. This affects - * Command Inhibit (data) bit. Just ignore it since - * MMC core driver has already polled card status - * with CMD13 after any command with busy siganl. - */ - if ((spec_reg == SDHCI_PRESENT_STATE) && - (esdhc->quirk_ignore_data_inhibit == true)) { - ret = value & ~SDHCI_DATA_INHIBIT; - return ret; - } - ret = value; return ret; } From 0fc1a90bcef75f9b65f6476670c35ce232e27990 Mon Sep 17 00:00:00 2001 From: "H. Nikolaus Schaller" Date: Fri, 13 Jan 2023 22:11:51 +0100 Subject: [PATCH 02/57] ARM: dts: gta04: fix excess dma channel usage [ Upstream commit a622310f7f0185da02e42cdb06475f533efaae60 ] OMAP processors support 32 channels but there is no check or inspect this except booting a device and looking at dmesg reports of not available channels. Recently some more subsystems with DMA (aes1+2) were added filling the list of dma channels beyond the limit of 32 (even if other parameters indicate 96 or 128 channels). This leads to random subsystem failures i(e.g. mcbsp for audio) after boot or boot messages that DMA can not be initialized. Another symptom is that /sys/kernel/debug/dmaengine/summary has 32 entries and does not show all required channels. Fix by disabling unused (on the GTA04 hardware) mcspi1...4. Each SPI channel allocates 4 DMA channels rapidly filling the available ones. Disabling unused SPI modules on the OMAP3 SoC may also save some energy (has not been checked). Fixes: c312f066314e ("ARM: dts: omap3: Migrate AES from hwmods to sysc-omap2") Signed-off-by: H. Nikolaus Schaller [re-enabled aes2, improved commit subject line] Signed-off-by: Andreas Kemnade Message-Id: <20230113211151.2314874-1-andreas@kemnade.info> Signed-off-by: Tony Lindgren Signed-off-by: Sasha Levin --- arch/arm/boot/dts/omap3-gta04.dtsi | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi index cc8a378dd076..e61e5ddbf202 100644 --- a/arch/arm/boot/dts/omap3-gta04.dtsi +++ b/arch/arm/boot/dts/omap3-gta04.dtsi @@ -609,6 +609,22 @@ clock-frequency = <100000>; }; +&mcspi1 { + status = "disabled"; +}; + +&mcspi2 { + status = "disabled"; +}; + +&mcspi3 { + status = "disabled"; +}; + +&mcspi4 { + status = "disabled"; +}; + &usb_otg_hs { interface-type = <0>; usb-phy = <&usb2_phy>; From e21c93b20a864c1624e3367a5abbc00f43e843e5 Mon Sep 17 00:00:00 2001 From: Harshit Mogalapalli Date: Mon, 13 Mar 2023 22:27:11 -0700 Subject: [PATCH 03/57] drm/lima/lima_drv: Add missing unwind goto in lima_pdev_probe() [ Upstream commit c5647cae2704e58d1c4e5fedbf63f11bca6376c9 ] Smatch reports: drivers/gpu/drm/lima/lima_drv.c:396 lima_pdev_probe() warn: missing unwind goto? Store return value in err and goto 'err_out0' which has lima_sched_slab_fini() before returning. Fixes: a1d2a6339961 ("drm/lima: driver for ARM Mali4xx GPUs") Signed-off-by: Harshit Mogalapalli Signed-off-by: Qiang Yu Link: https://patchwork.freedesktop.org/patch/msgid/20230314052711.4061652-1-harshit.m.mogalapalli@oracle.com Signed-off-by: Sasha Levin --- drivers/gpu/drm/lima/lima_drv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/lima/lima_drv.c b/drivers/gpu/drm/lima/lima_drv.c index ab460121fd52..65dc0dc2c119 100644 --- a/drivers/gpu/drm/lima/lima_drv.c +++ b/drivers/gpu/drm/lima/lima_drv.c @@ -392,8 +392,10 @@ static int lima_pdev_probe(struct platform_device *pdev) /* Allocate and initialize the DRM device. */ ddev = drm_dev_alloc(&lima_drm_driver, &pdev->dev); - if (IS_ERR(ddev)) - return PTR_ERR(ddev); + if (IS_ERR(ddev)) { + err = PTR_ERR(ddev); + goto err_out0; + } ddev->dev_private = ldev; ldev->ddev = ddev; From 4c0b98d87c3e0c1ba02c797cc83450938a6deaf5 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Wed, 29 Mar 2023 14:33:53 -0700 Subject: [PATCH 04/57] regulator: core: Consistently set mutex_owner when using ww_mutex_lock_slow() [ Upstream commit b83a1772be854f87602de14726737d3e5b06e1f4 ] When a codepath locks a rdev using ww_mutex_lock_slow() directly then that codepath is responsible for incrementing the "ref_cnt" and also setting the "mutex_owner" to "current". The regulator core consistently got that right for "ref_cnt" but didn't always get it right for "mutex_owner". Let's fix this. It's unlikely that this truly matters because the "mutex_owner" is only needed if we're going to do subsequent locking of the same rdev. However, even though it's not truly needed it seems less surprising if we consistently set "mutex_owner" properly. Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking") Signed-off-by: Douglas Anderson Link: https://lore.kernel.org/r/20230329143317.RFC.v2.1.I4e9d433ea26360c06dd1381d091c82bb1a4ce843@changeid Signed-off-by: Mark Brown Signed-off-by: Sasha Levin --- drivers/regulator/core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 6dd698b2d0af..bd2a3f44dd6e 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -343,6 +343,7 @@ static void regulator_lock_dependent(struct regulator_dev *rdev, ww_mutex_lock_slow(&new_contended_rdev->mutex, ww_ctx); old_contended_rdev = new_contended_rdev; old_contended_rdev->ref_cnt++; + old_contended_rdev->mutex_owner = current; } err = regulator_lock_recursive(rdev, @@ -5800,6 +5801,7 @@ static void regulator_summary_lock(struct ww_acquire_ctx *ww_ctx) ww_mutex_lock_slow(&new_contended_rdev->mutex, ww_ctx); old_contended_rdev = new_contended_rdev; old_contended_rdev->ref_cnt++; + old_contended_rdev->mutex_owner = current; } err = regulator_summary_lock_all(ww_ctx, From 06140d6dfe720d80566f792b4e28a9cd60a67970 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Wed, 29 Mar 2023 14:33:54 -0700 Subject: [PATCH 05/57] regulator: core: Avoid lockdep reports when resolving supplies [ Upstream commit cba6cfdc7c3f1516f0d08ddfb24e689af0932573 ] An automated bot told me that there was a potential lockdep problem with regulators. This was on the chromeos-5.15 kernel, but I see nothing that would be different downstream compared to upstream. The bot said: ============================================ WARNING: possible recursive locking detected 5.15.104-lockdep-17461-gc1e499ed6604 #1 Not tainted -------------------------------------------- kworker/u16:4/115 is trying to acquire lock: ffffff8083110170 (regulator_ww_class_mutex){+.+.}-{3:3}, at: create_regulator+0x398/0x7ec but task is already holding lock: ffffff808378e170 (regulator_ww_class_mutex){+.+.}-{3:3}, at: ww_mutex_trylock+0x3c/0x7b8 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(regulator_ww_class_mutex); lock(regulator_ww_class_mutex); *** DEADLOCK *** May be due to missing lock nesting notation 4 locks held by kworker/u16:4/115: #0: ffffff808006a948 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x520/0x1348 #1: ffffffc00e0a7cc0 ((work_completion)(&entry->work)){+.+.}-{0:0}, at: process_one_work+0x55c/0x1348 #2: ffffff80828a2260 (&dev->mutex){....}-{3:3}, at: __device_attach_async_helper+0xd0/0x2a4 #3: ffffff808378e170 (regulator_ww_class_mutex){+.+.}-{3:3}, at: ww_mutex_trylock+0x3c/0x7b8 stack backtrace: CPU: 2 PID: 115 Comm: kworker/u16:4 Not tainted 5.15.104-lockdep-17461-gc1e499ed6604 #1 9292e52fa83c0e23762b2b3aa1bacf5787a4d5da Hardware name: Google Quackingstick (rev0+) (DT) Workqueue: events_unbound async_run_entry_fn Call trace: dump_backtrace+0x0/0x4ec show_stack+0x34/0x50 dump_stack_lvl+0xdc/0x11c dump_stack+0x1c/0x48 __lock_acquire+0x16d4/0x6c74 lock_acquire+0x208/0x750 __mutex_lock_common+0x11c/0x11f8 ww_mutex_lock+0xc0/0x440 create_regulator+0x398/0x7ec regulator_resolve_supply+0x654/0x7c4 regulator_register_resolve_supply+0x30/0x120 class_for_each_device+0x1b8/0x230 regulator_register+0x17a4/0x1f40 devm_regulator_register+0x60/0xd0 reg_fixed_voltage_probe+0x728/0xaec platform_probe+0x150/0x1c8 really_probe+0x274/0xa20 __driver_probe_device+0x1dc/0x3f4 driver_probe_device+0x78/0x1c0 __device_attach_driver+0x1ac/0x2c8 bus_for_each_drv+0x11c/0x190 __device_attach_async_helper+0x1e4/0x2a4 async_run_entry_fn+0xa0/0x3ac process_one_work+0x638/0x1348 worker_thread+0x4a8/0x9c4 kthread+0x2e4/0x3a0 ret_from_fork+0x10/0x20 The problem was first reported soon after we made many of the regulators probe asynchronously, though nothing I've seen implies that the problems couldn't have also happened even without that. I haven't personally been able to reproduce the lockdep issue, but the issue does look somewhat legitimate. Specifically, it looks like in regulator_resolve_supply() we are holding a "rdev" lock while calling set_supply() -> create_regulator() which grabs the lock of a _different_ "rdev" (the one for our supply). This is not necessarily safe from a lockdep perspective since there is no documented ordering between these two locks. In reality, we should always be locking a regulator before the supplying regulator, so I don't expect there to be any real deadlocks in practice. However, the regulator framework in general doesn't express this to lockdep. Let's fix the issue by simply grabbing the two locks involved in the same way we grab multiple locks elsewhere in the regulator framework: using the "wound/wait" mechanisms. Fixes: eaa7995c529b ("regulator: core: avoid regulator_resolve_supply() race condition") Signed-off-by: Douglas Anderson Link: https://lore.kernel.org/r/20230329143317.RFC.v2.2.I30d8e1ca10cfbe5403884cdd192253a2e063eb9e@changeid Signed-off-by: Mark Brown Signed-off-by: Sasha Levin --- drivers/regulator/core.c | 91 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 83 insertions(+), 8 deletions(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index bd2a3f44dd6e..47a04c5f7a9b 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -216,6 +216,78 @@ static void regulator_unlock(struct regulator_dev *rdev) mutex_unlock(®ulator_nesting_mutex); } +/** + * regulator_lock_two - lock two regulators + * @rdev1: first regulator + * @rdev2: second regulator + * @ww_ctx: w/w mutex acquire context + * + * Locks both rdevs using the regulator_ww_class. + */ +static void regulator_lock_two(struct regulator_dev *rdev1, + struct regulator_dev *rdev2, + struct ww_acquire_ctx *ww_ctx) +{ + struct regulator_dev *tmp; + int ret; + + ww_acquire_init(ww_ctx, ®ulator_ww_class); + + /* Try to just grab both of them */ + ret = regulator_lock_nested(rdev1, ww_ctx); + WARN_ON(ret); + ret = regulator_lock_nested(rdev2, ww_ctx); + if (ret != -EDEADLOCK) { + WARN_ON(ret); + goto exit; + } + + while (true) { + /* + * Start of loop: rdev1 was locked and rdev2 was contended. + * Need to unlock rdev1, slowly lock rdev2, then try rdev1 + * again. + */ + regulator_unlock(rdev1); + + ww_mutex_lock_slow(&rdev2->mutex, ww_ctx); + rdev2->ref_cnt++; + rdev2->mutex_owner = current; + ret = regulator_lock_nested(rdev1, ww_ctx); + + if (ret == -EDEADLOCK) { + /* More contention; swap which needs to be slow */ + tmp = rdev1; + rdev1 = rdev2; + rdev2 = tmp; + } else { + WARN_ON(ret); + break; + } + } + +exit: + ww_acquire_done(ww_ctx); +} + +/** + * regulator_unlock_two - unlock two regulators + * @rdev1: first regulator + * @rdev2: second regulator + * @ww_ctx: w/w mutex acquire context + * + * The inverse of regulator_lock_two(). + */ + +static void regulator_unlock_two(struct regulator_dev *rdev1, + struct regulator_dev *rdev2, + struct ww_acquire_ctx *ww_ctx) +{ + regulator_unlock(rdev2); + regulator_unlock(rdev1); + ww_acquire_fini(ww_ctx); +} + static bool regulator_supply_is_couple(struct regulator_dev *rdev) { struct regulator_dev *c_rdev; @@ -1460,8 +1532,8 @@ static int set_machine_constraints(struct regulator_dev *rdev) /** * set_supply - set regulator supply regulator - * @rdev: regulator name - * @supply_rdev: supply regulator name + * @rdev: regulator (locked) + * @supply_rdev: supply regulator (locked)) * * Called by platform initialisation code to set the supply regulator for this * regulator. This ensures that a regulators supply will also be enabled by the @@ -1633,6 +1705,8 @@ static struct regulator *create_regulator(struct regulator_dev *rdev, struct regulator *regulator; int err = 0; + lockdep_assert_held_once(&rdev->mutex.base); + if (dev) { char buf[REG_STR_SIZE]; int size; @@ -1660,9 +1734,7 @@ static struct regulator *create_regulator(struct regulator_dev *rdev, regulator->rdev = rdev; regulator->supply_name = supply_name; - regulator_lock(rdev); list_add(®ulator->list, &rdev->consumer_list); - regulator_unlock(rdev); if (dev) { regulator->dev = dev; @@ -1828,6 +1900,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev) { struct regulator_dev *r; struct device *dev = rdev->dev.parent; + struct ww_acquire_ctx ww_ctx; int ret = 0; /* No supply to resolve? */ @@ -1894,23 +1967,23 @@ static int regulator_resolve_supply(struct regulator_dev *rdev) * between rdev->supply null check and setting rdev->supply in * set_supply() from concurrent tasks. */ - regulator_lock(rdev); + regulator_lock_two(rdev, r, &ww_ctx); /* Supply just resolved by a concurrent task? */ if (rdev->supply) { - regulator_unlock(rdev); + regulator_unlock_two(rdev, r, &ww_ctx); put_device(&r->dev); goto out; } ret = set_supply(rdev, r); if (ret < 0) { - regulator_unlock(rdev); + regulator_unlock_two(rdev, r, &ww_ctx); put_device(&r->dev); goto out; } - regulator_unlock(rdev); + regulator_unlock_two(rdev, r, &ww_ctx); /* * In set_machine_constraints() we may have turned this regulator on @@ -2023,7 +2096,9 @@ struct regulator *_regulator_get(struct device *dev, const char *id, return regulator; } + regulator_lock(rdev); regulator = create_regulator(rdev, dev, id); + regulator_unlock(rdev); if (regulator == NULL) { regulator = ERR_PTR(-ENOMEM); module_put(rdev->owner); From 4883f0f7ee448de1e15814497f21177481d03447 Mon Sep 17 00:00:00 2001 From: Uros Bizjak Date: Mon, 27 Feb 2023 17:09:17 +0100 Subject: [PATCH 06/57] x86/apic: Fix atomic update of offset in reserve_eilvt_offset() [ Upstream commit f96fb2df3eb31ede1b34b0521560967310267750 ] The detection of atomic update failure in reserve_eilvt_offset() is not correct. The value returned by atomic_cmpxchg() should be compared to the old value from the location to be updated. If these two are the same, then atomic update succeeded and "eilvt_offsets[offset]" location is updated to "new" in an atomic way. Otherwise, the atomic update failed and it should be retried with the value from "eilvt_offsets[offset]" - exactly what atomic_try_cmpxchg() does in a correct and more optimal way. Fixes: a68c439b1966c ("apic, x86: Check if EILVT APIC registers are available (AMD only)") Signed-off-by: Uros Bizjak Signed-off-by: Borislav Petkov (AMD) Link: https://lore.kernel.org/r/20230227160917.107820-1-ubizjak@gmail.com Signed-off-by: Sasha Levin --- arch/x86/kernel/apic/apic.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c index 1c96f2425eaf..25eb69f26e03 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -410,10 +410,9 @@ static unsigned int reserve_eilvt_offset(int offset, unsigned int new) if (vector && !eilvt_entry_is_changeable(vector, new)) /* may not change if vectors are different */ return rsvd; - rsvd = atomic_cmpxchg(&eilvt_offsets[offset], rsvd, new); - } while (rsvd != new); + } while (!atomic_try_cmpxchg(&eilvt_offsets[offset], &rsvd, new)); - rsvd &= ~APIC_EILVT_MASKED; + rsvd = new & ~APIC_EILVT_MASKED; if (rsvd && rsvd != vector) pr_info("LVT offset %d assigned for vector 0x%02x\n", offset, rsvd); From de19d02d734ef29f5dbd2c12fe810fa960ecd83f Mon Sep 17 00:00:00 2001 From: Zheng Wang Date: Mon, 13 Mar 2023 16:42:20 +0000 Subject: [PATCH 07/57] media: rkvdec: fix use after free bug in rkvdec_remove [ Upstream commit 3228cec23b8b29215e18090c6ba635840190993d ] In rkvdec_probe, rkvdec->watchdog_work is bound with rkvdec_watchdog_func. Then rkvdec_vp9_run may be called to start the work. If we remove the module which will call rkvdec_remove to make cleanup, there may be a unfinished work. The possible sequence is as follows, which will cause a typical UAF bug. Fix it by canceling the work before cleanup in rkvdec_remove. CPU0 CPU1 |rkvdec_watchdog_func rkvdec_remove | rkvdec_v4l2_cleanup| v4l2_m2m_release | kfree(m2m_dev); | | | v4l2_m2m_get_curr_priv | m2m_dev->curr_ctx //use Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver") Signed-off-by: Zheng Wang Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab Signed-off-by: Sasha Levin --- drivers/staging/media/rkvdec/rkvdec.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c index e384ea8d7280..f6a29a707862 100644 --- a/drivers/staging/media/rkvdec/rkvdec.c +++ b/drivers/staging/media/rkvdec/rkvdec.c @@ -1077,6 +1077,8 @@ static int rkvdec_remove(struct platform_device *pdev) { struct rkvdec_dev *rkvdec = platform_get_drvdata(pdev); + cancel_delayed_work_sync(&rkvdec->watchdog_work); + rkvdec_v4l2_cleanup(rkvdec); pm_runtime_disable(&pdev->dev); pm_runtime_dont_use_autosuspend(&pdev->dev); From e9d64e90a0ada4d00ac6562e351ef10ae7d9b911 Mon Sep 17 00:00:00 2001 From: Zheng Wang Date: Sat, 18 Mar 2023 16:15:06 +0800 Subject: [PATCH 08/57] media: dm1105: Fix use after free bug in dm1105_remove due to race condition [ Upstream commit 5abda7a16698d4d1f47af1168d8fa2c640116b4a ] In dm1105_probe, it called dm1105_ir_init and bound &dm1105->ir.work with dm1105_emit_key. When it handles IRQ request with dm1105_irq, it may call schedule_work to start the work. When we call dm1105_remove to remove the driver, there may be a sequence as follows: Fix it by finishing the work before cleanup in dm1105_remove CPU0 CPU1 |dm1105_emit_key dm1105_remove | dm1105_ir_exit | rc_unregister_device | rc_free_device | rc_dev_release | kfree(dev); | | | rc_keydown | //use Fixes: 34d2f9bf189c ("V4L/DVB: dm1105: use dm1105_dev & dev instead of dm1105dvb") Signed-off-by: Zheng Wang Signed-off-by: Hans Verkuil Signed-off-by: Sasha Levin --- drivers/media/pci/dm1105/dm1105.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/pci/dm1105/dm1105.c b/drivers/media/pci/dm1105/dm1105.c index 9dce31d2b525..d2e194a24e7e 100644 --- a/drivers/media/pci/dm1105/dm1105.c +++ b/drivers/media/pci/dm1105/dm1105.c @@ -1178,6 +1178,7 @@ static void dm1105_remove(struct pci_dev *pdev) struct dvb_demux *dvbdemux = &dev->demux; struct dmx_demux *dmx = &dvbdemux->dmx; + cancel_work_sync(&dev->ir.work); dm1105_ir_exit(dev); dmx->close(dmx); dvb_net_release(&dev->dvbnet); From 7dac96e9cc985328ec1fae92f0c245f559dc0e11 Mon Sep 17 00:00:00 2001 From: Zheng Wang Date: Sat, 18 Mar 2023 16:50:23 +0800 Subject: [PATCH 09/57] media: saa7134: fix use after free bug in saa7134_finidev due to race condition [ Upstream commit 30cf57da176cca80f11df0d9b7f71581fe601389 ] In saa7134_initdev, it will call saa7134_hwinit1. There are three function invoking here: saa7134_video_init1, saa7134_ts_init1 and saa7134_vbi_init1. All of them will init a timer with same function. Take saa7134_video_init1 as an example. It'll bound &dev->video_q.timeout with saa7134_buffer_timeout. In buffer_activate, the timer funtcion is started. If we remove the module or device which will call saa7134_finidev to make cleanup, there may be a unfinished work. The possible sequence is as follows, which will cause a typical UAF bug. Fix it by canceling the timer works accordingly before cleanup in saa7134_finidev. CPU0 CPU1 |saa7134_buffer_timeout saa7134_finidev | kfree(dev); | | | saa7134_buffer_next | //use dev Fixes: 1e7126b4a86a ("media: saa7134: Convert timers to use timer_setup()") Signed-off-by: Zheng Wang Signed-off-by: Hans Verkuil Signed-off-by: Sasha Levin --- drivers/media/pci/saa7134/saa7134-ts.c | 1 + drivers/media/pci/saa7134/saa7134-vbi.c | 1 + drivers/media/pci/saa7134/saa7134-video.c | 1 + 3 files changed, 3 insertions(+) diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c index 6a5053126237..437dbe5e75e2 100644 --- a/drivers/media/pci/saa7134/saa7134-ts.c +++ b/drivers/media/pci/saa7134/saa7134-ts.c @@ -300,6 +300,7 @@ int saa7134_ts_start(struct saa7134_dev *dev) int saa7134_ts_fini(struct saa7134_dev *dev) { + del_timer_sync(&dev->ts_q.timeout); saa7134_pgtable_free(dev->pci, &dev->ts_q.pt); return 0; } diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c index 3f0b0933eed6..3e773690468b 100644 --- a/drivers/media/pci/saa7134/saa7134-vbi.c +++ b/drivers/media/pci/saa7134/saa7134-vbi.c @@ -185,6 +185,7 @@ int saa7134_vbi_init1(struct saa7134_dev *dev) int saa7134_vbi_fini(struct saa7134_dev *dev) { /* nothing */ + del_timer_sync(&dev->vbi_q.timeout); return 0; } diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c index 85d082baaadc..df9e3293015a 100644 --- a/drivers/media/pci/saa7134/saa7134-video.c +++ b/drivers/media/pci/saa7134/saa7134-video.c @@ -2153,6 +2153,7 @@ int saa7134_video_init1(struct saa7134_dev *dev) void saa7134_video_fini(struct saa7134_dev *dev) { + del_timer_sync(&dev->video_q.timeout); /* free stuff */ saa7134_pgtable_free(dev->pci, &dev->video_q.pt); saa7134_pgtable_free(dev->pci, &dev->vbi_q.pt); From 21de9d0daab1a98635bfd2b2afb9abe346691d18 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Fri, 23 Apr 2021 16:59:34 +0200 Subject: [PATCH 10/57] media: rcar_fdp1: simplify error check logic at fdp_open() [ Upstream commit fa9f443f7c962d072d150472e2bb77de39817a9a ] Avoid some code duplication by moving the common error path logic at fdp_open(). Signed-off-by: Mauro Carvalho Chehab Stable-dep-of: c766c90faf93 ("media: rcar_fdp1: Fix refcount leak in probe and remove function") Signed-off-by: Sasha Levin --- drivers/media/platform/rcar_fdp1.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c index c9448de885b6..ac2e7b2c6082 100644 --- a/drivers/media/platform/rcar_fdp1.c +++ b/drivers/media/platform/rcar_fdp1.c @@ -2121,9 +2121,7 @@ static int fdp1_open(struct file *file) if (ctx->hdl.error) { ret = ctx->hdl.error; - v4l2_ctrl_handler_free(&ctx->hdl); - kfree(ctx); - goto done; + goto error_ctx; } ctx->fh.ctrl_handler = &ctx->hdl; @@ -2137,10 +2135,7 @@ static int fdp1_open(struct file *file) if (IS_ERR(ctx->fh.m2m_ctx)) { ret = PTR_ERR(ctx->fh.m2m_ctx); - - v4l2_ctrl_handler_free(&ctx->hdl); - kfree(ctx); - goto done; + goto error_ctx; } /* Perform any power management required */ @@ -2151,6 +2146,12 @@ static int fdp1_open(struct file *file) dprintk(fdp1, "Created instance: %p, m2m_ctx: %p\n", ctx, ctx->fh.m2m_ctx); + mutex_unlock(&fdp1->dev_mutex); + return 0; + +error_ctx: + v4l2_ctrl_handler_free(&ctx->hdl); + kfree(ctx); done: mutex_unlock(&fdp1->dev_mutex); return ret; From bf91fb6c1e9d20d965654f9b13cb23f22559ef12 Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Fri, 23 Apr 2021 16:59:34 +0200 Subject: [PATCH 11/57] media: rcar_fdp1: fix pm_runtime_get_sync() usage count [ Upstream commit 45e75a8c6fa455a5909ac04db76a4b15d6bb8368 ] The pm_runtime_get_sync() internally increments the dev->power.usage_count without decrementing it, even on errors. Replace it by the new pm_runtime_resume_and_get(), introduced by: commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter") in order to properly decrement the usage counter, avoiding a potential PM usage counter leak. Also, right now, the driver is ignoring any troubles when trying to do PM resume. So, add the proper error handling for the code. Reviewed-by: Jonathan Cameron Signed-off-by: Mauro Carvalho Chehab Stable-dep-of: c766c90faf93 ("media: rcar_fdp1: Fix refcount leak in probe and remove function") Signed-off-by: Sasha Levin --- drivers/media/platform/rcar_fdp1.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c index ac2e7b2c6082..cedae184c6ad 100644 --- a/drivers/media/platform/rcar_fdp1.c +++ b/drivers/media/platform/rcar_fdp1.c @@ -2139,7 +2139,9 @@ static int fdp1_open(struct file *file) } /* Perform any power management required */ - pm_runtime_get_sync(fdp1->dev); + ret = pm_runtime_resume_and_get(fdp1->dev); + if (ret < 0) + goto error_pm; v4l2_fh_add(&ctx->fh); @@ -2149,6 +2151,8 @@ static int fdp1_open(struct file *file) mutex_unlock(&fdp1->dev_mutex); return 0; +error_pm: + v4l2_m2m_ctx_release(ctx->fh.m2m_ctx); error_ctx: v4l2_ctrl_handler_free(&ctx->hdl); kfree(ctx); @@ -2356,7 +2360,9 @@ static int fdp1_probe(struct platform_device *pdev) /* Power up the cells to read HW */ pm_runtime_enable(&pdev->dev); - pm_runtime_get_sync(fdp1->dev); + ret = pm_runtime_resume_and_get(fdp1->dev); + if (ret < 0) + goto disable_pm; hw_version = fdp1_read(fdp1, FD1_IP_INTDATA); switch (hw_version) { @@ -2385,6 +2391,9 @@ static int fdp1_probe(struct platform_device *pdev) return 0; +disable_pm: + pm_runtime_disable(fdp1->dev); + release_m2m: v4l2_m2m_release(fdp1->m2m_dev); From ee24c9e23206aa3e04e5275b500e7b81995ec894 Mon Sep 17 00:00:00 2001 From: Cai Huoqing Date: Wed, 1 Sep 2021 07:55:24 +0200 Subject: [PATCH 12/57] media: rcar_fdp1: Make use of the helper function devm_platform_ioremap_resource() [ Upstream commit 736cce12fa630e28705de06570d74f0513d948d5 ] Use the devm_platform_ioremap_resource() helper instead of calling platform_get_resource() and devm_ioremap_resource() separately Signed-off-by: Cai Huoqing Reviewed-by: Kieran Bingham Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab Stable-dep-of: c766c90faf93 ("media: rcar_fdp1: Fix refcount leak in probe and remove function") Signed-off-by: Sasha Levin --- drivers/media/platform/rcar_fdp1.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c index cedae184c6ad..782e805b85da 100644 --- a/drivers/media/platform/rcar_fdp1.c +++ b/drivers/media/platform/rcar_fdp1.c @@ -2260,7 +2260,6 @@ static int fdp1_probe(struct platform_device *pdev) struct fdp1_dev *fdp1; struct video_device *vfd; struct device_node *fcp_node; - struct resource *res; struct clk *clk; unsigned int i; @@ -2287,8 +2286,7 @@ static int fdp1_probe(struct platform_device *pdev) platform_set_drvdata(pdev, fdp1); /* Memory-mapped registers */ - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - fdp1->regs = devm_ioremap_resource(&pdev->dev, res); + fdp1->regs = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(fdp1->regs)) return PTR_ERR(fdp1->regs); From ccc454881f0953c9bf8201e673d398cc320cdb8b Mon Sep 17 00:00:00 2001 From: Tang Bin Date: Thu, 21 Oct 2021 05:09:38 +0200 Subject: [PATCH 13/57] media: rcar_fdp1: Fix the correct variable assignments [ Upstream commit af88c2adbb72a09ab1bb5c37ba388c98fecca69b ] In the function fdp1_probe(), when get irq failed, the function platform_get_irq() log an error message, so remove redundant message here. And the variable type of "ret" is int, the "fdp1->irq" is unsigned int, when irq failed, this place maybe wrong, thus fix it. Signed-off-by: Tang Bin Reviewed-by: Geert Uytterhoeven Reviewed-by: Kieran Bingham Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab Stable-dep-of: c766c90faf93 ("media: rcar_fdp1: Fix refcount leak in probe and remove function") Signed-off-by: Sasha Levin --- drivers/media/platform/rcar_fdp1.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c index 782e805b85da..1dd8bebb66f5 100644 --- a/drivers/media/platform/rcar_fdp1.c +++ b/drivers/media/platform/rcar_fdp1.c @@ -2291,11 +2291,10 @@ static int fdp1_probe(struct platform_device *pdev) return PTR_ERR(fdp1->regs); /* Interrupt service routine registration */ - fdp1->irq = ret = platform_get_irq(pdev, 0); - if (ret < 0) { - dev_err(&pdev->dev, "cannot find IRQ\n"); + ret = platform_get_irq(pdev, 0); + if (ret < 0) return ret; - } + fdp1->irq = ret; ret = devm_request_irq(&pdev->dev, fdp1->irq, fdp1_irq_handler, 0, dev_name(&pdev->dev), fdp1); From 1acb982e3616e70128994fdecf2368a259c8a489 Mon Sep 17 00:00:00 2001 From: Miaoqian Lin Date: Fri, 6 Jan 2023 11:58:09 +0400 Subject: [PATCH 14/57] media: rcar_fdp1: Fix refcount leak in probe and remove function [ Upstream commit c766c90faf93897b77c9c5daa603cffab85ba907 ] rcar_fcp_get() take reference, which should be balanced with rcar_fcp_put(). Add missing rcar_fcp_put() in fdp1_remove and the error paths of fdp1_probe() to fix this. Fixes: 4710b752e029 ("[media] v4l: Add Renesas R-Car FDP1 Driver") Signed-off-by: Miaoqian Lin Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart [hverkuil: resolve merge conflict, remove() is now void] Signed-off-by: Hans Verkuil Signed-off-by: Sasha Levin --- drivers/media/platform/rcar_fdp1.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/rcar_fdp1.c b/drivers/media/platform/rcar_fdp1.c index 1dd8bebb66f5..44a57fa06e74 100644 --- a/drivers/media/platform/rcar_fdp1.c +++ b/drivers/media/platform/rcar_fdp1.c @@ -2317,8 +2317,10 @@ static int fdp1_probe(struct platform_device *pdev) /* Determine our clock rate */ clk = clk_get(&pdev->dev, NULL); - if (IS_ERR(clk)) - return PTR_ERR(clk); + if (IS_ERR(clk)) { + ret = PTR_ERR(clk); + goto put_dev; + } fdp1->clk_rate = clk_get_rate(clk); clk_put(clk); @@ -2327,7 +2329,7 @@ static int fdp1_probe(struct platform_device *pdev) ret = v4l2_device_register(&pdev->dev, &fdp1->v4l2_dev); if (ret) { v4l2_err(&fdp1->v4l2_dev, "Failed to register video device\n"); - return ret; + goto put_dev; } /* M2M registration */ @@ -2397,6 +2399,8 @@ release_m2m: unreg_dev: v4l2_device_unregister(&fdp1->v4l2_dev); +put_dev: + rcar_fcp_put(fdp1->fcp); return ret; } @@ -2408,6 +2412,7 @@ static int fdp1_remove(struct platform_device *pdev) video_unregister_device(&fdp1->vfd); v4l2_device_unregister(&fdp1->v4l2_dev); pm_runtime_disable(&pdev->dev); + rcar_fcp_put(fdp1->fcp); return 0; } From 025a34716c888551a7ffd2a1119f60cb628be7fd Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Fri, 24 Mar 2023 13:38:33 -0700 Subject: [PATCH 15/57] media: rc: gpio-ir-recv: Fix support for wake-up [ Upstream commit 9c592f8ab114875fdb3b2040f01818e53de44991 ] The driver was intended from the start to be a wake-up source for the system, however due to the absence of a suitable call to device_set_wakeup_capable(), the device_may_wakeup() call used to decide whether to enable the GPIO interrupt as a wake-up source would never happen. Lookup the DT standard "wakeup-source" property and call device_init_wakeup() to ensure the device is flagged as being wakeup capable. Reported-by: Matthew Lear Fixes: fd0f6851eb46 ("[media] rc: Add support for GPIO based IR Receiver driver") Signed-off-by: Florian Fainelli Signed-off-by: Sean Young Signed-off-by: Hans Verkuil Signed-off-by: Sasha Levin --- drivers/media/rc/gpio-ir-recv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c index a56c844d7f81..16795e07dc10 100644 --- a/drivers/media/rc/gpio-ir-recv.c +++ b/drivers/media/rc/gpio-ir-recv.c @@ -107,6 +107,8 @@ static int gpio_ir_recv_probe(struct platform_device *pdev) rcdev->map_name = RC_MAP_EMPTY; gpio_dev->rcdev = rcdev; + if (of_property_read_bool(np, "wakeup-source")) + device_init_wakeup(dev, true); rc = devm_rc_register_device(dev, rcdev); if (rc < 0) { From a30297bff881dd13bd74cfa4c93034e47396011e Mon Sep 17 00:00:00 2001 From: Stanimir Varbanov Date: Mon, 28 Sep 2020 18:44:29 +0200 Subject: [PATCH 16/57] media: venus: vdec: Fix non reliable setting of LAST flag [ Upstream commit acf8a57d8caf5ceabbe50774953fe04745ad1a50 ] In real use of dynamic-resolution-change it is observed that the LAST buffer flag (which marks the last decoded buffer with the resolution before the resolution-change event) is not reliably set. Fix this by set the LAST buffer flag on next queued capture buffer after the resolution-change event. Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab Stable-dep-of: 50248ad9f190 ("media: venus: dec: Fix handling of the start cmd") Signed-off-by: Sasha Levin --- drivers/media/platform/qcom/venus/core.h | 5 +-- drivers/media/platform/qcom/venus/helpers.c | 6 +++ drivers/media/platform/qcom/venus/vdec.c | 45 ++++++++++++--------- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h index f2a0ef9ee884..f78eed2c243a 100644 --- a/drivers/media/platform/qcom/venus/core.h +++ b/drivers/media/platform/qcom/venus/core.h @@ -283,7 +283,6 @@ enum venus_dec_state { VENUS_DEC_STATE_DRAIN = 5, VENUS_DEC_STATE_DECODING = 6, VENUS_DEC_STATE_DRC = 7, - VENUS_DEC_STATE_DRC_FLUSH_DONE = 8, }; struct venus_ts_metadata { @@ -348,7 +347,7 @@ struct venus_ts_metadata { * @priv: a private for HFI operations callbacks * @session_type: the type of the session (decoder or encoder) * @hprop: a union used as a holder by get property - * @last_buf: last capture buffer for dynamic-resoluton-change + * @next_buf_last: a flag to mark next queued capture buffer as last */ struct venus_inst { struct list_head list; @@ -410,7 +409,7 @@ struct venus_inst { union hfi_get_property hprop; unsigned int core_acquired: 1; unsigned int bit_depth; - struct vb2_buffer *last_buf; + bool next_buf_last; }; #define IS_V1(core) ((core)->res->hfi_version == HFI_VERSION_1XX) diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c index 50439eb1ffea..5ca3920237c5 100644 --- a/drivers/media/platform/qcom/venus/helpers.c +++ b/drivers/media/platform/qcom/venus/helpers.c @@ -1347,6 +1347,12 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer *vb) v4l2_m2m_buf_queue(m2m_ctx, vbuf); + /* Skip processing queued capture buffers after LAST flag */ + if (inst->session_type == VIDC_SESSION_TYPE_DEC && + V4L2_TYPE_IS_CAPTURE(vb->vb2_queue->type) && + inst->codec_state == VENUS_DEC_STATE_DRC) + goto unlock; + cache_payload(inst, vb); if (inst->session_type == VIDC_SESSION_TYPE_ENC && diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index de34a87d1130..b6b5ae0a457b 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -916,10 +916,6 @@ static int vdec_start_capture(struct venus_inst *inst) return 0; reconfigure: - ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT, true); - if (ret) - return ret; - ret = vdec_output_conf(inst); if (ret) return ret; @@ -947,6 +943,8 @@ reconfigure: venus_pm_load_scale(inst); + inst->next_buf_last = false; + ret = hfi_session_continue(inst); if (ret) goto free_dpb_bufs; @@ -987,6 +985,7 @@ static int vdec_start_output(struct venus_inst *inst) venus_helper_init_instance(inst); inst->sequence_out = 0; inst->reconfig = false; + inst->next_buf_last = false; ret = vdec_set_properties(inst); if (ret) @@ -1080,9 +1079,7 @@ static int vdec_stop_capture(struct venus_inst *inst) inst->codec_state = VENUS_DEC_STATE_STOPPED; break; case VENUS_DEC_STATE_DRC: - WARN_ON(1); - fallthrough; - case VENUS_DEC_STATE_DRC_FLUSH_DONE: + ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT, true); inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP; venus_helper_free_dpb_bufs(inst); break; @@ -1206,9 +1203,28 @@ static void vdec_buf_cleanup(struct vb2_buffer *vb) static void vdec_vb2_buf_queue(struct vb2_buffer *vb) { struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue); + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); + static const struct v4l2_event eos = { .type = V4L2_EVENT_EOS }; vdec_pm_get_put(inst); + mutex_lock(&inst->lock); + + if (inst->next_buf_last && V4L2_TYPE_IS_CAPTURE(vb->vb2_queue->type) && + inst->codec_state == VENUS_DEC_STATE_DRC) { + vbuf->flags |= V4L2_BUF_FLAG_LAST; + vbuf->sequence = inst->sequence_cap++; + vbuf->field = V4L2_FIELD_NONE; + vb2_set_plane_payload(vb, 0, 0); + v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_DONE); + v4l2_event_queue_fh(&inst->fh, &eos); + inst->next_buf_last = false; + mutex_unlock(&inst->lock); + return; + } + + mutex_unlock(&inst->lock); + venus_helper_vb2_buf_queue(vb); } @@ -1252,13 +1268,6 @@ static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type, vb->timestamp = timestamp_us * NSEC_PER_USEC; vbuf->sequence = inst->sequence_cap++; - if (inst->last_buf == vb) { - inst->last_buf = NULL; - vbuf->flags |= V4L2_BUF_FLAG_LAST; - vb2_set_plane_payload(vb, 0, 0); - vb->timestamp = 0; - } - if (vbuf->flags & V4L2_BUF_FLAG_LAST) { const struct v4l2_event ev = { .type = V4L2_EVENT_EOS }; @@ -1343,12 +1352,9 @@ static void vdec_event_change(struct venus_inst *inst, */ if (!sufficient && inst->codec_state == VENUS_DEC_STATE_DRC) { - struct vb2_v4l2_buffer *last; int ret; - last = v4l2_m2m_last_dst_buf(inst->m2m_ctx); - if (last) - inst->last_buf = &last->vb2_buf; + inst->next_buf_last = true; ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT, false); if (ret) @@ -1397,8 +1403,7 @@ static void vdec_event_notify(struct venus_inst *inst, u32 event, static void vdec_flush_done(struct venus_inst *inst) { - if (inst->codec_state == VENUS_DEC_STATE_DRC) - inst->codec_state = VENUS_DEC_STATE_DRC_FLUSH_DONE; + dev_dbg(inst->core->dev_dec, VDBGH "flush done\n"); } static const struct hfi_inst_ops vdec_hfi_ops = { From 4c1239274f41989d3f2b454f87847a351e3f6ba2 Mon Sep 17 00:00:00 2001 From: Stanimir Varbanov Date: Mon, 28 Sep 2020 18:44:30 +0200 Subject: [PATCH 17/57] media: venus: vdec: Make decoder return LAST flag for sufficient event [ Upstream commit a4ca67af8b831a781ac53060c5d5c3dccaf7676e ] This makes the decoder to behaives equally for sufficient and insufficient events. After this change the LAST buffer flag will be set when the new resolution (in dynamic-resolution-change state) is smaller then the old bitstream resolution. Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab Stable-dep-of: 50248ad9f190 ("media: venus: dec: Fix handling of the start cmd") Signed-off-by: Sasha Levin --- drivers/media/platform/qcom/venus/vdec.c | 42 ++++++++++++++++-------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index b6b5ae0a457b..3adff10fce6a 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -636,6 +636,7 @@ static int vdec_output_conf(struct venus_inst *inst) { struct venus_core *core = inst->core; struct hfi_enable en = { .enable = 1 }; + struct hfi_buffer_requirements bufreq; u32 width = inst->out_width; u32 height = inst->out_height; u32 out_fmt, out2_fmt; @@ -711,6 +712,23 @@ static int vdec_output_conf(struct venus_inst *inst) } if (IS_V3(core) || IS_V4(core)) { + ret = venus_helper_get_bufreq(inst, HFI_BUFFER_OUTPUT, &bufreq); + if (ret) + return ret; + + if (bufreq.size > inst->output_buf_size) + return -EINVAL; + + if (inst->dpb_fmt) { + ret = venus_helper_get_bufreq(inst, HFI_BUFFER_OUTPUT2, + &bufreq); + if (ret) + return ret; + + if (bufreq.size > inst->output2_buf_size) + return -EINVAL; + } + if (inst->output2_buf_size) { ret = venus_helper_set_bufsize(inst, inst->output2_buf_size, @@ -1330,19 +1348,15 @@ static void vdec_event_change(struct venus_inst *inst, dev_dbg(dev, VDBGM "event %s sufficient resources (%ux%u)\n", sufficient ? "" : "not", ev_data->width, ev_data->height); - if (sufficient) { - hfi_session_continue(inst); - } else { - switch (inst->codec_state) { - case VENUS_DEC_STATE_INIT: - inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP; - break; - case VENUS_DEC_STATE_DECODING: - inst->codec_state = VENUS_DEC_STATE_DRC; - break; - default: - break; - } + switch (inst->codec_state) { + case VENUS_DEC_STATE_INIT: + inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP; + break; + case VENUS_DEC_STATE_DECODING: + inst->codec_state = VENUS_DEC_STATE_DRC; + break; + default: + break; } /* @@ -1351,7 +1365,7 @@ static void vdec_event_change(struct venus_inst *inst, * itself doesn't mark the last decoder output buffer with HFI EOS flag. */ - if (!sufficient && inst->codec_state == VENUS_DEC_STATE_DRC) { + if (inst->codec_state == VENUS_DEC_STATE_DRC) { int ret; inst->next_buf_last = true; From 5bac3de7f49fe3a21f485d2cf64f847b5b4677dd Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Wed, 2 Dec 2020 06:34:24 +0100 Subject: [PATCH 18/57] media: venus: preserve DRC state across seeks [ Upstream commit d5ee32d7e5929592ad9b6e7a919dcdf89d05221b ] DRC events can happen virtually at anytime, including when we are starting a seek. Should this happen, we must make sure to return to the DRC state, otherwise the firmware will expect buffers of the new resolution whereas userspace will still work with the old one. Returning to the DRC state upon resume for seeking makes sure that the client will get the DRC event and will reallocate the buffers to fit the firmware's expectations. Signed-off-by: Alexandre Courbot Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab Stable-dep-of: 50248ad9f190 ("media: venus: dec: Fix handling of the start cmd") Signed-off-by: Sasha Levin --- drivers/media/platform/qcom/venus/vdec.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index 3adff10fce6a..1bb2350408cf 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -987,7 +987,10 @@ static int vdec_start_output(struct venus_inst *inst) if (inst->codec_state == VENUS_DEC_STATE_SEEK) { ret = venus_helper_process_initial_out_bufs(inst); - inst->codec_state = VENUS_DEC_STATE_DECODING; + if (inst->next_buf_last) + inst->codec_state = VENUS_DEC_STATE_DRC; + else + inst->codec_state = VENUS_DEC_STATE_DECODING; goto done; } @@ -1093,8 +1096,10 @@ static int vdec_stop_capture(struct venus_inst *inst) ret = hfi_session_flush(inst, HFI_FLUSH_ALL, true); fallthrough; case VENUS_DEC_STATE_DRAIN: - vdec_cancel_dst_buffers(inst); inst->codec_state = VENUS_DEC_STATE_STOPPED; + fallthrough; + case VENUS_DEC_STATE_SEEK: + vdec_cancel_dst_buffers(inst); break; case VENUS_DEC_STATE_DRC: ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT, true); @@ -1116,6 +1121,7 @@ static int vdec_stop_output(struct venus_inst *inst) case VENUS_DEC_STATE_DECODING: case VENUS_DEC_STATE_DRAIN: case VENUS_DEC_STATE_STOPPED: + case VENUS_DEC_STATE_DRC: ret = hfi_session_flush(inst, HFI_FLUSH_ALL, true); inst->codec_state = VENUS_DEC_STATE_SEEK; break; @@ -1375,6 +1381,7 @@ static void vdec_event_change(struct venus_inst *inst, dev_dbg(dev, VDBGH "flush output error %d\n", ret); } + inst->next_buf_last = true; inst->reconfig = true; v4l2_event_queue_fh(&inst->fh, &ev); wake_up(&inst->reconf_wait); From 3a227dc12b1af9cdca12da9ce486606138ee33b0 Mon Sep 17 00:00:00 2001 From: Fritz Koenig Date: Tue, 1 Dec 2020 05:23:23 +0100 Subject: [PATCH 19/57] media: venus: vdec: Handle DRC after drain [ Upstream commit c8e8dabcd1a8c7aaedc514052d383a8152119084 ] If the DRC is near the end of the stream the client may send a V4L2_DEC_CMD_STOP before the DRC occurs. V4L2_DEC_CMD_STOP puts the driver into the VENUS_DEC_STATE_DRAIN state. DRC must be aware so that after the DRC event the state can be restored correctly. Signed-off-by: Fritz Koenig Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab Stable-dep-of: 50248ad9f190 ("media: venus: dec: Fix handling of the start cmd") Signed-off-by: Sasha Levin --- drivers/media/platform/qcom/venus/core.h | 1 + drivers/media/platform/qcom/venus/vdec.c | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h index f78eed2c243a..aebd4c664bfa 100644 --- a/drivers/media/platform/qcom/venus/core.h +++ b/drivers/media/platform/qcom/venus/core.h @@ -410,6 +410,7 @@ struct venus_inst { unsigned int core_acquired: 1; unsigned int bit_depth; bool next_buf_last; + bool drain_active; }; #define IS_V1(core) ((core)->res->hfi_version == HFI_VERSION_1XX) diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index 1bb2350408cf..766c29291527 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -518,8 +518,10 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd) ret = hfi_session_process_buf(inst, &fdata); - if (!ret && inst->codec_state == VENUS_DEC_STATE_DECODING) + if (!ret && inst->codec_state == VENUS_DEC_STATE_DECODING) { inst->codec_state = VENUS_DEC_STATE_DRAIN; + inst->drain_active = true; + } } unlock: @@ -969,9 +971,13 @@ reconfigure: inst->codec_state = VENUS_DEC_STATE_DECODING; + if (inst->drain_active) + inst->codec_state = VENUS_DEC_STATE_DRAIN; + inst->streamon_cap = 1; inst->sequence_cap = 0; inst->reconfig = false; + inst->drain_active = false; return 0; @@ -1097,6 +1103,7 @@ static int vdec_stop_capture(struct venus_inst *inst) fallthrough; case VENUS_DEC_STATE_DRAIN: inst->codec_state = VENUS_DEC_STATE_STOPPED; + inst->drain_active = false; fallthrough; case VENUS_DEC_STATE_SEEK: vdec_cancel_dst_buffers(inst); @@ -1297,8 +1304,10 @@ static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type, v4l2_event_queue_fh(&inst->fh, &ev); - if (inst->codec_state == VENUS_DEC_STATE_DRAIN) + if (inst->codec_state == VENUS_DEC_STATE_DRAIN) { + inst->drain_active = false; inst->codec_state = VENUS_DEC_STATE_STOPPED; + } } if (!bytesused) @@ -1359,6 +1368,7 @@ static void vdec_event_change(struct venus_inst *inst, inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP; break; case VENUS_DEC_STATE_DECODING: + case VENUS_DEC_STATE_DRAIN: inst->codec_state = VENUS_DEC_STATE_DRC; break; default: From a6aeba550c5904fba261c92ec8e7bfd715694d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krawczyk?= Date: Mon, 30 Jan 2023 13:54:18 +0000 Subject: [PATCH 20/57] media: venus: dec: Fix handling of the start cmd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [ Upstream commit 50248ad9f190d527cbd578190ca769729518b703 ] The decoder driver should clear the last_buffer_dequeued flag of the capture queue upon receiving V4L2_DEC_CMD_START. The last_buffer_dequeued flag is set upon receiving EOS (which always happens upon receiving V4L2_DEC_CMD_STOP). Without this patch, after issuing the V4L2_DEC_CMD_STOP and V4L2_DEC_CMD_START, the vb2_dqbuf() function will always fail, even if the buffers are completed by the hardware. Fixes: beac82904a87 ("media: venus: make decoder compliant with stateful codec API") Signed-off-by: MichaƂ Krawczyk Signed-off-by: Stanimir Varbanov Signed-off-by: Hans Verkuil Signed-off-by: Sasha Levin --- drivers/media/platform/qcom/venus/vdec.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index 766c29291527..c437a929a545 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -495,6 +495,7 @@ static int vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd) { struct venus_inst *inst = to_inst(file); + struct vb2_queue *dst_vq; struct hfi_frame_data fdata = {0}; int ret; @@ -522,6 +523,13 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd) inst->codec_state = VENUS_DEC_STATE_DRAIN; inst->drain_active = true; } + } else if (cmd->cmd == V4L2_DEC_CMD_START && + inst->codec_state == VENUS_DEC_STATE_STOPPED) { + dst_vq = v4l2_m2m_get_vq(inst->fh.m2m_ctx, + V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); + vb2_clear_last_buffer_dequeued(dst_vq); + + inst->codec_state = VENUS_DEC_STATE_DECODING; } unlock: From dfce9bb3517a78507cf96f9b83948d0b81338afa Mon Sep 17 00:00:00 2001 From: YAN SHI Date: Wed, 12 Apr 2023 11:35:29 +0800 Subject: [PATCH 21/57] regulator: stm32-pwr: fix of_iomap leak [ Upstream commit c4a413e56d16a2ae84e6d8992f215c4dcc7fac20 ] Smatch reports: drivers/regulator/stm32-pwr.c:166 stm32_pwr_regulator_probe() warn: 'base' from of_iomap() not released on lines: 151,166. In stm32_pwr_regulator_probe(), base is not released when devm_kzalloc() fails to allocate memory or devm_regulator_register() fails to register a new regulator device, which may cause a leak. To fix this issue, replace of_iomap() with devm_platform_ioremap_resource(). devm_platform_ioremap_resource() is a specialized function for platform devices. It allows 'base' to be automatically released whether the probe function succeeds or fails. Besides, use IS_ERR(base) instead of !base as the return value of devm_platform_ioremap_resource() can either be a pointer to the remapped memory or an ERR_PTR() encoded error code if the operation fails. Fixes: dc62f951a6a8 ("regulator: stm32-pwr: Fix return value check in stm32_pwr_regulator_probe()") Signed-off-by: YAN SHI Reported-by: kernel test robot Link: https://lore.kernel.org/oe-kbuild-all/202304111750.o2643eJN-lkp@intel.com/ Reviewed-by: Dongliang Mu Link: https://lore.kernel.org/r/20230412033529.18890-1-m202071378@hust.edu.cn Signed-off-by: Mark Brown Signed-off-by: Sasha Levin --- drivers/regulator/stm32-pwr.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/regulator/stm32-pwr.c b/drivers/regulator/stm32-pwr.c index 2a42acb7c24e..e5dd4db6403b 100644 --- a/drivers/regulator/stm32-pwr.c +++ b/drivers/regulator/stm32-pwr.c @@ -129,17 +129,16 @@ static const struct regulator_desc stm32_pwr_desc[] = { static int stm32_pwr_regulator_probe(struct platform_device *pdev) { - struct device_node *np = pdev->dev.of_node; struct stm32_pwr_reg *priv; void __iomem *base; struct regulator_dev *rdev; struct regulator_config config = { }; int i, ret = 0; - base = of_iomap(np, 0); - if (!base) { + base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(base)) { dev_err(&pdev->dev, "Unable to map IO memory\n"); - return -ENOMEM; + return PTR_ERR(base); } config.dev = &pdev->dev; From 55fc2246c46d8cc2ea56497664eaa4d7412409c8 Mon Sep 17 00:00:00 2001 From: Saurabh Sengar Date: Tue, 28 Mar 2023 00:30:04 -0700 Subject: [PATCH 22/57] x86/ioapic: Don't return 0 from arch_dynirq_lower_bound() [ Upstream commit 5af507bef93c09a94fb8f058213b489178f4cbe5 ] arch_dynirq_lower_bound() is invoked by the core interrupt code to retrieve the lowest possible Linux interrupt number for dynamically allocated interrupts like MSI. The x86 implementation uses this to exclude the IO/APIC GSI space. This works correctly as long as there is an IO/APIC registered, but returns 0 if not. This has been observed in VMs where the BIOS does not advertise an IO/APIC. 0 is an invalid interrupt number except for the legacy timer interrupt on x86. The return value is unchecked in the core code, so it ends up to allocate interrupt number 0 which is subsequently considered to be invalid by the caller, e.g. the MSI allocation code. The function has already a check for 0 in the case that an IO/APIC is registered, as ioapic_dynirq_base is 0 in case of device tree setups. Consolidate this and zero check for both ioapic_dynirq_base and gsi_top, which is used in the case that no IO/APIC is registered. Fixes: 3e5bedc2c258 ("x86/apic: Fix arch_dynirq_lower_bound() bug for DT enabled machines") Signed-off-by: Saurabh Sengar Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/1679988604-20308-1-git-send-email-ssengar@linux.microsoft.com Signed-off-by: Sasha Levin --- arch/x86/kernel/apic/io_apic.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 25b1d5c6af96..74794387bf59 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2442,17 +2442,21 @@ static int io_apic_get_redir_entries(int ioapic) unsigned int arch_dynirq_lower_bound(unsigned int from) { + unsigned int ret; + /* * dmar_alloc_hwirq() may be called before setup_IO_APIC(), so use * gsi_top if ioapic_dynirq_base hasn't been initialized yet. */ - if (!ioapic_initialized) - return gsi_top; + ret = ioapic_dynirq_base ? : gsi_top; + /* - * For DT enabled machines ioapic_dynirq_base is irrelevant and not - * updated. So simply return @from if ioapic_dynirq_base == 0. + * For DT enabled machines ioapic_dynirq_base is irrelevant and + * always 0. gsi_top can be 0 if there is no IO/APIC registered. + * 0 is an invalid interrupt number for dynamic allocations. Return + * @from instead. */ - return ioapic_dynirq_base ? : from; + return ret ? : from; } #ifdef CONFIG_X86_32 From f16f065f8ce3651b875cc88fb7a8d048f7beef7a Mon Sep 17 00:00:00 2001 From: Sumit Garg Date: Thu, 2 Feb 2023 13:01:48 +0530 Subject: [PATCH 23/57] arm64: kgdb: Set PSTATE.SS to 1 to re-enable single-step [ Upstream commit af6c0bd59f4f3ad5daad2f7b777954b1954551d5 ] Currently only the first attempt to single-step has any effect. After that all further stepping remains "stuck" at the same program counter value. Refer to the ARM Architecture Reference Manual (ARM DDI 0487E.a) D2.12, PSTATE.SS=1 should be set at each step before transferring the PE to the 'Active-not-pending' state. The problem here is PSTATE.SS=1 is not set since the second single-step. After the first single-step, the PE transferes to the 'Inactive' state, with PSTATE.SS=0 and MDSCR.SS=1, thus PSTATE.SS won't be set to 1 due to kernel_active_single_step()=true. Then the PE transferes to the 'Active-pending' state when ERET and returns to the debugger by step exception. Before this patch: ================== Entering kdb (current=0xffff3376039f0000, pid 1) on processor 0 due to Keyboard Entry [0]kdb> [0]kdb> [0]kdb> bp write_sysrq_trigger Instruction(i) BP #0 at 0xffffa45c13d09290 (write_sysrq_trigger) is enabled addr at ffffa45c13d09290, hardtype=0 installed=0 [0]kdb> go $ echo h > /proc/sysrq-trigger Entering kdb (current=0xffff4f7e453f8000, pid 175) on processor 1 due to Breakpoint @ 0xffffad651a309290 [1]kdb> ss Entering kdb (current=0xffff4f7e453f8000, pid 175) on processor 1 due to SS trap @ 0xffffad651a309294 [1]kdb> ss Entering kdb (current=0xffff4f7e453f8000, pid 175) on processor 1 due to SS trap @ 0xffffad651a309294 [1]kdb> After this patch: ================= Entering kdb (current=0xffff6851c39f0000, pid 1) on processor 0 due to Keyboard Entry [0]kdb> bp write_sysrq_trigger Instruction(i) BP #0 at 0xffffc02d2dd09290 (write_sysrq_trigger) is enabled addr at ffffc02d2dd09290, hardtype=0 installed=0 [0]kdb> go $ echo h > /proc/sysrq-trigger Entering kdb (current=0xffff6851c53c1840, pid 174) on processor 1 due to Breakpoint @ 0xffffc02d2dd09290 [1]kdb> ss Entering kdb (current=0xffff6851c53c1840, pid 174) on processor 1 due to SS trap @ 0xffffc02d2dd09294 [1]kdb> ss Entering kdb (current=0xffff6851c53c1840, pid 174) on processor 1 due to SS trap @ 0xffffc02d2dd09298 [1]kdb> ss Entering kdb (current=0xffff6851c53c1840, pid 174) on processor 1 due to SS trap @ 0xffffc02d2dd0929c [1]kdb> Fixes: 44679a4f142b ("arm64: KGDB: Add step debugging support") Co-developed-by: Wei Li Signed-off-by: Wei Li Signed-off-by: Sumit Garg Tested-by: Douglas Anderson Acked-by: Daniel Thompson Tested-by: Daniel Thompson Link: https://lore.kernel.org/r/20230202073148.657746-3-sumit.garg@linaro.org Signed-off-by: Will Deacon Signed-off-by: Sasha Levin --- arch/arm64/include/asm/debug-monitors.h | 1 + arch/arm64/kernel/debug-monitors.c | 5 +++++ arch/arm64/kernel/kgdb.c | 2 ++ 3 files changed, 8 insertions(+) diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h index 657c921fd784..c16ed5b68768 100644 --- a/arch/arm64/include/asm/debug-monitors.h +++ b/arch/arm64/include/asm/debug-monitors.h @@ -116,6 +116,7 @@ void user_regs_reset_single_step(struct user_pt_regs *regs, void kernel_enable_single_step(struct pt_regs *regs); void kernel_disable_single_step(void); int kernel_active_single_step(void); +void kernel_rewind_single_step(struct pt_regs *regs); #ifdef CONFIG_HAVE_HW_BREAKPOINT int reinstall_suspended_bps(struct pt_regs *regs); diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c index fa76151de6ff..38a0213fdbee 100644 --- a/arch/arm64/kernel/debug-monitors.c +++ b/arch/arm64/kernel/debug-monitors.c @@ -439,6 +439,11 @@ int kernel_active_single_step(void) } NOKPROBE_SYMBOL(kernel_active_single_step); +void kernel_rewind_single_step(struct pt_regs *regs) +{ + set_regs_spsr_ss(regs); +} + /* ptrace API */ void user_enable_single_step(struct task_struct *task) { diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index 1a157ca33262..e4e95821b1f6 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -223,6 +223,8 @@ int kgdb_arch_handle_exception(int exception_vector, int signo, */ if (!kernel_active_single_step()) kernel_enable_single_step(linux_regs); + else + kernel_rewind_single_step(linux_regs); err = 0; break; default: From 9edf5518db252b7ea2e2e87133c891bad5d8955a Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 12 Apr 2023 09:54:39 +0200 Subject: [PATCH 24/57] debugobject: Prevent init race with static objects [ Upstream commit 63a759694eed61025713b3e14dd827c8548daadc ] Statically initialized objects are usually not initialized via the init() function of the subsystem. They are special cased and the subsystem provides a function to validate whether an object which is not yet tracked by debugobjects is statically initialized. This means the object is started to be tracked on first use, e.g. activation. This works perfectly fine, unless there are two concurrent operations on that object. Schspa decoded the problem: T0 T1 debug_object_assert_init(addr) lock_hash_bucket() obj = lookup_object(addr); if (!obj) { unlock_hash_bucket(); - > preemption lock_subsytem_object(addr); activate_object(addr) lock_hash_bucket(); obj = lookup_object(addr); if (!obj) { unlock_hash_bucket(); if (is_static_object(addr)) init_and_track(addr); lock_hash_bucket(); obj = lookup_object(addr); obj->state = ACTIVATED; unlock_hash_bucket(); subsys function modifies content of addr, so static object detection does not longer work. unlock_subsytem_object(addr); if (is_static_object(addr)) <- Fails debugobject emits a warning and invokes the fixup function which reinitializes the already active object in the worst case. This race exists forever, but was never observed until mod_timer() got a debug_object_assert_init() added which is outside of the timer base lock held section right at the beginning of the function to cover the lockless early exit points too. Rework the code so that the lookup, the static object check and the tracking object association happens atomically under the hash bucket lock. This prevents the issue completely as all callers are serialized on the hash bucket lock and therefore cannot observe inconsistent state. Fixes: 3ac7fe5a4aab ("infrastructure to debug (dynamic) objects") Reported-by: syzbot+5093ba19745994288b53@syzkaller.appspotmail.com Debugged-by: Schspa Shi Signed-off-by: Thomas Gleixner Reviewed-by: Stephen Boyd Link: https://syzkaller.appspot.com/bug?id=22c8a5938eab640d1c6bcc0e3dc7be519d878462 Link: https://lore.kernel.org/lkml/20230303161906.831686-1-schspa@gmail.com Link: https://lore.kernel.org/r/87zg7dzgao.ffs@tglx Signed-off-by: Sasha Levin --- lib/debugobjects.c | 125 ++++++++++++++++++++++++--------------------- 1 file changed, 66 insertions(+), 59 deletions(-) diff --git a/lib/debugobjects.c b/lib/debugobjects.c index 71bdc167a9ee..8282ae37db4e 100644 --- a/lib/debugobjects.c +++ b/lib/debugobjects.c @@ -219,10 +219,6 @@ static struct debug_obj *__alloc_object(struct hlist_head *list) return obj; } -/* - * Allocate a new object. If the pool is empty, switch off the debugger. - * Must be called with interrupts disabled. - */ static struct debug_obj * alloc_object(void *addr, struct debug_bucket *b, const struct debug_obj_descr *descr) { @@ -555,11 +551,49 @@ static void debug_object_is_on_stack(void *addr, int onstack) WARN_ON(1); } +static struct debug_obj *lookup_object_or_alloc(void *addr, struct debug_bucket *b, + const struct debug_obj_descr *descr, + bool onstack, bool alloc_ifstatic) +{ + struct debug_obj *obj = lookup_object(addr, b); + enum debug_obj_state state = ODEBUG_STATE_NONE; + + if (likely(obj)) + return obj; + + /* + * debug_object_init() unconditionally allocates untracked + * objects. It does not matter whether it is a static object or + * not. + * + * debug_object_assert_init() and debug_object_activate() allow + * allocation only if the descriptor callback confirms that the + * object is static and considered initialized. For non-static + * objects the allocation needs to be done from the fixup callback. + */ + if (unlikely(alloc_ifstatic)) { + if (!descr->is_static_object || !descr->is_static_object(addr)) + return ERR_PTR(-ENOENT); + /* Statically allocated objects are considered initialized */ + state = ODEBUG_STATE_INIT; + } + + obj = alloc_object(addr, b, descr); + if (likely(obj)) { + obj->state = state; + debug_object_is_on_stack(addr, onstack); + return obj; + } + + /* Out of memory. Do the cleanup outside of the locked region */ + debug_objects_enabled = 0; + return NULL; +} + static void __debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack) { enum debug_obj_state state; - bool check_stack = false; struct debug_bucket *db; struct debug_obj *obj; unsigned long flags; @@ -570,16 +604,11 @@ __debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack raw_spin_lock_irqsave(&db->lock, flags); - obj = lookup_object(addr, db); - if (!obj) { - obj = alloc_object(addr, db, descr); - if (!obj) { - debug_objects_enabled = 0; - raw_spin_unlock_irqrestore(&db->lock, flags); - debug_objects_oom(); - return; - } - check_stack = true; + obj = lookup_object_or_alloc(addr, db, descr, onstack, false); + if (unlikely(!obj)) { + raw_spin_unlock_irqrestore(&db->lock, flags); + debug_objects_oom(); + return; } switch (obj->state) { @@ -605,8 +634,6 @@ __debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack } raw_spin_unlock_irqrestore(&db->lock, flags); - if (check_stack) - debug_object_is_on_stack(addr, onstack); } /** @@ -646,14 +673,12 @@ EXPORT_SYMBOL_GPL(debug_object_init_on_stack); */ int debug_object_activate(void *addr, const struct debug_obj_descr *descr) { + struct debug_obj o = { .object = addr, .state = ODEBUG_STATE_NOTAVAILABLE, .descr = descr }; enum debug_obj_state state; struct debug_bucket *db; struct debug_obj *obj; unsigned long flags; int ret; - struct debug_obj o = { .object = addr, - .state = ODEBUG_STATE_NOTAVAILABLE, - .descr = descr }; if (!debug_objects_enabled) return 0; @@ -662,8 +687,8 @@ int debug_object_activate(void *addr, const struct debug_obj_descr *descr) raw_spin_lock_irqsave(&db->lock, flags); - obj = lookup_object(addr, db); - if (obj) { + obj = lookup_object_or_alloc(addr, db, descr, false, true); + if (likely(!IS_ERR_OR_NULL(obj))) { bool print_object = false; switch (obj->state) { @@ -696,24 +721,16 @@ int debug_object_activate(void *addr, const struct debug_obj_descr *descr) raw_spin_unlock_irqrestore(&db->lock, flags); - /* - * We are here when a static object is activated. We - * let the type specific code confirm whether this is - * true or not. if true, we just make sure that the - * static object is tracked in the object tracker. If - * not, this must be a bug, so we try to fix it up. - */ - if (descr->is_static_object && descr->is_static_object(addr)) { - /* track this static object */ - debug_object_init(addr, descr); - debug_object_activate(addr, descr); - } else { - debug_print_object(&o, "activate"); - ret = debug_object_fixup(descr->fixup_activate, addr, - ODEBUG_STATE_NOTAVAILABLE); - return ret ? 0 : -EINVAL; + /* If NULL the allocation has hit OOM */ + if (!obj) { + debug_objects_oom(); + return 0; } - return 0; + + /* Object is neither static nor tracked. It's not initialized */ + debug_print_object(&o, "activate"); + ret = debug_object_fixup(descr->fixup_activate, addr, ODEBUG_STATE_NOTAVAILABLE); + return ret ? 0 : -EINVAL; } EXPORT_SYMBOL_GPL(debug_object_activate); @@ -867,6 +884,7 @@ EXPORT_SYMBOL_GPL(debug_object_free); */ void debug_object_assert_init(void *addr, const struct debug_obj_descr *descr) { + struct debug_obj o = { .object = addr, .state = ODEBUG_STATE_NOTAVAILABLE, .descr = descr }; struct debug_bucket *db; struct debug_obj *obj; unsigned long flags; @@ -877,31 +895,20 @@ void debug_object_assert_init(void *addr, const struct debug_obj_descr *descr) db = get_bucket((unsigned long) addr); raw_spin_lock_irqsave(&db->lock, flags); + obj = lookup_object_or_alloc(addr, db, descr, false, true); + raw_spin_unlock_irqrestore(&db->lock, flags); + if (likely(!IS_ERR_OR_NULL(obj))) + return; - obj = lookup_object(addr, db); + /* If NULL the allocation has hit OOM */ if (!obj) { - struct debug_obj o = { .object = addr, - .state = ODEBUG_STATE_NOTAVAILABLE, - .descr = descr }; - - raw_spin_unlock_irqrestore(&db->lock, flags); - /* - * Maybe the object is static, and we let the type specific - * code confirm. Track this static object if true, else invoke - * fixup. - */ - if (descr->is_static_object && descr->is_static_object(addr)) { - /* Track this static object */ - debug_object_init(addr, descr); - } else { - debug_print_object(&o, "assert_init"); - debug_object_fixup(descr->fixup_assert_init, addr, - ODEBUG_STATE_NOTAVAILABLE); - } + debug_objects_oom(); return; } - raw_spin_unlock_irqrestore(&db->lock, flags); + /* Object is neither tracked nor static. It's not initialized. */ + debug_print_object(&o, "assert_init"); + debug_object_fixup(descr->fixup_assert_init, addr, ODEBUG_STATE_NOTAVAILABLE); } EXPORT_SYMBOL_GPL(debug_object_assert_init); From 780f303233c35eeb5132e3ee1cbc8f4cebe86dd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Thu, 13 Apr 2023 23:06:02 +0300 Subject: [PATCH 25/57] drm/i915: Make intel_get_crtc_new_encoder() less oopsy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [ Upstream commit 631420b06597a33c72b6dcef78d1c2dea17f452d ] The point of the WARN was to print something, not oops straight up. Currently that is precisely what happens if we can't find the connector for the crtc in the atomic state. Get the dev pointer from the atomic state instead of the potentially NULL encoder to avoid that. Signed-off-by: Ville SyrjĂ€lĂ€ Link: https://patchwork.freedesktop.org/patch/msgid/20230413200602.6037-2-ville.syrjala@linux.intel.com Fixes: 3a47ae201e07 ("drm/i915/display: Make WARN* drm specific where encoder ptr is available") Reviewed-by: Jani Nikula (cherry picked from commit 3b6692357f70498f617ea1b31a0378070a0acf1c) Signed-off-by: Joonas Lahtinen Signed-off-by: Sasha Levin --- drivers/gpu/drm/i915/display/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index d46011f7a838..9a06bd8cb200 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -5844,7 +5844,7 @@ intel_get_crtc_new_encoder(const struct intel_atomic_state *state, num_encoders++; } - drm_WARN(encoder->base.dev, num_encoders != 1, + drm_WARN(state->base.dev, num_encoders != 1, "%d encoders for pipe %c\n", num_encoders, pipe_name(crtc->pipe)); From ca721584e9a4492b92972fda884449eb0e38a312 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 17 Nov 2020 14:19:45 +0100 Subject: [PATCH 26/57] tick/sched: Use tick_next_period for lockless quick check [ Upstream commit 372acbbaa80940189593f9d69c7c069955f24f7a ] No point in doing calculations. tick_next_period = last_jiffies_update + tick_period Just check whether now is before tick_next_period to figure out whether jiffies need an update. Add a comment why the intentional data race in the quick check is safe or not so safe in a 32bit corner case and why we don't worry about it. Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201117132006.337366695@linutronix.de Stable-dep-of: e9523a0d8189 ("tick/common: Align tick period with the HZ tick.") Signed-off-by: Sasha Levin --- kernel/time/tick-sched.c | 46 ++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 378096fc8c56..4f1b6170b3a2 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -57,11 +57,29 @@ static void tick_do_update_jiffies64(ktime_t now) ktime_t delta; /* - * Do a quick check without holding jiffies_lock: - * The READ_ONCE() pairs with two updates done later in this function. + * Do a quick check without holding jiffies_lock. The READ_ONCE() + * pairs with the update done later in this function. + * + * This is also an intentional data race which is even safe on + * 32bit in theory. If there is a concurrent update then the check + * might give a random answer. It does not matter because if it + * returns then the concurrent update is already taking care, if it + * falls through then it will pointlessly contend on jiffies_lock. + * + * Though there is one nasty case on 32bit due to store tearing of + * the 64bit value. If the first 32bit store makes the quick check + * return on all other CPUs and the writing CPU context gets + * delayed to complete the second store (scheduled out on virt) + * then jiffies can become stale for up to ~2^32 nanoseconds + * without noticing. After that point all CPUs will wait for + * jiffies lock. + * + * OTOH, this is not any different than the situation with NOHZ=off + * where one CPU is responsible for updating jiffies and + * timekeeping. If that CPU goes out for lunch then all other CPUs + * will operate on stale jiffies until it decides to come back. */ - delta = ktime_sub(now, READ_ONCE(last_jiffies_update)); - if (delta < tick_period) + if (ktime_before(now, READ_ONCE(tick_next_period))) return; /* Reevaluate with jiffies_lock held */ @@ -72,9 +90,8 @@ static void tick_do_update_jiffies64(ktime_t now) if (delta >= tick_period) { delta = ktime_sub(delta, tick_period); - /* Pairs with the lockless read in this function. */ - WRITE_ONCE(last_jiffies_update, - ktime_add(last_jiffies_update, tick_period)); + last_jiffies_update = ktime_add(last_jiffies_update, + tick_period); /* Slow path for long timeouts */ if (unlikely(delta >= tick_period)) { @@ -82,15 +99,18 @@ static void tick_do_update_jiffies64(ktime_t now) ticks = ktime_divns(delta, incr); - /* Pairs with the lockless read in this function. */ - WRITE_ONCE(last_jiffies_update, - ktime_add_ns(last_jiffies_update, - incr * ticks)); + last_jiffies_update = ktime_add_ns(last_jiffies_update, + incr * ticks); } do_timer(++ticks); - /* Keep the tick_next_period variable up to date */ - tick_next_period = ktime_add(last_jiffies_update, tick_period); + /* + * Keep the tick_next_period variable up to date. + * WRITE_ONCE() pairs with the READ_ONCE() in the lockless + * quick check above. + */ + WRITE_ONCE(tick_next_period, + ktime_add(last_jiffies_update, tick_period)); } else { write_seqcount_end(&jiffies_seq); raw_spin_unlock(&jiffies_lock); From 93c43008368d60f2cb11cc239a3219a2f4142c30 Mon Sep 17 00:00:00 2001 From: Yunfeng Ye Date: Tue, 17 Nov 2020 14:19:46 +0100 Subject: [PATCH 27/57] tick/sched: Reduce seqcount held scope in tick_do_update_jiffies64() [ Upstream commit 94ad2e3cedb82af034f6d97c58022f162b669f9b ] If jiffies are up to date already (caller lost the race against another CPU) there is no point to change the sequence count. Doing that just forces other CPUs into the seqcount retry loop in tick_nohz_next_event() for nothing. Just bail out early. [ tglx: Rewrote most of it ] Signed-off-by: Yunfeng Ye Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201117132006.462195901@linutronix.de Stable-dep-of: e9523a0d8189 ("tick/common: Align tick period with the HZ tick.") Signed-off-by: Sasha Levin --- kernel/time/tick-sched.c | 55 +++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 4f1b6170b3a2..ac9953f6f92c 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -84,38 +84,35 @@ static void tick_do_update_jiffies64(ktime_t now) /* Reevaluate with jiffies_lock held */ raw_spin_lock(&jiffies_lock); - write_seqcount_begin(&jiffies_seq); - - delta = ktime_sub(now, last_jiffies_update); - if (delta >= tick_period) { - - delta = ktime_sub(delta, tick_period); - last_jiffies_update = ktime_add(last_jiffies_update, - tick_period); - - /* Slow path for long timeouts */ - if (unlikely(delta >= tick_period)) { - s64 incr = ktime_to_ns(tick_period); - - ticks = ktime_divns(delta, incr); - - last_jiffies_update = ktime_add_ns(last_jiffies_update, - incr * ticks); - } - do_timer(++ticks); - - /* - * Keep the tick_next_period variable up to date. - * WRITE_ONCE() pairs with the READ_ONCE() in the lockless - * quick check above. - */ - WRITE_ONCE(tick_next_period, - ktime_add(last_jiffies_update, tick_period)); - } else { - write_seqcount_end(&jiffies_seq); + if (ktime_before(now, tick_next_period)) { raw_spin_unlock(&jiffies_lock); return; } + + write_seqcount_begin(&jiffies_seq); + + last_jiffies_update = ktime_add(last_jiffies_update, tick_period); + + delta = ktime_sub(now, tick_next_period); + if (unlikely(delta >= tick_period)) { + /* Slow path for long idle sleep times */ + s64 incr = ktime_to_ns(tick_period); + + ticks = ktime_divns(delta, incr); + + last_jiffies_update = ktime_add_ns(last_jiffies_update, + incr * ticks); + } + + do_timer(++ticks); + + /* + * Keep the tick_next_period variable up to date. WRITE_ONCE() + * pairs with the READ_ONCE() in the lockless quick check above. + */ + WRITE_ONCE(tick_next_period, + ktime_add(last_jiffies_update, tick_period)); + write_seqcount_end(&jiffies_seq); raw_spin_unlock(&jiffies_lock); update_wall_time(); From fdc48767461a3ceeb1b6d587740d1430f7eb9c6f Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 17 Nov 2020 14:19:47 +0100 Subject: [PATCH 28/57] tick/sched: Optimize tick_do_update_jiffies64() further [ Upstream commit 7a35bf2a6a871cd0252cd371d741e7d070b53af9 ] Now that it's clear that there is always one tick to account, simplify the calculations some more. Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201117132006.565663056@linutronix.de Stable-dep-of: e9523a0d8189 ("tick/common: Align tick period with the HZ tick.") Signed-off-by: Sasha Levin --- kernel/time/tick-sched.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index ac9953f6f92c..5c3d4355266d 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -53,7 +53,7 @@ static ktime_t last_jiffies_update; */ static void tick_do_update_jiffies64(ktime_t now) { - unsigned long ticks = 0; + unsigned long ticks = 1; ktime_t delta; /* @@ -91,20 +91,21 @@ static void tick_do_update_jiffies64(ktime_t now) write_seqcount_begin(&jiffies_seq); - last_jiffies_update = ktime_add(last_jiffies_update, tick_period); - delta = ktime_sub(now, tick_next_period); if (unlikely(delta >= tick_period)) { /* Slow path for long idle sleep times */ s64 incr = ktime_to_ns(tick_period); - ticks = ktime_divns(delta, incr); + ticks += ktime_divns(delta, incr); last_jiffies_update = ktime_add_ns(last_jiffies_update, incr * ticks); + } else { + last_jiffies_update = ktime_add(last_jiffies_update, + tick_period); } - do_timer(++ticks); + do_timer(ticks); /* * Keep the tick_next_period variable up to date. WRITE_ONCE() From 107ea1f63b266afcf50b7a9496d04797d149c0cf Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 17 Nov 2020 14:19:49 +0100 Subject: [PATCH 29/57] tick: Get rid of tick_period [ Upstream commit b996544916429946bf4934c1c01a306d1690972c ] The variable tick_period is initialized to NSEC_PER_TICK / HZ during boot and never updated again. If NSEC_PER_TICK is not an integer multiple of HZ this computation is less accurate than TICK_NSEC which has proper rounding in place. Aside of the inaccuracy there is no reason for having this variable at all. It's just a pointless indirection and all usage sites can just use the TICK_NSEC constant. Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201117132006.766643526@linutronix.de Stable-dep-of: e9523a0d8189 ("tick/common: Align tick period with the HZ tick.") Signed-off-by: Sasha Levin --- kernel/time/tick-broadcast.c | 2 +- kernel/time/tick-common.c | 8 +++----- kernel/time/tick-internal.h | 1 - kernel/time/tick-sched.c | 22 +++++++++++----------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c index 36d7464c8962..a9530e866e5f 100644 --- a/kernel/time/tick-broadcast.c +++ b/kernel/time/tick-broadcast.c @@ -331,7 +331,7 @@ static void tick_handle_periodic_broadcast(struct clock_event_device *dev) bc_local = tick_do_periodic_broadcast(); if (clockevent_state_oneshot(dev)) { - ktime_t next = ktime_add(dev->next_event, tick_period); + ktime_t next = ktime_add_ns(dev->next_event, TICK_NSEC); clockevents_program_event(dev, next, true); } diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c index 6c9c342dd0e5..92bf99d558b4 100644 --- a/kernel/time/tick-common.c +++ b/kernel/time/tick-common.c @@ -30,7 +30,6 @@ DEFINE_PER_CPU(struct tick_device, tick_cpu_device); * Tick next event: keeps track of the tick time */ ktime_t tick_next_period; -ktime_t tick_period; /* * tick_do_timer_cpu is a timer core internal variable which holds the CPU NR @@ -88,7 +87,7 @@ static void tick_periodic(int cpu) write_seqcount_begin(&jiffies_seq); /* Keep track of the next tick event */ - tick_next_period = ktime_add(tick_next_period, tick_period); + tick_next_period = ktime_add_ns(tick_next_period, TICK_NSEC); do_timer(1); write_seqcount_end(&jiffies_seq); @@ -127,7 +126,7 @@ void tick_handle_periodic(struct clock_event_device *dev) * Setup the next period for devices, which do not have * periodic mode: */ - next = ktime_add(next, tick_period); + next = ktime_add_ns(next, TICK_NSEC); if (!clockevents_program_event(dev, next, false)) return; @@ -173,7 +172,7 @@ void tick_setup_periodic(struct clock_event_device *dev, int broadcast) for (;;) { if (!clockevents_program_event(dev, next, false)) return; - next = ktime_add(next, tick_period); + next = ktime_add_ns(next, TICK_NSEC); } } } @@ -220,7 +219,6 @@ static void tick_setup_device(struct tick_device *td, tick_do_timer_cpu = cpu; tick_next_period = ktime_get(); - tick_period = NSEC_PER_SEC / HZ; #ifdef CONFIG_NO_HZ_FULL /* * The boot CPU may be nohz_full, in which case set diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h index 5294f5b1f955..e61c1244e7d4 100644 --- a/kernel/time/tick-internal.h +++ b/kernel/time/tick-internal.h @@ -15,7 +15,6 @@ DECLARE_PER_CPU(struct tick_device, tick_cpu_device); extern ktime_t tick_next_period; -extern ktime_t tick_period; extern int tick_do_timer_cpu __read_mostly; extern void tick_setup_periodic(struct clock_event_device *dev, int broadcast); diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 5c3d4355266d..17dc3f53efef 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -92,17 +92,17 @@ static void tick_do_update_jiffies64(ktime_t now) write_seqcount_begin(&jiffies_seq); delta = ktime_sub(now, tick_next_period); - if (unlikely(delta >= tick_period)) { + if (unlikely(delta >= TICK_NSEC)) { /* Slow path for long idle sleep times */ - s64 incr = ktime_to_ns(tick_period); + s64 incr = TICK_NSEC; ticks += ktime_divns(delta, incr); last_jiffies_update = ktime_add_ns(last_jiffies_update, incr * ticks); } else { - last_jiffies_update = ktime_add(last_jiffies_update, - tick_period); + last_jiffies_update = ktime_add_ns(last_jiffies_update, + TICK_NSEC); } do_timer(ticks); @@ -112,7 +112,7 @@ static void tick_do_update_jiffies64(ktime_t now) * pairs with the READ_ONCE() in the lockless quick check above. */ WRITE_ONCE(tick_next_period, - ktime_add(last_jiffies_update, tick_period)); + ktime_add_ns(last_jiffies_update, TICK_NSEC)); write_seqcount_end(&jiffies_seq); raw_spin_unlock(&jiffies_lock); @@ -688,7 +688,7 @@ static void tick_nohz_restart(struct tick_sched *ts, ktime_t now) hrtimer_set_expires(&ts->sched_timer, ts->last_tick); /* Forward the time to expire in the future */ - hrtimer_forward(&ts->sched_timer, now, tick_period); + hrtimer_forward(&ts->sched_timer, now, TICK_NSEC); if (ts->nohz_mode == NOHZ_MODE_HIGHRES) { hrtimer_start_expires(&ts->sched_timer, @@ -1250,7 +1250,7 @@ static void tick_nohz_handler(struct clock_event_device *dev) if (unlikely(ts->tick_stopped)) return; - hrtimer_forward(&ts->sched_timer, now, tick_period); + hrtimer_forward(&ts->sched_timer, now, TICK_NSEC); tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1); } @@ -1287,7 +1287,7 @@ static void tick_nohz_switch_to_nohz(void) next = tick_init_jiffy_update(); hrtimer_set_expires(&ts->sched_timer, next); - hrtimer_forward_now(&ts->sched_timer, tick_period); + hrtimer_forward_now(&ts->sched_timer, TICK_NSEC); tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1); tick_nohz_activate(ts, NOHZ_MODE_LOWRES); } @@ -1353,7 +1353,7 @@ static enum hrtimer_restart tick_sched_timer(struct hrtimer *timer) if (unlikely(ts->tick_stopped)) return HRTIMER_NORESTART; - hrtimer_forward(timer, now, tick_period); + hrtimer_forward(timer, now, TICK_NSEC); return HRTIMER_RESTART; } @@ -1387,13 +1387,13 @@ void tick_setup_sched_timer(void) /* Offset the tick to avert jiffies_lock contention. */ if (sched_skew_tick) { - u64 offset = ktime_to_ns(tick_period) >> 1; + u64 offset = TICK_NSEC >> 1; do_div(offset, num_possible_cpus()); offset *= smp_processor_id(); hrtimer_add_expires_ns(&ts->sched_timer, offset); } - hrtimer_forward(&ts->sched_timer, now, tick_period); + hrtimer_forward(&ts->sched_timer, now, TICK_NSEC); hrtimer_start_expires(&ts->sched_timer, HRTIMER_MODE_ABS_PINNED_HARD); tick_nohz_activate(ts, NOHZ_MODE_HIGHRES); } From c4013689269df584efe9e42f7a8e42c871fc32db Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Tue, 18 Apr 2023 14:26:39 +0200 Subject: [PATCH 30/57] tick/common: Align tick period with the HZ tick. [ Upstream commit e9523a0d81899361214d118ad60ef76f0e92f71d ] With HIGHRES enabled tick_sched_timer() is programmed every jiffy to expire the timer_list timers. This timer is programmed accurate in respect to CLOCK_MONOTONIC so that 0 seconds and nanoseconds is the first tick and the next one is 1000/CONFIG_HZ ms later. For HZ=250 it is every 4 ms and so based on the current time the next tick can be computed. This accuracy broke since the commit mentioned below because the jiffy based clocksource is initialized with higher accuracy in read_persistent_wall_and_boot_offset(). This higher accuracy is inherited during the setup in tick_setup_device(). The timer still fires every 4ms with HZ=250 but timer is no longer aligned with CLOCK_MONOTONIC with 0 as it origin but has an offset in the us/ns part of the timestamp. The offset differs with every boot and makes it impossible for user land to align with the tick. Align the tick period with CLOCK_MONOTONIC ensuring that it is always a multiple of 1000/CONFIG_HZ ms. Fixes: 857baa87b6422 ("sched/clock: Enable sched clock early") Reported-by: Gusenleitner Klaus Signed-off-by: Sebastian Andrzej Siewior Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/20230406095735.0_14edn3@linutronix.de Link: https://lore.kernel.org/r/20230418122639.ikgfvu3f@linutronix.de Signed-off-by: Sasha Levin --- kernel/time/tick-common.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c index 92bf99d558b4..2b7448ae5b47 100644 --- a/kernel/time/tick-common.c +++ b/kernel/time/tick-common.c @@ -216,9 +216,19 @@ static void tick_setup_device(struct tick_device *td, * this cpu: */ if (tick_do_timer_cpu == TICK_DO_TIMER_BOOT) { + ktime_t next_p; + u32 rem; + tick_do_timer_cpu = cpu; - tick_next_period = ktime_get(); + next_p = ktime_get(); + div_u64_rem(next_p, TICK_NSEC, &rem); + if (rem) { + next_p -= rem; + next_p += TICK_NSEC; + } + + tick_next_period = next_p; #ifdef CONFIG_NO_HZ_FULL /* * The boot CPU may be nohz_full, in which case set From 129c3fb5795d062eb962453a84a84d6365194c48 Mon Sep 17 00:00:00 2001 From: "Alexey V. Vissarionov" Date: Wed, 15 Feb 2023 20:31:37 +0200 Subject: [PATCH 31/57] wifi: ath6kl: minor fix for allocation size [ Upstream commit 778f83f889e7fca37780d9640fcbd0229ae38eaa ] Although the "param" pointer occupies more or equal space compared to "*param", the allocation size should use the size of variable itself. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: bdcd81707973cf8a ("Add ath6kl cleaned up driver") Signed-off-by: Alexey V. Vissarionov Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/20230117110414.GC12547@altlinux.org Signed-off-by: Sasha Levin --- drivers/net/wireless/ath/ath6kl/bmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath6kl/bmi.c b/drivers/net/wireless/ath/ath6kl/bmi.c index bde5a10d470c..af98e871199d 100644 --- a/drivers/net/wireless/ath/ath6kl/bmi.c +++ b/drivers/net/wireless/ath/ath6kl/bmi.c @@ -246,7 +246,7 @@ int ath6kl_bmi_execute(struct ath6kl *ar, u32 addr, u32 *param) return -EACCES; } - size = sizeof(cid) + sizeof(addr) + sizeof(param); + size = sizeof(cid) + sizeof(addr) + sizeof(*param); if (size > ar->bmi.max_cmd_size) { WARN_ON(1); return -EINVAL; From 320d760a35273aa815d58b57e4fd9ba5279a3489 Mon Sep 17 00:00:00 2001 From: Fedor Pchelkin Date: Thu, 16 Feb 2023 22:23:01 +0300 Subject: [PATCH 32/57] wifi: ath9k: hif_usb: fix memory leak of remain_skbs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [ Upstream commit 7654cc03eb699297130b693ec34e25f77b17c947 ] hif_dev->remain_skb is allocated and used exclusively in ath9k_hif_usb_rx_stream(). It is implied that an allocated remain_skb is processed and subsequently freed (in error paths) only during the next call of ath9k_hif_usb_rx_stream(). So, if the urbs are deallocated between those two calls due to the device deinitialization or suspend, it is possible that ath9k_hif_usb_rx_stream() is not called next time and the allocated remain_skb is leaked. Our local Syzkaller instance was able to trigger that. remain_skb makes sense when receiving two consecutive urbs which are logically linked together, i.e. a specific data field from the first skb indicates a cached skb to be allocated, memcpy'd with some data and subsequently processed in the next call to ath9k_hif_usb_rx_stream(). Urbs deallocation supposedly makes that link irrelevant so we need to free the cached skb in those cases. Fix the leak by introducing a function to explicitly free remain_skb (if it is not NULL) when the rx urbs have been deallocated. remain_skb is NULL when it has not been allocated at all (hif_dev struct is kzalloced) or when it has been processed in next call to ath9k_hif_usb_rx_stream(). Found by Linux Verification Center (linuxtesting.org) with Syzkaller. Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.") Signed-off-by: Fedor Pchelkin Signed-off-by: Alexey Khoroshilov Acked-by: Toke HĂžiland-JĂžrgensen Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/20230216192301.171225-1-pchelkin@ispras.ru Signed-off-by: Sasha Levin --- drivers/net/wireless/ath/ath9k/hif_usb.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c index f521dfa2f194..e0130beb304d 100644 --- a/drivers/net/wireless/ath/ath9k/hif_usb.c +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c @@ -534,6 +534,24 @@ static struct ath9k_htc_hif hif_usb = { .send = hif_usb_send, }; +/* Need to free remain_skb allocated in ath9k_hif_usb_rx_stream + * in case ath9k_hif_usb_rx_stream wasn't called next time to + * process the buffer and subsequently free it. + */ +static void ath9k_hif_usb_free_rx_remain_skb(struct hif_device_usb *hif_dev) +{ + unsigned long flags; + + spin_lock_irqsave(&hif_dev->rx_lock, flags); + if (hif_dev->remain_skb) { + dev_kfree_skb_any(hif_dev->remain_skb); + hif_dev->remain_skb = NULL; + hif_dev->rx_remain_len = 0; + RX_STAT_INC(hif_dev, skb_dropped); + } + spin_unlock_irqrestore(&hif_dev->rx_lock, flags); +} + static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev, struct sk_buff *skb) { @@ -868,6 +886,7 @@ err: static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb *hif_dev) { usb_kill_anchored_urbs(&hif_dev->rx_submitted); + ath9k_hif_usb_free_rx_remain_skb(hif_dev); } static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev) From 6c91b3b57b1f5aed73053e8992a6badc907218d3 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 6 Feb 2023 16:15:48 +0300 Subject: [PATCH 33/57] wifi: ath5k: fix an off by one check in ath5k_eeprom_read_freq_list() [ Upstream commit 4c856ee12df85aabd437c3836ed9f68d94268358 ] This loop checks that i < max at the start of loop but then it does i++ which could put it past the end of the array. It's harmless to check again and prevent a potential out of bounds. Fixes: 1048643ea94d ("ath5k: Clean up eeprom parsing and add missing calibration data") Signed-off-by: Dan Carpenter Reviewed-by: Luis Chamberlain Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/Y+D9hPQrHfWBJhXz@kili Signed-off-by: Sasha Levin --- drivers/net/wireless/ath/ath5k/eeprom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath5k/eeprom.c b/drivers/net/wireless/ath/ath5k/eeprom.c index d444b3d70ba2..58d3e86f6256 100644 --- a/drivers/net/wireless/ath/ath5k/eeprom.c +++ b/drivers/net/wireless/ath/ath5k/eeprom.c @@ -529,7 +529,7 @@ ath5k_eeprom_read_freq_list(struct ath5k_hw *ah, int *offset, int max, ee->ee_n_piers[mode]++; freq2 = (val >> 8) & 0xff; - if (!freq2) + if (!freq2 || i >= max) break; pc[i++].freq = ath5k_eeprom_bin2freq(ee, From 1300517e371e4d0acdb0f1237477e1ed223c3a9a Mon Sep 17 00:00:00 2001 From: Fedor Pchelkin Date: Fri, 24 Feb 2023 12:28:05 +0200 Subject: [PATCH 34/57] wifi: ath6kl: reduce WARN to dev_dbg() in callback [ Upstream commit 75c4a8154cb6c7239fb55d5550f481f6765fb83c ] The warn is triggered on a known race condition, documented in the code above the test, that is correctly handled. Using WARN() hinders automated testing. Reducing severity. Fixes: de2070fc4aa7 ("ath6kl: Fix kernel panic on continuous driver load/unload") Reported-and-tested-by: syzbot+555908813b2ea35dae9a@syzkaller.appspotmail.com Signed-off-by: Oliver Neukum Signed-off-by: Fedor Pchelkin Signed-off-by: Alexey Khoroshilov Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/20230126182431.867984-1-pchelkin@ispras.ru Signed-off-by: Sasha Levin --- drivers/net/wireless/ath/ath6kl/htc_pipe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath6kl/htc_pipe.c b/drivers/net/wireless/ath/ath6kl/htc_pipe.c index c68848819a52..9b88d96bfe96 100644 --- a/drivers/net/wireless/ath/ath6kl/htc_pipe.c +++ b/drivers/net/wireless/ath/ath6kl/htc_pipe.c @@ -960,8 +960,8 @@ static int ath6kl_htc_pipe_rx_complete(struct ath6kl *ar, struct sk_buff *skb, * Thus the possibility of ar->htc_target being NULL * via ath6kl_recv_complete -> ath6kl_usb_io_comp_work. */ - if (WARN_ON_ONCE(!target)) { - ath6kl_err("Target not yet initialized\n"); + if (!target) { + ath6kl_dbg(ATH6KL_DBG_HTC, "Target not yet initialized\n"); status = -EINVAL; goto free_skb; } From 3ab6ec6c485b6a0b8811faa5e6166d67584001cc Mon Sep 17 00:00:00 2001 From: Luis Gerhorst Date: Mon, 27 Feb 2023 16:08:54 +0100 Subject: [PATCH 35/57] tools: bpftool: Remove invalid \' json escape [ Upstream commit c679bbd611c08b0559ffae079330bc4e5574696a ] RFC8259 ("The JavaScript Object Notation (JSON) Data Interchange Format") only specifies \", \\, \/, \b, \f, \n, \r, and \r as valid two-character escape sequences. This does not include \', which is not required in JSON because it exclusively uses double quotes as string separators. Solidus (/) may be escaped, but does not have to. Only reverse solidus (\), double quotes ("), and the control characters have to be escaped. Therefore, with this fix, bpftool correctly supports all valid two-character escape sequences (but still does not support characters that require multi-character escape sequences). Witout this fix, attempting to load a JSON file generated by bpftool using Python 3.10.6's default json.load() may fail with the error "Invalid \escape" if the file contains the invalid escaped single quote (\'). Fixes: b66e907cfee2 ("tools: bpftool: copy JSON writer from iproute2 repository") Signed-off-by: Luis Gerhorst Signed-off-by: Andrii Nakryiko Reviewed-by: Quentin Monnet Link: https://lore.kernel.org/bpf/20230227150853.16863-1-gerhorst@cs.fau.de Signed-off-by: Sasha Levin --- tools/bpf/bpftool/json_writer.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/bpf/bpftool/json_writer.c b/tools/bpf/bpftool/json_writer.c index 7fea83bedf48..bca5dd0a59e3 100644 --- a/tools/bpf/bpftool/json_writer.c +++ b/tools/bpf/bpftool/json_writer.c @@ -80,9 +80,6 @@ static void jsonw_puts(json_writer_t *self, const char *str) case '"': fputs("\\\"", self->out); break; - case '\'': - fputs("\\\'", self->out); - break; default: putc(*str, self->out); } From 1980dd8c53ec6383358645073463e7a155f16f0a Mon Sep 17 00:00:00 2001 From: Martin Blumenstingl Date: Sun, 26 Feb 2023 23:10:03 +0100 Subject: [PATCH 36/57] wifi: rtw88: mac: Return the original error from rtw_pwr_seq_parser() [ Upstream commit b7ed9fa2cb76ca7a3c3cd4a6d35748fe1fbda9f6 ] rtw_pwr_seq_parser() calls rtw_sub_pwr_seq_parser() which can either return -EBUSY, -EINVAL or 0. Propagate the original error code instead of unconditionally returning -EBUSY in case of an error. Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver") Signed-off-by: Martin Blumenstingl Reviewed-by: Ping-Ke Shih Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/20230226221004.138331-2-martin.blumenstingl@googlemail.com Signed-off-by: Sasha Levin --- drivers/net/wireless/realtek/rtw88/mac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c index 59028b121b00..fd427c606fa2 100644 --- a/drivers/net/wireless/realtek/rtw88/mac.c +++ b/drivers/net/wireless/realtek/rtw88/mac.c @@ -233,7 +233,7 @@ static int rtw_pwr_seq_parser(struct rtw_dev *rtwdev, ret = rtw_sub_pwr_seq_parser(rtwdev, intf_mask, cut_mask, cmd); if (ret) - return -EBUSY; + return ret; idx++; } while (1); From 243fab8e37d4aa3ac91f05bd157819f97ab38d88 Mon Sep 17 00:00:00 2001 From: Martin Blumenstingl Date: Sun, 26 Feb 2023 23:10:04 +0100 Subject: [PATCH 37/57] wifi: rtw88: mac: Return the original error from rtw_mac_power_switch() [ Upstream commit 15c8e267dfa62f207ee1db666c822324e3362b84 ] rtw_mac_power_switch() calls rtw_pwr_seq_parser() which can return -EINVAL, -EBUSY or 0. Propagate the original error code instead of unconditionally returning -EINVAL in case of an error. Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver") Signed-off-by: Martin Blumenstingl Reviewed-by: Ping-Ke Shih Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/20230226221004.138331-3-martin.blumenstingl@googlemail.com Signed-off-by: Sasha Levin --- drivers/net/wireless/realtek/rtw88/mac.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c index fd427c606fa2..a6d554a9dd99 100644 --- a/drivers/net/wireless/realtek/rtw88/mac.c +++ b/drivers/net/wireless/realtek/rtw88/mac.c @@ -247,6 +247,7 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on) const struct rtw_pwr_seq_cmd **pwr_seq; u8 rpwm; bool cur_pwr; + int ret; if (rtw_chip_wcpu_11ac(rtwdev)) { rpwm = rtw_read8(rtwdev, rtwdev->hci.rpwm_addr); @@ -270,8 +271,9 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on) return -EALREADY; pwr_seq = pwr_on ? chip->pwr_on_seq : chip->pwr_off_seq; - if (rtw_pwr_seq_parser(rtwdev, pwr_seq)) - return -EINVAL; + ret = rtw_pwr_seq_parser(rtwdev, pwr_seq); + if (ret) + return ret; return 0; } From 6be8ad4cdcac2fda572dd3b9f2ea064ad8b8a679 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Thu, 9 Mar 2023 14:41:31 -0800 Subject: [PATCH 38/57] bpf: take into account liveness when propagating precision [ Upstream commit 52c2b005a3c18c565fc70cfd0ca49375f301e952 ] When doing state comparison, if old state has register that is not marked as REG_LIVE_READ, then we just skip comparison, regardless what's the state of corresponing register in current state. This is because not REG_LIVE_READ register is irrelevant for further program execution and correctness. All good here. But when we get to precision propagation, after two states were declared equivalent, we don't take into account old register's liveness, and thus attempt to propagate precision for register in current state even if that register in old state was not REG_LIVE_READ anymore. This is bad, because register in current state could be anything at all and this could cause -EFAULT due to internal logic bugs. Fix by taking into account REG_LIVE_READ liveness mark to keep the logic in state comparison in sync with precision propagation. Fixes: a3ce685dd01a ("bpf: fix precision tracking") Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/r/20230309224131.57449-1-andrii@kernel.org Signed-off-by: Alexei Starovoitov Signed-off-by: Sasha Levin --- kernel/bpf/verifier.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 5a96a9dd51e4..e2488a00efc5 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9550,7 +9550,8 @@ static int propagate_precision(struct bpf_verifier_env *env, state_reg = state->regs; for (i = 0; i < BPF_REG_FP; i++, state_reg++) { if (state_reg->type != SCALAR_VALUE || - !state_reg->precise) + !state_reg->precise || + !(state_reg->live & REG_LIVE_READ)) continue; if (env->log.level & BPF_LOG_LEVEL2) verbose(env, "frame %d: propagating r%d\n", i, fr); @@ -9564,7 +9565,8 @@ static int propagate_precision(struct bpf_verifier_env *env, continue; state_reg = &state->stack[i].spilled_ptr; if (state_reg->type != SCALAR_VALUE || - !state_reg->precise) + !state_reg->precise || + !(state_reg->live & REG_LIVE_READ)) continue; if (env->log.level & BPF_LOG_LEVEL2) verbose(env, "frame %d: propagating fp%d\n", From 10702be8b3732b31ce4d584a23fefbf1a6f8a0f7 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Mon, 13 Mar 2023 11:40:17 -0700 Subject: [PATCH 39/57] bpf: fix precision propagation verbose logging [ Upstream commit 34f0677e7afd3a292bc1aadda7ce8e35faedb204 ] Fix wrong order of frame index vs register/slot index in precision propagation verbose (level 2) output. It's wrong and very confusing as is. Fixes: 529409ea92d5 ("bpf: propagate precision across all frames, not just the last one") Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/r/20230313184017.4083374-1-andrii@kernel.org Signed-off-by: Alexei Starovoitov Signed-off-by: Sasha Levin --- kernel/bpf/verifier.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index e2488a00efc5..232f720104cd 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9554,7 +9554,7 @@ static int propagate_precision(struct bpf_verifier_env *env, !(state_reg->live & REG_LIVE_READ)) continue; if (env->log.level & BPF_LOG_LEVEL2) - verbose(env, "frame %d: propagating r%d\n", i, fr); + verbose(env, "frame %d: propagating r%d\n", fr, i); err = mark_chain_precision_frame(env, fr, i); if (err < 0) return err; @@ -9570,7 +9570,7 @@ static int propagate_precision(struct bpf_verifier_env *env, continue; if (env->log.level & BPF_LOG_LEVEL2) verbose(env, "frame %d: propagating fp%d\n", - (-i - 1) * BPF_REG_SIZE, fr); + fr, (-i - 1) * BPF_REG_SIZE); err = mark_chain_precision_stack_frame(env, fr, i); if (err < 0) return err; From 10c1051267879ddf3e4aff1d0bd7d2debf28d85a Mon Sep 17 00:00:00 2001 From: Alexander Mikhalitsyn Date: Mon, 13 Mar 2023 12:32:11 +0100 Subject: [PATCH 40/57] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC [ Upstream commit a02d83f9947d8f71904eda4de046630c3eb6802c ] Currently, kernel would set MSG_CTRUNC flag if msg_control buffer wasn't provided and SO_PASSCRED was set or if there was pending SCM_RIGHTS. For some reason we have no corresponding check for SO_PASSSEC. In the recvmsg(2) doc we have: MSG_CTRUNC indicates that some control data was discarded due to lack of space in the buffer for ancillary data. So, we need to set MSG_CTRUNC flag for all types of SCM. This change can break applications those don't check MSG_CTRUNC flag. Cc: "David S. Miller" Cc: Eric Dumazet Cc: Jakub Kicinski Cc: Paolo Abeni Cc: Leon Romanovsky Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Alexander Mikhalitsyn v2: - commit message was rewritten according to Eric's suggestion Acked-by: Paul Moore Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- include/net/scm.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/include/net/scm.h b/include/net/scm.h index 1ce365f4c256..585adc1346bd 100644 --- a/include/net/scm.h +++ b/include/net/scm.h @@ -105,16 +105,27 @@ static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct sc } } } + +static inline bool scm_has_secdata(struct socket *sock) +{ + return test_bit(SOCK_PASSSEC, &sock->flags); +} #else static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm) { } + +static inline bool scm_has_secdata(struct socket *sock) +{ + return false; +} #endif /* CONFIG_SECURITY_NETWORK */ static __inline__ void scm_recv(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm, int flags) { if (!msg->msg_control) { - if (test_bit(SOCK_PASSCRED, &sock->flags) || scm->fp) + if (test_bit(SOCK_PASSCRED, &sock->flags) || scm->fp || + scm_has_secdata(sock)) msg->msg_flags |= MSG_CTRUNC; scm_destroy(scm); return; From 118df5df1b4b046870f166b4b88ee59b94985c93 Mon Sep 17 00:00:00 2001 From: Luis Gerhorst Date: Wed, 15 Mar 2023 17:54:00 +0100 Subject: [PATCH 41/57] bpf: Remove misleading spec_v1 check on var-offset stack read [ Upstream commit 082cdc69a4651dd2a77539d69416a359ed1214f5 ] For every BPF_ADD/SUB involving a pointer, adjust_ptr_min_max_vals() ensures that the resulting pointer has a constant offset if bypass_spec_v1 is false. This is ensured by calling sanitize_check_bounds() which in turn calls check_stack_access_for_ptr_arithmetic(). There, -EACCESS is returned if the register's offset is not constant, thereby rejecting the program. In summary, an unprivileged user must never be able to create stack pointers with a variable offset. That is also the case, because a respective check in check_stack_write() is missing. If they were able to create a variable-offset pointer, users could still use it in a stack-write operation to trigger unsafe speculative behavior [1]. Because unprivileged users must already be prevented from creating variable-offset stack pointers, viable options are to either remove this check (replacing it with a clarifying comment), or to turn it into a "verifier BUG"-message, also adding a similar check in check_stack_write() (for consistency, as a second-level defense). This patch implements the first option to reduce verifier bloat. This check was introduced by commit 01f810ace9ed ("bpf: Allow variable-offset stack access") which correctly notes that "variable-offset reads and writes are disallowed (they were already disallowed for the indirect access case) because the speculative execution checking code doesn't support them". However, it does not further discuss why the check in check_stack_read() is necessary. The code which made this check obsolete was also introduced in this commit. I have compiled ~650 programs from the Linux selftests, Linux samples, Cilium, and libbpf/examples projects and confirmed that none of these trigger the check in check_stack_read() [2]. Instead, all of these programs are, as expected, already rejected when constructing the variable-offset pointers. Note that the check in check_stack_access_for_ptr_arithmetic() also prints "off=%d" while the code removed by this patch does not (the error removed does not appear in the "verification_error" values). For reproducibility, the repository linked includes the raw data and scripts used to create the plot. [1] https://arxiv.org/pdf/1807.03757.pdf [2] https://gitlab.cs.fau.de/un65esoq/bpf-spectre/-/raw/53dc19fcf459c186613b1156a81504b39c8d49db/data/plots/23-02-26_23-56_bpftool/bpftool/0004-errors.pdf?inline=false Fixes: 01f810ace9ed ("bpf: Allow variable-offset stack access") Signed-off-by: Luis Gerhorst Signed-off-by: Daniel Borkmann Acked-by: Daniel Borkmann Link: https://lore.kernel.org/bpf/20230315165358.23701-1-gerhorst@cs.fau.de Signed-off-by: Sasha Levin --- kernel/bpf/verifier.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 232f720104cd..6876796a8de0 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -2775,17 +2775,13 @@ static int check_stack_read(struct bpf_verifier_env *env, } /* Variable offset is prohibited for unprivileged mode for simplicity * since it requires corresponding support in Spectre masking for stack - * ALU. See also retrieve_ptr_limit(). + * ALU. See also retrieve_ptr_limit(). The check in + * check_stack_access_for_ptr_arithmetic() called by + * adjust_ptr_min_max_vals() prevents users from creating stack pointers + * with variable offsets, therefore no check is required here. Further, + * just checking it here would be insufficient as speculative stack + * writes could still lead to unsafe speculative behaviour. */ - if (!env->bypass_spec_v1 && var_off) { - char tn_buf[48]; - - tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off); - verbose(env, "R%d variable offset stack access prohibited for !root, var_off=%s\n", - ptr_regno, tn_buf); - return -EACCES; - } - if (!var_off) { off += reg->var_off.value; err = check_stack_read_fixed_off(env, state, off, size, From ba6d56b20e8acb0d83020580ce28b1127e6d4b88 Mon Sep 17 00:00:00 2001 From: Vadim Fedorenko Date: Wed, 15 Mar 2023 08:33:02 -0700 Subject: [PATCH 42/57] vlan: partially enable SIOCSHWTSTAMP in container [ Upstream commit 731b73dba359e3ff00517c13aa0daa82b34ff466 ] Setting timestamp filter was explicitly disabled on vlan devices in containers because it might affect other processes on the host. But it's absolutely legit in case when real device is in the same namespace. Fixes: 873017af7784 ("vlan: disable SIOCSHWTSTAMP in container") Signed-off-by: Vadim Fedorenko Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- net/8021q/vlan_dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c index 86a1c99025ea..929f85c6cf11 100644 --- a/net/8021q/vlan_dev.c +++ b/net/8021q/vlan_dev.c @@ -365,7 +365,7 @@ static int vlan_dev_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) switch (cmd) { case SIOCSHWTSTAMP: - if (!net_eq(dev_net(dev), &init_net)) + if (!net_eq(dev_net(dev), dev_net(real_dev))) break; fallthrough; case SIOCGMIIPHY: From 8bb81a925a9fd1b14633653146530528ef297ce1 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 16 Mar 2023 01:10:06 +0000 Subject: [PATCH 43/57] net/packet: annotate accesses to po->xmit [ Upstream commit b9d83ab8a708f23a4001d60e9d8d0b3be3d9f607 ] po->xmit can be set from setsockopt(PACKET_QDISC_BYPASS), while read locklessly. Use READ_ONCE()/WRITE_ONCE() to avoid potential load/store tearing issues. Fixes: d346a3fae3ff ("packet: introduce PACKET_QDISC_BYPASS socket option") Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- net/packet/af_packet.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 3716797c5564..2b435d9916e6 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -269,7 +269,8 @@ static void packet_cached_dev_reset(struct packet_sock *po) static bool packet_use_direct_xmit(const struct packet_sock *po) { - return po->xmit == packet_direct_xmit; + /* Paired with WRITE_ONCE() in packet_setsockopt() */ + return READ_ONCE(po->xmit) == packet_direct_xmit; } static u16 packet_pick_tx_queue(struct sk_buff *skb) @@ -2825,7 +2826,8 @@ tpacket_error: packet_inc_pending(&po->tx_ring); status = TP_STATUS_SEND_REQUEST; - err = po->xmit(skb); + /* Paired with WRITE_ONCE() in packet_setsockopt() */ + err = READ_ONCE(po->xmit)(skb); if (unlikely(err != 0)) { if (err > 0) err = net_xmit_errno(err); @@ -3028,7 +3030,8 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) virtio_net_hdr_set_proto(skb, &vnet_hdr); } - err = po->xmit(skb); + /* Paired with WRITE_ONCE() in packet_setsockopt() */ + err = READ_ONCE(po->xmit)(skb); if (unlikely(err != 0)) { if (err > 0) err = net_xmit_errno(err); @@ -3979,7 +3982,8 @@ packet_setsockopt(struct socket *sock, int level, int optname, sockptr_t optval, if (copy_from_sockptr(&val, optval, sizeof(val))) return -EFAULT; - po->xmit = val ? packet_direct_xmit : dev_queue_xmit; + /* Paired with all lockless reads of po->xmit */ + WRITE_ONCE(po->xmit, val ? packet_direct_xmit : dev_queue_xmit); return 0; } default: From 5ca1be3658cda035ab82cfeeb93c6dadbed502c4 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 16 Mar 2023 01:10:07 +0000 Subject: [PATCH 44/57] net/packet: convert po->origdev to an atomic flag [ Upstream commit ee5675ecdf7a4e713ed21d98a70c2871d6ebed01 ] syzbot/KCAN reported that po->origdev can be read while another thread is changing its value. We can avoid this splat by converting this field to an actual bit. Following patches will convert remaining 1bit fields. Fixes: 80feaacb8a64 ("[AF_PACKET]: Add option to return orig_dev to userspace.") Signed-off-by: Eric Dumazet Reported-by: syzbot Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- net/packet/af_packet.c | 10 ++++------ net/packet/diag.c | 2 +- net/packet/internal.h | 22 +++++++++++++++++++++- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 2b435d9916e6..df93f4b09ab9 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2146,7 +2146,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev, sll = &PACKET_SKB_CB(skb)->sa.ll; sll->sll_hatype = dev->type; sll->sll_pkttype = skb->pkt_type; - if (unlikely(po->origdev)) + if (unlikely(packet_sock_flag(po, PACKET_SOCK_ORIGDEV))) sll->sll_ifindex = orig_dev->ifindex; else sll->sll_ifindex = dev->ifindex; @@ -2419,7 +2419,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, sll->sll_hatype = dev->type; sll->sll_protocol = skb->protocol; sll->sll_pkttype = skb->pkt_type; - if (unlikely(po->origdev)) + if (unlikely(packet_sock_flag(po, PACKET_SOCK_ORIGDEV))) sll->sll_ifindex = orig_dev->ifindex; else sll->sll_ifindex = dev->ifindex; @@ -3883,9 +3883,7 @@ packet_setsockopt(struct socket *sock, int level, int optname, sockptr_t optval, if (copy_from_sockptr(&val, optval, sizeof(val))) return -EFAULT; - lock_sock(sk); - po->origdev = !!val; - release_sock(sk); + packet_sock_flag_set(po, PACKET_SOCK_ORIGDEV, val); return 0; } case PACKET_VNET_HDR: @@ -4037,7 +4035,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname, val = po->auxdata; break; case PACKET_ORIGDEV: - val = po->origdev; + val = packet_sock_flag(po, PACKET_SOCK_ORIGDEV); break; case PACKET_VNET_HDR: val = po->has_vnet_hdr; diff --git a/net/packet/diag.c b/net/packet/diag.c index 07812ae5ca07..e1ac9bb375b3 100644 --- a/net/packet/diag.c +++ b/net/packet/diag.c @@ -25,7 +25,7 @@ static int pdiag_put_info(const struct packet_sock *po, struct sk_buff *nlskb) pinfo.pdi_flags |= PDI_RUNNING; if (po->auxdata) pinfo.pdi_flags |= PDI_AUXDATA; - if (po->origdev) + if (packet_sock_flag(po, PACKET_SOCK_ORIGDEV)) pinfo.pdi_flags |= PDI_ORIGDEV; if (po->has_vnet_hdr) pinfo.pdi_flags |= PDI_VNETHDR; diff --git a/net/packet/internal.h b/net/packet/internal.h index 7af1e9179385..7fea453dc721 100644 --- a/net/packet/internal.h +++ b/net/packet/internal.h @@ -116,9 +116,9 @@ struct packet_sock { int copy_thresh; spinlock_t bind_lock; struct mutex pg_vec_lock; + unsigned long flags; unsigned int running; /* bind_lock must be held */ unsigned int auxdata:1, /* writer must hold sock lock */ - origdev:1, has_vnet_hdr:1, tp_loss:1, tp_tx_has_off:1; @@ -144,4 +144,24 @@ static struct packet_sock *pkt_sk(struct sock *sk) return (struct packet_sock *)sk; } +enum packet_sock_flags { + PACKET_SOCK_ORIGDEV, +}; + +static inline void packet_sock_flag_set(struct packet_sock *po, + enum packet_sock_flags flag, + bool val) +{ + if (val) + set_bit(flag, &po->flags); + else + clear_bit(flag, &po->flags); +} + +static inline bool packet_sock_flag(const struct packet_sock *po, + enum packet_sock_flags flag) +{ + return test_bit(flag, &po->flags); +} + #endif From 05c6db12aece625cd5de696c8b036c55d4ef8290 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 16 Mar 2023 01:10:08 +0000 Subject: [PATCH 45/57] net/packet: convert po->auxdata to an atomic flag [ Upstream commit fd53c297aa7b077ae98a3d3d2d3aa278a1686ba6 ] po->auxdata can be read while another thread is changing its value, potentially raising KCSAN splat. Convert it to PACKET_SOCK_AUXDATA flag. Fixes: 8dc419447415 ("[PACKET]: Add optional checksum computation for recvmsg") Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- net/packet/af_packet.c | 8 +++----- net/packet/diag.c | 2 +- net/packet/internal.h | 4 ++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index df93f4b09ab9..9b6f6a5e0b14 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -3485,7 +3485,7 @@ static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, memcpy(msg->msg_name, &PACKET_SKB_CB(skb)->sa, copy_len); } - if (pkt_sk(sk)->auxdata) { + if (packet_sock_flag(pkt_sk(sk), PACKET_SOCK_AUXDATA)) { struct tpacket_auxdata aux; aux.tp_status = TP_STATUS_USER; @@ -3869,9 +3869,7 @@ packet_setsockopt(struct socket *sock, int level, int optname, sockptr_t optval, if (copy_from_sockptr(&val, optval, sizeof(val))) return -EFAULT; - lock_sock(sk); - po->auxdata = !!val; - release_sock(sk); + packet_sock_flag_set(po, PACKET_SOCK_AUXDATA, val); return 0; } case PACKET_ORIGDEV: @@ -4032,7 +4030,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname, break; case PACKET_AUXDATA: - val = po->auxdata; + val = packet_sock_flag(po, PACKET_SOCK_AUXDATA); break; case PACKET_ORIGDEV: val = packet_sock_flag(po, PACKET_SOCK_ORIGDEV); diff --git a/net/packet/diag.c b/net/packet/diag.c index e1ac9bb375b3..d704c7bf51b2 100644 --- a/net/packet/diag.c +++ b/net/packet/diag.c @@ -23,7 +23,7 @@ static int pdiag_put_info(const struct packet_sock *po, struct sk_buff *nlskb) pinfo.pdi_flags = 0; if (po->running) pinfo.pdi_flags |= PDI_RUNNING; - if (po->auxdata) + if (packet_sock_flag(po, PACKET_SOCK_AUXDATA)) pinfo.pdi_flags |= PDI_AUXDATA; if (packet_sock_flag(po, PACKET_SOCK_ORIGDEV)) pinfo.pdi_flags |= PDI_ORIGDEV; diff --git a/net/packet/internal.h b/net/packet/internal.h index 7fea453dc721..3938cb413d5d 100644 --- a/net/packet/internal.h +++ b/net/packet/internal.h @@ -118,8 +118,7 @@ struct packet_sock { struct mutex pg_vec_lock; unsigned long flags; unsigned int running; /* bind_lock must be held */ - unsigned int auxdata:1, /* writer must hold sock lock */ - has_vnet_hdr:1, + unsigned int has_vnet_hdr:1, /* writer must hold sock lock */ tp_loss:1, tp_tx_has_off:1; int pressure; @@ -146,6 +145,7 @@ static struct packet_sock *pkt_sk(struct sock *sk) enum packet_sock_flags { PACKET_SOCK_ORIGDEV, + PACKET_SOCK_AUXDATA, }; static inline void packet_sock_flag_set(struct packet_sock *po, From 621c89a0216ac227a0fe146b42d60893f256c1fd Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Sun, 1 Nov 2020 00:32:08 +0100 Subject: [PATCH 46/57] scsi: target: Rename struct sense_info to sense_detail [ Upstream commit b455233dcc403e3eb955a642dd69b6676e12b245 ] This helps distinguish it from the SCSI sense INFORMATION field. Link: https://lore.kernel.org/r/20201031233211.5207-2-ddiss@suse.de Reviewed-by: Mike Christie Signed-off-by: David Disseldorp Signed-off-by: Martin K. Petersen Stable-dep-of: 673db054d7a2 ("scsi: target: Fix multiple LUN_RESET handling") Signed-off-by: Sasha Levin --- drivers/target/target_core_transport.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index bca3a32a4bfb..ce521d3d3047 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -3131,14 +3131,14 @@ bool transport_wait_for_tasks(struct se_cmd *cmd) } EXPORT_SYMBOL(transport_wait_for_tasks); -struct sense_info { +struct sense_detail { u8 key; u8 asc; u8 ascq; bool add_sector_info; }; -static const struct sense_info sense_info_table[] = { +static const struct sense_detail sense_detail_table[] = { [TCM_NO_SENSE] = { .key = NOT_READY }, @@ -3298,39 +3298,39 @@ static const struct sense_info sense_info_table[] = { */ static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason) { - const struct sense_info *si; + const struct sense_detail *sd; u8 *buffer = cmd->sense_buffer; int r = (__force int)reason; u8 key, asc, ascq; bool desc_format = target_sense_desc_format(cmd->se_dev); - if (r < ARRAY_SIZE(sense_info_table) && sense_info_table[r].key) - si = &sense_info_table[r]; + if (r < ARRAY_SIZE(sense_detail_table) && sense_detail_table[r].key) + sd = &sense_detail_table[r]; else - si = &sense_info_table[(__force int) + sd = &sense_detail_table[(__force int) TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE]; - key = si->key; + key = sd->key; if (reason == TCM_CHECK_CONDITION_UNIT_ATTENTION) { if (!core_scsi3_ua_for_check_condition(cmd, &key, &asc, &ascq)) { cmd->scsi_status = SAM_STAT_BUSY; return; } - } else if (si->asc == 0) { + } else if (sd->asc == 0) { WARN_ON_ONCE(cmd->scsi_asc == 0); asc = cmd->scsi_asc; ascq = cmd->scsi_ascq; } else { - asc = si->asc; - ascq = si->ascq; + asc = sd->asc; + ascq = sd->ascq; } cmd->se_cmd_flags |= SCF_EMULATED_TASK_SENSE; cmd->scsi_status = SAM_STAT_CHECK_CONDITION; cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; scsi_build_sense_buffer(desc_format, buffer, key, asc, ascq); - if (si->add_sector_info) + if (sd->add_sector_info) WARN_ON_ONCE(scsi_set_sense_information(buffer, cmd->scsi_sense_length, cmd->bad_sector) < 0); From 7c8a29f1b22e68978e0449ebb2e30a123074bea1 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Sun, 1 Nov 2020 00:32:09 +0100 Subject: [PATCH 47/57] scsi: target: Rename cmd.bad_sector to cmd.sense_info [ Upstream commit 8dd992fb67f33a0777fb4bee1e22a5ee5530f024 ] cmd.bad_sector currently gets packed into the sense INFORMATION field for TCM_LOGICAL_BLOCK_{GUARD,APP_TAG,REF_TAG}_CHECK_FAILED errors, which carry an .add_sector_info flag in the sense_detail_table to ensure this. In preparation for propagating a byte offset on COMPARE AND WRITE TCM_MISCOMPARE_VERIFY error, rename cmd.bad_sector to cmd.sense_info and sense_detail.add_sector_info to sense_detail.add_sense_info so that it better reflects the sense INFORMATION field destination. [ddiss: update previously overlooked ib_isert] Link: https://lore.kernel.org/r/20201031233211.5207-3-ddiss@suse.de Reviewed-by: Mike Christie Signed-off-by: David Disseldorp Signed-off-by: Martin K. Petersen Stable-dep-of: 673db054d7a2 ("scsi: target: Fix multiple LUN_RESET handling") Signed-off-by: Sasha Levin --- drivers/infiniband/ulp/isert/ib_isert.c | 4 ++-- drivers/target/target_core_sbc.c | 2 +- drivers/target/target_core_transport.c | 12 ++++++------ include/target/target_core_base.h | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c index edea37da8a5b..2d0d966fba2c 100644 --- a/drivers/infiniband/ulp/isert/ib_isert.c +++ b/drivers/infiniband/ulp/isert/ib_isert.c @@ -1553,12 +1553,12 @@ isert_check_pi_status(struct se_cmd *se_cmd, struct ib_mr *sig_mr) } sec_offset_err = mr_status.sig_err.sig_err_offset; do_div(sec_offset_err, block_size); - se_cmd->bad_sector = sec_offset_err + se_cmd->t_task_lba; + se_cmd->sense_info = sec_offset_err + se_cmd->t_task_lba; isert_err("PI error found type %d at sector 0x%llx " "expected 0x%x vs actual 0x%x\n", mr_status.sig_err.err_type, - (unsigned long long)se_cmd->bad_sector, + (unsigned long long)se_cmd->sense_info, mr_status.sig_err.expected, mr_status.sig_err.actual); ret = 1; diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index eaf8551ebc61..47c5f26e6012 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -1438,7 +1438,7 @@ sbc_dif_verify(struct se_cmd *cmd, sector_t start, unsigned int sectors, if (rc) { kunmap_atomic(daddr - dsg->offset); kunmap_atomic(paddr - psg->offset); - cmd->bad_sector = sector; + cmd->sense_info = sector; return rc; } next: diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index ce521d3d3047..84d654c14917 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -3135,7 +3135,7 @@ struct sense_detail { u8 key; u8 asc; u8 ascq; - bool add_sector_info; + bool add_sense_info; }; static const struct sense_detail sense_detail_table[] = { @@ -3238,19 +3238,19 @@ static const struct sense_detail sense_detail_table[] = { .key = ABORTED_COMMAND, .asc = 0x10, .ascq = 0x01, /* LOGICAL BLOCK GUARD CHECK FAILED */ - .add_sector_info = true, + .add_sense_info = true, }, [TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED] = { .key = ABORTED_COMMAND, .asc = 0x10, .ascq = 0x02, /* LOGICAL BLOCK APPLICATION TAG CHECK FAILED */ - .add_sector_info = true, + .add_sense_info = true, }, [TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED] = { .key = ABORTED_COMMAND, .asc = 0x10, .ascq = 0x03, /* LOGICAL BLOCK REFERENCE TAG CHECK FAILED */ - .add_sector_info = true, + .add_sense_info = true, }, [TCM_COPY_TARGET_DEVICE_NOT_REACHABLE] = { .key = COPY_ABORTED, @@ -3330,10 +3330,10 @@ static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason) cmd->scsi_status = SAM_STAT_CHECK_CONDITION; cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; scsi_build_sense_buffer(desc_format, buffer, key, asc, ascq); - if (sd->add_sector_info) + if (sd->add_sense_info) WARN_ON_ONCE(scsi_set_sense_information(buffer, cmd->scsi_sense_length, - cmd->bad_sector) < 0); + cmd->sense_info) < 0); } int diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 18a5dcd275f8..16992ba455de 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -540,7 +540,7 @@ struct se_cmd { struct scatterlist *t_prot_sg; unsigned int t_prot_nents; sense_reason_t pi_err; - sector_t bad_sector; + u64 sense_info; int cpuid; }; From 008b936bbde3e87a611b3828a0d5d2a4f99026a0 Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Sun, 1 Nov 2020 12:59:33 -0600 Subject: [PATCH 48/57] scsi: target: Make state_list per CPU [ Upstream commit 1526d9f10c6184031e42afad0adbdde1213e8ad1 ] Do a state_list/execute_task_lock per CPU, so we can do submissions from different CPUs without contention with each other. Note: tcm_fc was passing TARGET_SCF_USE_CPUID, but never set cpuid. The assumption is that it wanted to set the cpuid to the CPU it was submitting from so it will get this behavior with this patch. [mkp: s/printk/pr_err/ + resolve COMPARE AND WRITE patch conflict] Link: https://lore.kernel.org/r/1604257174-4524-8-git-send-email-michael.christie@oracle.com Reviewed-by: Himanshu Madhani Signed-off-by: Mike Christie Signed-off-by: Martin K. Petersen Stable-dep-of: 673db054d7a2 ("scsi: target: Fix multiple LUN_RESET handling") Signed-off-by: Sasha Levin --- drivers/target/target_core_device.c | 16 ++- drivers/target/target_core_tmr.c | 138 +++++++++++++------------ drivers/target/target_core_transport.c | 27 +++-- drivers/target/tcm_fc/tfc_cmd.c | 2 +- include/target/target_core_base.h | 13 ++- 5 files changed, 112 insertions(+), 84 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 1eded5c4ebda..bd3f1192c75a 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -724,11 +724,24 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name) { struct se_device *dev; struct se_lun *xcopy_lun; + int i; dev = hba->backend->ops->alloc_device(hba, name); if (!dev) return NULL; + dev->queues = kcalloc(nr_cpu_ids, sizeof(*dev->queues), GFP_KERNEL); + if (!dev->queues) { + dev->transport->free_device(dev); + return NULL; + } + + dev->queue_cnt = nr_cpu_ids; + for (i = 0; i < dev->queue_cnt; i++) { + INIT_LIST_HEAD(&dev->queues[i].state_list); + spin_lock_init(&dev->queues[i].lock); + } + dev->se_hba = hba; dev->transport = hba->backend->ops; dev->transport_flags = dev->transport->transport_flags_default; @@ -738,9 +751,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name) INIT_LIST_HEAD(&dev->dev_sep_list); INIT_LIST_HEAD(&dev->dev_tmr_list); INIT_LIST_HEAD(&dev->delayed_cmd_list); - INIT_LIST_HEAD(&dev->state_list); INIT_LIST_HEAD(&dev->qf_cmd_list); - spin_lock_init(&dev->execute_task_lock); spin_lock_init(&dev->delayed_cmd_lock); spin_lock_init(&dev->dev_reservation_lock); spin_lock_init(&dev->se_port_lock); @@ -1014,6 +1025,7 @@ void target_free_device(struct se_device *dev) if (dev->transport->free_prot) dev->transport->free_prot(dev); + kfree(dev->queues); dev->transport->free_device(dev); } diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 3efd5a3bd69d..5da384bb88c8 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -121,57 +121,61 @@ void core_tmr_abort_task( unsigned long flags; bool rc; u64 ref_tag; + int i; - spin_lock_irqsave(&dev->execute_task_lock, flags); - list_for_each_entry_safe(se_cmd, next, &dev->state_list, state_list) { + for (i = 0; i < dev->queue_cnt; i++) { + spin_lock_irqsave(&dev->queues[i].lock, flags); + list_for_each_entry_safe(se_cmd, next, &dev->queues[i].state_list, + state_list) { + if (se_sess != se_cmd->se_sess) + continue; - if (se_sess != se_cmd->se_sess) - continue; + /* + * skip task management functions, including + * tmr->task_cmd + */ + if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB) + continue; - /* skip task management functions, including tmr->task_cmd */ - if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB) - continue; + ref_tag = se_cmd->tag; + if (tmr->ref_task_tag != ref_tag) + continue; - ref_tag = se_cmd->tag; - if (tmr->ref_task_tag != ref_tag) - continue; + pr_err("ABORT_TASK: Found referenced %s task_tag: %llu\n", + se_cmd->se_tfo->fabric_name, ref_tag); - printk("ABORT_TASK: Found referenced %s task_tag: %llu\n", - se_cmd->se_tfo->fabric_name, ref_tag); + spin_lock(&se_sess->sess_cmd_lock); + rc = __target_check_io_state(se_cmd, se_sess, 0); + spin_unlock(&se_sess->sess_cmd_lock); + if (!rc) + continue; - spin_lock(&se_sess->sess_cmd_lock); - rc = __target_check_io_state(se_cmd, se_sess, 0); - spin_unlock(&se_sess->sess_cmd_lock); - if (!rc) - continue; + list_move_tail(&se_cmd->state_list, &aborted_list); + se_cmd->state_active = false; + spin_unlock_irqrestore(&dev->queues[i].lock, flags); - list_move_tail(&se_cmd->state_list, &aborted_list); - se_cmd->state_active = false; + /* + * Ensure that this ABORT request is visible to the LU + * RESET code. + */ + if (!tmr->tmr_dev) + WARN_ON_ONCE(transport_lookup_tmr_lun(tmr->task_cmd) < 0); - spin_unlock_irqrestore(&dev->execute_task_lock, flags); + if (dev->transport->tmr_notify) + dev->transport->tmr_notify(dev, TMR_ABORT_TASK, + &aborted_list); - /* - * Ensure that this ABORT request is visible to the LU RESET - * code. - */ - if (!tmr->tmr_dev) - WARN_ON_ONCE(transport_lookup_tmr_lun(tmr->task_cmd) < - 0); + list_del_init(&se_cmd->state_list); + target_put_cmd_and_wait(se_cmd); - if (dev->transport->tmr_notify) - dev->transport->tmr_notify(dev, TMR_ABORT_TASK, - &aborted_list); - - list_del_init(&se_cmd->state_list); - target_put_cmd_and_wait(se_cmd); - - printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for" - " ref_tag: %llu\n", ref_tag); - tmr->response = TMR_FUNCTION_COMPLETE; - atomic_long_inc(&dev->aborts_complete); - return; + pr_err("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for ref_tag: %llu\n", + ref_tag); + tmr->response = TMR_FUNCTION_COMPLETE; + atomic_long_inc(&dev->aborts_complete); + return; + } + spin_unlock_irqrestore(&dev->queues[i].lock, flags); } - spin_unlock_irqrestore(&dev->execute_task_lock, flags); if (dev->transport->tmr_notify) dev->transport->tmr_notify(dev, TMR_ABORT_TASK, &aborted_list); @@ -273,7 +277,7 @@ static void core_tmr_drain_state_list( struct se_session *sess; struct se_cmd *cmd, *next; unsigned long flags; - int rc; + int rc, i; /* * Complete outstanding commands with TASK_ABORTED SAM status. @@ -297,35 +301,39 @@ static void core_tmr_drain_state_list( * Note that this seems to be independent of TAS (Task Aborted Status) * in the Control Mode Page. */ - spin_lock_irqsave(&dev->execute_task_lock, flags); - list_for_each_entry_safe(cmd, next, &dev->state_list, state_list) { - /* - * For PREEMPT_AND_ABORT usage, only process commands - * with a matching reservation key. - */ - if (target_check_cdb_and_preempt(preempt_and_abort_list, cmd)) - continue; + for (i = 0; i < dev->queue_cnt; i++) { + spin_lock_irqsave(&dev->queues[i].lock, flags); + list_for_each_entry_safe(cmd, next, &dev->queues[i].state_list, + state_list) { + /* + * For PREEMPT_AND_ABORT usage, only process commands + * with a matching reservation key. + */ + if (target_check_cdb_and_preempt(preempt_and_abort_list, + cmd)) + continue; - /* - * Not aborting PROUT PREEMPT_AND_ABORT CDB.. - */ - if (prout_cmd == cmd) - continue; + /* + * Not aborting PROUT PREEMPT_AND_ABORT CDB.. + */ + if (prout_cmd == cmd) + continue; - sess = cmd->se_sess; - if (WARN_ON_ONCE(!sess)) - continue; + sess = cmd->se_sess; + if (WARN_ON_ONCE(!sess)) + continue; - spin_lock(&sess->sess_cmd_lock); - rc = __target_check_io_state(cmd, tmr_sess, tas); - spin_unlock(&sess->sess_cmd_lock); - if (!rc) - continue; + spin_lock(&sess->sess_cmd_lock); + rc = __target_check_io_state(cmd, tmr_sess, tas); + spin_unlock(&sess->sess_cmd_lock); + if (!rc) + continue; - list_move_tail(&cmd->state_list, &drain_task_list); - cmd->state_active = false; + list_move_tail(&cmd->state_list, &drain_task_list); + cmd->state_active = false; + } + spin_unlock_irqrestore(&dev->queues[i].lock, flags); } - spin_unlock_irqrestore(&dev->execute_task_lock, flags); if (dev->transport->tmr_notify) dev->transport->tmr_notify(dev, preempt_and_abort_list ? diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 84d654c14917..230fffa993c0 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -650,12 +650,12 @@ static void target_remove_from_state_list(struct se_cmd *cmd) if (!dev) return; - spin_lock_irqsave(&dev->execute_task_lock, flags); + spin_lock_irqsave(&dev->queues[cmd->cpuid].lock, flags); if (cmd->state_active) { list_del(&cmd->state_list); cmd->state_active = false; } - spin_unlock_irqrestore(&dev->execute_task_lock, flags); + spin_unlock_irqrestore(&dev->queues[cmd->cpuid].lock, flags); } /* @@ -866,10 +866,7 @@ void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status) INIT_WORK(&cmd->work, success ? target_complete_ok_work : target_complete_failure_work); - if (cmd->se_cmd_flags & SCF_USE_CPUID) - queue_work_on(cmd->cpuid, target_completion_wq, &cmd->work); - else - queue_work(target_completion_wq, &cmd->work); + queue_work_on(cmd->cpuid, target_completion_wq, &cmd->work); } EXPORT_SYMBOL(target_complete_cmd); @@ -904,12 +901,13 @@ static void target_add_to_state_list(struct se_cmd *cmd) struct se_device *dev = cmd->se_dev; unsigned long flags; - spin_lock_irqsave(&dev->execute_task_lock, flags); + spin_lock_irqsave(&dev->queues[cmd->cpuid].lock, flags); if (!cmd->state_active) { - list_add_tail(&cmd->state_list, &dev->state_list); + list_add_tail(&cmd->state_list, + &dev->queues[cmd->cpuid].state_list); cmd->state_active = true; } - spin_unlock_irqrestore(&dev->execute_task_lock, flags); + spin_unlock_irqrestore(&dev->queues[cmd->cpuid].lock, flags); } /* @@ -1397,6 +1395,9 @@ void transport_init_se_cmd( cmd->sense_buffer = sense_buffer; cmd->orig_fe_lun = unpacked_lun; + if (!(cmd->se_cmd_flags & SCF_USE_CPUID)) + cmd->cpuid = smp_processor_id(); + cmd->state_active = false; } EXPORT_SYMBOL(transport_init_se_cmd); @@ -1614,6 +1615,9 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess BUG_ON(!se_tpg); BUG_ON(se_cmd->se_tfo || se_cmd->se_sess); BUG_ON(in_interrupt()); + + if (flags & TARGET_SCF_USE_CPUID) + se_cmd->se_cmd_flags |= SCF_USE_CPUID; /* * Initialize se_cmd for target operation. From this point * exceptions are handled by sending exception status via @@ -1623,11 +1627,6 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess data_length, data_dir, task_attr, sense, unpacked_lun); - if (flags & TARGET_SCF_USE_CPUID) - se_cmd->se_cmd_flags |= SCF_USE_CPUID; - else - se_cmd->cpuid = WORK_CPU_UNBOUND; - if (flags & TARGET_SCF_UNKNOWN_SIZE) se_cmd->unknown_data_length = 1; /* diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c index a7ed56602c6c..8936a094f846 100644 --- a/drivers/target/tcm_fc/tfc_cmd.c +++ b/drivers/target/tcm_fc/tfc_cmd.c @@ -551,7 +551,7 @@ static void ft_send_work(struct work_struct *work) if (target_submit_cmd(&cmd->se_cmd, cmd->sess->se_sess, fcp->fc_cdb, &cmd->ft_sense_buffer[0], scsilun_to_int(&fcp->fc_lun), ntohl(fcp->fc_dl), task_attr, data_dir, - TARGET_SCF_ACK_KREF | TARGET_SCF_USE_CPUID)) + TARGET_SCF_ACK_KREF)) goto err; pr_debug("r_ctl %x target_submit_cmd %p\n", fh->fh_r_ctl, cmd); diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 16992ba455de..0109adcdfa0b 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -541,6 +541,10 @@ struct se_cmd { unsigned int t_prot_nents; sense_reason_t pi_err; u64 sense_info; + /* + * CPU LIO will execute the cmd on. Defaults to the CPU the cmd is + * initialized on. Drivers can override. + */ int cpuid; }; @@ -761,6 +765,11 @@ struct se_dev_stat_grps { struct config_group scsi_lu_group; }; +struct se_device_queue { + struct list_head state_list; + spinlock_t lock; +}; + struct se_device { /* RELATIVE TARGET PORT IDENTIFER Counter */ u16 dev_rpti_counter; @@ -794,7 +803,6 @@ struct se_device { atomic_t dev_qf_count; u32 export_count; spinlock_t delayed_cmd_lock; - spinlock_t execute_task_lock; spinlock_t dev_reservation_lock; unsigned int dev_reservation_flags; #define DRF_SPC2_RESERVATIONS 0x00000001 @@ -814,7 +822,6 @@ struct se_device { struct work_struct qf_work_queue; struct work_struct delayed_cmd_work; struct list_head delayed_cmd_list; - struct list_head state_list; struct list_head qf_cmd_list; /* Pointer to associated SE HBA */ struct se_hba *se_hba; @@ -841,6 +848,8 @@ struct se_device { /* For se_lun->lun_se_dev RCU read-side critical access */ u32 hba_index; struct rcu_head rcu_head; + int queue_cnt; + struct se_device_queue *queues; }; struct se_hba { From e1f59cd18a10969d08a082264b557876ca38766e Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Sat, 18 Mar 2023 20:56:18 -0500 Subject: [PATCH 49/57] scsi: target: Fix multiple LUN_RESET handling [ Upstream commit 673db054d7a2b5a470d7a25baf65956d005ad729 ] This fixes a bug where an initiator thinks a LUN_RESET has cleaned up running commands when it hasn't. The bug was added in commit 51ec502a3266 ("target: Delete tmr from list before processing"). The problem occurs when: 1. We have N I/O cmds running in the target layer spread over 2 sessions. 2. The initiator sends a LUN_RESET for each session. 3. session1's LUN_RESET loops over all the running commands from both sessions and moves them to its local drain_task_list. 4. session2's LUN_RESET does not see the LUN_RESET from session1 because the commit above has it remove itself. session2 also does not see any commands since the other reset moved them off the state lists. 5. sessions2's LUN_RESET will then complete with a successful response. 6. sessions2's inititor believes the running commands on its session are now cleaned up due to the successful response and cleans up the running commands from its side. It then restarts them. 7. The commands do eventually complete on the backend and the target starts to return aborted task statuses for them. The initiator will either throw a invalid ITT error or might accidentally lookup a new task if the ITT has been reallocated already. Fix the bug by reverting the patch, and serialize the execution of LUN_RESETs and Preempt and Aborts. Also prevent us from waiting on LUN_RESETs in core_tmr_drain_tmr_list, because it turns out the original patch fixed a bug that was not mentioned. For LUN_RESET1 core_tmr_drain_tmr_list can see a second LUN_RESET and wait on it. Then the second reset will run core_tmr_drain_tmr_list and see the first reset and wait on it resulting in a deadlock. Fixes: 51ec502a3266 ("target: Delete tmr from list before processing") Signed-off-by: Mike Christie Link: https://lore.kernel.org/r/20230319015620.96006-8-michael.christie@oracle.com Signed-off-by: Martin K. Petersen Signed-off-by: Sasha Levin --- drivers/target/target_core_device.c | 1 + drivers/target/target_core_tmr.c | 26 +++++++++++++++++++++++--- include/target/target_core_base.h | 1 + 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index bd3f1192c75a..4664330fb55d 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -770,6 +770,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name) spin_lock_init(&dev->t10_alua.lba_map_lock); INIT_WORK(&dev->delayed_cmd_work, target_do_delayed_work); + mutex_init(&dev->lun_reset_mutex); dev->t10_wwn.t10_dev = dev; dev->t10_alua.t10_dev = dev; diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 5da384bb88c8..a2b18a98d671 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -202,14 +202,23 @@ static void core_tmr_drain_tmr_list( * LUN_RESET tmr.. */ spin_lock_irqsave(&dev->se_tmr_lock, flags); - if (tmr) - list_del_init(&tmr->tmr_list); list_for_each_entry_safe(tmr_p, tmr_pp, &dev->dev_tmr_list, tmr_list) { + if (tmr_p == tmr) + continue; + cmd = tmr_p->task_cmd; if (!cmd) { pr_err("Unable to locate struct se_cmd for TMR\n"); continue; } + + /* + * We only execute one LUN_RESET at a time so we can't wait + * on them below. + */ + if (tmr_p->function == TMR_LUN_RESET) + continue; + /* * If this function was called with a valid pr_res_key * parameter (eg: for PROUT PREEMPT_AND_ABORT service action @@ -390,14 +399,25 @@ int core_tmr_lun_reset( tmr_nacl->initiatorname); } } + + + /* + * We only allow one reset or preempt and abort to execute at a time + * to prevent one call from claiming all the cmds causing a second + * call from returning while cmds it should have waited on are still + * running. + */ + mutex_lock(&dev->lun_reset_mutex); + pr_debug("LUN_RESET: %s starting for [%s], tas: %d\n", (preempt_and_abort_list) ? "Preempt" : "TMR", dev->transport->name, tas); - core_tmr_drain_tmr_list(dev, tmr, preempt_and_abort_list); core_tmr_drain_state_list(dev, prout_cmd, tmr_sess, tas, preempt_and_abort_list); + mutex_unlock(&dev->lun_reset_mutex); + /* * Clear any legacy SPC-2 reservation when called during * LOGICAL UNIT RESET diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 0109adcdfa0b..e681c712fcca 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -850,6 +850,7 @@ struct se_device { struct rcu_head rcu_head; int queue_cnt; struct se_device_queue *queues; + struct mutex lun_reset_mutex; }; struct se_hba { From bfe67e05632751e9d904d9addc8f361ea2e6e788 Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Sat, 18 Mar 2023 20:56:19 -0500 Subject: [PATCH 50/57] scsi: target: iscsit: Fix TAS handling during conn cleanup [ Upstream commit cc79da306ebb2edb700c3816b90219223182ac3c ] Fix a bug added in commit f36199355c64 ("scsi: target: iscsi: Fix cmd abort fabric stop race"). If CMD_T_TAS is set on the se_cmd we must call iscsit_free_cmd() to do the last put on the cmd and free it, because the connection is down and we will not up sending the response and doing the put from the normal I/O path. Add a check for CMD_T_TAS in iscsit_release_commands_from_conn() so we now detect this case and run iscsit_free_cmd(). Fixes: f36199355c64 ("scsi: target: iscsi: Fix cmd abort fabric stop race") Signed-off-by: Mike Christie Link: https://lore.kernel.org/r/20230319015620.96006-9-michael.christie@oracle.com Signed-off-by: Martin K. Petersen Signed-off-by: Sasha Levin --- drivers/target/iscsi/iscsi_target.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index a237f1cf9bd6..6bb840358072 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -4084,9 +4084,12 @@ static void iscsit_release_commands_from_conn(struct iscsi_conn *conn) list_for_each_entry_safe(cmd, cmd_tmp, &tmp_list, i_conn_node) { struct se_cmd *se_cmd = &cmd->se_cmd; - if (se_cmd->se_tfo != NULL) { - spin_lock_irq(&se_cmd->t_state_lock); - if (se_cmd->transport_state & CMD_T_ABORTED) { + if (!se_cmd->se_tfo) + continue; + + spin_lock_irq(&se_cmd->t_state_lock); + if (se_cmd->transport_state & CMD_T_ABORTED) { + if (!(se_cmd->transport_state & CMD_T_TAS)) /* * LIO's abort path owns the cleanup for this, * so put it back on the list and let @@ -4094,11 +4097,10 @@ static void iscsit_release_commands_from_conn(struct iscsi_conn *conn) */ list_move_tail(&cmd->i_conn_node, &conn->conn_cmd_list); - } else { - se_cmd->transport_state |= CMD_T_FABRIC_STOP; - } - spin_unlock_irq(&se_cmd->t_state_lock); + } else { + se_cmd->transport_state |= CMD_T_FABRIC_STOP; } + spin_unlock_irq(&se_cmd->t_state_lock); } spin_unlock_bh(&conn->cmd_lock); From 202048ec1ee531bdef179c32d54bb51d26bfff5b Mon Sep 17 00:00:00 2001 From: Danila Chernetsov Date: Fri, 17 Mar 2023 17:51:09 +0000 Subject: [PATCH 51/57] scsi: megaraid: Fix mega_cmd_done() CMDID_INT_CMDS [ Upstream commit 75cb113cd43f06aaf4f1bda0069cfd5b98e909eb ] When cmdid == CMDID_INT_CMDS, the 'cmds' pointer is NULL but is dereferenced below. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 0f2bb84d2a68 ("[SCSI] megaraid: simplify internal command handling") Signed-off-by: Danila Chernetsov Link: https://lore.kernel.org/r/20230317175109.18585-1-listdansp@mail.ru Signed-off-by: Martin K. Petersen Signed-off-by: Sasha Levin --- drivers/scsi/megaraid.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c index daffa36988ae..810d80340676 100644 --- a/drivers/scsi/megaraid.c +++ b/drivers/scsi/megaraid.c @@ -1443,6 +1443,7 @@ mega_cmd_done(adapter_t *adapter, u8 completed[], int nstatus, int status) */ if (cmdid == CMDID_INT_CMDS) { scb = &adapter->int_scb; + cmd = scb->cmd; list_del_init(&scb->list); scb->state = SCB_FREE; From c3a1914b96784e9c9125732ca036072ef4b948eb Mon Sep 17 00:00:00 2001 From: Yangtao Li Date: Tue, 21 Feb 2023 22:45:50 +0800 Subject: [PATCH 52/57] f2fs: handle dqget error in f2fs_transfer_project_quota() [ Upstream commit 8051692f5f23260215bfe9a72e712d93606acc5f ] We should set the error code when dqget() failed. Fixes: 2c1d03056991 ("f2fs: support F2FS_IOC_FS{GET,SET}XATTR") Signed-off-by: Yangtao Li Signed-off-by: Jaegeuk Kim Signed-off-by: Sasha Levin --- fs/f2fs/file.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index d56fcace1821..a0d8aa52b696 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -3013,15 +3013,16 @@ int f2fs_transfer_project_quota(struct inode *inode, kprojid_t kprojid) struct dquot *transfer_to[MAXQUOTAS] = {}; struct f2fs_sb_info *sbi = F2FS_I_SB(inode); struct super_block *sb = sbi->sb; - int err = 0; + int err; transfer_to[PRJQUOTA] = dqget(sb, make_kqid_projid(kprojid)); - if (!IS_ERR(transfer_to[PRJQUOTA])) { - err = __dquot_transfer(inode, transfer_to); - if (err) - set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR); - dqput(transfer_to[PRJQUOTA]); - } + if (IS_ERR(transfer_to[PRJQUOTA])) + return PTR_ERR(transfer_to[PRJQUOTA]); + + err = __dquot_transfer(inode, transfer_to); + if (err) + set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR); + dqput(transfer_to[PRJQUOTA]); return err; } From d420c4a06d8f3dc3dc34ff96cc298125e53dd5ac Mon Sep 17 00:00:00 2001 From: Jaegeuk Kim Date: Tue, 28 Jun 2022 10:57:24 -0700 Subject: [PATCH 53/57] f2fs: enforce single zone capacity [ Upstream commit b771aadc6e4c221a468fe4a2dfcfffec01a06722 ] In order to simplify the complicated per-zone capacity, let's support only one capacity for entire zoned device. Reviewed-by: Chao Yu Signed-off-by: Jaegeuk Kim Stable-dep-of: 0b37ed21e336 ("f2fs: apply zone capacity to all zone type") Signed-off-by: Sasha Levin --- fs/f2fs/f2fs.h | 2 +- fs/f2fs/segment.c | 19 ++++++------------- fs/f2fs/segment.h | 3 +++ fs/f2fs/super.c | 33 ++++++++++++--------------------- 4 files changed, 22 insertions(+), 35 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index c03fdda1bddf..62b7848f1f71 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -1153,7 +1153,6 @@ struct f2fs_dev_info { #ifdef CONFIG_BLK_DEV_ZONED unsigned int nr_blkz; /* Total number of zones */ unsigned long *blkz_seq; /* Bitmap indicating sequential zones */ - block_t *zone_capacity_blocks; /* Array of zone capacity in blks */ #endif }; @@ -1422,6 +1421,7 @@ struct f2fs_sb_info { unsigned int meta_ino_num; /* meta inode number*/ unsigned int log_blocks_per_seg; /* log2 blocks per segment */ unsigned int blocks_per_seg; /* blocks per segment */ + unsigned int unusable_blocks_per_sec; /* unusable blocks per section */ unsigned int segs_per_sec; /* segments per section */ unsigned int secs_per_zone; /* sections per zone */ unsigned int total_sections; /* total section count */ diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 7c90d93f4e43..5305913fe0d5 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -4982,7 +4982,7 @@ static unsigned int get_zone_idx(struct f2fs_sb_info *sbi, unsigned int secno, static inline unsigned int f2fs_usable_zone_segs_in_sec( struct f2fs_sb_info *sbi, unsigned int segno) { - unsigned int dev_idx, zone_idx, unusable_segs_in_sec; + unsigned int dev_idx, zone_idx; dev_idx = f2fs_target_device_index(sbi, START_BLOCK(sbi, segno)); zone_idx = get_zone_idx(sbi, GET_SEC_FROM_SEG(sbi, segno), dev_idx); @@ -4991,18 +4991,12 @@ static inline unsigned int f2fs_usable_zone_segs_in_sec( if (is_conv_zone(sbi, zone_idx, dev_idx)) return sbi->segs_per_sec; - /* - * If the zone_capacity_blocks array is NULL, then zone capacity - * is equal to the zone size for all zones - */ - if (!FDEV(dev_idx).zone_capacity_blocks) + if (!sbi->unusable_blocks_per_sec) return sbi->segs_per_sec; /* Get the segment count beyond zone capacity block */ - unusable_segs_in_sec = (sbi->blocks_per_blkz - - FDEV(dev_idx).zone_capacity_blocks[zone_idx]) >> - sbi->log_blocks_per_seg; - return sbi->segs_per_sec - unusable_segs_in_sec; + return sbi->segs_per_sec - (sbi->unusable_blocks_per_sec >> + sbi->log_blocks_per_seg); } /* @@ -5031,12 +5025,11 @@ static inline unsigned int f2fs_usable_zone_blks_in_seg( if (is_conv_zone(sbi, zone_idx, dev_idx)) return sbi->blocks_per_seg; - if (!FDEV(dev_idx).zone_capacity_blocks) + if (!sbi->unusable_blocks_per_sec) return sbi->blocks_per_seg; sec_start_blkaddr = START_BLOCK(sbi, GET_SEG_FROM_SEC(sbi, secno)); - sec_cap_blkaddr = sec_start_blkaddr + - FDEV(dev_idx).zone_capacity_blocks[zone_idx]; + sec_cap_blkaddr = sec_start_blkaddr + CAP_BLKS_PER_SEC(sbi); /* * If segment starts before zone capacity and spans beyond diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h index eafd89f0c77e..aca6c9a3aad0 100644 --- a/fs/f2fs/segment.h +++ b/fs/f2fs/segment.h @@ -101,6 +101,9 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi, GET_SEGNO_FROM_SEG0(sbi, blk_addr))) #define BLKS_PER_SEC(sbi) \ ((sbi)->segs_per_sec * (sbi)->blocks_per_seg) +#define CAP_BLKS_PER_SEC(sbi) \ + ((sbi)->segs_per_sec * (sbi)->blocks_per_seg - \ + (sbi)->unusable_blocks_per_sec) #define GET_SEC_FROM_SEG(sbi, segno) \ (((segno) == -1) ? -1: (segno) / (sbi)->segs_per_sec) #define GET_SEG_FROM_SEC(sbi, secno) \ diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 0bba5c72fc77..9a74d60f61db 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -1242,7 +1242,6 @@ static void destroy_device_list(struct f2fs_sb_info *sbi) blkdev_put(FDEV(i).bdev, FMODE_EXCL); #ifdef CONFIG_BLK_DEV_ZONED kvfree(FDEV(i).blkz_seq); - kfree(FDEV(i).zone_capacity_blocks); #endif } kvfree(sbi->devs); @@ -3199,24 +3198,29 @@ static int init_percpu_info(struct f2fs_sb_info *sbi) #ifdef CONFIG_BLK_DEV_ZONED struct f2fs_report_zones_args { + struct f2fs_sb_info *sbi; struct f2fs_dev_info *dev; - bool zone_cap_mismatch; }; static int f2fs_report_zone_cb(struct blk_zone *zone, unsigned int idx, void *data) { struct f2fs_report_zones_args *rz_args = data; + block_t unusable_blocks = (zone->len - zone->capacity) >> + F2FS_LOG_SECTORS_PER_BLOCK; if (zone->type == BLK_ZONE_TYPE_CONVENTIONAL) return 0; set_bit(idx, rz_args->dev->blkz_seq); - rz_args->dev->zone_capacity_blocks[idx] = zone->capacity >> - F2FS_LOG_SECTORS_PER_BLOCK; - if (zone->len != zone->capacity && !rz_args->zone_cap_mismatch) - rz_args->zone_cap_mismatch = true; - + if (!rz_args->sbi->unusable_blocks_per_sec) { + rz_args->sbi->unusable_blocks_per_sec = unusable_blocks; + return 0; + } + if (rz_args->sbi->unusable_blocks_per_sec != unusable_blocks) { + f2fs_err(rz_args->sbi, "F2FS supports single zone capacity\n"); + return -EINVAL; + } return 0; } @@ -3250,26 +3254,13 @@ static int init_blkz_info(struct f2fs_sb_info *sbi, int devi) if (!FDEV(devi).blkz_seq) return -ENOMEM; - /* Get block zones type and zone-capacity */ - FDEV(devi).zone_capacity_blocks = f2fs_kzalloc(sbi, - FDEV(devi).nr_blkz * sizeof(block_t), - GFP_KERNEL); - if (!FDEV(devi).zone_capacity_blocks) - return -ENOMEM; - + rep_zone_arg.sbi = sbi; rep_zone_arg.dev = &FDEV(devi); - rep_zone_arg.zone_cap_mismatch = false; ret = blkdev_report_zones(bdev, 0, BLK_ALL_ZONES, f2fs_report_zone_cb, &rep_zone_arg); if (ret < 0) return ret; - - if (!rep_zone_arg.zone_cap_mismatch) { - kfree(FDEV(devi).zone_capacity_blocks); - FDEV(devi).zone_capacity_blocks = NULL; - } - return 0; } #endif From d11a74577c21cf6dc1db65b6178ef3d03953ccf0 Mon Sep 17 00:00:00 2001 From: Jaegeuk Kim Date: Tue, 21 Mar 2023 15:58:04 -0700 Subject: [PATCH 54/57] f2fs: apply zone capacity to all zone type [ Upstream commit 0b37ed21e3367539b79284e0b0af2246ffcf0dca ] If we manage the zone capacity per zone type, it'll break the GC assumption. And, the current logic complains valid block count mismatch. Let's apply zone capacity to all zone type, if specified. Fixes: de881df97768 ("f2fs: support zone capacity less than zone size") Reviewed-by: Chao Yu Signed-off-by: Jaegeuk Kim Signed-off-by: Sasha Levin --- fs/f2fs/segment.c | 65 +++-------------------------------------------- fs/f2fs/segment.h | 3 +++ 2 files changed, 7 insertions(+), 61 deletions(-) diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 5305913fe0d5..a27a93429271 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -4957,48 +4957,6 @@ int f2fs_check_write_pointer(struct f2fs_sb_info *sbi) return 0; } -static bool is_conv_zone(struct f2fs_sb_info *sbi, unsigned int zone_idx, - unsigned int dev_idx) -{ - if (!bdev_is_zoned(FDEV(dev_idx).bdev)) - return true; - return !test_bit(zone_idx, FDEV(dev_idx).blkz_seq); -} - -/* Return the zone index in the given device */ -static unsigned int get_zone_idx(struct f2fs_sb_info *sbi, unsigned int secno, - int dev_idx) -{ - block_t sec_start_blkaddr = START_BLOCK(sbi, GET_SEG_FROM_SEC(sbi, secno)); - - return (sec_start_blkaddr - FDEV(dev_idx).start_blk) >> - sbi->log_blocks_per_blkz; -} - -/* - * Return the usable segments in a section based on the zone's - * corresponding zone capacity. Zone is equal to a section. - */ -static inline unsigned int f2fs_usable_zone_segs_in_sec( - struct f2fs_sb_info *sbi, unsigned int segno) -{ - unsigned int dev_idx, zone_idx; - - dev_idx = f2fs_target_device_index(sbi, START_BLOCK(sbi, segno)); - zone_idx = get_zone_idx(sbi, GET_SEC_FROM_SEG(sbi, segno), dev_idx); - - /* Conventional zone's capacity is always equal to zone size */ - if (is_conv_zone(sbi, zone_idx, dev_idx)) - return sbi->segs_per_sec; - - if (!sbi->unusable_blocks_per_sec) - return sbi->segs_per_sec; - - /* Get the segment count beyond zone capacity block */ - return sbi->segs_per_sec - (sbi->unusable_blocks_per_sec >> - sbi->log_blocks_per_seg); -} - /* * Return the number of usable blocks in a segment. The number of blocks * returned is always equal to the number of blocks in a segment for @@ -5011,23 +4969,13 @@ static inline unsigned int f2fs_usable_zone_blks_in_seg( struct f2fs_sb_info *sbi, unsigned int segno) { block_t seg_start, sec_start_blkaddr, sec_cap_blkaddr; - unsigned int zone_idx, dev_idx, secno; - - secno = GET_SEC_FROM_SEG(sbi, segno); - seg_start = START_BLOCK(sbi, segno); - dev_idx = f2fs_target_device_index(sbi, seg_start); - zone_idx = get_zone_idx(sbi, secno, dev_idx); - - /* - * Conventional zone's capacity is always equal to zone size, - * so, blocks per segment is unchanged. - */ - if (is_conv_zone(sbi, zone_idx, dev_idx)) - return sbi->blocks_per_seg; + unsigned int secno; if (!sbi->unusable_blocks_per_sec) return sbi->blocks_per_seg; + secno = GET_SEC_FROM_SEG(sbi, segno); + seg_start = START_BLOCK(sbi, segno); sec_start_blkaddr = START_BLOCK(sbi, GET_SEG_FROM_SEC(sbi, secno)); sec_cap_blkaddr = sec_start_blkaddr + CAP_BLKS_PER_SEC(sbi); @@ -5061,11 +5009,6 @@ static inline unsigned int f2fs_usable_zone_blks_in_seg(struct f2fs_sb_info *sbi return 0; } -static inline unsigned int f2fs_usable_zone_segs_in_sec(struct f2fs_sb_info *sbi, - unsigned int segno) -{ - return 0; -} #endif unsigned int f2fs_usable_blks_in_seg(struct f2fs_sb_info *sbi, unsigned int segno) @@ -5080,7 +5023,7 @@ unsigned int f2fs_usable_segs_in_sec(struct f2fs_sb_info *sbi, unsigned int segno) { if (f2fs_sb_has_blkzoned(sbi)) - return f2fs_usable_zone_segs_in_sec(sbi, segno); + return CAP_SEGS_PER_SEC(sbi); return sbi->segs_per_sec; } diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h index aca6c9a3aad0..979296b835b5 100644 --- a/fs/f2fs/segment.h +++ b/fs/f2fs/segment.h @@ -104,6 +104,9 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi, #define CAP_BLKS_PER_SEC(sbi) \ ((sbi)->segs_per_sec * (sbi)->blocks_per_seg - \ (sbi)->unusable_blocks_per_sec) +#define CAP_SEGS_PER_SEC(sbi) \ + ((sbi)->segs_per_sec - ((sbi)->unusable_blocks_per_sec >>\ + (sbi)->log_blocks_per_seg)) #define GET_SEC_FROM_SEG(sbi, segno) \ (((segno) == -1) ? -1: (segno) / (sbi)->segs_per_sec) #define GET_SEG_FROM_SEC(sbi, secno) \ From a8226a45b2a9ce83ba7a167a387a00fecc319e71 Mon Sep 17 00:00:00 2001 From: Yangtao Li Date: Tue, 21 Mar 2023 01:22:18 +0800 Subject: [PATCH 55/57] f2fs: compress: fix to call f2fs_wait_on_page_writeback() in f2fs_write_raw_pages() [ Upstream commit babedcbac164cec970872b8097401ca913a80e61 ] BUG_ON() will be triggered when writing files concurrently, because the same page is writtenback multiple times. 1597 void folio_end_writeback(struct folio *folio) 1598 { ...... 1618 if (!__folio_end_writeback(folio)) 1619 BUG(); ...... 1625 } kernel BUG at mm/filemap.c:1619! Call Trace: f2fs_write_end_io+0x1a0/0x370 blk_update_request+0x6c/0x410 blk_mq_end_request+0x15/0x130 blk_complete_reqs+0x3c/0x50 __do_softirq+0xb8/0x29b ? sort_range+0x20/0x20 run_ksoftirqd+0x19/0x20 smpboot_thread_fn+0x10b/0x1d0 kthread+0xde/0x110 ? kthread_complete_and_exit+0x20/0x20 ret_from_fork+0x22/0x30 Below is the concurrency scenario: [Process A] [Process B] [Process C] f2fs_write_raw_pages() - redirty_page_for_writepage() - unlock page() f2fs_do_write_data_page() - lock_page() - clear_page_dirty_for_io() - set_page_writeback() [1st writeback] ..... - unlock page() generic_perform_write() - f2fs_write_begin() - wait_for_stable_page() - f2fs_write_end() - set_page_dirty() - lock_page() - f2fs_do_write_data_page() - set_page_writeback() [2st writeback] This problem was introduced by the previous commit 7377e853967b ("f2fs: compress: fix potential deadlock of compress file"). All pagelocks were released in f2fs_write_raw_pages(), but whether the page was in the writeback state was ignored in the subsequent writing process. Let's fix it by waiting for the page to writeback before writing. Cc: Christoph Hellwig Fixes: 4c8ff7095bef ("f2fs: support data compression") Fixes: 7377e853967b ("f2fs: compress: fix potential deadlock of compress file") Signed-off-by: Qi Han Signed-off-by: Yangtao Li Reviewed-by: Chao Yu Signed-off-by: Jaegeuk Kim Signed-off-by: Sasha Levin --- fs/f2fs/compress.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c index 1541da5ace85..1be9de40f0b5 100644 --- a/fs/f2fs/compress.c +++ b/fs/f2fs/compress.c @@ -1391,6 +1391,12 @@ continue_unlock: if (!PageDirty(cc->rpages[i])) goto continue_unlock; + if (PageWriteback(cc->rpages[i])) { + if (wbc->sync_mode == WB_SYNC_NONE) + goto continue_unlock; + f2fs_wait_on_page_writeback(cc->rpages[i], DATA, true, true); + } + if (!clear_page_dirty_for_io(cc->rpages[i])) goto continue_unlock; From de3908e1515224b0e65cdfbe246eda933be3cdbf Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Tue, 21 Mar 2023 07:59:30 +0100 Subject: [PATCH 56/57] crypto: caam - Clear some memory in instantiate_rng [ Upstream commit 9c19fb86a8cb2ee82a832c95e139f29ea05c4d08 ] According to the comment at the end of the 'for' loop just a few lines below, it looks needed to clear 'desc'. So it should also be cleared for the first iteration. Move the memset() to the beginning of the loop to be safe. Fixes: 281922a1d4f5 ("crypto: caam - add support for SEC v5.x RNG4") Signed-off-by: Christophe JAILLET Reviewed-by: Gaurav Jain Signed-off-by: Herbert Xu Signed-off-by: Sasha Levin --- drivers/crypto/caam/ctrl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index f87aa2169e5f..f9a1ec3c8485 100644 --- a/drivers/crypto/caam/ctrl.c +++ b/drivers/crypto/caam/ctrl.c @@ -284,6 +284,10 @@ static int instantiate_rng(struct device *ctrldev, int state_handle_mask, const u32 rdsta_if = RDSTA_IF0 << sh_idx; const u32 rdsta_pr = RDSTA_PR0 << sh_idx; const u32 rdsta_mask = rdsta_if | rdsta_pr; + + /* Clear the contents before using the descriptor */ + memset(desc, 0x00, CAAM_CMD_SZ * 7); + /* * If the corresponding bit is set, this state handle * was initialized by somebody else, so it's left alone. @@ -327,8 +331,6 @@ static int instantiate_rng(struct device *ctrldev, int state_handle_mask, } dev_info(ctrldev, "Instantiated RNG4 SH%d\n", sh_idx); - /* Clear the contents before recreating the descriptor */ - memset(desc, 0x00, CAAM_CMD_SZ * 7); } kfree(desc); From adc2d82eee221bd9c36e0b17c857652fe27a871b Mon Sep 17 00:00:00 2001 From: Suman Anna Date: Fri, 24 Mar 2023 20:28:12 +0530 Subject: [PATCH 57/57] crypto: sa2ul - Select CRYPTO_DES [ Upstream commit 8832023efd20966e29944dac92118dfbf1fa1bc0 ] The SA2UL Crypto driver provides support for couple of DES3 algos "cbc(des3_ede)" and "ecb(des3_ede)", and enabling the crypto selftest throws the following errors (as seen on K3 J721E SoCs): saul-crypto 4e00000.crypto: Error allocating fallback algo cbc(des3_ede) alg: skcipher: failed to allocate transform for cbc-des3-sa2ul: -2 saul-crypto 4e00000.crypto: Error allocating fallback algo ecb(des3_ede) alg: skcipher: failed to allocate transform for ecb-des3-sa2ul: -2 Fix this by selecting CRYPTO_DES which was missed while adding base driver support. Fixes: 7694b6ca649f ("crypto: sa2ul - Add crypto driver") Signed-off-by: Suman Anna Signed-off-by: Jayesh Choudhary Signed-off-by: Herbert Xu Signed-off-by: Sasha Levin --- drivers/crypto/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 0a3dd0793f30..dbdee43c6ffc 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -897,6 +897,7 @@ config CRYPTO_DEV_SA2UL select CRYPTO_AES_ARM64 select CRYPTO_ALGAPI select CRYPTO_AUTHENC + select CRYPTO_DES select CRYPTO_SHA1 select CRYPTO_SHA256 select CRYPTO_SHA512