From aab8aa0d35fc59e81c367cf34f990aaf48d24419 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 3 Oct 2023 17:21:19 +0300 Subject: [PATCH 01/21] driver core: platform: Drop redundant check in platform_device_add() Starting from the commit 37c12e7497b6 ("[DRIVER MODEL] Improved dynamically allocated platform_device interface") the pdev expects to be allocated beforehand or guaranteed to be non-NULL. Hence the leftover check is now redundant (as we have no combined calls like platform_device_add(platform_device_alloc(...)) in the entire kernel source code. Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20231003142122.3072824-1-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 76bfcba25003..d81f05c4fccd 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -658,9 +658,6 @@ int platform_device_add(struct platform_device *pdev) u32 i; int ret; - if (!pdev) - return -EINVAL; - if (!pdev->dev.parent) pdev->dev.parent = &platform_bus; From a549e3aac29cde86c1ade76909df759918c11653 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 3 Oct 2023 17:21:20 +0300 Subject: [PATCH 02/21] driver core: platform: Refactor error path in a couple places The usual pattern is to bail out on the error case. Besides that one of the labels is redundant as we may return directly. Refactor platform_device_add() and platform_dma_configure() accordingly. Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20231003142122.3072824-2-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index d81f05c4fccd..2b8645911d51 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -678,7 +678,7 @@ int platform_device_add(struct platform_device *pdev) */ ret = ida_alloc(&platform_devid_ida, GFP_KERNEL); if (ret < 0) - goto err_out; + return ret; pdev->id = ret; pdev->id_auto = true; dev_set_name(&pdev->dev, "%s.%d.auto", pdev->name, pdev->id); @@ -712,8 +712,10 @@ int platform_device_add(struct platform_device *pdev) dev_name(&pdev->dev), dev_name(pdev->dev.parent)); ret = device_add(&pdev->dev); - if (ret == 0) - return ret; + if (ret) + goto failed; + + return 0; failed: if (pdev->id_auto) { @@ -727,7 +729,6 @@ int platform_device_add(struct platform_device *pdev) release_resource(r); } - err_out: return ret; } EXPORT_SYMBOL_GPL(platform_device_add); @@ -1453,12 +1454,12 @@ static int platform_dma_configure(struct device *dev) attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode)); ret = acpi_dma_configure(dev, attr); } + if (ret || drv->driver_managed_dma) + return ret; - if (!ret && !drv->driver_managed_dma) { - ret = iommu_device_use_default_domain(dev); - if (ret) - arch_teardown_dma_ops(dev); - } + ret = iommu_device_use_default_domain(dev); + if (ret) + arch_teardown_dma_ops(dev); return ret; } From 6136597c8feae2cfefd80973b966c092c4ab42d9 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 3 Oct 2023 17:21:21 +0300 Subject: [PATCH 03/21] driver core: platform: Use temporary variable in platform_device_add() With the temporary variable for the struct device pointer the code looks better and slightly easier to read and parse by human being. Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20231003142122.3072824-3-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 2b8645911d51..55891c11dd03 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -655,20 +655,21 @@ EXPORT_SYMBOL_GPL(platform_device_add_data); */ int platform_device_add(struct platform_device *pdev) { + struct device *dev = &pdev->dev; u32 i; int ret; - if (!pdev->dev.parent) - pdev->dev.parent = &platform_bus; + if (!dev->parent) + dev->parent = &platform_bus; - pdev->dev.bus = &platform_bus_type; + dev->bus = &platform_bus_type; switch (pdev->id) { default: - dev_set_name(&pdev->dev, "%s.%d", pdev->name, pdev->id); + dev_set_name(dev, "%s.%d", pdev->name, pdev->id); break; case PLATFORM_DEVID_NONE: - dev_set_name(&pdev->dev, "%s", pdev->name); + dev_set_name(dev, "%s", pdev->name); break; case PLATFORM_DEVID_AUTO: /* @@ -681,7 +682,7 @@ int platform_device_add(struct platform_device *pdev) return ret; pdev->id = ret; pdev->id_auto = true; - dev_set_name(&pdev->dev, "%s.%d.auto", pdev->name, pdev->id); + dev_set_name(dev, "%s.%d.auto", pdev->name, pdev->id); break; } @@ -689,7 +690,7 @@ int platform_device_add(struct platform_device *pdev) struct resource *p, *r = &pdev->resource[i]; if (r->name == NULL) - r->name = dev_name(&pdev->dev); + r->name = dev_name(dev); p = r->parent; if (!p) { @@ -702,16 +703,16 @@ int platform_device_add(struct platform_device *pdev) if (p) { ret = insert_resource(p, r); if (ret) { - dev_err(&pdev->dev, "failed to claim resource %d: %pR\n", i, r); + dev_err(dev, "failed to claim resource %d: %pR\n", i, r); goto failed; } } } - pr_debug("Registering platform device '%s'. Parent at %s\n", - dev_name(&pdev->dev), dev_name(pdev->dev.parent)); + pr_debug("Registering platform device '%s'. Parent at %s\n", dev_name(dev), + dev_name(dev->parent)); - ret = device_add(&pdev->dev); + ret = device_add(dev); if (ret) goto failed; From 243e1b776f613501cd8de4e56c2eb415c942bb04 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 3 Oct 2023 17:21:22 +0300 Subject: [PATCH 04/21] driver core: platform: Unify the firmware node type check OF and ACPI currently are using asymmetrical APIs to check for the firmware node type. Unify them by using is_*_node() against struct fwnode_handle pointer. Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20231003142122.3072824-4-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 55891c11dd03..828908d1d5b9 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -178,18 +178,19 @@ int platform_get_irq_optional(struct platform_device *dev, unsigned int num) ret = dev->archdata.irqs[num]; goto out; #else + struct fwnode_handle *fwnode = dev_fwnode(&dev->dev); struct resource *r; - if (IS_ENABLED(CONFIG_OF_IRQ) && dev->dev.of_node) { - ret = of_irq_get(dev->dev.of_node, num); + if (is_of_node(fwnode)) { + ret = of_irq_get(to_of_node(fwnode), num); if (ret > 0 || ret == -EPROBE_DEFER) goto out; } r = platform_get_resource(dev, IORESOURCE_IRQ, num); - if (has_acpi_companion(&dev->dev)) { + if (is_acpi_device_node(fwnode)) { if (r && r->flags & IORESOURCE_DISABLED) { - ret = acpi_irq_get(ACPI_HANDLE(&dev->dev), num, r); + ret = acpi_irq_get(ACPI_HANDLE_FWNODE(fwnode), num, r); if (ret) goto out; } @@ -222,8 +223,8 @@ int platform_get_irq_optional(struct platform_device *dev, unsigned int num) * the device will only expose one IRQ, and this fallback * allows a common code path across either kind of resource. */ - if (num == 0 && has_acpi_companion(&dev->dev)) { - ret = acpi_dev_gpio_irq_get(ACPI_COMPANION(&dev->dev), num); + if (num == 0 && is_acpi_device_node(fwnode)) { + ret = acpi_dev_gpio_irq_get(to_acpi_device_node(fwnode), num); /* Our callers expect -ENXIO for missing IRQs. */ if (ret >= 0 || ret == -EPROBE_DEFER) goto out; @@ -312,7 +313,7 @@ static void devm_platform_get_irqs_affinity_release(struct device *dev, for (i = 0; i < ptr->count; i++) { irq_dispose_mapping(ptr->irq[i]); - if (has_acpi_companion(dev)) + if (is_acpi_device_node(dev_fwnode(dev))) platform_disable_acpi_irq(to_platform_device(dev), i); } } @@ -1446,13 +1447,14 @@ static void platform_shutdown(struct device *_dev) static int platform_dma_configure(struct device *dev) { struct platform_driver *drv = to_platform_driver(dev->driver); + struct fwnode_handle *fwnode = dev_fwnode(dev); enum dev_dma_attr attr; int ret = 0; - if (dev->of_node) { - ret = of_dma_configure(dev, dev->of_node, true); - } else if (has_acpi_companion(dev)) { - attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode)); + if (is_of_node(fwnode)) { + ret = of_dma_configure(dev, to_of_node(fwnode), true); + } else if (is_acpi_device_node(fwnode)) { + attr = acpi_get_dma_attr(to_acpi_device_node(fwnode)); ret = acpi_dma_configure(dev, attr); } if (ret || drv->driver_managed_dma) From a083c755e136844a934bc9b4416cd23b5c19c617 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 8 Sep 2023 22:58:40 +0900 Subject: [PATCH 05/21] devres: rename the first parameter of devm_add_action(_or_reset) The first parameter of devm_add_action(_or_reset) is a device. The name 'release' is confusing because it is often used for dr_release_t in the devres context. Rename it to 'dev'. No functional change intended. Signed-off-by: Masahiro Yamada Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20230908135840.2362708-1-masahiroy@kernel.org Signed-off-by: Greg Kroah-Hartman --- include/linux/device.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/linux/device.h b/include/linux/device.h index 56d93a1ffb7b..d7a72a8749ea 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -389,8 +389,8 @@ void devm_remove_action(struct device *dev, void (*action)(void *), void *data); void devm_release_action(struct device *dev, void (*action)(void *), void *data); int __devm_add_action(struct device *dev, void (*action)(void *), void *data, const char *name); -#define devm_add_action(release, action, data) \ - __devm_add_action(release, action, data, #action) +#define devm_add_action(dev, action, data) \ + __devm_add_action(dev, action, data, #action) static inline int __devm_add_action_or_reset(struct device *dev, void (*action)(void *), void *data, const char *name) @@ -403,8 +403,8 @@ static inline int __devm_add_action_or_reset(struct device *dev, void (*action)( return ret; } -#define devm_add_action_or_reset(release, action, data) \ - __devm_add_action_or_reset(release, action, data, #action) +#define devm_add_action_or_reset(dev, action, data) \ + __devm_add_action_or_reset(dev, action, data, #action) /** * devm_alloc_percpu - Resource-managed alloc_percpu From 7523d330aac7190f738998a52df8d5aa14293280 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 4 Sep 2023 13:40:46 +0300 Subject: [PATCH 06/21] device property: Clarify usage scope of some struct fwnode_handle members Most of the struct fwnode_handle members are for exclusive use with device links framework. Clarify this by adding a respective comment. Signed-off-by: Andy Shevchenko Reviewed-by: Heikki Krogerus Reviewed-by: Sakari Ailus Link: https://lore.kernel.org/r/20230904104046.1682875-1-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- include/linux/fwnode.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index 5700451b300f..2a72f55d26eb 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -41,6 +41,8 @@ struct device; struct fwnode_handle { struct fwnode_handle *secondary; const struct fwnode_operations *ops; + + /* The below is used solely by device links, don't use otherwise */ struct device *dev; struct list_head suppliers; struct list_head consumers; From f1ac370cdda304d2fed44455ae685c7ee9539c0a Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 19 Sep 2023 22:50:48 +0300 Subject: [PATCH 07/21] driver core: Add missing parameter description to __fwnode_link_add() The kernel documentation validator is not happy with: drivers/base/core.c:67: warning: Function parameter or member 'flags' not described in '__fwnode_link_add' Add missing parameter description. Fixes: 6a6dfdf8b3ff ("driver core: fw_devlink: Allow marking a fwnode link as being part of a cycle") Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20230919195048.3197551-1-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index 4d8b315c48a1..67ba592afc77 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -49,6 +49,7 @@ static bool fw_devlink_best_effort; * __fwnode_link_add - Create a link between two fwnode_handles. * @con: Consumer end of the link. * @sup: Supplier end of the link. + * @flags: Link flags. * * Create a fwnode link between fwnode handles @con and @sup. The fwnode link * represents the detail that the firmware lists @sup fwnode as supplying a From 98ad1dd06a02096fff6c65703a85b9f3c3de1a7d Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Wed, 27 Sep 2023 15:20:10 +0200 Subject: [PATCH 08/21] drivers: base: test: Make property entry API test modular There is no reason why the KUnit Tests for the property entry API can only be built-in. Add support for building these tests as a loadable module, like is supported by most other tests. Signed-off-by: Geert Uytterhoeven Link: https://lore.kernel.org/r/98388154383df9d4ced73946efd18318aeea50e2.1695820382.git.geert+renesas@glider.be Signed-off-by: Greg Kroah-Hartman --- drivers/base/test/Kconfig | 4 ++-- drivers/base/test/property-entry-test.c | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/base/test/Kconfig b/drivers/base/test/Kconfig index 9d42051f8f8e..5c7fac80611c 100644 --- a/drivers/base/test/Kconfig +++ b/drivers/base/test/Kconfig @@ -14,6 +14,6 @@ config DM_KUNIT_TEST depends on KUNIT config DRIVER_PE_KUNIT_TEST - bool "KUnit Tests for property entry API" if !KUNIT_ALL_TESTS - depends on KUNIT=y + tristate "KUnit Tests for property entry API" if !KUNIT_ALL_TESTS + depends on KUNIT default KUNIT_ALL_TESTS diff --git a/drivers/base/test/property-entry-test.c b/drivers/base/test/property-entry-test.c index dd2b606d76a3..a8657eb06f94 100644 --- a/drivers/base/test/property-entry-test.c +++ b/drivers/base/test/property-entry-test.c @@ -506,3 +506,7 @@ static struct kunit_suite property_entry_test_suite = { }; kunit_test_suite(property_entry_test_suite); + +MODULE_DESCRIPTION("Test module for the property entry API"); +MODULE_AUTHOR("Dmitry Torokhov "); +MODULE_LICENSE("GPL"); From 1dc05a274a7b13fd61b6c43f0136153752e6f731 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 20 Sep 2023 18:38:19 +0300 Subject: [PATCH 09/21] device property: Replace custom implementation of COUNT_ARGS() Replace custom and non-portable implementation of COUNT_ARGS(). Fixes: e64b674bc9d7 ("software node: implement reference properties") Reported-by: Nick Desaulniers Closes: https://lore.kernel.org/r/ZQoILN6QCjzosCOs@google.com Signed-off-by: Andy Shevchenko Reviewed-by: Takashi Iwai Closes: https://github.com/ClangBuiltLinux/linux/issues/1935 Reviewed-by: Nick Desaulniers Link: https://lore.kernel.org/r/20230920153819.2069869-1-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- include/linux/property.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/linux/property.h b/include/linux/property.h index 8c3c6685a2ae..9f2585d705a8 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -10,6 +10,7 @@ #ifndef _LINUX_PROPERTY_H_ #define _LINUX_PROPERTY_H_ +#include #include #include #include @@ -288,7 +289,7 @@ struct software_node_ref_args { #define SOFTWARE_NODE_REFERENCE(_ref_, ...) \ (const struct software_node_ref_args) { \ .node = _ref_, \ - .nargs = ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1, \ + .nargs = COUNT_ARGS(__VA_ARGS__), \ .args = { __VA_ARGS__ }, \ } From 7360a48bd0f5e62b2d00c387d5d3f2821eb290ce Mon Sep 17 00:00:00 2001 From: Mike Tipton Date: Fri, 22 Sep 2023 06:45:12 -0700 Subject: [PATCH 10/21] debugfs: Fix __rcu type comparison warning Sparse reports the following: fs/debugfs/file.c:942:9: sparse: sparse: incompatible types in comparison expression (different address spaces): fs/debugfs/file.c:942:9: sparse: char [noderef] __rcu * fs/debugfs/file.c:942:9: sparse: char * rcu_assign_pointer() expects that it's assigning to pointers annotated with __rcu. We can't annotate the generic struct file::private_data, so cast it instead. Fixes: 86b5488121db ("debugfs: Add write support to debugfs_create_str()") Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202309091933.BRWlSnCq-lkp@intel.com/ Signed-off-by: Mike Tipton Link: https://lore.kernel.org/r/20230922134512.5126-1-quic_mdtipton@quicinc.com Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 87b3753aa4b1..c45e8c2d62e1 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -939,7 +939,7 @@ static ssize_t debugfs_write_file_str(struct file *file, const char __user *user new[pos + count] = '\0'; strim(new); - rcu_assign_pointer(*(char **)file->private_data, new); + rcu_assign_pointer(*(char __rcu **)file->private_data, new); synchronize_rcu(); kfree(old); From 0fedefd4c4e33dd24f726b13b5d7c143e2b483be Mon Sep 17 00:00:00 2001 From: Valentine Sinitsyn Date: Mon, 25 Sep 2023 11:40:12 +0300 Subject: [PATCH 11/21] kernfs: sysfs: support custom llseek method for sysfs entries As of now, seeking in sysfs files is handled by generic_file_llseek(). There are situations where one may want to customize seeking logic: - Many sysfs entries are fixed files while generic_file_llseek() accepts past-the-end positions. Not only being useless by itself, this also means a bug in userspace code will trigger not at lseek(), but at some later point making debugging harder. - generic_file_llseek() relies on f_mapping->host to get the file size which might not be correct for all sysfs entries. See commit 636b21b50152 ("PCI: Revoke mappings like devmem") as an example. Implement llseek method to override this behavior at sysfs attribute level. The method is optional, and if it is absent, generic_file_llseek() is called to preserve backwards compatibility. Signed-off-by: Valentine Sinitsyn Link: https://lore.kernel.org/r/20230925084013.309399-1-valesini@yandex-team.ru Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/file.c | 29 ++++++++++++++++++++++++++++- fs/sysfs/file.c | 13 +++++++++++++ include/linux/kernfs.h | 1 + include/linux/sysfs.h | 2 ++ 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c index 180906c36f51..855e3f9d8dcc 100644 --- a/fs/kernfs/file.c +++ b/fs/kernfs/file.c @@ -903,6 +903,33 @@ static __poll_t kernfs_fop_poll(struct file *filp, poll_table *wait) return ret; } +static loff_t kernfs_fop_llseek(struct file *file, loff_t offset, int whence) +{ + struct kernfs_open_file *of = kernfs_of(file); + const struct kernfs_ops *ops; + loff_t ret; + + /* + * @of->mutex nests outside active ref and is primarily to ensure that + * the ops aren't called concurrently for the same open file. + */ + mutex_lock(&of->mutex); + if (!kernfs_get_active(of->kn)) { + mutex_unlock(&of->mutex); + return -ENODEV; + } + + ops = kernfs_ops(of->kn); + if (ops->llseek) + ret = ops->llseek(of, offset, whence); + else + ret = generic_file_llseek(file, offset, whence); + + kernfs_put_active(of->kn); + mutex_unlock(&of->mutex); + return ret; +} + static void kernfs_notify_workfn(struct work_struct *work) { struct kernfs_node *kn; @@ -1005,7 +1032,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify); const struct file_operations kernfs_file_fops = { .read_iter = kernfs_fop_read_iter, .write_iter = kernfs_fop_write_iter, - .llseek = generic_file_llseek, + .llseek = kernfs_fop_llseek, .mmap = kernfs_fop_mmap, .open = kernfs_fop_open, .release = kernfs_fop_release, diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index a12ac0356c69..6b7652fb8050 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -167,6 +167,18 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of, return battr->mmap(of->file, kobj, battr, vma); } +static loff_t sysfs_kf_bin_llseek(struct kernfs_open_file *of, loff_t offset, + int whence) +{ + struct bin_attribute *battr = of->kn->priv; + struct kobject *kobj = of->kn->parent->priv; + + if (battr->llseek) + return battr->llseek(of->file, kobj, battr, offset, whence); + else + return generic_file_llseek(of->file, offset, whence); +} + static int sysfs_kf_bin_open(struct kernfs_open_file *of) { struct bin_attribute *battr = of->kn->priv; @@ -249,6 +261,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = { .write = sysfs_kf_bin_write, .mmap = sysfs_kf_bin_mmap, .open = sysfs_kf_bin_open, + .llseek = sysfs_kf_bin_llseek, }; int sysfs_add_file_mode_ns(struct kernfs_node *parent, diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h index 2a36f3218b51..99aaa050ccb7 100644 --- a/include/linux/kernfs.h +++ b/include/linux/kernfs.h @@ -316,6 +316,7 @@ struct kernfs_ops { struct poll_table_struct *pt); int (*mmap)(struct kernfs_open_file *of, struct vm_area_struct *vma); + loff_t (*llseek)(struct kernfs_open_file *of, loff_t offset, int whence); }; /* diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index fd3fe5c8c17f..b717a70219f6 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -181,6 +181,8 @@ struct bin_attribute { char *, loff_t, size_t); ssize_t (*write)(struct file *, struct kobject *, struct bin_attribute *, char *, loff_t, size_t); + loff_t (*llseek)(struct file *, struct kobject *, struct bin_attribute *, + loff_t, int); int (*mmap)(struct file *, struct kobject *, struct bin_attribute *attr, struct vm_area_struct *vma); }; From 24de09c16f974dce70e2c56e3a4325117221ed12 Mon Sep 17 00:00:00 2001 From: Valentine Sinitsyn Date: Mon, 25 Sep 2023 11:40:13 +0300 Subject: [PATCH 12/21] PCI: Implement custom llseek for sysfs resource entries Since commit 636b21b50152 ("PCI: Revoke mappings like devmem"), mmappable sysfs entries have started to receive their f_mapping from the iomem pseudo filesystem, so that CONFIG_IO_STRICT_DEVMEM is honored in sysfs (and procfs) as well as in /dev/[k]mem. This resulted in a userspace-visible regression: 1. Open a sysfs PCI resource file (eg. /sys/bus/pci/devices/*/resource0) 2. Use lseek(fd, 0, SEEK_END) to determine its size Expected result: a PCI region size is returned. Actual result: 0 is returned. The reason is that PCI resource files residing in sysfs use generic_file_llseek(), which relies on f_mapping->host inode to get the file size. As f_mapping is now redefined, f_mapping->host points to an anonymous zero-sized iomem_inode which has nothing to do with sysfs file in question. Implement a custom llseek method for sysfs PCI resources, which is almost the same as proc_bus_pci_lseek() used for procfs entries. This makes sysfs and procfs entries consistent with regards to seeking, but also introduces userspace-visible changes to seeking PCI resources in sysfs: - SEEK_DATA and SEEK_HOLE are no longer supported; - Seeking past the end of the file is prohibited while previously offsets up to MAX_NON_LFS were accepted (reading from these offsets was always invalid). Signed-off-by: Valentine Sinitsyn Acked-by: Bjorn Helgaas Link: https://lore.kernel.org/r/20230925084013.309399-2-valesini@yandex-team.ru Signed-off-by: Greg Kroah-Hartman --- drivers/pci/pci-sysfs.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index d9eede2dbc0e..97c9c62d5e3e 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -835,6 +835,19 @@ static const struct attribute_group pci_dev_config_attr_group = { .is_bin_visible = pci_dev_config_attr_is_visible, }; +/* + * llseek operation for mmappable PCI resources. + * May be left unused if the arch doesn't provide them. + */ +static __maybe_unused loff_t +pci_llseek_resource(struct file *filep, + struct kobject *kobj __always_unused, + struct bin_attribute *attr, + loff_t offset, int whence) +{ + return fixed_size_llseek(filep, offset, whence, attr->size); +} + #ifdef HAVE_PCI_LEGACY /** * pci_read_legacy_io - read byte(s) from legacy I/O port space @@ -967,6 +980,8 @@ void pci_create_legacy_files(struct pci_bus *b) b->legacy_io->attr.mode = 0600; b->legacy_io->read = pci_read_legacy_io; b->legacy_io->write = pci_write_legacy_io; + /* See pci_create_attr() for motivation */ + b->legacy_io->llseek = pci_llseek_resource; b->legacy_io->mmap = pci_mmap_legacy_io; b->legacy_io->f_mapping = iomem_get_mapping; pci_adjust_legacy_attr(b, pci_mmap_io); @@ -981,6 +996,8 @@ void pci_create_legacy_files(struct pci_bus *b) b->legacy_mem->size = 1024*1024; b->legacy_mem->attr.mode = 0600; b->legacy_mem->mmap = pci_mmap_legacy_mem; + /* See pci_create_attr() for motivation */ + b->legacy_mem->llseek = pci_llseek_resource; b->legacy_mem->f_mapping = iomem_get_mapping; pci_adjust_legacy_attr(b, pci_mmap_mem); error = device_create_bin_file(&b->dev, b->legacy_mem); @@ -1199,8 +1216,15 @@ static int pci_create_attr(struct pci_dev *pdev, int num, int write_combine) res_attr->mmap = pci_mmap_resource_uc; } } - if (res_attr->mmap) + if (res_attr->mmap) { res_attr->f_mapping = iomem_get_mapping; + /* + * generic_file_llseek() consults f_mapping->host to determine + * the file size. As iomem_inode knows nothing about the + * attribute, it's not going to work, so override it as well. + */ + res_attr->llseek = pci_llseek_resource; + } res_attr->attr.name = res_attr_name; res_attr->attr.mode = 0600; res_attr->size = pci_resource_len(pdev, num); From 441f0dd8fa035a2c7cfe972047bb905d3be05c1b Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 12 Sep 2023 19:53:10 +0300 Subject: [PATCH 13/21] resource: Reuse for_each_resource() macro We have a few places where for_each_resource() is open coded. Replace that by the macro. This makes code easier to read and understand. With this, compile r_next() only for CONFIG_PROC_FS=y. Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20230912165312.402422-1-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- kernel/resource.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/kernel/resource.c b/kernel/resource.c index b1763b2fd7ef..86716cd566e9 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -77,13 +77,6 @@ static struct resource *next_resource_skip_children(struct resource *p) (_p) = (_skip_children) ? next_resource_skip_children(_p) : \ next_resource(_p)) -static void *r_next(struct seq_file *m, void *v, loff_t *pos) -{ - struct resource *p = v; - (*pos)++; - return (void *)next_resource(p); -} - #ifdef CONFIG_PROC_FS enum { MAX_IORES_LEVEL = 5 }; @@ -91,14 +84,26 @@ enum { MAX_IORES_LEVEL = 5 }; static void *r_start(struct seq_file *m, loff_t *pos) __acquires(resource_lock) { - struct resource *p = pde_data(file_inode(m->file)); - loff_t l = 0; + struct resource *root = pde_data(file_inode(m->file)); + struct resource *p; + loff_t l = *pos; + read_lock(&resource_lock); - for (p = p->child; p && l < *pos; p = r_next(m, p, &l)) - ; + for_each_resource(root, p, false) { + if (l-- == 0) + break; + } + return p; } +static void *r_next(struct seq_file *m, void *v, loff_t *pos) +{ + struct resource *p = v; + (*pos)++; + return (void *)next_resource(p); +} + static void r_stop(struct seq_file *m, void *v) __releases(resource_lock) { @@ -336,7 +341,7 @@ static int find_next_iomem_res(resource_size_t start, resource_size_t end, read_lock(&resource_lock); - for (p = iomem_resource.child; p; p = next_resource(p)) { + for_each_resource(&iomem_resource, p, false) { /* If we passed the resource we are looking for, stop */ if (p->start > end) { p = NULL; @@ -1641,13 +1646,12 @@ __setup("reserve=", reserve_setup); */ int iomem_map_sanity_check(resource_size_t addr, unsigned long size) { - struct resource *p = &iomem_resource; resource_size_t end = addr + size - 1; + struct resource *p; int err = 0; - loff_t l; read_lock(&resource_lock); - for (p = p->child; p ; p = r_next(NULL, p, &l)) { + for_each_resource(&iomem_resource, p, false) { /* * We can probably skip the resources without * IORESOURCE_IO attribute? From 10dabdf45ed34caaaad97978306fe6e9ee7581d9 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 12 Sep 2023 19:53:11 +0300 Subject: [PATCH 14/21] resource: Unify next_resource() and next_resource_skip_children() We have the next_resource() is used once and no user for the next_resource_skip_children() outside of the for_each_resource(). Unify them by adding skip_children parameter to the next_resource(). Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20230912165312.402422-2-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- kernel/resource.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/kernel/resource.c b/kernel/resource.c index 86716cd566e9..866ef3663a0b 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -56,26 +56,17 @@ struct resource_constraint { static DEFINE_RWLOCK(resource_lock); -static struct resource *next_resource(struct resource *p) +static struct resource *next_resource(struct resource *p, bool skip_children) { - if (p->child) + if (!skip_children && p->child) return p->child; while (!p->sibling && p->parent) p = p->parent; return p->sibling; } -static struct resource *next_resource_skip_children(struct resource *p) -{ - while (!p->sibling && p->parent) - p = p->parent; - return p->sibling; -} - #define for_each_resource(_root, _p, _skip_children) \ - for ((_p) = (_root)->child; (_p); \ - (_p) = (_skip_children) ? next_resource_skip_children(_p) : \ - next_resource(_p)) + for ((_p) = (_root)->child; (_p); (_p) = next_resource(_p, _skip_children)) #ifdef CONFIG_PROC_FS @@ -100,8 +91,10 @@ static void *r_start(struct seq_file *m, loff_t *pos) static void *r_next(struct seq_file *m, void *v, loff_t *pos) { struct resource *p = v; + (*pos)++; - return (void *)next_resource(p); + + return (void *)next_resource(p, false); } static void r_stop(struct seq_file *m, void *v) From eb7581deb4c2eef77f6368e1891e123b69349bb0 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 12 Sep 2023 19:53:12 +0300 Subject: [PATCH 15/21] resource: Constify resource crosscheck APIs Constify APIs: _contains(), _overlaps(), _intersection(), _union(). Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20230912165312.402422-3-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- include/linux/ioport.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/linux/ioport.h b/include/linux/ioport.h index 25d768d48970..14f5cfabbbc8 100644 --- a/include/linux/ioport.h +++ b/include/linux/ioport.h @@ -229,7 +229,7 @@ static inline unsigned long resource_ext_type(const struct resource *res) return res->flags & IORESOURCE_EXT_TYPE_BITS; } /* True iff r1 completely contains r2 */ -static inline bool resource_contains(struct resource *r1, struct resource *r2) +static inline bool resource_contains(const struct resource *r1, const struct resource *r2) { if (resource_type(r1) != resource_type(r2)) return false; @@ -239,13 +239,13 @@ static inline bool resource_contains(struct resource *r1, struct resource *r2) } /* True if any part of r1 overlaps r2 */ -static inline bool resource_overlaps(struct resource *r1, struct resource *r2) +static inline bool resource_overlaps(const struct resource *r1, const struct resource *r2) { return r1->start <= r2->end && r1->end >= r2->start; } -static inline bool -resource_intersection(struct resource *r1, struct resource *r2, struct resource *r) +static inline bool resource_intersection(const struct resource *r1, const struct resource *r2, + struct resource *r) { if (!resource_overlaps(r1, r2)) return false; @@ -254,8 +254,8 @@ resource_intersection(struct resource *r1, struct resource *r2, struct resource return true; } -static inline bool -resource_union(struct resource *r1, struct resource *r2, struct resource *r) +static inline bool resource_union(const struct resource *r1, const struct resource *r2, + struct resource *r) { if (!resource_overlaps(r1, r2)) return false; From 36b2d7dd5a8ac95c8c1e69bdc93c4a6e2dc28a23 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 6 Oct 2023 13:17:49 -0700 Subject: [PATCH 16/21] driver core: platform: Annotate struct irq_affinity_devres with __counted_by Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct irq_affinity_devres. Cc: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" Cc: "Gustavo A. R. Silva" Cc: linux-hardening@vger.kernel.org Link: https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1] Signed-off-by: Kees Cook Reviewed-by: "Gustavo A. R. Silva" Link: https://lore.kernel.org/r/20231006201749.work.432-kees@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 828908d1d5b9..10c577963418 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -292,7 +292,7 @@ EXPORT_SYMBOL_GPL(platform_irq_count); struct irq_affinity_devres { unsigned int count; - unsigned int irq[]; + unsigned int irq[] __counted_by(count); }; static void platform_disable_acpi_irq(struct platform_device *pdev, int index) From 28f2d57d88a71353eb1f1952d1ac4a7816612087 Mon Sep 17 00:00:00 2001 From: Maurizio Lombardi Date: Fri, 20 Oct 2023 18:00:42 +0200 Subject: [PATCH 17/21] driver core: class: remove boilerplate code Jump to err_out to avoid duplicating the code. Signed-off-by: Maurizio Lombardi Link: https://lore.kernel.org/r/20231020160042.759439-1-mlombard@redhat.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/class.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/base/class.c b/drivers/base/class.c index 05d9df90f621..7e78aee0fd6c 100644 --- a/drivers/base/class.c +++ b/drivers/base/class.c @@ -193,10 +193,8 @@ int class_register(const struct class *cls) lockdep_register_key(key); __mutex_init(&cp->mutex, "subsys mutex", key); error = kobject_set_name(&cp->subsys.kobj, "%s", cls->name); - if (error) { - kfree(cp); - return error; - } + if (error) + goto err_out; cp->subsys.kobj.kset = class_kset; cp->subsys.kobj.ktype = &class_ktype; From 2e84dc37920012b458e9458b19fc4ed33f81bc74 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Tue, 17 Oct 2023 18:38:50 -0700 Subject: [PATCH 18/21] driver core: Release all resources during unbind before updating device links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes a bug in commit 9ed9895370ae ("driver core: Functional dependencies tracking support") where the device link status was incorrectly updated in the driver unbind path before all the device's resources were released. Fixes: 9ed9895370ae ("driver core: Functional dependencies tracking support") Cc: stable Reported-by: Uwe Kleine-König Closes: https://lore.kernel.org/all/20231014161721.f4iqyroddkcyoefo@pengutronix.de/ Signed-off-by: Saravana Kannan Cc: Thierry Reding Cc: Yang Yingliang Cc: Andy Shevchenko Cc: Mark Brown Cc: Matti Vaittinen Cc: James Clark Acked-by: "Rafael J. Wysocki" Tested-by: Uwe Kleine-König Acked-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20231018013851.3303928-1-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index a528cec24264..0c3725c3eefa 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -1274,8 +1274,8 @@ static void __device_release_driver(struct device *dev, struct device *parent) if (dev->bus && dev->bus->dma_cleanup) dev->bus->dma_cleanup(dev); - device_links_driver_cleanup(dev); device_unbind_cleanup(dev); + device_links_driver_cleanup(dev); klist_remove(&dev->p->knode_driver); device_pm_check_callbacks(dev); From 0217f3944aebad1d4beec5894ec80472b94b4139 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Sun, 15 Oct 2023 15:09:59 +0200 Subject: [PATCH 19/21] Documentation: security-bugs.rst: linux-distros relaxed their rules The linux-distros list relaxed their rules to try to adapt better to how the Linux kernel works. Let's update the Coordination part to explain why and when to contact them or not to and how to avoid trouble in the future. Link: https://www.openwall.com/lists/oss-security/2023/09/08/4 Cc: Kees Cook Cc: Solar Designer Cc: Vegard Nossum Acked-by: Jiri Kosina Signed-off-by: Willy Tarreau Link: https://lore.kernel.org/r/20231015130959.26242-1-w@1wt.eu Signed-off-by: Greg Kroah-Hartman --- Documentation/process/security-bugs.rst | 33 +++++++++++++++++++------ 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/Documentation/process/security-bugs.rst b/Documentation/process/security-bugs.rst index 5a6993795bd2..692a3ba56cca 100644 --- a/Documentation/process/security-bugs.rst +++ b/Documentation/process/security-bugs.rst @@ -66,15 +66,32 @@ lifted, in perpetuity. Coordination with other groups ------------------------------ -The kernel security team strongly recommends that reporters of potential -security issues NEVER contact the "linux-distros" mailing list until -AFTER discussing it with the kernel security team. Do not Cc: both -lists at once. You may contact the linux-distros mailing list after a -fix has been agreed on and you fully understand the requirements that -doing so will impose on you and the kernel community. +While the kernel security team solely focuses on getting bugs fixed, +other groups focus on fixing issues in distros and coordinating +disclosure between operating system vendors. Coordination is usually +handled by the "linux-distros" mailing list and disclosure by the +public "oss-security" mailing list, both of which are closely related +and presented in the linux-distros wiki: + -The different lists have different goals and the linux-distros rules do -not contribute to actually fixing any potential security problems. +Please note that the respective policies and rules are different since +the 3 lists pursue different goals. Coordinating between the kernel +security team and other teams is difficult since for the kernel security +team occasional embargoes (as subject to a maximum allowed number of +days) start from the availability of a fix, while for "linux-distros" +they start from the initial post to the list regardless of the +availability of a fix. + +As such, the kernel security team strongly recommends that as a reporter +of a potential security issue you DO NOT contact the "linux-distros" +mailing list UNTIL a fix is accepted by the affected code's maintainers +and you have read the distros wiki page above and you fully understand +the requirements that contacting "linux-distros" will impose on you and +the kernel community. This also means that in general it doesn't make +sense to Cc: both lists at once, except maybe for coordination if and +while an accepted fix has not yet been merged. In other words, until a +fix is accepted do not Cc: "linux-distros", and after it's merged do not +Cc: the kernel security team. CVE assignment -------------- From 87ffa98eeee8d62a56afdad80ea697e7a6e5c354 Mon Sep 17 00:00:00 2001 From: Mukesh Ojha Date: Thu, 26 Oct 2023 19:57:38 +0530 Subject: [PATCH 20/21] firmware_loader: Refactor kill_pending_fw_fallback_reqs() Rename 'only_kill_custom' and refactor logic related to it to be more meaningful. Signed-off-by: Mukesh Ojha Acked-by: Luis Chamberlain Link: https://lore.kernel.org/r/1698330459-31776-1-git-send-email-quic_mojha@quicinc.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/fallback.c | 4 ++-- drivers/base/firmware_loader/fallback.h | 4 ++-- drivers/base/firmware_loader/main.c | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index bf68e3947814..b3ce07160281 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -46,7 +46,7 @@ static inline int fw_sysfs_wait_timeout(struct fw_priv *fw_priv, long timeout) static LIST_HEAD(pending_fw_head); -void kill_pending_fw_fallback_reqs(bool only_kill_custom) +void kill_pending_fw_fallback_reqs(bool kill_all) { struct fw_priv *fw_priv; struct fw_priv *next; @@ -54,7 +54,7 @@ void kill_pending_fw_fallback_reqs(bool only_kill_custom) mutex_lock(&fw_lock); list_for_each_entry_safe(fw_priv, next, &pending_fw_head, pending_list) { - if (!fw_priv->need_uevent || !only_kill_custom) + if (kill_all || !fw_priv->need_uevent) __fw_load_abort(fw_priv); } mutex_unlock(&fw_lock); diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h index 144148595660..ccf912bef6ca 100644 --- a/drivers/base/firmware_loader/fallback.h +++ b/drivers/base/firmware_loader/fallback.h @@ -13,7 +13,7 @@ int firmware_fallback_sysfs(struct firmware *fw, const char *name, struct device *device, u32 opt_flags, int ret); -void kill_pending_fw_fallback_reqs(bool only_kill_custom); +void kill_pending_fw_fallback_reqs(bool kill_all); void fw_fallback_set_cache_timeout(void); void fw_fallback_set_default_timeout(void); @@ -28,7 +28,7 @@ static inline int firmware_fallback_sysfs(struct firmware *fw, const char *name, return ret; } -static inline void kill_pending_fw_fallback_reqs(bool only_kill_custom) { } +static inline void kill_pending_fw_fallback_reqs(bool kill_all) { } static inline void fw_fallback_set_cache_timeout(void) { } static inline void fw_fallback_set_default_timeout(void) { } #endif /* CONFIG_FW_LOADER_USER_HELPER */ diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index b58c42f1b1ce..522ccee781b4 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -1524,10 +1524,10 @@ static int fw_pm_notify(struct notifier_block *notify_block, case PM_SUSPEND_PREPARE: case PM_RESTORE_PREPARE: /* - * kill pending fallback requests with a custom fallback - * to avoid stalling suspend. + * Here, kill pending fallback requests will only kill + * non-uevent firmware request to avoid stalling suspend. */ - kill_pending_fw_fallback_reqs(true); + kill_pending_fw_fallback_reqs(false); device_cache_fw_images(); break; @@ -1612,7 +1612,7 @@ static int fw_shutdown_notify(struct notifier_block *unused1, * Kill all pending fallback requests to avoid both stalling shutdown, * and avoid a deadlock with the usermode_lock. */ - kill_pending_fw_fallback_reqs(false); + kill_pending_fw_fallback_reqs(true); return NOTIFY_DONE; } From effd7c70eaa0440688b60b9d419243695ede3c45 Mon Sep 17 00:00:00 2001 From: Mukesh Ojha Date: Thu, 26 Oct 2023 19:57:39 +0530 Subject: [PATCH 21/21] firmware_loader: Abort all upcoming firmware load request once reboot triggered There could be following scenario where there is a ongoing reboot is going from processA which tries to call all the reboot notifier callback and one of them is firmware reboot call which tries to abort all the ongoing firmware userspace request under fw_lock but there could be another processB which tries to do request firmware, which came just after abort done from ProcessA and ask for userspace to load the firmware and this can stop the ongoing reboot ProcessA to stall for next 60s(default timeout) which may not be expected behaviour everyone like to see, instead we should abort any firmware load request which came once firmware knows about the reboot through notification. ProcessA ProcessB kernel_restart_prepare blocking_notifier_call_chain fw_shutdown_notify kill_pending_fw_fallback_reqs __fw_load_abort fw_state_aborted request_firmware __fw_state_set firmware_fallback_sysfs ... fw_load_from_user_helper .. ... . .. usermodehelper_read_trylock fw_load_sysfs_fallback fw_sysfs_wait_timeout usermodehelper_disable __usermodehelper_disable down_write() Signed-off-by: Mukesh Ojha Acked-by: Luis Chamberlain Link: https://lore.kernel.org/r/1698330459-31776-2-git-send-email-quic_mojha@quicinc.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/fallback.c | 6 +++++- drivers/base/firmware_loader/firmware.h | 1 + drivers/base/firmware_loader/main.c | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index b3ce07160281..3ef0b312ae71 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -57,6 +57,10 @@ void kill_pending_fw_fallback_reqs(bool kill_all) if (kill_all || !fw_priv->need_uevent) __fw_load_abort(fw_priv); } + + if (kill_all) + fw_load_abort_all = true; + mutex_unlock(&fw_lock); } @@ -86,7 +90,7 @@ static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs, long timeout) } mutex_lock(&fw_lock); - if (fw_state_is_aborted(fw_priv)) { + if (fw_load_abort_all || fw_state_is_aborted(fw_priv)) { mutex_unlock(&fw_lock); retval = -EINTR; goto out; diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h index bf549d6500d7..e891742ba264 100644 --- a/drivers/base/firmware_loader/firmware.h +++ b/drivers/base/firmware_loader/firmware.h @@ -86,6 +86,7 @@ struct fw_priv { extern struct mutex fw_lock; extern struct firmware_cache fw_cache; +extern bool fw_load_abort_all; static inline bool __fw_state_check(struct fw_priv *fw_priv, enum fw_status status) diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 522ccee781b4..ea28102d421e 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -93,6 +93,7 @@ static inline struct fw_priv *to_fw_priv(struct kref *ref) DEFINE_MUTEX(fw_lock); struct firmware_cache fw_cache; +bool fw_load_abort_all; void fw_state_init(struct fw_priv *fw_priv) {