* [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-20 17:12 ` Rafael J. Wysocki
2022-04-19 11:34 ` [PATCH v7 02/12] amba: Use driver_set_override() instead of open-coding Krzysztof Kozlowski
` (11 subsequent siblings)
12 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, virtualization,
Heiko Carstens, Vasily Gorbik, linux-arm-msm, Haiyang Zhang,
Rasmus Villemoes, Bjorn Helgaas, Bjorn Andersson,
linux-arm-kernel, Mathieu Poirier, linux-kernel, linux-spi,
Krzysztof Kozlowski, Sven Schnelle, Linus Torvalds
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).
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.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 8c0d33e182fd..1b9d47b10bd0 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 8cc272fd5c99..b684157b7f2f 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1275,31 +1275,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 15e7c5e15d62..700453017e1c 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -151,6 +151,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 7c96f169d274..582d83ed9a91 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.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override
2022-04-19 11:34 ` [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override Krzysztof Kozlowski
@ 2022-04-20 17:12 ` Rafael J. Wysocki
0 siblings, 0 replies; 17+ messages in thread
From: Rafael J. Wysocki @ 2022-04-20 17:12 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Linux on Hyper-V List, Stuart Yoder, Rafael J. Wysocki, Linux PCI,
linux-remoteproc,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Bjorn Andersson, Vineeth Vijayan, Alexander Gordeev,
K. Y. Srinivasan, linux-clk, linux-s390, Wei Liu,
Stephen Hemminger, Dexuan Cui, Andy Shevchenko, Andy Gross,
NXP Linux Team, Christian Borntraeger, Heiko Carstens,
Vasily Gorbik, linux-arm-msm, Haiyang Zhang, linux-spi,
Rasmus Villemoes, Bjorn Helgaas, virtualization, Linux ARM,
Mathieu Poirier, Greg Kroah-Hartman, Peter Oberparleiter,
Linux Kernel Mailing List, Sven Schnelle, Linus Torvalds
On Tue, Apr 19, 2022 at 1:34 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> 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).
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.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 8c0d33e182fd..1b9d47b10bd0 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.
I would stick to one-line description here and possibly expand them in
the body of the comment.
Regardless, I think that the series is an improvement, so please feel
free to add
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
to this patch and
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
to the other patches in the series.
> + * @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 8cc272fd5c99..b684157b7f2f 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -1275,31 +1275,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 15e7c5e15d62..700453017e1c 100644
> --- a/include/linux/device/driver.h
> +++ b/include/linux/device/driver.h
> @@ -151,6 +151,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 7c96f169d274..582d83ed9a91 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.32.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 02/12] amba: Use driver_set_override() instead of open-coding
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 03/12] fsl-mc: " Krzysztof Kozlowski
` (10 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, virtualization,
Heiko Carstens, Vasily Gorbik, linux-arm-msm, Haiyang Zhang,
Rasmus Villemoes, Bjorn Helgaas, Bjorn Andersson,
linux-arm-kernel, Mathieu Poirier, linux-kernel, linux-spi,
Krzysztof Kozlowski, Sven Schnelle, Linus Torvalds
Use a helper to set driver_override to reduce the amount of duplicated
code. Make the driver_override field const char, because it is not
modified by the core and it matches other subsystems.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
drivers/amba/bus.c | 28 ++++------------------------
include/linux/amba/bus.h | 6 +++++-
2 files changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index d3bd14aaabf6..f3d26d698b77 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -94,31 +94,11 @@ static ssize_t driver_override_store(struct device *_dev,
const char *buf, size_t count)
{
struct amba_device *dev = to_amba_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 = dev->driver_override;
- if (strlen(driver_override)) {
- dev->driver_override = driver_override;
- } else {
- kfree(driver_override);
- dev->driver_override = NULL;
- }
- device_unlock(_dev);
+ int ret;
- kfree(old);
+ ret = driver_set_override(_dev, &dev->driver_override, buf, count);
+ if (ret)
+ return ret;
return count;
}
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index 6562f543c3e0..93799a72ff82 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -70,7 +70,11 @@ struct amba_device {
unsigned int cid;
struct amba_cs_uci_id uci;
unsigned int irq[AMBA_NR_IRQS];
- char *driver_override;
+ /*
+ * 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;
};
struct amba_driver {
--
2.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 03/12] fsl-mc: Use driver_set_override() instead of open-coding
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 02/12] amba: Use driver_set_override() instead of open-coding Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 04/12] hv: " Krzysztof Kozlowski
` (9 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, virtualization,
Heiko Carstens, Vasily Gorbik, linux-arm-msm, Haiyang Zhang,
Rasmus Villemoes, Bjorn Helgaas, Bjorn Andersson,
linux-arm-kernel, Mathieu Poirier, linux-kernel, linux-spi,
Krzysztof Kozlowski, Sven Schnelle, Linus Torvalds
Use a helper to set driver_override to reduce the amount of duplicated
code. Make the driver_override field const char, because it is not
modified by the core and it matches other subsystems.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
drivers/bus/fsl-mc/fsl-mc-bus.c | 25 ++++---------------------
include/linux/fsl/mc.h | 6 ++++--
2 files changed, 8 insertions(+), 23 deletions(-)
diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index 8fd4a356a86e..ba01c7f4de92 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -166,31 +166,14 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
- char *driver_override, *old = mc_dev->driver_override;
- char *cp;
+ int ret;
if (WARN_ON(dev->bus != &fsl_mc_bus_type))
return -EINVAL;
- 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';
-
- if (strlen(driver_override)) {
- mc_dev->driver_override = driver_override;
- } else {
- kfree(driver_override);
- mc_dev->driver_override = NULL;
- }
-
- kfree(old);
+ ret = driver_set_override(dev, &mc_dev->driver_override, buf, count);
+ if (ret)
+ return ret;
return count;
}
diff --git a/include/linux/fsl/mc.h b/include/linux/fsl/mc.h
index 7b6c42bfb660..7a87ab9eba99 100644
--- a/include/linux/fsl/mc.h
+++ b/include/linux/fsl/mc.h
@@ -170,7 +170,9 @@ struct fsl_mc_obj_desc {
* @regions: pointer to array of MMIO region entries
* @irqs: pointer to array of pointers to interrupts allocated to this device
* @resource: generic resource associated with this MC object device, if any.
- * @driver_override: driver name to force a match
+ * @driver_override: driver name to force a match; do not set directly,
+ * because core frees it; use driver_set_override() to
+ * set or clear it.
*
* Generic device object for MC object devices that are "attached" to a
* MC bus.
@@ -204,7 +206,7 @@ struct fsl_mc_device {
struct fsl_mc_device_irq **irqs;
struct fsl_mc_resource *resource;
struct device_link *consumer_link;
- char *driver_override;
+ const char *driver_override;
};
#define to_fsl_mc_device(_dev) \
--
2.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 04/12] hv: Use driver_set_override() instead of open-coding
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
` (2 preceding siblings ...)
2022-04-19 11:34 ` [PATCH v7 03/12] fsl-mc: " Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 05/12] PCI: " Krzysztof Kozlowski
` (8 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, virtualization,
Heiko Carstens, Vasily Gorbik, linux-arm-msm, Haiyang Zhang,
Rasmus Villemoes, Bjorn Helgaas, Bjorn Andersson,
linux-arm-kernel, Michael Kelley, Mathieu Poirier, linux-kernel,
linux-spi, Krzysztof Kozlowski, Sven Schnelle, Linus Torvalds
Use a helper to set driver_override to the reduce amount of duplicated
code. Make the driver_override field const char, because it is not
modified by the core and it matches other subsystems.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/hv/vmbus_drv.c | 28 ++++------------------------
include/linux/hyperv.h | 6 +++++-
2 files changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 14de17087864..607e40aba18e 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -575,31 +575,11 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct hv_device *hv_dev = device_to_hv_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 = hv_dev->driver_override;
- if (strlen(driver_override)) {
- hv_dev->driver_override = driver_override;
- } else {
- kfree(driver_override);
- hv_dev->driver_override = NULL;
- }
- device_unlock(dev);
+ int ret;
- kfree(old);
+ ret = driver_set_override(dev, &hv_dev->driver_override, buf, count);
+ if (ret)
+ return ret;
return count;
}
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index fe2e0179ed51..12e2336b23b7 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1257,7 +1257,11 @@ struct hv_device {
u16 device_id;
struct device device;
- 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;
struct vmbus_channel *channel;
struct kset *channels_kset;
--
2.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 05/12] PCI: Use driver_set_override() instead of open-coding
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
` (3 preceding siblings ...)
2022-04-19 11:34 ` [PATCH v7 04/12] hv: " Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 06/12] s390/cio: " Krzysztof Kozlowski
` (7 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, virtualization,
Heiko Carstens, Vasily Gorbik, linux-arm-msm, Haiyang Zhang,
Rasmus Villemoes, Bjorn Helgaas, Bjorn Andersson,
linux-arm-kernel, Mathieu Poirier, linux-kernel, linux-spi,
Krzysztof Kozlowski, Sven Schnelle, Linus Torvalds
Use a helper to set driver_override to the reduce amount of duplicated
code. Make the driver_override field const char, because it is not
modified by the core and it matches other subsystems.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
drivers/pci/pci-sysfs.c | 28 ++++------------------------
include/linux/pci.h | 6 +++++-
2 files changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index c263ffc5884a..fc804e08e3cb 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -567,31 +567,11 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct pci_dev *pdev = to_pci_dev(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/pci.h b/include/linux/pci.h
index 60adf42460ab..844d38f589cf 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -516,7 +516,11 @@ struct pci_dev {
u16 acs_cap; /* ACS Capability offset */
phys_addr_t rom; /* Physical address if not from BAR */
size_t romlen; /* Length if not from BAR */
- 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;
unsigned long priv_flags; /* Private flags for the PCI driver */
--
2.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 06/12] s390/cio: Use driver_set_override() instead of open-coding
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
` (4 preceding siblings ...)
2022-04-19 11:34 ` [PATCH v7 05/12] PCI: " Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 07/12] spi: Use helper for safer setting of driver_override Krzysztof Kozlowski
` (6 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, virtualization,
Heiko Carstens, Vasily Gorbik, linux-arm-msm, Haiyang Zhang,
Rasmus Villemoes, Bjorn Helgaas, Bjorn Andersson,
linux-arm-kernel, Mathieu Poirier, linux-kernel, linux-spi,
Krzysztof Kozlowski, Sven Schnelle, Linus Torvalds
Use a helper to set driver_override to the reduce amount of duplicated
code. Make the driver_override field const char, because it is not
modified by the core and it matches other subsystems.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Vineeth Vijayan <vneethv@linux.ibm.com>
---
drivers/s390/cio/cio.h | 6 +++++-
drivers/s390/cio/css.c | 28 ++++------------------------
2 files changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/s390/cio/cio.h b/drivers/s390/cio/cio.h
index 1cb9daf9c645..fa8df50bb49e 100644
--- a/drivers/s390/cio/cio.h
+++ b/drivers/s390/cio/cio.h
@@ -103,7 +103,11 @@ struct subchannel {
struct work_struct todo_work;
struct schib_config config;
u64 dma_mask;
- 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;
} __attribute__ ((aligned(8)));
DECLARE_PER_CPU_ALIGNED(struct irb, cio_irb);
diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c
index fa8293335077..913b6ddd040b 100644
--- a/drivers/s390/cio/css.c
+++ b/drivers/s390/cio/css.c
@@ -338,31 +338,11 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct subchannel *sch = to_subchannel(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 = sch->driver_override;
- if (strlen(driver_override)) {
- sch->driver_override = driver_override;
- } else {
- kfree(driver_override);
- sch->driver_override = NULL;
- }
- device_unlock(dev);
+ int ret;
- kfree(old);
+ ret = driver_set_override(dev, &sch->driver_override, buf, count);
+ if (ret)
+ return ret;
return count;
}
--
2.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 07/12] spi: Use helper for safer setting of driver_override
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
` (5 preceding siblings ...)
2022-04-19 11:34 ` [PATCH v7 06/12] s390/cio: " Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 08/12] vdpa: " Krzysztof Kozlowski
` (5 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, virtualization,
Heiko Carstens, Vasily Gorbik, linux-arm-msm, Haiyang Zhang,
Mark Brown, Rasmus Villemoes, Bjorn Helgaas, Bjorn Andersson,
linux-arm-kernel, Mathieu Poirier, linux-kernel, linux-spi,
Krzysztof Kozlowski, Sven Schnelle, Linus Torvalds
Use a helper to set driver_override to the reduce amount of duplicated
code.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi.c | 26 ++++----------------------
include/linux/spi/spi.h | 2 ++
2 files changed, 6 insertions(+), 22 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 890ff46c784a..be8f1a1e21b2 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -71,29 +71,11 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct spi_device *spi = to_spi_device(dev);
- const char *end = memchr(buf, '\n', count);
- const size_t len = end ? end - buf : count;
- const char *driver_override, *old;
-
- /* We need to keep extra room for a newline when displaying value */
- if (len >= (PAGE_SIZE - 1))
- return -EINVAL;
-
- driver_override = kstrndup(buf, len, GFP_KERNEL);
- if (!driver_override)
- return -ENOMEM;
+ int ret;
- device_lock(dev);
- old = spi->driver_override;
- if (len) {
- spi->driver_override = driver_override;
- } else {
- /* Empty string, disable driver override */
- spi->driver_override = NULL;
- kfree(driver_override);
- }
- device_unlock(dev);
- kfree(old);
+ ret = driver_set_override(dev, &spi->driver_override, buf, count);
+ if (ret)
+ return ret;
return count;
}
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 5f8c063ddff4..f0177f9b6e13 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -138,6 +138,8 @@ extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer);
* for driver coldplugging, and in uevents used for hotplugging
* @driver_override: If the name of a driver is written to this attribute, then
* the device will bind to the named driver and only the named driver.
+ * Do not set directly, because core frees it; use driver_set_override() to
+ * set or clear it.
* @cs_gpiod: gpio descriptor of the chipselect line (optional, NULL when
* not using a GPIO line)
* @word_delay: delay to be inserted between consecutive
--
2.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 08/12] vdpa: Use helper for safer setting of driver_override
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
` (6 preceding siblings ...)
2022-04-19 11:34 ` [PATCH v7 07/12] spi: Use helper for safer setting of driver_override Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 09/12] clk: imx: scu: Fix kfree() of static memory on setting driver_override Krzysztof Kozlowski
` (4 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, Michael S . Tsirkin, linux-pci,
linux-remoteproc, alsa-devel, Peter Oberparleiter,
Vineeth Vijayan, Alexander Gordeev, K. Y. Srinivasan, linux-clk,
linux-s390, Wei Liu, Stephen Hemminger, Dexuan Cui,
Andy Shevchenko, Andy Gross, NXP Linux Team,
Christian Borntraeger, virtualization, Heiko Carstens,
Vasily Gorbik, linux-arm-msm, Haiyang Zhang, Rasmus Villemoes,
Bjorn Helgaas, Bjorn Andersson, linux-arm-kernel, Mathieu Poirier,
linux-kernel, linux-spi, Krzysztof Kozlowski, Sven Schnelle,
Linus Torvalds
Use a helper to set driver_override to the reduce amount of duplicated
code.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vdpa/vdpa.c | 29 ++++-------------------------
include/linux/vdpa.h | 4 +++-
2 files changed, 7 insertions(+), 26 deletions(-)
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 2b75c00b1005..33d1ad60cba7 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -77,32 +77,11 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct vdpa_device *vdev = dev_to_vdpa(dev);
- const char *driver_override, *old;
- char *cp;
+ int ret;
- /* 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 = vdev->driver_override;
- if (strlen(driver_override)) {
- vdev->driver_override = driver_override;
- } else {
- kfree(driver_override);
- vdev->driver_override = NULL;
- }
- device_unlock(dev);
-
- kfree(old);
+ ret = driver_set_override(dev, &vdev->driver_override, buf, count);
+ if (ret)
+ return ret;
return count;
}
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 8943a209202e..c0a5083632ab 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -64,7 +64,9 @@ struct vdpa_mgmt_dev;
* struct vdpa_device - representation of a vDPA device
* @dev: underlying device
* @dma_dev: the actual device that is performing DMA
- * @driver_override: driver name to force a match
+ * @driver_override: driver name to force a match; do not set directly,
+ * because core frees it; use driver_set_override() to
+ * set or clear it.
* @config: the configuration ops for this device.
* @cf_mutex: Protects get and set access to configuration layout.
* @index: device index
--
2.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 09/12] clk: imx: scu: Fix kfree() of static memory on setting driver_override
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
` (7 preceding siblings ...)
2022-04-19 11:34 ` [PATCH v7 08/12] vdpa: " Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-23 16:09 ` Abel Vesa
2022-04-19 11:34 ` [PATCH v7 10/12] slimbus: qcom-ngd: " Krzysztof Kozlowski
` (3 subsequent siblings)
12 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, virtualization,
Heiko Carstens, Vasily Gorbik, linux-arm-msm, Haiyang Zhang,
Rasmus Villemoes, Bjorn Helgaas, Bjorn Andersson,
linux-arm-kernel, Mathieu Poirier, Stephen Boyd, linux-kernel,
linux-spi, Krzysztof Kozlowski, Sven Schnelle, Linus Torvalds
The driver_override field from platform driver should not be initialized
from static memory (string literal) because the core later kfree() it,
for example when driver_override is set via sysfs.
Use dedicated helper to set driver_override properly.
Fixes: 77d8f3068c63 ("clk: imx: scu: add two cells binding support")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Stephen Boyd <sboyd@kernel.org>
---
drivers/clk/imx/clk-scu.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index ed3c01d2e8ae..4996f1d94657 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -683,7 +683,12 @@ struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
return ERR_PTR(ret);
}
- pdev->driver_override = "imx-scu-clk";
+ ret = driver_set_override(&pdev->dev, &pdev->driver_override,
+ "imx-scu-clk", strlen("imx-scu-clk"));
+ if (ret) {
+ platform_device_put(pdev);
+ return ERR_PTR(ret);
+ }
ret = imx_clk_scu_attach_pd(&pdev->dev, rsrc_id);
if (ret)
--
2.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v7 09/12] clk: imx: scu: Fix kfree() of static memory on setting driver_override
2022-04-19 11:34 ` [PATCH v7 09/12] clk: imx: scu: Fix kfree() of static memory on setting driver_override Krzysztof Kozlowski
@ 2022-04-23 16:09 ` Abel Vesa
0 siblings, 0 replies; 17+ messages in thread
From: Abel Vesa @ 2022-04-23 16:09 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-hyperv, Stuart Yoder, Rafael J. Wysocki, linux-pci,
linux-remoteproc, alsa-devel, Bjorn Andersson, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, Heiko Carstens,
Vasily Gorbik, linux-arm-msm, Haiyang Zhang, linux-spi,
Rasmus Villemoes, Bjorn Helgaas, virtualization, linux-arm-kernel,
Mathieu Poirier, Stephen Boyd, Greg Kroah-Hartman,
Peter Oberparleiter, linux-kernel, Sven Schnelle, Linus Torvalds
On 22-04-19 13:34:32, Krzysztof Kozlowski wrote:
> The driver_override field from platform driver should not be initialized
> from static memory (string literal) because the core later kfree() it,
> for example when driver_override is set via sysfs.
>
> Use dedicated helper to set driver_override properly.
>
> Fixes: 77d8f3068c63 ("clk: imx: scu: add two cells binding support")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Acked-by: Stephen Boyd <sboyd@kernel.org>
Reviewed-by: Abel Vesa <abel.vesa@nxp.com>
> ---
> drivers/clk/imx/clk-scu.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> index ed3c01d2e8ae..4996f1d94657 100644
> --- a/drivers/clk/imx/clk-scu.c
> +++ b/drivers/clk/imx/clk-scu.c
> @@ -683,7 +683,12 @@ struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
> return ERR_PTR(ret);
> }
>
> - pdev->driver_override = "imx-scu-clk";
> + ret = driver_set_override(&pdev->dev, &pdev->driver_override,
> + "imx-scu-clk", strlen("imx-scu-clk"));
> + if (ret) {
> + platform_device_put(pdev);
> + return ERR_PTR(ret);
> + }
>
> ret = imx_clk_scu_attach_pd(&pdev->dev, rsrc_id);
> if (ret)
> --
> 2.32.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 10/12] slimbus: qcom-ngd: Fix kfree() of static memory on setting driver_override
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
` (8 preceding siblings ...)
2022-04-19 11:34 ` [PATCH v7 09/12] clk: imx: scu: Fix kfree() of static memory on setting driver_override Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 11/12] rpmsg: Constify local variable in field store macro Krzysztof Kozlowski
` (2 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Srinivas Kandagatla,
Vineeth Vijayan, Alexander Gordeev, K. Y. Srinivasan, linux-clk,
linux-s390, Wei Liu, Stephen Hemminger, Dexuan Cui,
Andy Shevchenko, Andy Gross, NXP Linux Team,
Christian Borntraeger, virtualization, Heiko Carstens,
Vasily Gorbik, linux-arm-msm, Haiyang Zhang, Rasmus Villemoes,
Bjorn Helgaas, Bjorn Andersson, linux-arm-kernel, Mathieu Poirier,
linux-kernel, linux-spi, Krzysztof Kozlowski, Sven Schnelle,
Linus Torvalds
The driver_override field from platform driver should not be initialized
from static memory (string literal) because the core later kfree() it,
for example when driver_override is set via sysfs.
Use dedicated helper to set driver_override properly.
Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
drivers/slimbus/qcom-ngd-ctrl.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index 0f29a08b4c09..0aa8408464ad 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -1434,6 +1434,7 @@ static int of_qcom_slim_ngd_register(struct device *parent,
const struct of_device_id *match;
struct device_node *node;
u32 id;
+ int ret;
match = of_match_node(qcom_slim_ngd_dt_match, parent->of_node);
data = match->data;
@@ -1455,7 +1456,17 @@ static int of_qcom_slim_ngd_register(struct device *parent,
}
ngd->id = id;
ngd->pdev->dev.parent = parent;
- ngd->pdev->driver_override = QCOM_SLIM_NGD_DRV_NAME;
+
+ ret = driver_set_override(&ngd->pdev->dev,
+ &ngd->pdev->driver_override,
+ QCOM_SLIM_NGD_DRV_NAME,
+ strlen(QCOM_SLIM_NGD_DRV_NAME));
+ if (ret) {
+ platform_device_put(ngd->pdev);
+ kfree(ngd);
+ of_node_put(node);
+ return ret;
+ }
ngd->pdev->dev.of_node = node;
ctrl->ngd = ngd;
--
2.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 11/12] rpmsg: Constify local variable in field store macro
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
` (9 preceding siblings ...)
2022-04-19 11:34 ` [PATCH v7 10/12] slimbus: qcom-ngd: " Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-19 11:34 ` [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override Krzysztof Kozlowski
2022-04-20 9:20 ` [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
12 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, virtualization,
Heiko Carstens, Vasily Gorbik, linux-arm-msm, Haiyang Zhang,
Rasmus Villemoes, Bjorn Helgaas, Bjorn Andersson,
linux-arm-kernel, Mathieu Poirier, linux-kernel, linux-spi,
Krzysztof Kozlowski, Sven Schnelle, Linus Torvalds
Memory pointed by variable 'old' in field store macro is not modified,
so it can be made a pointer to const.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
drivers/rpmsg/rpmsg_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 79368a957d89..95fc283f6af7 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -400,7 +400,8 @@ field##_store(struct device *dev, struct device_attribute *attr, \
const char *buf, size_t sz) \
{ \
struct rpmsg_device *rpdev = to_rpmsg_device(dev); \
- char *new, *old; \
+ const char *old; \
+ char *new; \
\
new = kstrndup(buf, sz, GFP_KERNEL); \
if (!new) \
--
2.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
` (10 preceding siblings ...)
2022-04-19 11:34 ` [PATCH v7 11/12] rpmsg: Constify local variable in field store macro Krzysztof Kozlowski
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
2022-04-20 9:20 ` [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
12 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, virtualization,
Heiko Carstens, Vasily Gorbik, linux-arm-msm, Haiyang Zhang,
Rasmus Villemoes, Bjorn Helgaas, Bjorn Andersson,
linux-arm-kernel, Mathieu Poirier, linux-kernel, linux-spi,
Krzysztof Kozlowski, Sven Schnelle, Linus Torvalds
The driver_override field from platform driver should not be initialized
from static memory (string literal) because the core later kfree() it,
for example when driver_override is set via sysfs.
Use dedicated helper to set driver_override properly.
Fixes: 950a7388f02b ("rpmsg: Turn name service into a stand alone driver")
Fixes: c0cdc19f84a4 ("rpmsg: Driver for user space endpoint interface")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/rpmsg/rpmsg_internal.h | 13 +++++++++++--
drivers/rpmsg/rpmsg_ns.c | 14 ++++++++++++--
include/linux/rpmsg.h | 6 ++++--
3 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index d4b23fd019a8..3e81642238d2 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -94,10 +94,19 @@ int rpmsg_release_channel(struct rpmsg_device *rpdev,
*/
static inline int rpmsg_ctrldev_register_device(struct rpmsg_device *rpdev)
{
+ int ret;
+
strcpy(rpdev->id.name, "rpmsg_ctrl");
- rpdev->driver_override = "rpmsg_ctrl";
+ ret = driver_set_override(&rpdev->dev, &rpdev->driver_override,
+ rpdev->id.name, strlen(rpdev->id.name));
+ if (ret)
+ return ret;
+
+ ret = rpmsg_register_device(rpdev);
+ if (ret)
+ kfree(rpdev->driver_override);
- return rpmsg_register_device(rpdev);
+ return ret;
}
#endif
diff --git a/drivers/rpmsg/rpmsg_ns.c b/drivers/rpmsg/rpmsg_ns.c
index 762ff1ae279f..8eb8f328237e 100644
--- a/drivers/rpmsg/rpmsg_ns.c
+++ b/drivers/rpmsg/rpmsg_ns.c
@@ -20,12 +20,22 @@
*/
int rpmsg_ns_register_device(struct rpmsg_device *rpdev)
{
+ int ret;
+
strcpy(rpdev->id.name, "rpmsg_ns");
- rpdev->driver_override = "rpmsg_ns";
+ ret = driver_set_override(&rpdev->dev, &rpdev->driver_override,
+ rpdev->id.name, strlen(rpdev->id.name));
+ if (ret)
+ return ret;
+
rpdev->src = RPMSG_NS_ADDR;
rpdev->dst = RPMSG_NS_ADDR;
- return rpmsg_register_device(rpdev);
+ ret = rpmsg_register_device(rpdev);
+ if (ret)
+ kfree(rpdev->driver_override);
+
+ return ret;
}
EXPORT_SYMBOL(rpmsg_ns_register_device);
diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
index 02fa9116cd60..20c8cd1cde21 100644
--- a/include/linux/rpmsg.h
+++ b/include/linux/rpmsg.h
@@ -41,7 +41,9 @@ struct rpmsg_channel_info {
* rpmsg_device - device that belong to the rpmsg bus
* @dev: the device struct
* @id: device id (used to match between rpmsg drivers and devices)
- * @driver_override: driver name to force a match
+ * @driver_override: driver name to force a match; do not set directly,
+ * because core frees it; use driver_set_override() to
+ * set or clear it.
* @src: local address
* @dst: destination address
* @ept: the rpmsg endpoint of this channel
@@ -51,7 +53,7 @@ struct rpmsg_channel_info {
struct rpmsg_device {
struct device dev;
struct rpmsg_device_id id;
- char *driver_override;
+ const char *driver_override;
u32 src;
u32 dst;
struct rpmsg_endpoint *ept;
--
2.32.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory)
2022-04-19 11:34 [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
` (11 preceding siblings ...)
2022-04-19 11:34 ` [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override Krzysztof Kozlowski
@ 2022-04-20 9:20 ` Krzysztof Kozlowski
2022-04-22 14:54 ` Greg Kroah-Hartman
12 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-20 9:20 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: linux-hyperv, Stuart Yoder, linux-pci, linux-remoteproc,
alsa-devel, Peter Oberparleiter, Vineeth Vijayan,
Alexander Gordeev, K. Y. Srinivasan, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, Christian Borntraeger, virtualization,
Heiko Carstens, Vasily Gorbik, linux-arm-msm, Haiyang Zhang,
Rasmus Villemoes, Bjorn Helgaas, Bjorn Andersson,
linux-arm-kernel, Mathieu Poirier, linux-kernel, linux-spi,
Sven Schnelle, Linus Torvalds
On 19/04/2022 13:34, Krzysztof Kozlowski wrote:
Hi Greg, Rafael,
The patchset was for some time on the lists, got some reviews, some
changes/feedback which I hope I applied/responded.
Entire set depends on the driver core changes, so maybe you could pick
up everything via drivers core tree?
> Dependencies (and stable):
> ==========================
> 1. All patches, including last three fixes, depend on the first patch
> introducing the helper.
> 2. The last three commits - fixes - are probably not backportable
> directly, because of this dependency. I don't know how to express
> this dependency here, since stable-kernel-rules.rst mentions only commits as
> possible dependencies.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory)
2022-04-20 9:20 ` [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory) Krzysztof Kozlowski
@ 2022-04-22 14:54 ` Greg Kroah-Hartman
0 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2022-04-22 14:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-hyperv, Stuart Yoder, Rafael J. Wysocki, linux-pci,
linux-remoteproc, alsa-devel, Peter Oberparleiter,
Vineeth Vijayan, Alexander Gordeev, K. Y. Srinivasan, linux-clk,
linux-s390, Wei Liu, Stephen Hemminger, Dexuan Cui,
Andy Shevchenko, Andy Gross, NXP Linux Team,
Christian Borntraeger, virtualization, Heiko Carstens,
Vasily Gorbik, linux-arm-msm, Haiyang Zhang, Rasmus Villemoes,
Bjorn Helgaas, Bjorn Andersson, linux-arm-kernel, Mathieu Poirier,
linux-kernel, linux-spi, Sven Schnelle, Linus Torvalds
On Wed, Apr 20, 2022 at 11:20:06AM +0200, Krzysztof Kozlowski wrote:
> On 19/04/2022 13:34, Krzysztof Kozlowski wrote:
>
> Hi Greg, Rafael,
>
> The patchset was for some time on the lists, got some reviews, some
> changes/feedback which I hope I applied/responded.
>
> Entire set depends on the driver core changes, so maybe you could pick
> up everything via drivers core tree?
Ok, will do, thanks.
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread