October 2023 - Linux-stable-mirror

[PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override

by Lee Jones

From: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> commit 6c2f421174273de8f83cde4286d1c076d43a2d35 upstream. Several core drivers and buses expect that driver_override is a dynamically allocated memory thus later they can kfree() it. However such assumption is not documented, there were in the past and there are already users setting it to a string literal. This leads to kfree() of static memory during device release (e.g. in error paths or during unbind): kernel BUG at ../mm/slub.c:3960! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM ... (kfree) from [<c058da50>] (platform_device_release+0x88/0xb4) (platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90) (device_release) from [<c0a69050>] (kobject_put+0xec/0x20c) (kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c) (exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4) (platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414) (really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4) (driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8) (bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c) (__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90) (bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c) (device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc) (of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc) (of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc) (of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118) (of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8) (of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404) Provide a helper which clearly documents the usage of driver_override. This will allow later to reuse the helper and reduce the amount of duplicated code. Convert the platform driver to use a new helper and make the driver_override field const char (it is not modified by the core). Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Acked-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> Link: https://lore.kernel.org/r/20220419113435.246203-2-krzysztof.kozlowski@linar… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Signed-off-by: Lee Jones <lee(a)kernel.org> --- drivers/base/driver.c | 69 +++++++++++++++++++++++++++++++++ drivers/base/platform.c | 28 ++----------- include/linux/device.h | 2 + include/linux/platform_device.h | 6 ++- 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/drivers/base/driver.c b/drivers/base/driver.c index 857c8f1b876e5..668c6c8c22f1f 100644 --- a/drivers/base/driver.c +++ b/drivers/base/driver.c @@ -29,6 +29,75 @@ static struct device *next_device(struct klist_iter *i) return dev; } +/** + * driver_set_override() - Helper to set or clear driver override. + * @dev: Device to change + * @override: Address of string to change (e.g. &device->driver_override); + * The contents will be freed and hold newly allocated override. + * @s: NUL-terminated string, new driver name to force a match, pass empty + * string to clear it ("" or "\n", where the latter is only for sysfs + * interface). + * @len: length of @s + * + * Helper to set or clear driver override in a device, intended for the cases + * when the driver_override field is allocated by driver/bus code. + * + * Returns: 0 on success or a negative error code on failure. + */ +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len) +{ + const char *new, *old; + char *cp; + + if (!override || !s) + return -EINVAL; + + /* + * The stored value will be used in sysfs show callback (sysfs_emit()), + * which has a length limit of PAGE_SIZE and adds a trailing newline. + * Thus we can store one character less to avoid truncation during sysfs + * show. + */ + if (len >= (PAGE_SIZE - 1)) + return -EINVAL; + + if (!len) { + /* Empty string passed - clear override */ + device_lock(dev); + old = *override; + *override = NULL; + device_unlock(dev); + kfree(old); + + return 0; + } + + cp = strnchr(s, len, '\n'); + if (cp) + len = cp - s; + + new = kstrndup(s, len, GFP_KERNEL); + if (!new) + return -ENOMEM; + + device_lock(dev); + old = *override; + if (cp != s) { + *override = new; + } else { + /* "\n" passed - clear override */ + kfree(new); + *override = NULL; + } + device_unlock(dev); + + kfree(old); + + return 0; +} +EXPORT_SYMBOL_GPL(driver_set_override); + /** * driver_for_each_device - Iterator for devices bound to a driver. * @drv: Driver we're iterating. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 2f89e618b142c..a09e7a681f7a7 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -891,31 +891,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct platform_device *pdev = to_platform_device(dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = pdev->driver_override; - if (strlen(driver_override)) { - pdev->driver_override = driver_override; - } else { - kfree(driver_override); - pdev->driver_override = NULL; - } - device_unlock(dev); + int ret; - kfree(old); + ret = driver_set_override(dev, &pdev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/device.h b/include/linux/device.h index 37e359d81a86f..bccd367c11de5 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -330,6 +330,8 @@ extern int __must_check driver_create_file(struct device_driver *driver, extern void driver_remove_file(struct device_driver *driver, const struct driver_attribute *attr); +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len); extern int __must_check driver_for_each_device(struct device_driver *drv, struct device *start, void *data, diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 9e5c98fcea8c6..8268439975b21 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -29,7 +29,11 @@ struct platform_device { struct resource *resource; const struct platform_device_id *id_entry; - char *driver_override; /* Driver name to force a match */ + /* + * Driver name to force a match. Do not set directly, because core + * frees it. Use driver_set_override() to set or clear it. + */ + const char *driver_override; /* MFD cell pointer */ struct mfd_cell *mfd_cell; -- 2.42.0.820.g83a721a137-goog

2 years, 1 month

1
5
0 0

[PATCH v5.4.y 1/6] driver: platform: Add helper for safer setting of driver_override

by Lee Jones

From: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> commit 6c2f421174273de8f83cde4286d1c076d43a2d35 upstream. Several core drivers and buses expect that driver_override is a dynamically allocated memory thus later they can kfree() it. However such assumption is not documented, there were in the past and there are already users setting it to a string literal. This leads to kfree() of static memory during device release (e.g. in error paths or during unbind): kernel BUG at ../mm/slub.c:3960! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM ... (kfree) from [<c058da50>] (platform_device_release+0x88/0xb4) (platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90) (device_release) from [<c0a69050>] (kobject_put+0xec/0x20c) (kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c) (exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4) (platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414) (really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4) (driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8) (bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c) (__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90) (bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c) (device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc) (of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc) (of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc) (of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118) (of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8) (of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404) Provide a helper which clearly documents the usage of driver_override. This will allow later to reuse the helper and reduce the amount of duplicated code. Convert the platform driver to use a new helper and make the driver_override field const char (it is not modified by the core). Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Acked-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> Link: https://lore.kernel.org/r/20220419113435.246203-2-krzysztof.kozlowski@linar… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Signed-off-by: Lee Jones <lee(a)kernel.org> --- drivers/base/driver.c | 69 +++++++++++++++++++++++++++++++++ drivers/base/platform.c | 28 ++----------- include/linux/device.h | 2 + include/linux/platform_device.h | 6 ++- 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/drivers/base/driver.c b/drivers/base/driver.c index 4e5ca632f35e8..ef14566a49710 100644 --- a/drivers/base/driver.c +++ b/drivers/base/driver.c @@ -29,6 +29,75 @@ static struct device *next_device(struct klist_iter *i) return dev; } +/** + * driver_set_override() - Helper to set or clear driver override. + * @dev: Device to change + * @override: Address of string to change (e.g. &device->driver_override); + * The contents will be freed and hold newly allocated override. + * @s: NUL-terminated string, new driver name to force a match, pass empty + * string to clear it ("" or "\n", where the latter is only for sysfs + * interface). + * @len: length of @s + * + * Helper to set or clear driver override in a device, intended for the cases + * when the driver_override field is allocated by driver/bus code. + * + * Returns: 0 on success or a negative error code on failure. + */ +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len) +{ + const char *new, *old; + char *cp; + + if (!override || !s) + return -EINVAL; + + /* + * The stored value will be used in sysfs show callback (sysfs_emit()), + * which has a length limit of PAGE_SIZE and adds a trailing newline. + * Thus we can store one character less to avoid truncation during sysfs + * show. + */ + if (len >= (PAGE_SIZE - 1)) + return -EINVAL; + + if (!len) { + /* Empty string passed - clear override */ + device_lock(dev); + old = *override; + *override = NULL; + device_unlock(dev); + kfree(old); + + return 0; + } + + cp = strnchr(s, len, '\n'); + if (cp) + len = cp - s; + + new = kstrndup(s, len, GFP_KERNEL); + if (!new) + return -ENOMEM; + + device_lock(dev); + old = *override; + if (cp != s) { + *override = new; + } else { + /* "\n" passed - clear override */ + kfree(new); + *override = NULL; + } + device_unlock(dev); + + kfree(old); + + return 0; +} +EXPORT_SYMBOL_GPL(driver_set_override); + /** * driver_for_each_device - Iterator for devices bound to a driver. * @drv: Driver we're iterating. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 75623b914b8c2..0ed43d185a900 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -973,31 +973,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct platform_device *pdev = to_platform_device(dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = pdev->driver_override; - if (strlen(driver_override)) { - pdev->driver_override = driver_override; - } else { - kfree(driver_override); - pdev->driver_override = NULL; - } - device_unlock(dev); + int ret; - kfree(old); + ret = driver_set_override(dev, &pdev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/device.h b/include/linux/device.h index c7be3a8073ec3..af4ecbf889107 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -422,6 +422,8 @@ extern int __must_check driver_create_file(struct device_driver *driver, extern void driver_remove_file(struct device_driver *driver, const struct driver_attribute *attr); +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len); extern int __must_check driver_for_each_device(struct device_driver *drv, struct device *start, void *data, diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 569f446502bed..c7bd8a1a60976 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -29,7 +29,11 @@ struct platform_device { struct resource *resource; const struct platform_device_id *id_entry; - char *driver_override; /* Driver name to force a match */ + /* + * Driver name to force a match. Do not set directly, because core + * frees it. Use driver_set_override() to set or clear it. + */ + const char *driver_override; /* MFD cell pointer */ struct mfd_cell *mfd_cell; -- 2.42.0.820.g83a721a137-goog

2 years, 1 month

1
5
0 0

[PATCH v5.10.y 1/6] driver: platform: Add helper for safer setting of driver_override

by Lee Jones

From: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> commit 6c2f421174273de8f83cde4286d1c076d43a2d35 upstream. Several core drivers and buses expect that driver_override is a dynamically allocated memory thus later they can kfree() it. However such assumption is not documented, there were in the past and there are already users setting it to a string literal. This leads to kfree() of static memory during device release (e.g. in error paths or during unbind): kernel BUG at ../mm/slub.c:3960! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM ... (kfree) from [<c058da50>] (platform_device_release+0x88/0xb4) (platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90) (device_release) from [<c0a69050>] (kobject_put+0xec/0x20c) (kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c) (exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4) (platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414) (really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4) (driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8) (bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c) (__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90) (bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c) (device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc) (of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc) (of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc) (of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118) (of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8) (of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404) Provide a helper which clearly documents the usage of driver_override. This will allow later to reuse the helper and reduce the amount of duplicated code. Convert the platform driver to use a new helper and make the driver_override field const char (it is not modified by the core). Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Acked-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> Link: https://lore.kernel.org/r/20220419113435.246203-2-krzysztof.kozlowski@linar… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Signed-off-by: Lee Jones <lee(a)kernel.org> --- drivers/base/driver.c | 69 +++++++++++++++++++++++++++++++++ drivers/base/platform.c | 28 ++----------- include/linux/device/driver.h | 2 + include/linux/platform_device.h | 6 ++- 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/drivers/base/driver.c b/drivers/base/driver.c index 8c0d33e182fd5..1b9d47b10bd0a 100644 --- a/drivers/base/driver.c +++ b/drivers/base/driver.c @@ -30,6 +30,75 @@ static struct device *next_device(struct klist_iter *i) return dev; } +/** + * driver_set_override() - Helper to set or clear driver override. + * @dev: Device to change + * @override: Address of string to change (e.g. &device->driver_override); + * The contents will be freed and hold newly allocated override. + * @s: NUL-terminated string, new driver name to force a match, pass empty + * string to clear it ("" or "\n", where the latter is only for sysfs + * interface). + * @len: length of @s + * + * Helper to set or clear driver override in a device, intended for the cases + * when the driver_override field is allocated by driver/bus code. + * + * Returns: 0 on success or a negative error code on failure. + */ +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len) +{ + const char *new, *old; + char *cp; + + if (!override || !s) + return -EINVAL; + + /* + * The stored value will be used in sysfs show callback (sysfs_emit()), + * which has a length limit of PAGE_SIZE and adds a trailing newline. + * Thus we can store one character less to avoid truncation during sysfs + * show. + */ + if (len >= (PAGE_SIZE - 1)) + return -EINVAL; + + if (!len) { + /* Empty string passed - clear override */ + device_lock(dev); + old = *override; + *override = NULL; + device_unlock(dev); + kfree(old); + + return 0; + } + + cp = strnchr(s, len, '\n'); + if (cp) + len = cp - s; + + new = kstrndup(s, len, GFP_KERNEL); + if (!new) + return -ENOMEM; + + device_lock(dev); + old = *override; + if (cp != s) { + *override = new; + } else { + /* "\n" passed - clear override */ + kfree(new); + *override = NULL; + } + device_unlock(dev); + + kfree(old); + + return 0; +} +EXPORT_SYMBOL_GPL(driver_set_override); + /** * driver_for_each_device - Iterator for devices bound to a driver. * @drv: Driver we're iterating. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 88aef93eb4ddf..647066229fec3 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -1046,31 +1046,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct platform_device *pdev = to_platform_device(dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = pdev->driver_override; - if (strlen(driver_override)) { - pdev->driver_override = driver_override; - } else { - kfree(driver_override); - pdev->driver_override = NULL; - } - device_unlock(dev); + int ret; - kfree(old); + ret = driver_set_override(dev, &pdev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h index ee7ba5b5417e5..a44f5adeaef5a 100644 --- a/include/linux/device/driver.h +++ b/include/linux/device/driver.h @@ -150,6 +150,8 @@ extern int __must_check driver_create_file(struct device_driver *driver, extern void driver_remove_file(struct device_driver *driver, const struct driver_attribute *attr); +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len); extern int __must_check driver_for_each_device(struct device_driver *drv, struct device *start, void *data, diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 17f9cd5626c83..e7a83b0218077 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -30,7 +30,11 @@ struct platform_device { struct resource *resource; const struct platform_device_id *id_entry; - char *driver_override; /* Driver name to force a match */ + /* + * Driver name to force a match. Do not set directly, because core + * frees it. Use driver_set_override() to set or clear it. + */ + const char *driver_override; /* MFD cell pointer */ struct mfd_cell *mfd_cell; -- 2.42.0.820.g83a721a137-goog

2 years, 1 month

1
5
0 0

[PATCH v5.15.y 1/6] driver: platform: Add helper for safer setting of driver_override

by Lee Jones

From: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> commit 6c2f421174273de8f83cde4286d1c076d43a2d35 upstream. Several core drivers and buses expect that driver_override is a dynamically allocated memory thus later they can kfree() it. However such assumption is not documented, there were in the past and there are already users setting it to a string literal. This leads to kfree() of static memory during device release (e.g. in error paths or during unbind): kernel BUG at ../mm/slub.c:3960! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM ... (kfree) from [<c058da50>] (platform_device_release+0x88/0xb4) (platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90) (device_release) from [<c0a69050>] (kobject_put+0xec/0x20c) (kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c) (exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4) (platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414) (really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4) (driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8) (bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c) (__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90) (bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c) (device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc) (of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc) (of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc) (of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118) (of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8) (of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404) Provide a helper which clearly documents the usage of driver_override. This will allow later to reuse the helper and reduce the amount of duplicated code. Convert the platform driver to use a new helper and make the driver_override field const char (it is not modified by the core). Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Acked-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> Link: https://lore.kernel.org/r/20220419113435.246203-2-krzysztof.kozlowski@linar… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Signed-off-by: Lee Jones <lee(a)kernel.org> --- drivers/base/driver.c | 69 +++++++++++++++++++++++++++++++++ drivers/base/platform.c | 28 ++----------- include/linux/device/driver.h | 2 + include/linux/platform_device.h | 6 ++- 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/drivers/base/driver.c b/drivers/base/driver.c index 8c0d33e182fd5..1b9d47b10bd0a 100644 --- a/drivers/base/driver.c +++ b/drivers/base/driver.c @@ -30,6 +30,75 @@ static struct device *next_device(struct klist_iter *i) return dev; } +/** + * driver_set_override() - Helper to set or clear driver override. + * @dev: Device to change + * @override: Address of string to change (e.g. &device->driver_override); + * The contents will be freed and hold newly allocated override. + * @s: NUL-terminated string, new driver name to force a match, pass empty + * string to clear it ("" or "\n", where the latter is only for sysfs + * interface). + * @len: length of @s + * + * Helper to set or clear driver override in a device, intended for the cases + * when the driver_override field is allocated by driver/bus code. + * + * Returns: 0 on success or a negative error code on failure. + */ +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len) +{ + const char *new, *old; + char *cp; + + if (!override || !s) + return -EINVAL; + + /* + * The stored value will be used in sysfs show callback (sysfs_emit()), + * which has a length limit of PAGE_SIZE and adds a trailing newline. + * Thus we can store one character less to avoid truncation during sysfs + * show. + */ + if (len >= (PAGE_SIZE - 1)) + return -EINVAL; + + if (!len) { + /* Empty string passed - clear override */ + device_lock(dev); + old = *override; + *override = NULL; + device_unlock(dev); + kfree(old); + + return 0; + } + + cp = strnchr(s, len, '\n'); + if (cp) + len = cp - s; + + new = kstrndup(s, len, GFP_KERNEL); + if (!new) + return -ENOMEM; + + device_lock(dev); + old = *override; + if (cp != s) { + *override = new; + } else { + /* "\n" passed - clear override */ + kfree(new); + *override = NULL; + } + device_unlock(dev); + + kfree(old); + + return 0; +} +EXPORT_SYMBOL_GPL(driver_set_override); + /** * driver_for_each_device - Iterator for devices bound to a driver. * @drv: Driver we're iterating. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index ac5cf1a8d79ab..596fbe6b701a5 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -1270,31 +1270,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct platform_device *pdev = to_platform_device(dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = pdev->driver_override; - if (strlen(driver_override)) { - pdev->driver_override = driver_override; - } else { - kfree(driver_override); - pdev->driver_override = NULL; - } - device_unlock(dev); + int ret; - kfree(old); + ret = driver_set_override(dev, &pdev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h index a498ebcf49933..abf948e102f5d 100644 --- a/include/linux/device/driver.h +++ b/include/linux/device/driver.h @@ -150,6 +150,8 @@ extern int __must_check driver_create_file(struct device_driver *driver, extern void driver_remove_file(struct device_driver *driver, const struct driver_attribute *attr); +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len); extern int __must_check driver_for_each_device(struct device_driver *drv, struct device *start, void *data, diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 8aefdc0099c86..72cf70857b85f 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -31,7 +31,11 @@ struct platform_device { struct resource *resource; const struct platform_device_id *id_entry; - char *driver_override; /* Driver name to force a match */ + /* + * Driver name to force a match. Do not set directly, because core + * frees it. Use driver_set_override() to set or clear it. + */ + const char *driver_override; /* MFD cell pointer */ struct mfd_cell *mfd_cell; -- 2.42.0.820.g83a721a137-goog

2 years, 1 month

1
5
0 0

FAILED: patch "[PATCH] mm/damon/sysfs: check DAMOS regions update progress from" failed to apply to 6.5-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 6.5-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.5.y git checkout FETCH_HEAD git cherry-pick -x 76b7069bcc89dec33f03eb08abee165d0306b754 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '2023102716-prudishly-reggae-1b29@gregkh' --subject-prefix 'PATCH 6.5.y' HEAD^.. Possible dependencies: thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 76b7069bcc89dec33f03eb08abee165d0306b754 Mon Sep 17 00:00:00 2001 From: SeongJae Park <sj(a)kernel.org> Date: Sat, 7 Oct 2023 20:04:32 +0000 Subject: [PATCH] mm/damon/sysfs: check DAMOS regions update progress from before_terminate() DAMON_SYSFS can receive DAMOS tried regions update request while kdamond is already out of the main loop and before_terminate callback (damon_sysfs_before_terminate() in this case) is not yet called. And damon_sysfs_handle_cmd() can further be finished before the callback is invoked. Then, damon_sysfs_before_terminate() unlocks damon_sysfs_lock, which is not locked by anyone. This happens because the callback function assumes damon_sysfs_cmd_request_callback() should be called before it. Check if the assumption was true before doing the unlock, to avoid this problem. Link: https://lkml.kernel.org/r/20231007200432.3110-1-sj@kernel.org Fixes: f1d13cacabe1 ("mm/damon/sysfs: implement DAMOS tried regions update command") Signed-off-by: SeongJae Park <sj(a)kernel.org> Cc: <stable(a)vger.kernel.org> [6.2.x] Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c index b86ba7b0a921..f60e56150feb 100644 --- a/mm/damon/sysfs.c +++ b/mm/damon/sysfs.c @@ -1208,6 +1208,8 @@ static int damon_sysfs_set_targets(struct damon_ctx *ctx, return 0; } +static bool damon_sysfs_schemes_regions_updating; + static void damon_sysfs_before_terminate(struct damon_ctx *ctx) { struct damon_target *t, *next; @@ -1219,8 +1221,10 @@ static void damon_sysfs_before_terminate(struct damon_ctx *ctx) cmd = damon_sysfs_cmd_request.cmd; if (kdamond && ctx == kdamond->damon_ctx && (cmd == DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_REGIONS || - cmd == DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_BYTES)) { + cmd == DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_BYTES) && + damon_sysfs_schemes_regions_updating) { damon_sysfs_schemes_update_regions_stop(ctx); + damon_sysfs_schemes_regions_updating = false; mutex_unlock(&damon_sysfs_lock); } @@ -1340,7 +1344,6 @@ static int damon_sysfs_commit_input(struct damon_sysfs_kdamond *kdamond) static int damon_sysfs_cmd_request_callback(struct damon_ctx *c) { struct damon_sysfs_kdamond *kdamond; - static bool damon_sysfs_schemes_regions_updating; bool total_bytes_only = false; int err = 0;

2 years, 1 month

3
3
0 0

FAILED: patch "[PATCH] drm/dp_mst: Fix NULL deref in" failed to apply to 5.4-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 5.4-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.4.y git checkout FETCH_HEAD git cherry-pick -x 3d887d512494d678b17c57b835c32f4e48d34f26 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '2023102723-steerable-trench-2f00@gregkh' --subject-prefix 'PATCH 5.4.y' HEAD^.. Possible dependencies: thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 3d887d512494d678b17c57b835c32f4e48d34f26 Mon Sep 17 00:00:00 2001 From: Lukasz Majczak <lma(a)semihalf.com> Date: Fri, 22 Sep 2023 08:34:10 +0200 Subject: [PATCH] drm/dp_mst: Fix NULL deref in get_mst_branch_device_by_guid_helper() As drm_dp_get_mst_branch_device_by_guid() is called from drm_dp_get_mst_branch_device_by_guid(), mstb parameter has to be checked, otherwise NULL dereference may occur in the call to the memcpy() and cause following: [12579.365869] BUG: kernel NULL pointer dereference, address: 0000000000000049 [12579.365878] #PF: supervisor read access in kernel mode [12579.365880] #PF: error_code(0x0000) - not-present page [12579.365882] PGD 0 P4D 0 [12579.365887] Oops: 0000 [#1] PREEMPT SMP NOPTI ... [12579.365895] Workqueue: events_long drm_dp_mst_up_req_work [12579.365899] RIP: 0010:memcmp+0xb/0x29 [12579.365921] Call Trace: [12579.365927] get_mst_branch_device_by_guid_helper+0x22/0x64 [12579.365930] drm_dp_mst_up_req_work+0x137/0x416 [12579.365933] process_one_work+0x1d0/0x419 [12579.365935] worker_thread+0x11a/0x289 [12579.365938] kthread+0x13e/0x14f [12579.365941] ? process_one_work+0x419/0x419 [12579.365943] ? kthread_blkcg+0x31/0x31 [12579.365946] ret_from_fork+0x1f/0x30 As get_mst_branch_device_by_guid_helper() is recursive, moving condition to the first line allow to remove a similar one for step over of NULL elements inside a loop. Fixes: 5e93b8208d3c ("drm/dp/mst: move GUID storage from mgr, port to only mst branch") Cc: <stable(a)vger.kernel.org> # 4.14+ Signed-off-by: Lukasz Majczak <lma(a)semihalf.com> Reviewed-by: Radoslaw Biernacki <rad(a)chromium.org> Signed-off-by: Manasi Navare <navaremanasi(a)chromium.org> Link: https://patchwork.freedesktop.org/patch/msgid/20230922063410.23626-1-lma@se… diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index ed96cfcfa304..8c929ef72c72 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -2574,14 +2574,14 @@ static struct drm_dp_mst_branch *get_mst_branch_device_by_guid_helper( struct drm_dp_mst_branch *found_mstb; struct drm_dp_mst_port *port; + if (!mstb) + return NULL; + if (memcmp(mstb->guid, guid, 16) == 0) return mstb; list_for_each_entry(port, &mstb->ports, next) { - if (!port->mstb) - continue; - found_mstb = get_mst_branch_device_by_guid_helper(port->mstb, guid); if (found_mstb)

2 years, 1 month

3
2
0 0

[PATCH 6.1.y] platform/x86: Add s2idle quirk for more Lenovo laptops

by David Lazar

commit 3bde7ec13c971445faade32172cb0b4370b841d9 upstream. When suspending to idle and resuming on some Lenovo laptops using the Mendocino APU, multiple NVME IOMMU page faults occur, showing up in dmesg as repeated errors: nvme 0000:01:00.0: AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x000b address=0xb6674000 flags=0x0000] The system is unstable afterwards. Applying the s2idle quirk introduced by commit 455cd867b85b ("platform/x86: thinkpad_acpi: Add a s2idle resume quirk for a number of laptops") allows these systems to work with the IOMMU enabled and s2idle resume to work. Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218024 Suggested-by: Mario Limonciello <mario.limonciello(a)amd.com> Suggested-by: Mark Pearson <mpearson-lenovo(a)squebb.ca> Signed-off-by: David Lazar <dlazar(a)gmail.com> Reviewed-by: Mario Limonciello <mario.limonciello(a)amd.com> Reviewed-by: Mark Pearson <mpearson-lenovo(a)squebb.ca> Link: https://lore.kernel.org/r/ZTlsyOaFucF2pWrL@localhost Signed-off-by: Hans de Goede <hdegoede(a)redhat.com> --- drivers/platform/x86/thinkpad_acpi.c | 73 ++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index e7ece2738de9..3bb60687f2e4 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -4513,6 +4513,79 @@ static const struct dmi_system_id fwbug_list[] __initconst = { DMI_MATCH(DMI_PRODUCT_NAME, "21A1"), } }, + /* https://bugzilla.kernel.org/show_bug.cgi?id=218024 */ + { + .ident = "V14 G4 AMN", + .driver_data = &quirk_s2idle_bug, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "82YT"), + } + }, + { + .ident = "V14 G4 AMN", + .driver_data = &quirk_s2idle_bug, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "83GE"), + } + }, + { + .ident = "V15 G4 AMN", + .driver_data = &quirk_s2idle_bug, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "82YU"), + } + }, + { + .ident = "V15 G4 AMN", + .driver_data = &quirk_s2idle_bug, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "83CQ"), + } + }, + { + .ident = "IdeaPad 1 14AMN7", + .driver_data = &quirk_s2idle_bug, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "82VF"), + } + }, + { + .ident = "IdeaPad 1 15AMN7", + .driver_data = &quirk_s2idle_bug, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "82VG"), + } + }, + { + .ident = "IdeaPad 1 15AMN7", + .driver_data = &quirk_s2idle_bug, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "82X5"), + } + }, + { + .ident = "IdeaPad Slim 3 14AMN8", + .driver_data = &quirk_s2idle_bug, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "82XN"), + } + }, + { + .ident = "IdeaPad Slim 3 15AMN8", + .driver_data = &quirk_s2idle_bug, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "82XQ"), + } + }, {} }; -- 2.39.2

