From: Greg Kroah-Hartman Date: Thu, 15 Jul 2021 12:39:06 +0000 (+0200) Subject: 4.14-stable patches X-Git-Tag: v5.4.133~42 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f54a0397f0686860b2d4a5f561a4687a83a3170e;p=thirdparty%2Fkernel%2Fstable-queue.git 4.14-stable patches added patches: ata-ahci_sunxi-disable-dipm.patch cpu-hotplug-cure-the-cpusets-trainwreck.patch drm-radeon-add-the-missed-drm_gem_object_put-in-radeon_user_framebuffer_create.patch mmc-core-allow-uhs-i-voltage-switch-for-sdsc-cards-if-supported.patch mmc-core-clear-flags-before-allowing-to-retune.patch mmc-sdhci-fix-warning-message-when-accessing-rpmb-in-hs400-mode.patch pinctrl-amd-add-device-hid-for-new-amd-gpio-controller.patch usb-gadget-f_fs-fix-setting-of-device-and-driver-data-cross-references.patch --- diff --git a/queue-4.14/ata-ahci_sunxi-disable-dipm.patch b/queue-4.14/ata-ahci_sunxi-disable-dipm.patch new file mode 100644 index 00000000000..6e522c56949 --- /dev/null +++ b/queue-4.14/ata-ahci_sunxi-disable-dipm.patch @@ -0,0 +1,43 @@ +From f6bca4d91b2ea052e917cca3f9d866b5cc1d500a Mon Sep 17 00:00:00 2001 +From: Timo Sigurdsson +Date: Mon, 14 Jun 2021 09:25:39 +0200 +Subject: ata: ahci_sunxi: Disable DIPM + +From: Timo Sigurdsson + +commit f6bca4d91b2ea052e917cca3f9d866b5cc1d500a upstream. + +DIPM is unsupported or broken on sunxi. Trying to enable the power +management policy med_power_with_dipm on an Allwinner A20 SoC based board +leads to immediate I/O errors and the attached SATA disk disappears from +the /dev filesystem. A reset (power cycle) is required to make the SATA +controller or disk work again. The A10 and A20 SoC data sheets and manuals +don't mention DIPM at all [1], so it's fair to assume that it's simply not +supported. But even if it was, it should be considered broken and best be +disabled in the ahci_sunxi driver. + +[1] https://github.com/allwinner-zh/documents/tree/master/ + +Fixes: c5754b5220f0 ("ARM: sunxi: Add support for Allwinner SUNXi SoCs sata to ahci_platform") +Cc: stable@vger.kernel.org +Signed-off-by: Timo Sigurdsson +Tested-by: Timo Sigurdsson +Link: https://lore.kernel.org/r/20210614072539.3307-1-public_timo.s@silentcreek.de +Signed-off-by: Jens Axboe +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/ata/ahci_sunxi.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +--- a/drivers/ata/ahci_sunxi.c ++++ b/drivers/ata/ahci_sunxi.c +@@ -165,7 +165,7 @@ static void ahci_sunxi_start_engine(stru + } + + static const struct ata_port_info ahci_sunxi_port_info = { +- .flags = AHCI_FLAG_COMMON | ATA_FLAG_NCQ, ++ .flags = AHCI_FLAG_COMMON | ATA_FLAG_NCQ | ATA_FLAG_NO_DIPM, + .pio_mask = ATA_PIO4, + .udma_mask = ATA_UDMA6, + .port_ops = &ahci_platform_ops, diff --git a/queue-4.14/cpu-hotplug-cure-the-cpusets-trainwreck.patch b/queue-4.14/cpu-hotplug-cure-the-cpusets-trainwreck.patch new file mode 100644 index 00000000000..ab85a6c3e93 --- /dev/null +++ b/queue-4.14/cpu-hotplug-cure-the-cpusets-trainwreck.patch @@ -0,0 +1,157 @@ +From b22afcdf04c96ca58327784e280e10288cfd3303 Mon Sep 17 00:00:00 2001 +From: Thomas Gleixner +Date: Sat, 27 Mar 2021 22:01:36 +0100 +Subject: cpu/hotplug: Cure the cpusets trainwreck + +From: Thomas Gleixner + +commit b22afcdf04c96ca58327784e280e10288cfd3303 upstream. + +Alexey and Joshua tried to solve a cpusets related hotplug problem which is +user space visible and results in unexpected behaviour for some time after +a CPU has been plugged in and the corresponding uevent was delivered. + +cpusets delegate the hotplug work (rebuilding cpumasks etc.) to a +workqueue. This is done because the cpusets code has already a lock +nesting of cgroups_mutex -> cpu_hotplug_lock. A synchronous callback or +waiting for the work to finish with cpu_hotplug_lock held can and will +deadlock because that results in the reverse lock order. + +As a consequence the uevent can be delivered before cpusets have consistent +state which means that a user space invocation of sched_setaffinity() to +move a task to the plugged CPU fails up to the point where the scheduled +work has been processed. + +The same is true for CPU unplug, but that does not create user observable +failure (yet). + +It's still inconsistent to claim that an operation is finished before it +actually is and that's the real issue at hand. uevents just make it +reliably observable. + +Obviously the problem should be fixed in cpusets/cgroups, but untangling +that is pretty much impossible because according to the changelog of the +commit which introduced this 8 years ago: + + 3a5a6d0c2b03("cpuset: don't nest cgroup_mutex inside get_online_cpus()") + +the lock order cgroups_mutex -> cpu_hotplug_lock is a design decision and +the whole code is built around that. + +So bite the bullet and invoke the relevant cpuset function, which waits for +the work to finish, in _cpu_up/down() after dropping cpu_hotplug_lock and +only when tasks are not frozen by suspend/hibernate because that would +obviously wait forever. + +Waiting there with cpu_add_remove_lock, which is protecting the present +and possible CPU maps, held is not a problem at all because neither work +queues nor cpusets/cgroups have any lockchains related to that lock. + +Waiting in the hotplug machinery is not problematic either because there +are already state callbacks which wait for hardware queues to drain. It +makes the operations slightly slower, but hotplug is slow anyway. + +This ensures that state is consistent before returning from a hotplug +up/down operation. It's still inconsistent during the operation, but that's +a different story. + +Add a large comment which explains why this is done and why this is not a +dump ground for the hack of the day to work around half thought out locking +schemes. Document also the implications vs. hotplug operations and +serialization or the lack of it. + +Thanks to Alexy and Joshua for analyzing why this temporary +sched_setaffinity() failure happened. + +Fixes: 3a5a6d0c2b03("cpuset: don't nest cgroup_mutex inside get_online_cpus()") +Reported-by: Alexey Klimov +Reported-by: Joshua Baker +Signed-off-by: Thomas Gleixner +Tested-by: Alexey Klimov +Cc: stable@vger.kernel.org +Link: https://lore.kernel.org/r/87tuowcnv3.ffs@nanos.tec.linutronix.de +Signed-off-by: Greg Kroah-Hartman + +--- + kernel/cpu.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 49 insertions(+) + +--- a/kernel/cpu.c ++++ b/kernel/cpu.c +@@ -30,6 +30,7 @@ + #include + #include + #include ++#include + + #include + #define CREATE_TRACE_POINTS +@@ -771,6 +772,52 @@ void __init cpuhp_threads_init(void) + kthread_unpark(this_cpu_read(cpuhp_state.thread)); + } + ++/* ++ * ++ * Serialize hotplug trainwrecks outside of the cpu_hotplug_lock ++ * protected region. ++ * ++ * The operation is still serialized against concurrent CPU hotplug via ++ * cpu_add_remove_lock, i.e. CPU map protection. But it is _not_ ++ * serialized against other hotplug related activity like adding or ++ * removing of state callbacks and state instances, which invoke either the ++ * startup or the teardown callback of the affected state. ++ * ++ * This is required for subsystems which are unfixable vs. CPU hotplug and ++ * evade lock inversion problems by scheduling work which has to be ++ * completed _before_ cpu_up()/_cpu_down() returns. ++ * ++ * Don't even think about adding anything to this for any new code or even ++ * drivers. It's only purpose is to keep existing lock order trainwrecks ++ * working. ++ * ++ * For cpu_down() there might be valid reasons to finish cleanups which are ++ * not required to be done under cpu_hotplug_lock, but that's a different ++ * story and would be not invoked via this. ++ */ ++static void cpu_up_down_serialize_trainwrecks(bool tasks_frozen) ++{ ++ /* ++ * cpusets delegate hotplug operations to a worker to "solve" the ++ * lock order problems. Wait for the worker, but only if tasks are ++ * _not_ frozen (suspend, hibernate) as that would wait forever. ++ * ++ * The wait is required because otherwise the hotplug operation ++ * returns with inconsistent state, which could even be observed in ++ * user space when a new CPU is brought up. The CPU plug uevent ++ * would be delivered and user space reacting on it would fail to ++ * move tasks to the newly plugged CPU up to the point where the ++ * work has finished because up to that point the newly plugged CPU ++ * is not assignable in cpusets/cgroups. On unplug that's not ++ * necessarily a visible issue, but it is still inconsistent state, ++ * which is the real problem which needs to be "fixed". This can't ++ * prevent the transient state between scheduling the work and ++ * returning from waiting for it. ++ */ ++ if (!tasks_frozen) ++ cpuset_wait_for_hotplug(); ++} ++ + #ifdef CONFIG_HOTPLUG_CPU + #ifndef arch_clear_mm_cpumask_cpu + #define arch_clear_mm_cpumask_cpu(cpu, mm) cpumask_clear_cpu(cpu, mm_cpumask(mm)) +@@ -1010,6 +1057,7 @@ out: + */ + lockup_detector_cleanup(); + arch_smt_update(); ++ cpu_up_down_serialize_trainwrecks(tasks_frozen); + return ret; + } + +@@ -1145,6 +1193,7 @@ static int _cpu_up(unsigned int cpu, int + out: + cpus_write_unlock(); + arch_smt_update(); ++ cpu_up_down_serialize_trainwrecks(tasks_frozen); + return ret; + } + diff --git a/queue-4.14/drm-radeon-add-the-missed-drm_gem_object_put-in-radeon_user_framebuffer_create.patch b/queue-4.14/drm-radeon-add-the-missed-drm_gem_object_put-in-radeon_user_framebuffer_create.patch new file mode 100644 index 00000000000..22a18ba470d --- /dev/null +++ b/queue-4.14/drm-radeon-add-the-missed-drm_gem_object_put-in-radeon_user_framebuffer_create.patch @@ -0,0 +1,35 @@ +From 9ba85914c36c8fed9bf3e8b69c0782908c1247b7 Mon Sep 17 00:00:00 2001 +From: Jing Xiangfeng +Date: Tue, 29 Jun 2021 19:44:55 +0800 +Subject: drm/radeon: Add the missed drm_gem_object_put() in radeon_user_framebuffer_create() +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Jing Xiangfeng + +commit 9ba85914c36c8fed9bf3e8b69c0782908c1247b7 upstream. + +radeon_user_framebuffer_create() misses to call drm_gem_object_put() in +an error path. Add the missed function call to fix it. + +Reviewed-by: Christian König +Signed-off-by: Jing Xiangfeng +Signed-off-by: Alex Deucher +Cc: stable@vger.kernel.org +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/gpu/drm/radeon/radeon_display.c | 1 + + 1 file changed, 1 insertion(+) + +--- a/drivers/gpu/drm/radeon/radeon_display.c ++++ b/drivers/gpu/drm/radeon/radeon_display.c +@@ -1347,6 +1347,7 @@ radeon_user_framebuffer_create(struct dr + /* Handle is imported dma-buf, so cannot be migrated to VRAM for scanout */ + if (obj->import_attach) { + DRM_DEBUG_KMS("Cannot create framebuffer from imported dma_buf\n"); ++ drm_gem_object_put(obj); + return ERR_PTR(-EINVAL); + } + diff --git a/queue-4.14/mmc-core-allow-uhs-i-voltage-switch-for-sdsc-cards-if-supported.patch b/queue-4.14/mmc-core-allow-uhs-i-voltage-switch-for-sdsc-cards-if-supported.patch new file mode 100644 index 00000000000..4f8ab91b035 --- /dev/null +++ b/queue-4.14/mmc-core-allow-uhs-i-voltage-switch-for-sdsc-cards-if-supported.patch @@ -0,0 +1,57 @@ +From 09247e110b2efce3a104e57e887c373e0a57a412 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Christian=20L=C3=B6hle?= +Date: Wed, 12 May 2021 16:03:24 +0000 +Subject: mmc: core: Allow UHS-I voltage switch for SDSC cards if supported + +From: Christian Löhle + +commit 09247e110b2efce3a104e57e887c373e0a57a412 upstream. + +While initializing an UHS-I SD card, the mmc core first tries to switch to +1.8V I/O voltage, before it continues to change the settings for the bus +speed mode. + +However, the current behaviour in the mmc core is inconsistent and doesn't +conform to the SD spec. More precisely, an SD card that supports UHS-I must +set both the SD_OCR_CCS bit and the SD_OCR_S18R bit in the OCR register +response. When switching to 1.8V I/O the mmc core correctly checks both of +the bits, but only the SD_OCR_S18R bit when changing the settings for bus +speed mode. + +Rather than actually fixing the code to confirm to the SD spec, let's +deliberately deviate from it by requiring only the SD_OCR_S18R bit for both +parts. This enables us to support UHS-I for SDSC cards (outside spec), +which is actually being supported by some existing SDSC cards. Moreover, +this fixes the inconsistent behaviour. + +Signed-off-by: Christian Loehle +Link: https://lore.kernel.org/r/CWXP265MB26803AE79E0AD5ED083BF2A6C4529@CWXP265MB2680.GBRP265.PROD.OUTLOOK.COM +Cc: stable@vger.kernel.org +[Ulf: Rewrote commit message and comments to clarify the changes] +Signed-off-by: Ulf Hansson +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/mmc/core/sd.c | 10 ++++++---- + 1 file changed, 6 insertions(+), 4 deletions(-) + +--- a/drivers/mmc/core/sd.c ++++ b/drivers/mmc/core/sd.c +@@ -787,11 +787,13 @@ try_again: + return err; + + /* +- * In case CCS and S18A in the response is set, start Signal Voltage +- * Switch procedure. SPI mode doesn't support CMD11. ++ * In case the S18A bit is set in the response, let's start the signal ++ * voltage switch procedure. SPI mode doesn't support CMD11. ++ * Note that, according to the spec, the S18A bit is not valid unless ++ * the CCS bit is set as well. We deliberately deviate from the spec in ++ * regards to this, which allows UHS-I to be supported for SDSC cards. + */ +- if (!mmc_host_is_spi(host) && rocr && +- ((*rocr & 0x41000000) == 0x41000000)) { ++ if (!mmc_host_is_spi(host) && rocr && (*rocr & 0x01000000)) { + err = mmc_set_uhs_voltage(host, pocr); + if (err == -EAGAIN) { + retries--; diff --git a/queue-4.14/mmc-core-clear-flags-before-allowing-to-retune.patch b/queue-4.14/mmc-core-clear-flags-before-allowing-to-retune.patch new file mode 100644 index 00000000000..fcdd8455e43 --- /dev/null +++ b/queue-4.14/mmc-core-clear-flags-before-allowing-to-retune.patch @@ -0,0 +1,56 @@ +From 77347eda64ed5c9383961d1de9165f9d0b7d8df6 Mon Sep 17 00:00:00 2001 +From: Wolfram Sang +Date: Thu, 24 Jun 2021 17:16:14 +0200 +Subject: mmc: core: clear flags before allowing to retune + +From: Wolfram Sang + +commit 77347eda64ed5c9383961d1de9165f9d0b7d8df6 upstream. + +It might be that something goes wrong during tuning so the MMC core will +immediately trigger a retune. In our case it was: + + - we sent a tuning block + - there was an error so we need to send an abort cmd to the eMMC + - the abort cmd had a CRC error + - retune was set by the MMC core + +This lead to a vicious circle causing a performance regression of 75%. +So, clear retuning flags before we enable retuning to start with a known +cleared state. + +Reported-by Yoshihiro Shimoda +Suggested-by: Adrian Hunter +Signed-off-by: Wolfram Sang +Acked-by: Adrian Hunter +Reviewed-by: Yoshihiro Shimoda +Tested-by: Yoshihiro Shimoda +Fixes: bd11e8bd03ca ("mmc: core: Flag re-tuning is needed on CRC errors") +Cc: stable@vger.kernel.org +Link: https://lore.kernel.org/r/20210624151616.38770-2-wsa+renesas@sang-engineering.com +Signed-off-by: Ulf Hansson +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/mmc/core/core.c | 7 +++++-- + 1 file changed, 5 insertions(+), 2 deletions(-) + +--- a/drivers/mmc/core/core.c ++++ b/drivers/mmc/core/core.c +@@ -992,11 +992,14 @@ int mmc_execute_tuning(struct mmc_card * + + err = host->ops->execute_tuning(host, opcode); + +- if (err) ++ if (err) { + pr_err("%s: tuning execution failed: %d\n", + mmc_hostname(host), err); +- else ++ } else { ++ host->retune_now = 0; ++ host->need_retune = 0; + mmc_retune_enable(host); ++ } + + return err; + } diff --git a/queue-4.14/mmc-sdhci-fix-warning-message-when-accessing-rpmb-in-hs400-mode.patch b/queue-4.14/mmc-sdhci-fix-warning-message-when-accessing-rpmb-in-hs400-mode.patch new file mode 100644 index 00000000000..04083875a43 --- /dev/null +++ b/queue-4.14/mmc-sdhci-fix-warning-message-when-accessing-rpmb-in-hs400-mode.patch @@ -0,0 +1,61 @@ +From d0244847f9fc5e20df8b7483c8a4717fe0432d38 Mon Sep 17 00:00:00 2001 +From: Al Cooper +Date: Thu, 24 Jun 2021 12:30:45 -0400 +Subject: mmc: sdhci: Fix warning message when accessing RPMB in HS400 mode + +From: Al Cooper + +commit d0244847f9fc5e20df8b7483c8a4717fe0432d38 upstream. + +When an eMMC device is being run in HS400 mode, any access to the +RPMB device will cause the error message "mmc1: Invalid UHS-I mode +selected". This happens as a result of tuning being disabled before +RPMB access and then re-enabled after the RPMB access is complete. +When tuning is re-enabled, the system has to switch from HS400 +to HS200 to do the tuning and then back to HS400. As part of +sequence to switch from HS400 to HS200 the system is temporarily +put into HS mode. When switching to HS mode, sdhci_get_preset_value() +is called and does not have support for HS mode and prints the warning +message and returns the preset for SDR12. The fix is to add support +for MMC and SD HS modes to sdhci_get_preset_value(). + +This can be reproduced on any system running eMMC in HS400 mode +(not HS400ES) by using the "mmc" utility to run the following +command: "mmc rpmb read-counter /dev/mmcblk0rpmb". + +Signed-off-by: Al Cooper +Acked-by: Adrian Hunter +Fixes: 52983382c74f ("mmc: sdhci: enhance preset value function") +Cc: stable@vger.kernel.org +Link: https://lore.kernel.org/r/20210624163045.33651-1-alcooperx@gmail.com +Signed-off-by: Ulf Hansson +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/mmc/host/sdhci.c | 4 ++++ + drivers/mmc/host/sdhci.h | 1 + + 2 files changed, 5 insertions(+) + +--- a/drivers/mmc/host/sdhci.c ++++ b/drivers/mmc/host/sdhci.c +@@ -1293,6 +1293,10 @@ static u16 sdhci_get_preset_value(struct + u16 preset = 0; + + switch (host->timing) { ++ case MMC_TIMING_MMC_HS: ++ case MMC_TIMING_SD_HS: ++ preset = sdhci_readw(host, SDHCI_PRESET_FOR_HIGH_SPEED); ++ break; + case MMC_TIMING_UHS_SDR12: + preset = sdhci_readw(host, SDHCI_PRESET_FOR_SDR12); + break; +--- a/drivers/mmc/host/sdhci.h ++++ b/drivers/mmc/host/sdhci.h +@@ -252,6 +252,7 @@ + + /* 60-FB reserved */ + ++#define SDHCI_PRESET_FOR_HIGH_SPEED 0x64 + #define SDHCI_PRESET_FOR_SDR12 0x66 + #define SDHCI_PRESET_FOR_SDR25 0x68 + #define SDHCI_PRESET_FOR_SDR50 0x6A diff --git a/queue-4.14/pinctrl-amd-add-device-hid-for-new-amd-gpio-controller.patch b/queue-4.14/pinctrl-amd-add-device-hid-for-new-amd-gpio-controller.patch new file mode 100644 index 00000000000..b448688214b --- /dev/null +++ b/queue-4.14/pinctrl-amd-add-device-hid-for-new-amd-gpio-controller.patch @@ -0,0 +1,34 @@ +From 1ca46d3e43569186bd1decfb02a6b4c4ddb4304b Mon Sep 17 00:00:00 2001 +From: Maximilian Luz +Date: Wed, 12 May 2021 23:03:16 +0200 +Subject: pinctrl/amd: Add device HID for new AMD GPIO controller + +From: Maximilian Luz + +commit 1ca46d3e43569186bd1decfb02a6b4c4ddb4304b upstream. + +Add device HID AMDI0031 to the AMD GPIO controller driver match table. +This controller can be found on Microsoft Surface Laptop 4 devices and +seems similar enough that we can just copy the existing AMDI0030 entry. + +Cc: # 5.10+ +Tested-by: Sachi King +Signed-off-by: Maximilian Luz +Link: https://lore.kernel.org/r/20210512210316.1982416-1-luzmaximilian@gmail.com +Signed-off-by: Linus Walleij +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/pinctrl/pinctrl-amd.c | 1 + + 1 file changed, 1 insertion(+) + +--- a/drivers/pinctrl/pinctrl-amd.c ++++ b/drivers/pinctrl/pinctrl-amd.c +@@ -920,6 +920,7 @@ static int amd_gpio_remove(struct platfo + static const struct acpi_device_id amd_gpio_acpi_match[] = { + { "AMD0030", 0 }, + { "AMDI0030", 0}, ++ { "AMDI0031", 0}, + { }, + }; + MODULE_DEVICE_TABLE(acpi, amd_gpio_acpi_match); diff --git a/queue-4.14/series b/queue-4.14/series index d3443c8f289..200f9ea5d53 100644 --- a/queue-4.14/series +++ b/queue-4.14/series @@ -205,3 +205,11 @@ bdi-do-not-use-freezable-workqueue.patch fuse-reject-internal-errno.patch mac80211-fix-memory-corruption-in-eapol-handling.patch powerpc-barrier-avoid-collision-with-clang-s-__lwsync-macro.patch +usb-gadget-f_fs-fix-setting-of-device-and-driver-data-cross-references.patch +drm-radeon-add-the-missed-drm_gem_object_put-in-radeon_user_framebuffer_create.patch +pinctrl-amd-add-device-hid-for-new-amd-gpio-controller.patch +mmc-sdhci-fix-warning-message-when-accessing-rpmb-in-hs400-mode.patch +mmc-core-clear-flags-before-allowing-to-retune.patch +mmc-core-allow-uhs-i-voltage-switch-for-sdsc-cards-if-supported.patch +ata-ahci_sunxi-disable-dipm.patch +cpu-hotplug-cure-the-cpusets-trainwreck.patch diff --git a/queue-4.14/usb-gadget-f_fs-fix-setting-of-device-and-driver-data-cross-references.patch b/queue-4.14/usb-gadget-f_fs-fix-setting-of-device-and-driver-data-cross-references.patch new file mode 100644 index 00000000000..78a3480cea1 --- /dev/null +++ b/queue-4.14/usb-gadget-f_fs-fix-setting-of-device-and-driver-data-cross-references.patch @@ -0,0 +1,563 @@ +From ecfbd7b9054bddb12cea07fda41bb3a79a7b0149 Mon Sep 17 00:00:00 2001 +From: Andrew Gabbasov +Date: Thu, 3 Jun 2021 12:15:07 -0500 +Subject: usb: gadget: f_fs: Fix setting of device and driver data cross-references + +From: Andrew Gabbasov + +commit ecfbd7b9054bddb12cea07fda41bb3a79a7b0149 upstream. + +FunctionFS device structure 'struct ffs_dev' and driver data structure +'struct ffs_data' are bound to each other with cross-reference pointers +'ffs_data->private_data' and 'ffs_dev->ffs_data'. While the first one +is supposed to be valid through the whole life of 'struct ffs_data' +(and while 'struct ffs_dev' exists non-freed), the second one is cleared +in 'ffs_closed()' (called from 'ffs_data_reset()' or the last +'ffs_data_put()'). This can be called several times, alternating in +different order with 'ffs_free_inst()', that, if possible, clears +the other cross-reference. + +As a result, different cases of these calls order may leave stale +cross-reference pointers, used when the pointed structure is already +freed. Even if it occasionally doesn't cause kernel crash, this error +is reported by KASAN-enabled kernel configuration. + +For example, the case [last 'ffs_data_put()' - 'ffs_free_inst()'] was +fixed by commit cdafb6d8b8da ("usb: gadget: f_fs: Fix use-after-free in +ffs_free_inst"). + +The other case ['ffs_data_reset()' - 'ffs_free_inst()' - 'ffs_data_put()'] +now causes KASAN reported error [1], when 'ffs_data_reset()' clears +'ffs_dev->ffs_data', then 'ffs_free_inst()' frees the 'struct ffs_dev', +but can't clear 'ffs_data->private_data', which is then accessed +in 'ffs_closed()' called from 'ffs_data_put()'. This happens since +'ffs_dev->ffs_data' reference is cleared too early. + +Moreover, one more use case, when 'ffs_free_inst()' is called immediately +after mounting FunctionFS device (that is before the descriptors are +written and 'ffs_ready()' is called), and then 'ffs_data_reset()' +or 'ffs_data_put()' is called from accessing "ep0" file or unmounting +the device. This causes KASAN error report like [2], since +'ffs_dev->ffs_data' is not yet set when 'ffs_free_inst()' can't properly +clear 'ffs_data->private_data', that is later accessed to freed structure. + +Fix these (and may be other) cases of stale pointers access by moving +setting and clearing of the mentioned cross-references to the single +places, setting both of them when 'struct ffs_data' is created and +bound to 'struct ffs_dev', and clearing both of them when one of the +structures is destroyed. It seems convenient to make this pointer +initialization and structures binding in 'ffs_acquire_dev()' and +make pointers clearing in 'ffs_release_dev()'. This required some +changes in these functions parameters and return types. + +Also, 'ffs_release_dev()' calling requires some cleanup, fixing minor +issues, like (1) 'ffs_release_dev()' is not called if 'ffs_free_inst()' +is called without unmounting the device, and "release_dev" callback +is not called at all, or (2) "release_dev" callback is called before +"ffs_closed" callback on unmounting, which seems to be not correctly +nested with "acquire_dev" and "ffs_ready" callbacks. +Make this cleanup togther with other mentioned 'ffs_release_dev()' changes. + +[1] +================================================================== +root@rcar-gen3:~# mkdir /dev/cfs +root@rcar-gen3:~# mkdir /dev/ffs +root@rcar-gen3:~# modprobe libcomposite +root@rcar-gen3:~# mount -t configfs none /dev/cfs +root@rcar-gen3:~# mkdir /dev/cfs/usb_gadget/g1 +root@rcar-gen3:~# mkdir /dev/cfs/usb_gadget/g1/functions/ffs.ffs +[ 64.340664] file system registered +root@rcar-gen3:~# mount -t functionfs ffs /dev/ffs +root@rcar-gen3:~# cd /dev/ffs +root@rcar-gen3:/dev/ffs# /home/root/ffs-test +ffs-test: info: ep0: writing descriptors (in v2 format) +[ 83.181442] read descriptors +[ 83.186085] read strings +ffs-test: info: ep0: writing strings +ffs-test: dbg: ep1: starting +ffs-test: dbg: ep2: starting +ffs-test: info: ep1: starts +ffs-test: info: ep2: starts +ffs-test: info: ep0: starts + +^C +root@rcar-gen3:/dev/ffs# cd /home/root/ +root@rcar-gen3:~# rmdir /dev/cfs/usb_gadget/g1/functions/ffs.ffs +[ 98.935061] unloading +root@rcar-gen3:~# umount /dev/ffs +[ 102.734301] ================================================================== +[ 102.742059] BUG: KASAN: use-after-free in ffs_release_dev+0x64/0xa8 [usb_f_fs] +[ 102.749683] Write of size 1 at addr ffff0004d46ff549 by task umount/2997 +[ 102.756709] +[ 102.758311] CPU: 0 PID: 2997 Comm: umount Not tainted 5.13.0-rc4+ #8 +[ 102.764971] Hardware name: Renesas Salvator-X board based on r8a77951 (DT) +[ 102.772179] Call trace: +[ 102.774779] dump_backtrace+0x0/0x330 +[ 102.778653] show_stack+0x20/0x2c +[ 102.782152] dump_stack+0x11c/0x1ac +[ 102.785833] print_address_description.constprop.0+0x30/0x274 +[ 102.791862] kasan_report+0x14c/0x1c8 +[ 102.795719] __asan_report_store1_noabort+0x34/0x58 +[ 102.800840] ffs_release_dev+0x64/0xa8 [usb_f_fs] +[ 102.805801] ffs_fs_kill_sb+0x50/0x84 [usb_f_fs] +[ 102.810663] deactivate_locked_super+0xa0/0xf0 +[ 102.815339] deactivate_super+0x98/0xac +[ 102.819378] cleanup_mnt+0xd0/0x1b0 +[ 102.823057] __cleanup_mnt+0x1c/0x28 +[ 102.826823] task_work_run+0x104/0x180 +[ 102.830774] do_notify_resume+0x458/0x14e0 +[ 102.835083] work_pending+0xc/0x5f8 +[ 102.838762] +[ 102.840357] Allocated by task 2988: +[ 102.844032] kasan_save_stack+0x28/0x58 +[ 102.848071] kasan_set_track+0x28/0x3c +[ 102.852016] ____kasan_kmalloc+0x84/0x9c +[ 102.856142] __kasan_kmalloc+0x10/0x1c +[ 102.860088] __kmalloc+0x214/0x2f8 +[ 102.863678] kzalloc.constprop.0+0x14/0x20 [usb_f_fs] +[ 102.868990] ffs_alloc_inst+0x8c/0x208 [usb_f_fs] +[ 102.873942] try_get_usb_function_instance+0xf0/0x164 [libcomposite] +[ 102.880629] usb_get_function_instance+0x64/0x68 [libcomposite] +[ 102.886858] function_make+0x128/0x1ec [libcomposite] +[ 102.892185] configfs_mkdir+0x330/0x590 [configfs] +[ 102.897245] vfs_mkdir+0x12c/0x1bc +[ 102.900835] do_mkdirat+0x180/0x1d0 +[ 102.904513] __arm64_sys_mkdirat+0x80/0x94 +[ 102.908822] invoke_syscall+0xf8/0x25c +[ 102.912772] el0_svc_common.constprop.0+0x150/0x1a0 +[ 102.917891] do_el0_svc+0xa0/0xd4 +[ 102.921386] el0_svc+0x24/0x34 +[ 102.924613] el0_sync_handler+0xcc/0x154 +[ 102.928743] el0_sync+0x198/0x1c0 +[ 102.932238] +[ 102.933832] Freed by task 2996: +[ 102.937144] kasan_save_stack+0x28/0x58 +[ 102.941181] kasan_set_track+0x28/0x3c +[ 102.945128] kasan_set_free_info+0x28/0x4c +[ 102.949435] ____kasan_slab_free+0x104/0x118 +[ 102.953921] __kasan_slab_free+0x18/0x24 +[ 102.958047] slab_free_freelist_hook+0x148/0x1f0 +[ 102.962897] kfree+0x318/0x440 +[ 102.966123] ffs_free_inst+0x164/0x2d8 [usb_f_fs] +[ 102.971075] usb_put_function_instance+0x84/0xa4 [libcomposite] +[ 102.977302] ffs_attr_release+0x18/0x24 [usb_f_fs] +[ 102.982344] config_item_put+0x140/0x1a4 [configfs] +[ 102.987486] configfs_rmdir+0x3fc/0x518 [configfs] +[ 102.992535] vfs_rmdir+0x114/0x234 +[ 102.996122] do_rmdir+0x274/0x2b0 +[ 102.999617] __arm64_sys_unlinkat+0x94/0xc8 +[ 103.004015] invoke_syscall+0xf8/0x25c +[ 103.007961] el0_svc_common.constprop.0+0x150/0x1a0 +[ 103.013080] do_el0_svc+0xa0/0xd4 +[ 103.016575] el0_svc+0x24/0x34 +[ 103.019801] el0_sync_handler+0xcc/0x154 +[ 103.023930] el0_sync+0x198/0x1c0 +[ 103.027426] +[ 103.029020] The buggy address belongs to the object at ffff0004d46ff500 +[ 103.029020] which belongs to the cache kmalloc-128 of size 128 +[ 103.042079] The buggy address is located 73 bytes inside of +[ 103.042079] 128-byte region [ffff0004d46ff500, ffff0004d46ff580) +[ 103.054236] The buggy address belongs to the page: +[ 103.059262] page:0000000021aa849b refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff0004d46fee00 pfn:0x5146fe +[ 103.070437] head:0000000021aa849b order:1 compound_mapcount:0 +[ 103.076456] flags: 0x8000000000010200(slab|head|zone=2) +[ 103.081948] raw: 8000000000010200 fffffc0013521a80 0000000d0000000d ffff0004c0002300 +[ 103.090052] raw: ffff0004d46fee00 000000008020001e 00000001ffffffff 0000000000000000 +[ 103.098150] page dumped because: kasan: bad access detected +[ 103.103985] +[ 103.105578] Memory state around the buggy address: +[ 103.110602] ffff0004d46ff400: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb +[ 103.118161] ffff0004d46ff480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc +[ 103.125726] >ffff0004d46ff500: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb +[ 103.133284] ^ +[ 103.139120] ffff0004d46ff580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc +[ 103.146679] ffff0004d46ff600: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb +[ 103.154238] ================================================================== +[ 103.161792] Disabling lock debugging due to kernel taint +[ 103.167319] Unable to handle kernel paging request at virtual address 0037801d6000018e +[ 103.175406] Mem abort info: +[ 103.178457] ESR = 0x96000004 +[ 103.181609] EC = 0x25: DABT (current EL), IL = 32 bits +[ 103.187020] SET = 0, FnV = 0 +[ 103.190185] EA = 0, S1PTW = 0 +[ 103.193417] Data abort info: +[ 103.196385] ISV = 0, ISS = 0x00000004 +[ 103.200315] CM = 0, WnR = 0 +[ 103.203366] [0037801d6000018e] address between user and kernel address ranges +[ 103.210611] Internal error: Oops: 96000004 [#1] PREEMPT SMP +[ 103.216231] Modules linked in: usb_f_fs libcomposite configfs ath9k_htc led_class mac80211 libarc4 ath9k_common ath9k_hw ath cfg80211 aes_ce_blk sata_rc4 +[ 103.259233] CPU: 0 PID: 2997 Comm: umount Tainted: G B 5.13.0-rc4+ #8 +[ 103.267031] Hardware name: Renesas Salvator-X board based on r8a77951 (DT) +[ 103.273951] pstate: 00000005 (nzcv daif -PAN -UAO -TCO BTYPE=--) +[ 103.280001] pc : ffs_data_clear+0x138/0x370 [usb_f_fs] +[ 103.285197] lr : ffs_data_clear+0x124/0x370 [usb_f_fs] +[ 103.290385] sp : ffff800014777a80 +[ 103.293725] x29: ffff800014777a80 x28: ffff0004d7649c80 x27: 0000000000000000 +[ 103.300931] x26: ffff800014777fb0 x25: ffff60009aec9394 x24: ffff0004d7649ca4 +[ 103.308136] x23: 1fffe0009a3d063a x22: dfff800000000000 x21: ffff0004d1e831d0 +[ 103.315340] x20: e1c000eb00000bb4 x19: ffff0004d1e83000 x18: 0000000000000000 +[ 103.322545] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 +[ 103.329748] x14: 0720072007200720 x13: 0720072007200720 x12: 1ffff000012ef658 +[ 103.336952] x11: ffff7000012ef658 x10: 0720072007200720 x9 : ffff800011322648 +[ 103.344157] x8 : ffff800014777818 x7 : ffff80000977b2c7 x6 : 0000000000000000 +[ 103.351359] x5 : 0000000000000001 x4 : ffff7000012ef659 x3 : 0000000000000001 +[ 103.358562] x2 : 0000000000000000 x1 : 1c38001d6000018e x0 : e1c000eb00000c70 +[ 103.365766] Call trace: +[ 103.368235] ffs_data_clear+0x138/0x370 [usb_f_fs] +[ 103.373076] ffs_data_reset+0x20/0x304 [usb_f_fs] +[ 103.377829] ffs_data_closed+0x1ec/0x244 [usb_f_fs] +[ 103.382755] ffs_fs_kill_sb+0x70/0x84 [usb_f_fs] +[ 103.387420] deactivate_locked_super+0xa0/0xf0 +[ 103.391905] deactivate_super+0x98/0xac +[ 103.395776] cleanup_mnt+0xd0/0x1b0 +[ 103.399299] __cleanup_mnt+0x1c/0x28 +[ 103.402906] task_work_run+0x104/0x180 +[ 103.406691] do_notify_resume+0x458/0x14e0 +[ 103.410823] work_pending+0xc/0x5f8 +[ 103.414351] Code: b4000a54 9102f280 12000802 d343fc01 (38f66821) +[ 103.420490] ---[ end trace 57b43a50e8244f57 ]--- +Segmentation fault +root@rcar-gen3:~# +================================================================== + +[2] +================================================================== +root@rcar-gen3:~# mkdir /dev/ffs +root@rcar-gen3:~# modprobe libcomposite +root@rcar-gen3:~# +root@rcar-gen3:~# mount -t configfs none /dev/cfs +root@rcar-gen3:~# mkdir /dev/cfs/usb_gadget/g1 +root@rcar-gen3:~# mkdir /dev/cfs/usb_gadget/g1/functions/ffs.ffs +[ 54.766480] file system registered +root@rcar-gen3:~# mount -t functionfs ffs /dev/ffs +root@rcar-gen3:~# rmdir /dev/cfs/usb_gadget/g1/functions/ffs.ffs +[ 63.197597] unloading +root@rcar-gen3:~# cat /dev/ffs/ep0 +cat: read error:[ 67.213506] ================================================================== +[ 67.222095] BUG: KASAN: use-after-free in ffs_data_clear+0x70/0x370 [usb_f_fs] +[ 67.229699] Write of size 1 at addr ffff0004c26e974a by task cat/2994 +[ 67.236446] +[ 67.238045] CPU: 0 PID: 2994 Comm: cat Not tainted 5.13.0-rc4+ #8 +[ 67.244431] Hardware name: Renesas Salvator-X board based on r8a77951 (DT) +[ 67.251624] Call trace: +[ 67.254212] dump_backtrace+0x0/0x330 +[ 67.258081] show_stack+0x20/0x2c +[ 67.261579] dump_stack+0x11c/0x1ac +[ 67.265260] print_address_description.constprop.0+0x30/0x274 +[ 67.271286] kasan_report+0x14c/0x1c8 +[ 67.275143] __asan_report_store1_noabort+0x34/0x58 +[ 67.280265] ffs_data_clear+0x70/0x370 [usb_f_fs] +[ 67.285220] ffs_data_reset+0x20/0x304 [usb_f_fs] +[ 67.290172] ffs_data_closed+0x240/0x244 [usb_f_fs] +[ 67.295305] ffs_ep0_release+0x40/0x54 [usb_f_fs] +[ 67.300256] __fput+0x304/0x580 +[ 67.303576] ____fput+0x18/0x24 +[ 67.306893] task_work_run+0x104/0x180 +[ 67.310846] do_notify_resume+0x458/0x14e0 +[ 67.315154] work_pending+0xc/0x5f8 +[ 67.318834] +[ 67.320429] Allocated by task 2988: +[ 67.324105] kasan_save_stack+0x28/0x58 +[ 67.328144] kasan_set_track+0x28/0x3c +[ 67.332090] ____kasan_kmalloc+0x84/0x9c +[ 67.336217] __kasan_kmalloc+0x10/0x1c +[ 67.340163] __kmalloc+0x214/0x2f8 +[ 67.343754] kzalloc.constprop.0+0x14/0x20 [usb_f_fs] +[ 67.349066] ffs_alloc_inst+0x8c/0x208 [usb_f_fs] +[ 67.354017] try_get_usb_function_instance+0xf0/0x164 [libcomposite] +[ 67.360705] usb_get_function_instance+0x64/0x68 [libcomposite] +[ 67.366934] function_make+0x128/0x1ec [libcomposite] +[ 67.372260] configfs_mkdir+0x330/0x590 [configfs] +[ 67.377320] vfs_mkdir+0x12c/0x1bc +[ 67.380911] do_mkdirat+0x180/0x1d0 +[ 67.384589] __arm64_sys_mkdirat+0x80/0x94 +[ 67.388899] invoke_syscall+0xf8/0x25c +[ 67.392850] el0_svc_common.constprop.0+0x150/0x1a0 +[ 67.397969] do_el0_svc+0xa0/0xd4 +[ 67.401464] el0_svc+0x24/0x34 +[ 67.404691] el0_sync_handler+0xcc/0x154 +[ 67.408819] el0_sync+0x198/0x1c0 +[ 67.412315] +[ 67.413909] Freed by task 2993: +[ 67.417220] kasan_save_stack+0x28/0x58 +[ 67.421257] kasan_set_track+0x28/0x3c +[ 67.425204] kasan_set_free_info+0x28/0x4c +[ 67.429513] ____kasan_slab_free+0x104/0x118 +[ 67.434001] __kasan_slab_free+0x18/0x24 +[ 67.438128] slab_free_freelist_hook+0x148/0x1f0 +[ 67.442978] kfree+0x318/0x440 +[ 67.446205] ffs_free_inst+0x164/0x2d8 [usb_f_fs] +[ 67.451156] usb_put_function_instance+0x84/0xa4 [libcomposite] +[ 67.457385] ffs_attr_release+0x18/0x24 [usb_f_fs] +[ 67.462428] config_item_put+0x140/0x1a4 [configfs] +[ 67.467570] configfs_rmdir+0x3fc/0x518 [configfs] +[ 67.472626] vfs_rmdir+0x114/0x234 +[ 67.476215] do_rmdir+0x274/0x2b0 +[ 67.479710] __arm64_sys_unlinkat+0x94/0xc8 +[ 67.484108] invoke_syscall+0xf8/0x25c +[ 67.488055] el0_svc_common.constprop.0+0x150/0x1a0 +[ 67.493175] do_el0_svc+0xa0/0xd4 +[ 67.496671] el0_svc+0x24/0x34 +[ 67.499896] el0_sync_handler+0xcc/0x154 +[ 67.504024] el0_sync+0x198/0x1c0 +[ 67.507520] +[ 67.509114] The buggy address belongs to the object at ffff0004c26e9700 +[ 67.509114] which belongs to the cache kmalloc-128 of size 128 +[ 67.522171] The buggy address is located 74 bytes inside of +[ 67.522171] 128-byte region [ffff0004c26e9700, ffff0004c26e9780) +[ 67.534328] The buggy address belongs to the page: +[ 67.539355] page:000000003177a217 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x5026e8 +[ 67.549175] head:000000003177a217 order:1 compound_mapcount:0 +[ 67.555195] flags: 0x8000000000010200(slab|head|zone=2) +[ 67.560687] raw: 8000000000010200 fffffc0013037100 0000000c00000002 ffff0004c0002300 +[ 67.568791] raw: 0000000000000000 0000000080200020 00000001ffffffff 0000000000000000 +[ 67.576890] page dumped because: kasan: bad access detected +[ 67.582725] +[ 67.584318] Memory state around the buggy address: +[ 67.589343] ffff0004c26e9600: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb +[ 67.596903] ffff0004c26e9680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc +[ 67.604463] >ffff0004c26e9700: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb +[ 67.612022] ^ +[ 67.617860] ffff0004c26e9780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc +[ 67.625421] ffff0004c26e9800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 +[ 67.632981] ================================================================== +[ 67.640535] Disabling lock debugging due to kernel taint + File descriptor[ 67.646100] Unable to handle kernel paging request at virtual address fabb801d4000018d + in bad state +[ 67.655456] Mem abort info: +[ 67.659619] ESR = 0x96000004 +[ 67.662801] EC = 0x25: DABT (current EL), IL = 32 bits +[ 67.668225] SET = 0, FnV = 0 +[ 67.671375] EA = 0, S1PTW = 0 +[ 67.674613] Data abort info: +[ 67.677587] ISV = 0, ISS = 0x00000004 +[ 67.681522] CM = 0, WnR = 0 +[ 67.684588] [fabb801d4000018d] address between user and kernel address ranges +[ 67.691849] Internal error: Oops: 96000004 [#1] PREEMPT SMP +[ 67.697470] Modules linked in: usb_f_fs libcomposite configfs ath9k_htc led_class mac80211 libarc4 ath9k_common ath9k_hw ath cfg80211 aes_ce_blk crypto_simd cryptd aes_ce_cipher ghash_ce gf128mul sha2_ce sha1_ce evdev sata_rcar libata xhci_plat_hcd scsi_mod xhci_hcd rene4 +[ 67.740467] CPU: 0 PID: 2994 Comm: cat Tainted: G B 5.13.0-rc4+ #8 +[ 67.748005] Hardware name: Renesas Salvator-X board based on r8a77951 (DT) +[ 67.754924] pstate: 00000005 (nzcv daif -PAN -UAO -TCO BTYPE=--) +[ 67.760974] pc : ffs_data_clear+0x138/0x370 [usb_f_fs] +[ 67.766178] lr : ffs_data_clear+0x124/0x370 [usb_f_fs] +[ 67.771365] sp : ffff800014767ad0 +[ 67.774706] x29: ffff800014767ad0 x28: ffff800009cf91c0 x27: ffff0004c54861a0 +[ 67.781913] x26: ffff0004dc90b288 x25: 1fffe00099ec10f5 x24: 00000000000a801d +[ 67.789118] x23: 1fffe00099f6953a x22: dfff800000000000 x21: ffff0004cfb4a9d0 +[ 67.796322] x20: d5e000ea00000bb1 x19: ffff0004cfb4a800 x18: 0000000000000000 +[ 67.803526] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 +[ 67.810730] x14: 0720072007200720 x13: 0720072007200720 x12: 1ffff000028ecefa +[ 67.817934] x11: ffff7000028ecefa x10: 0720072007200720 x9 : ffff80001132c014 +[ 67.825137] x8 : ffff8000147677d8 x7 : ffff8000147677d7 x6 : 0000000000000000 +[ 67.832341] x5 : 0000000000000001 x4 : ffff7000028ecefb x3 : 0000000000000001 +[ 67.839544] x2 : 0000000000000005 x1 : 1abc001d4000018d x0 : d5e000ea00000c6d +[ 67.846748] Call trace: +[ 67.849218] ffs_data_clear+0x138/0x370 [usb_f_fs] +[ 67.854058] ffs_data_reset+0x20/0x304 [usb_f_fs] +[ 67.858810] ffs_data_closed+0x240/0x244 [usb_f_fs] +[ 67.863736] ffs_ep0_release+0x40/0x54 [usb_f_fs] +[ 67.868488] __fput+0x304/0x580 +[ 67.871665] ____fput+0x18/0x24 +[ 67.874837] task_work_run+0x104/0x180 +[ 67.878622] do_notify_resume+0x458/0x14e0 +[ 67.882754] work_pending+0xc/0x5f8 +[ 67.886282] Code: b4000a54 9102f280 12000802 d343fc01 (38f66821) +[ 67.892422] ---[ end trace 6d7cedf53d7abbea ]--- +Segmentation fault +root@rcar-gen3:~# +================================================================== + +Fixes: 4b187fceec3c ("usb: gadget: FunctionFS: add devices management code") +Fixes: 3262ad824307 ("usb: gadget: f_fs: Stop ffs_closed NULL pointer dereference") +Fixes: cdafb6d8b8da ("usb: gadget: f_fs: Fix use-after-free in ffs_free_inst") +Reported-by: Bhuvanesh Surachari +Tested-by: Eugeniu Rosca +Reviewed-by: Eugeniu Rosca +Signed-off-by: Andrew Gabbasov +Link: https://lore.kernel.org/r/20210603171507.22514-1-andrew_gabbasov@mentor.com +[agabbasov: Backported to earlier mount API, resolved context conflicts] +Signed-off-by: Greg Kroah-Hartman +--- + drivers/usb/gadget/function/f_fs.c | 67 +++++++++++++++++-------------------- + 1 file changed, 32 insertions(+), 35 deletions(-) + +--- a/drivers/usb/gadget/function/f_fs.c ++++ b/drivers/usb/gadget/function/f_fs.c +@@ -247,8 +247,8 @@ EXPORT_SYMBOL_GPL(ffs_lock); + static struct ffs_dev *_ffs_find_dev(const char *name); + static struct ffs_dev *_ffs_alloc_dev(void); + static void _ffs_free_dev(struct ffs_dev *dev); +-static void *ffs_acquire_dev(const char *dev_name); +-static void ffs_release_dev(struct ffs_data *ffs_data); ++static int ffs_acquire_dev(const char *dev_name, struct ffs_data *ffs_data); ++static void ffs_release_dev(struct ffs_dev *ffs_dev); + static int ffs_ready(struct ffs_data *ffs); + static void ffs_closed(struct ffs_data *ffs); + +@@ -1505,7 +1505,6 @@ ffs_fs_mount(struct file_system_type *t, + }; + struct dentry *rv; + int ret; +- void *ffs_dev; + struct ffs_data *ffs; + + ENTER(); +@@ -1526,19 +1525,16 @@ ffs_fs_mount(struct file_system_type *t, + return ERR_PTR(-ENOMEM); + } + +- ffs_dev = ffs_acquire_dev(dev_name); +- if (IS_ERR(ffs_dev)) { ++ ret = ffs_acquire_dev(dev_name, ffs); ++ if (ret) { + ffs_data_put(ffs); +- return ERR_CAST(ffs_dev); ++ return ERR_PTR(ret); + } +- ffs->private_data = ffs_dev; + data.ffs_data = ffs; + + rv = mount_nodev(t, flags, &data, ffs_sb_fill); +- if (IS_ERR(rv) && data.ffs_data) { +- ffs_release_dev(data.ffs_data); ++ if (IS_ERR(rv) && data.ffs_data) + ffs_data_put(data.ffs_data); +- } + return rv; + } + +@@ -1548,10 +1544,8 @@ ffs_fs_kill_sb(struct super_block *sb) + ENTER(); + + kill_litter_super(sb); +- if (sb->s_fs_info) { +- ffs_release_dev(sb->s_fs_info); ++ if (sb->s_fs_info) + ffs_data_closed(sb->s_fs_info); +- } + } + + static struct file_system_type ffs_fs_type = { +@@ -1620,6 +1614,7 @@ static void ffs_data_put(struct ffs_data + if (unlikely(refcount_dec_and_test(&ffs->ref))) { + pr_info("%s(): freeing\n", __func__); + ffs_data_clear(ffs); ++ ffs_release_dev(ffs->private_data); + BUG_ON(waitqueue_active(&ffs->ev.waitq) || + waitqueue_active(&ffs->ep0req_completion.wait) || + waitqueue_active(&ffs->wait)); +@@ -2924,6 +2919,7 @@ static inline struct f_fs_opts *ffs_do_f + struct ffs_function *func = ffs_func_from_usb(f); + struct f_fs_opts *ffs_opts = + container_of(f->fi, struct f_fs_opts, func_inst); ++ struct ffs_data *ffs_data; + int ret; + + ENTER(); +@@ -2938,12 +2934,13 @@ static inline struct f_fs_opts *ffs_do_f + if (!ffs_opts->no_configfs) + ffs_dev_lock(); + ret = ffs_opts->dev->desc_ready ? 0 : -ENODEV; +- func->ffs = ffs_opts->dev->ffs_data; ++ ffs_data = ffs_opts->dev->ffs_data; + if (!ffs_opts->no_configfs) + ffs_dev_unlock(); + if (ret) + return ERR_PTR(ret); + ++ func->ffs = ffs_data; + func->conf = c; + func->gadget = c->cdev->gadget; + +@@ -3398,6 +3395,7 @@ static void ffs_free_inst(struct usb_fun + struct f_fs_opts *opts; + + opts = to_f_fs_opts(f); ++ ffs_release_dev(opts->dev); + ffs_dev_lock(); + _ffs_free_dev(opts->dev); + ffs_dev_unlock(); +@@ -3585,47 +3583,48 @@ static void _ffs_free_dev(struct ffs_dev + { + list_del(&dev->entry); + +- /* Clear the private_data pointer to stop incorrect dev access */ +- if (dev->ffs_data) +- dev->ffs_data->private_data = NULL; +- + kfree(dev); + if (list_empty(&ffs_devices)) + functionfs_cleanup(); + } + +-static void *ffs_acquire_dev(const char *dev_name) ++static int ffs_acquire_dev(const char *dev_name, struct ffs_data *ffs_data) + { ++ int ret = 0; + struct ffs_dev *ffs_dev; + + ENTER(); + ffs_dev_lock(); + + ffs_dev = _ffs_find_dev(dev_name); +- if (!ffs_dev) +- ffs_dev = ERR_PTR(-ENOENT); +- else if (ffs_dev->mounted) +- ffs_dev = ERR_PTR(-EBUSY); +- else if (ffs_dev->ffs_acquire_dev_callback && +- ffs_dev->ffs_acquire_dev_callback(ffs_dev)) +- ffs_dev = ERR_PTR(-ENOENT); +- else ++ if (!ffs_dev) { ++ ret = -ENOENT; ++ } else if (ffs_dev->mounted) { ++ ret = -EBUSY; ++ } else if (ffs_dev->ffs_acquire_dev_callback && ++ ffs_dev->ffs_acquire_dev_callback(ffs_dev)) { ++ ret = -ENOENT; ++ } else { + ffs_dev->mounted = true; ++ ffs_dev->ffs_data = ffs_data; ++ ffs_data->private_data = ffs_dev; ++ } + + ffs_dev_unlock(); +- return ffs_dev; ++ return ret; + } + +-static void ffs_release_dev(struct ffs_data *ffs_data) ++static void ffs_release_dev(struct ffs_dev *ffs_dev) + { +- struct ffs_dev *ffs_dev; +- + ENTER(); + ffs_dev_lock(); + +- ffs_dev = ffs_data->private_data; +- if (ffs_dev) { ++ if (ffs_dev && ffs_dev->mounted) { + ffs_dev->mounted = false; ++ if (ffs_dev->ffs_data) { ++ ffs_dev->ffs_data->private_data = NULL; ++ ffs_dev->ffs_data = NULL; ++ } + + if (ffs_dev->ffs_release_dev_callback) + ffs_dev->ffs_release_dev_callback(ffs_dev); +@@ -3653,7 +3652,6 @@ static int ffs_ready(struct ffs_data *ff + } + + ffs_obj->desc_ready = true; +- ffs_obj->ffs_data = ffs; + + if (ffs_obj->ffs_ready_callback) { + ret = ffs_obj->ffs_ready_callback(ffs); +@@ -3681,7 +3679,6 @@ static void ffs_closed(struct ffs_data * + goto done; + + ffs_obj->desc_ready = false; +- ffs_obj->ffs_data = NULL; + + if (test_and_clear_bit(FFS_FL_CALL_CLOSED_CALLBACK, &ffs->flags) && + ffs_obj->ffs_closed_callback)