2 years, 1 month

3
2
0 0

[PATCH v3] drm/i915: Flush WC GGTT only on required platforms

by Nirmoy Das

gen8_ggtt_invalidate() is only needed for limited set of platforms where GGTT is mapped as WC. This was added as way to fix WC based GGTT in commit 0f9b91c754b7 ("drm/i915: flush system agent TLBs on SNB") and there are no reference in HW docs that forces us to use this on non-WC backed GGTT. This can also cause unwanted side-effects on XE_HP platforms where GFX_FLSH_CNTL_GEN6 is not valid anymore. v2: Add a func to detect wc ggtt detection (Ville) v3: Improve commit log and add reference commit (Daniel) Fixes: d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement") Cc: Rodrigo Vivi <rodrigo.vivi(a)intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin(a)linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen(a)linux.intel.com> Cc: Jani Nikula <jani.nikula(a)linux.intel.com> Cc: Jonathan Cavitt <jonathan.cavitt(a)intel.com> Cc: John Harrison <john.c.harrison(a)intel.com> Cc: Andi Shyti <andi.shyti(a)linux.intel.com> Cc: Ville Syrjälä <ville.syrjala(a)linux.intel.com> Cc: Daniel Vetter <daniel(a)ffwll.ch> Cc: <stable(a)vger.kernel.org> # v6.2+ Suggested-by: Matt Roper <matthew.d.roper(a)intel.com> Signed-off-by: Nirmoy Das <nirmoy.das(a)intel.com> Reviewed-by: Matt Roper <matthew.d.roper(a)intel.com> Reviewed-by: Andi Shyti <andi.shyti(a)linux.intel.com> --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 35 +++++++++++++++++++--------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 1c93e84278a0..15fc8e4703f4 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -195,6 +195,21 @@ void gen6_ggtt_invalidate(struct i915_ggtt *ggtt) spin_unlock_irq(&uncore->lock); } +static bool needs_wc_ggtt_mapping(struct drm_i915_private *i915) +{ + /* + * On BXT+/ICL+ writes larger than 64 bit to the GTT pagetable range + * will be dropped. For WC mappings in general we have 64 byte burst + * writes when the WC buffer is flushed, so we can't use it, but have to + * resort to an uncached mapping. The WC issue is easily caught by the + * readback check when writing GTT PTE entries. + */ + if (!IS_GEN9_LP(i915) && GRAPHICS_VER(i915) < 11) + return true; + + return false; +} + static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt) { struct intel_uncore *uncore = ggtt->vm.gt->uncore; @@ -202,8 +217,12 @@ static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt) /* * Note that as an uncached mmio write, this will flush the * WCB of the writes into the GGTT before it triggers the invalidate. + * + * Only perform this when GGTT is mapped as WC, see ggtt_probe_common(). */ - intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN); + if (needs_wc_ggtt_mapping(ggtt->vm.i915)) + intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, + GFX_FLSH_CNTL_EN); } static void guc_ggtt_ct_invalidate(struct intel_gt *gt) @@ -1140,17 +1159,11 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size) GEM_WARN_ON(pci_resource_len(pdev, GEN4_GTTMMADR_BAR) != gen6_gttmmadr_size(i915)); phys_addr = pci_resource_start(pdev, GEN4_GTTMMADR_BAR) + gen6_gttadr_offset(i915); - /* - * On BXT+/ICL+ writes larger than 64 bit to the GTT pagetable range - * will be dropped. For WC mappings in general we have 64 byte burst - * writes when the WC buffer is flushed, so we can't use it, but have to - * resort to an uncached mapping. The WC issue is easily caught by the - * readback check when writing GTT PTE entries. - */ - if (IS_GEN9_LP(i915) || GRAPHICS_VER(i915) >= 11) - ggtt->gsm = ioremap(phys_addr, size); - else + if (needs_wc_ggtt_mapping(i915)) ggtt->gsm = ioremap_wc(phys_addr, size); + else + ggtt->gsm = ioremap(phys_addr, size); + if (!ggtt->gsm) { drm_err(&i915->drm, "Failed to map the ggtt page table\n"); return -ENOMEM; -- 2.41.0

2 years, 1 month

3
5
0 0

[PATCH 1/6] driver: platform: Add helper for safer setting of driver_override

by Lee Jones

From: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> commit 6c2f421174273de8f83cde4286d1c076d43a2d35 upstream. Several core drivers and buses expect that driver_override is a dynamically allocated memory thus later they can kfree() it. However such assumption is not documented, there were in the past and there are already users setting it to a string literal. This leads to kfree() of static memory during device release (e.g. in error paths or during unbind): kernel BUG at ../mm/slub.c:3960! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM ... (kfree) from [<c058da50>] (platform_device_release+0x88/0xb4) (platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90) (device_release) from [<c0a69050>] (kobject_put+0xec/0x20c) (kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c) (exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4) (platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414) (really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4) (driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8) (bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c) (__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90) (bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c) (device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc) (of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc) (of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc) (of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118) (of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8) (of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404) Provide a helper which clearly documents the usage of driver_override. This will allow later to reuse the helper and reduce the amount of duplicated code. Convert the platform driver to use a new helper and make the driver_override field const char (it is not modified by the core). Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Acked-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> Link: https://lore.kernel.org/r/20220419113435.246203-2-krzysztof.kozlowski@linar… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Signed-off-by: Lee Jones <lee(a)kernel.org> Change-Id: I131f04170f2f76d468565b27929e0ee6fd0e7adf --- drivers/base/driver.c | 69 +++++++++++++++++++++++++++++++++ drivers/base/platform.c | 28 ++----------- include/linux/device/driver.h | 2 + include/linux/platform_device.h | 6 ++- 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/drivers/base/driver.c b/drivers/base/driver.c index 8c0d33e182fd5..1b9d47b10bd0a 100644 --- a/drivers/base/driver.c +++ b/drivers/base/driver.c @@ -30,6 +30,75 @@ static struct device *next_device(struct klist_iter *i) return dev; } +/** + * driver_set_override() - Helper to set or clear driver override. + * @dev: Device to change + * @override: Address of string to change (e.g. &device->driver_override); + * The contents will be freed and hold newly allocated override. + * @s: NUL-terminated string, new driver name to force a match, pass empty + * string to clear it ("" or "\n", where the latter is only for sysfs + * interface). + * @len: length of @s + * + * Helper to set or clear driver override in a device, intended for the cases + * when the driver_override field is allocated by driver/bus code. + * + * Returns: 0 on success or a negative error code on failure. + */ +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len) +{ + const char *new, *old; + char *cp; + + if (!override || !s) + return -EINVAL; + + /* + * The stored value will be used in sysfs show callback (sysfs_emit()), + * which has a length limit of PAGE_SIZE and adds a trailing newline. + * Thus we can store one character less to avoid truncation during sysfs + * show. + */ + if (len >= (PAGE_SIZE - 1)) + return -EINVAL; + + if (!len) { + /* Empty string passed - clear override */ + device_lock(dev); + old = *override; + *override = NULL; + device_unlock(dev); + kfree(old); + + return 0; + } + + cp = strnchr(s, len, '\n'); + if (cp) + len = cp - s; + + new = kstrndup(s, len, GFP_KERNEL); + if (!new) + return -ENOMEM; + + device_lock(dev); + old = *override; + if (cp != s) { + *override = new; + } else { + /* "\n" passed - clear override */ + kfree(new); + *override = NULL; + } + device_unlock(dev); + + kfree(old); + + return 0; +} +EXPORT_SYMBOL_GPL(driver_set_override); + /** * driver_for_each_device - Iterator for devices bound to a driver. * @drv: Driver we're iterating. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index ac5cf1a8d79ab..596fbe6b701a5 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -1270,31 +1270,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct platform_device *pdev = to_platform_device(dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = pdev->driver_override; - if (strlen(driver_override)) { - pdev->driver_override = driver_override; - } else { - kfree(driver_override); - pdev->driver_override = NULL; - } - device_unlock(dev); + int ret; - kfree(old); + ret = driver_set_override(dev, &pdev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h index a498ebcf49933..abf948e102f5d 100644 --- a/include/linux/device/driver.h +++ b/include/linux/device/driver.h @@ -150,6 +150,8 @@ extern int __must_check driver_create_file(struct device_driver *driver, extern void driver_remove_file(struct device_driver *driver, const struct driver_attribute *attr); +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len); extern int __must_check driver_for_each_device(struct device_driver *drv, struct device *start, void *data, diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 8aefdc0099c86..72cf70857b85f 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -31,7 +31,11 @@ struct platform_device { struct resource *resource; const struct platform_device_id *id_entry; - char *driver_override; /* Driver name to force a match */ + /* + * Driver name to force a match. Do not set directly, because core + * frees it. Use driver_set_override() to set or clear it. + */ + const char *driver_override; /* MFD cell pointer */ struct mfd_cell *mfd_cell; -- 2.42.0.820.g83a721a137-goog

2 years, 1 month

1
7
0 0

[PATCH v5.10.y 1/6] driver: platform: Add helper for safer setting of driver_override

by Lee Jones

From: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> commit 6c2f421174273de8f83cde4286d1c076d43a2d35 upstream. Several core drivers and buses expect that driver_override is a dynamically allocated memory thus later they can kfree() it. However such assumption is not documented, there were in the past and there are already users setting it to a string literal. This leads to kfree() of static memory during device release (e.g. in error paths or during unbind): kernel BUG at ../mm/slub.c:3960! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM ... (kfree) from [<c058da50>] (platform_device_release+0x88/0xb4) (platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90) (device_release) from [<c0a69050>] (kobject_put+0xec/0x20c) (kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c) (exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4) (platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414) (really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4) (driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8) (bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c) (__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90) (bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c) (device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc) (of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc) (of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc) (of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118) (of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8) (of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404) Provide a helper which clearly documents the usage of driver_override. This will allow later to reuse the helper and reduce the amount of duplicated code. Convert the platform driver to use a new helper and make the driver_override field const char (it is not modified by the core). Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Acked-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> Link: https://lore.kernel.org/r/20220419113435.246203-2-krzysztof.kozlowski@linar… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Signed-off-by: Lee Jones <lee(a)kernel.org> Change-Id: Ic3c1fefbb9bf1a27f7adcca5bf0405e02d2f7775 --- drivers/base/driver.c | 69 +++++++++++++++++++++++++++++++++ drivers/base/platform.c | 28 ++----------- include/linux/device/driver.h | 2 + include/linux/platform_device.h | 6 ++- 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/drivers/base/driver.c b/drivers/base/driver.c index 8c0d33e182fd5..1b9d47b10bd0a 100644 --- a/drivers/base/driver.c +++ b/drivers/base/driver.c @@ -30,6 +30,75 @@ static struct device *next_device(struct klist_iter *i) return dev; } +/** + * driver_set_override() - Helper to set or clear driver override. + * @dev: Device to change + * @override: Address of string to change (e.g. &device->driver_override); + * The contents will be freed and hold newly allocated override. + * @s: NUL-terminated string, new driver name to force a match, pass empty + * string to clear it ("" or "\n", where the latter is only for sysfs + * interface). + * @len: length of @s + * + * Helper to set or clear driver override in a device, intended for the cases + * when the driver_override field is allocated by driver/bus code. + * + * Returns: 0 on success or a negative error code on failure. + */ +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len) +{ + const char *new, *old; + char *cp; + + if (!override || !s) + return -EINVAL; + + /* + * The stored value will be used in sysfs show callback (sysfs_emit()), + * which has a length limit of PAGE_SIZE and adds a trailing newline. + * Thus we can store one character less to avoid truncation during sysfs + * show. + */ + if (len >= (PAGE_SIZE - 1)) + return -EINVAL; + + if (!len) { + /* Empty string passed - clear override */ + device_lock(dev); + old = *override; + *override = NULL; + device_unlock(dev); + kfree(old); + + return 0; + } + + cp = strnchr(s, len, '\n'); + if (cp) + len = cp - s; + + new = kstrndup(s, len, GFP_KERNEL); + if (!new) + return -ENOMEM; + + device_lock(dev); + old = *override; + if (cp != s) { + *override = new; + } else { + /* "\n" passed - clear override */ + kfree(new); + *override = NULL; + } + device_unlock(dev); + + kfree(old); + + return 0; +} +EXPORT_SYMBOL_GPL(driver_set_override); + /** * driver_for_each_device - Iterator for devices bound to a driver. * @drv: Driver we're iterating. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 88aef93eb4ddf..647066229fec3 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -1046,31 +1046,11 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct platform_device *pdev = to_platform_device(dev); - char *driver_override, *old, *cp; - - /* We need to keep extra room for a newline */ - if (count >= (PAGE_SIZE - 1)) - return -EINVAL; - - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = pdev->driver_override; - if (strlen(driver_override)) { - pdev->driver_override = driver_override; - } else { - kfree(driver_override); - pdev->driver_override = NULL; - } - device_unlock(dev); + int ret; - kfree(old); + ret = driver_set_override(dev, &pdev->driver_override, buf, count); + if (ret) + return ret; return count; } diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h index ee7ba5b5417e5..a44f5adeaef5a 100644 --- a/include/linux/device/driver.h +++ b/include/linux/device/driver.h @@ -150,6 +150,8 @@ extern int __must_check driver_create_file(struct device_driver *driver, extern void driver_remove_file(struct device_driver *driver, const struct driver_attribute *attr); +int driver_set_override(struct device *dev, const char **override, + const char *s, size_t len); extern int __must_check driver_for_each_device(struct device_driver *drv, struct device *start, void *data, diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 17f9cd5626c83..e7a83b0218077 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -30,7 +30,11 @@ struct platform_device { struct resource *resource; const struct platform_device_id *id_entry; - char *driver_override; /* Driver name to force a match */ + /* + * Driver name to force a match. Do not set directly, because core + * frees it. Use driver_set_override() to set or clear it. + */ + const char *driver_override; /* MFD cell pointer */ struct mfd_cell *mfd_cell; -- 2.42.0.820.g83a721a137-goog

2 years, 1 month

1
6
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror October 2023