* [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
Hi,
This is a continuation of my old patchset from 2019. [1]
Back then, few drivers set driver_override wrong. I fixed Exynos
in a different way after discussions. QCOM NGD was not fixed
and a new user appeared - IMX SCU.
It seems "char *" in driver_override looks too consty, so we
tend to make a mistake of storing there string literals.
Changes since latest v7
=======================
1. Patch #1: remove out_free label, document clearing override in kerneldoc and
in code-comments (Andy).
2. Patch #12 (rpmsg): do not duplicate string (Biju).
Changes since latest v6
=======================
1. Patch #1: Don't check for !dev and handle len==0 (Andy).
2. New patch #11 (rpmsg): split constifying of local variable to a new patch.
Changes since latest v5
=======================
1. Patch #11 (rpmsg): split from previous patch 11 - easier to understand the
need of it.
2. Fix build issue in patch 12 (rpmsg).
Changes since latest v4
=======================
1. Correct commit msgs and comments after Andy's review.
2. Re-order code in new helper (patch #1) (Andy).
3. Add tags.
Changes since latest v3
=======================
1. Wrap comments, extend comment in driver_set_override() about newline.
2. Minor commit msg fixes.
3. Add tags.
Changes since latest v2
=======================
1. Make all driver_override fields as "const char *", just like SPI
and VDPA. (Mark)
2. Move "count" check to the new helper and add "count" argument. (Michael)
3. Fix typos in docs, patch subject. Extend doc. (Michael, Bjorn)
4. Compare pointers to reduce number of string readings in the helper.
5. Fix clk-imx return value.
Changes since latest v1 (not the old 2019 solution):
====================================================
https://lore.kernel.org/all/708eabb1-7b35-d525-d4c3-451d4a3de84f@rasmusvillemoes.dk/
1. Add helper for setting driver_override.
2. Use the helper.
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.
[1] https://lore.kernel.org/all/1550484960-2392-3-git-send-email-krzk@kernel.org/
Best regards,
Krzysztof
Krzysztof Kozlowski (12):
driver: platform: Add helper for safer setting of driver_override
amba: Use driver_set_override() instead of open-coding
fsl-mc: Use driver_set_override() instead of open-coding
hv: Use driver_set_override() instead of open-coding
PCI: Use driver_set_override() instead of open-coding
s390/cio: Use driver_set_override() instead of open-coding
spi: Use helper for safer setting of driver_override
vdpa: Use helper for safer setting of driver_override
clk: imx: scu: Fix kfree() of static memory on setting driver_override
slimbus: qcom-ngd: Fix kfree() of static memory on setting
driver_override
rpmsg: Constify local variable in field store macro
rpmsg: Fix kfree() of static memory on setting driver_override
drivers/amba/bus.c | 28 ++-----------
drivers/base/driver.c | 69 +++++++++++++++++++++++++++++++++
drivers/base/platform.c | 28 ++-----------
drivers/bus/fsl-mc/fsl-mc-bus.c | 25 ++----------
drivers/clk/imx/clk-scu.c | 7 +++-
drivers/hv/vmbus_drv.c | 28 ++-----------
drivers/pci/pci-sysfs.c | 28 ++-----------
drivers/rpmsg/rpmsg_core.c | 3 +-
drivers/rpmsg/rpmsg_internal.h | 13 ++++++-
drivers/rpmsg/rpmsg_ns.c | 14 ++++++-
drivers/s390/cio/cio.h | 6 ++-
drivers/s390/cio/css.c | 28 ++-----------
drivers/slimbus/qcom-ngd-ctrl.c | 13 ++++++-
drivers/spi/spi.c | 26 ++-----------
drivers/vdpa/vdpa.c | 29 ++------------
include/linux/amba/bus.h | 6 ++-
include/linux/device/driver.h | 2 +
include/linux/fsl/mc.h | 6 ++-
include/linux/hyperv.h | 6 ++-
include/linux/pci.h | 6 ++-
include/linux/platform_device.h | 6 ++-
include/linux/rpmsg.h | 6 ++-
include/linux/spi/spi.h | 2 +
include/linux/vdpa.h | 4 +-
24 files changed, 184 insertions(+), 205 deletions(-)
--
2.32.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 61+ messages in thread* [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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
Hi,
This is a continuation of my old patchset from 2019. [1]
Back then, few drivers set driver_override wrong. I fixed Exynos
in a different way after discussions. QCOM NGD was not fixed
and a new user appeared - IMX SCU.
It seems "char *" in driver_override looks too consty, so we
tend to make a mistake of storing there string literals.
Changes since latest v7
=======================
1. Patch #1: remove out_free label, document clearing override in kerneldoc and
in code-comments (Andy).
2. Patch #12 (rpmsg): do not duplicate string (Biju).
Changes since latest v6
=======================
1. Patch #1: Don't check for !dev and handle len==0 (Andy).
2. New patch #11 (rpmsg): split constifying of local variable to a new patch.
Changes since latest v5
=======================
1. Patch #11 (rpmsg): split from previous patch 11 - easier to understand the
need of it.
2. Fix build issue in patch 12 (rpmsg).
Changes since latest v4
=======================
1. Correct commit msgs and comments after Andy's review.
2. Re-order code in new helper (patch #1) (Andy).
3. Add tags.
Changes since latest v3
=======================
1. Wrap comments, extend comment in driver_set_override() about newline.
2. Minor commit msg fixes.
3. Add tags.
Changes since latest v2
=======================
1. Make all driver_override fields as "const char *", just like SPI
and VDPA. (Mark)
2. Move "count" check to the new helper and add "count" argument. (Michael)
3. Fix typos in docs, patch subject. Extend doc. (Michael, Bjorn)
4. Compare pointers to reduce number of string readings in the helper.
5. Fix clk-imx return value.
Changes since latest v1 (not the old 2019 solution):
====================================================
https://lore.kernel.org/all/708eabb1-7b35-d525-d4c3-451d4a3de84f@rasmusvillemoes.dk/
1. Add helper for setting driver_override.
2. Use the helper.
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.
[1] https://lore.kernel.org/all/1550484960-2392-3-git-send-email-krzk@kernel.org/
Best regards,
Krzysztof
Krzysztof Kozlowski (12):
driver: platform: Add helper for safer setting of driver_override
amba: Use driver_set_override() instead of open-coding
fsl-mc: Use driver_set_override() instead of open-coding
hv: Use driver_set_override() instead of open-coding
PCI: Use driver_set_override() instead of open-coding
s390/cio: Use driver_set_override() instead of open-coding
spi: Use helper for safer setting of driver_override
vdpa: Use helper for safer setting of driver_override
clk: imx: scu: Fix kfree() of static memory on setting driver_override
slimbus: qcom-ngd: Fix kfree() of static memory on setting
driver_override
rpmsg: Constify local variable in field store macro
rpmsg: Fix kfree() of static memory on setting driver_override
drivers/amba/bus.c | 28 ++-----------
drivers/base/driver.c | 69 +++++++++++++++++++++++++++++++++
drivers/base/platform.c | 28 ++-----------
drivers/bus/fsl-mc/fsl-mc-bus.c | 25 ++----------
drivers/clk/imx/clk-scu.c | 7 +++-
drivers/hv/vmbus_drv.c | 28 ++-----------
drivers/pci/pci-sysfs.c | 28 ++-----------
drivers/rpmsg/rpmsg_core.c | 3 +-
drivers/rpmsg/rpmsg_internal.h | 13 ++++++-
drivers/rpmsg/rpmsg_ns.c | 14 ++++++-
drivers/s390/cio/cio.h | 6 ++-
drivers/s390/cio/css.c | 28 ++-----------
drivers/slimbus/qcom-ngd-ctrl.c | 13 ++++++-
drivers/spi/spi.c | 26 ++-----------
drivers/vdpa/vdpa.c | 29 ++------------
include/linux/amba/bus.h | 6 ++-
include/linux/device/driver.h | 2 +
include/linux/fsl/mc.h | 6 ++-
include/linux/hyperv.h | 6 ++-
include/linux/pci.h | 6 ++-
include/linux/platform_device.h | 6 ++-
include/linux/rpmsg.h | 6 ++-
include/linux/spi/spi.h | 2 +
include/linux/vdpa.h | 4 +-
24 files changed, 184 insertions(+), 205 deletions(-)
--
2.32.0
^ permalink raw reply [flat|nested] 61+ messages in thread* [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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] 61+ messages in thread* [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread* Re: [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
(?)
@ 2022-04-20 17:12 ` Rafael J. Wysocki
-1 siblings, 0 replies; 61+ messages in thread
From: Rafael J. Wysocki @ 2022-04-20 17:12 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Stuart Yoder,
K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Bjorn Helgaas, Bjorn Andersson, Mathieu Poirier,
Vineeth Vijayan, Peter Oberparleiter, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Andy Gross, Linux Kernel Mailing List, linux-clk,
NXP Linux Team, Linux ARM, Linux on Hyper-V List, Linux PCI,
linux-remoteproc, linux-s390, linux-arm-msm,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
linux-spi, virtualization, Linus Torvalds, Rasmus Villemoes,
Andy Shevchenko
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] 61+ messages in thread* Re: [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override
@ 2022-04-20 17:12 ` Rafael J. Wysocki
0 siblings, 0 replies; 61+ 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] 61+ messages in thread* Re: [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override
@ 2022-04-20 17:12 ` Rafael J. Wysocki
0 siblings, 0 replies; 61+ messages in thread
From: Rafael J. Wysocki @ 2022-04-20 17:12 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Stuart Yoder,
K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Bjorn Helgaas, Bjorn Andersson, Mathieu Poirier,
Vineeth Vijayan, Peter Oberparleiter, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Andy Gross, Linux Kernel Mailing List, linux-clk,
NXP Linux Team, Linux ARM, Linux on Hyper-V List, Linux PCI,
linux-remoteproc, linux-s390, linux-arm-msm,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
linux-spi, virtualization, Linus Torvalds, Rasmus Villemoes,
Andy Shevchenko
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
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v7 01/12] driver: platform: Add helper for safer setting of driver_override
@ 2022-04-20 17:12 ` Rafael J. Wysocki
0 siblings, 0 replies; 61+ 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, linux-clk,
linux-s390, Wei Liu, Stephen Hemminger, Dexuan Cui,
Andy Shevchenko, Andy Gross, NXP Linux Team, 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
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v7 02/12] amba: Use driver_set_override() instead of open-coding
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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] 61+ messages in thread* [PATCH v7 02/12] amba: Use driver_set_override() instead of open-coding
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 02/12] amba: Use driver_set_override() instead of open-coding
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread
* [PATCH v7 03/12] fsl-mc: Use driver_set_override() instead of open-coding
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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] 61+ messages in thread* [PATCH v7 03/12] fsl-mc: Use driver_set_override() instead of open-coding
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 03/12] fsl-mc: Use driver_set_override() instead of open-coding
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread
* [PATCH v7 04/12] hv: Use driver_set_override() instead of open-coding
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski,
Michael Kelley
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] 61+ messages in thread* [PATCH v7 04/12] hv: Use driver_set_override() instead of open-coding
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski,
Michael Kelley
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 04/12] hv: Use driver_set_override() instead of open-coding
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread
* [PATCH v7 05/12] PCI: Use driver_set_override() instead of open-coding
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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] 61+ messages in thread* [PATCH v7 05/12] PCI: Use driver_set_override() instead of open-coding
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 05/12] PCI: Use driver_set_override() instead of open-coding
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread
* [PATCH v7 06/12] s390/cio: Use driver_set_override() instead of open-coding
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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] 61+ messages in thread* [PATCH v7 06/12] s390/cio: Use driver_set_override() instead of open-coding
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 06/12] s390/cio: Use driver_set_override() instead of open-coding
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread
* [PATCH v7 07/12] spi: Use helper for safer setting of driver_override
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski,
Mark Brown
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] 61+ messages in thread* [PATCH v7 07/12] spi: Use helper for safer setting of driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski,
Mark Brown
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 07/12] spi: Use helper for safer setting of driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread
* [PATCH v7 08/12] vdpa: Use helper for safer setting of driver_override
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski,
Michael S . Tsirkin
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] 61+ messages in thread* [PATCH v7 08/12] vdpa: Use helper for safer setting of driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski,
Michael S . Tsirkin
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 08/12] vdpa: Use helper for safer setting of driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread
* [PATCH v7 09/12] clk: imx: scu: Fix kfree() of static memory on setting driver_override
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski,
Stephen Boyd
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] 61+ messages in thread* [PATCH v7 09/12] clk: imx: scu: Fix kfree() of static memory on setting driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski,
Stephen Boyd
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 09/12] clk: imx: scu: Fix kfree() of static memory on setting driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ 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 ` Krzysztof Kozlowski
(?)
@ 2022-04-23 16:09 ` Abel Vesa
-1 siblings, 0 replies; 61+ messages in thread
From: Abel Vesa @ 2022-04-23 16:09 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Stuart Yoder,
K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Bjorn Helgaas, Bjorn Andersson, Mathieu Poirier,
Vineeth Vijayan, Peter Oberparleiter, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Andy Gross, linux-kernel, linux-clk,
NXP Linux Team, linux-arm-kernel, linux-hyperv, linux-pci,
linux-remoteproc, linux-s390, linux-arm-msm, alsa-devel,
linux-spi, virtualization, Linus Torvalds, Rasmus Villemoes,
Andy Shevchenko, Stephen Boyd
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] 61+ messages in thread* Re: [PATCH v7 09/12] clk: imx: scu: Fix kfree() of static memory on setting driver_override
@ 2022-04-23 16:09 ` Abel Vesa
0 siblings, 0 replies; 61+ 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] 61+ messages in thread* Re: [PATCH v7 09/12] clk: imx: scu: Fix kfree() of static memory on setting driver_override
@ 2022-04-23 16:09 ` Abel Vesa
0 siblings, 0 replies; 61+ messages in thread
From: Abel Vesa @ 2022-04-23 16:09 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Stuart Yoder,
K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Bjorn Helgaas, Bjorn Andersson, Mathieu Poirier,
Vineeth Vijayan, Peter Oberparleiter, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Andy Gross, linux-kernel, linux-clk,
NXP Linux Team, linux-arm-kernel, linux-hyperv, linux-pci,
linux-remoteproc, linux-s390, linux-arm-msm, alsa-devel,
linux-spi, virtualization, Linus Torvalds, Rasmus Villemoes,
Andy Shevchenko, Stephen Boyd
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
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 61+ messages in thread
* [PATCH v7 10/12] slimbus: qcom-ngd: Fix kfree() of static memory on setting driver_override
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski,
Srinivas Kandagatla
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] 61+ messages in thread* [PATCH v7 10/12] slimbus: qcom-ngd: Fix kfree() of static memory on setting driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski,
Srinivas Kandagatla
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 10/12] slimbus: qcom-ngd: Fix kfree() of static memory on setting driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread
* [PATCH v7 11/12] rpmsg: Constify local variable in field store macro
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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] 61+ messages in thread* [PATCH v7 11/12] rpmsg: Constify local variable in field store macro
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 11/12] rpmsg: Constify local variable in field store macro
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread
* [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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] 61+ messages in thread* [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-19 11:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko, Krzysztof Kozlowski
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
@ 2022-04-19 11:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread[parent not found: <CGME20220429122942eucas1p1820d0cd17a871d4953bac2b3de1dcdd9@eucas1p1.samsung.com>]
* Re: [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
[not found] ` <CGME20220429122942eucas1p1820d0cd17a871d4953bac2b3de1dcdd9@eucas1p1.samsung.com>
@ 2022-04-29 12:29 ` Marek Szyprowski
0 siblings, 0 replies; 61+ messages in thread
From: Marek Szyprowski @ 2022-04-29 12:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko
Hi Krzysztof,
On 19.04.2022 13:34, 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: 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>
This patch landed recently in linux-next as commit 42cd402b8fd4 ("rpmsg:
Fix kfree() of static memory on setting driver_override"). In my tests I
found that it triggers the following issue during boot of the
DragonBoard410c SBC (arch/arm64/boot/dts/qcom/apq8016-sbc.dtb):
------------[ cut here ]------------
DEBUG_LOCKS_WARN_ON(lock->magic != lock)
WARNING: CPU: 1 PID: 8 at kernel/locking/mutex.c:582
__mutex_lock+0x1ec/0x430
Modules linked in:
CPU: 1 PID: 8 Comm: kworker/u8:0 Not tainted 5.18.0-rc4-next-20220429 #11815
Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __mutex_lock+0x1ec/0x430
lr : __mutex_lock+0x1ec/0x430
..
Call trace:
__mutex_lock+0x1ec/0x430
mutex_lock_nested+0x38/0x64
driver_set_override+0x124/0x150
qcom_smd_register_edge+0x2a8/0x4ec
qcom_smd_probe+0x54/0x80
platform_probe+0x68/0xe0
really_probe.part.0+0x9c/0x29c
__driver_probe_device+0x98/0x144
driver_probe_device+0xac/0x14c
__device_attach_driver+0xb8/0x120
bus_for_each_drv+0x78/0xd0
__device_attach+0xd8/0x180
device_initial_probe+0x14/0x20
bus_probe_device+0x9c/0xa4
deferred_probe_work_func+0x88/0xc4
process_one_work+0x288/0x6bc
worker_thread+0x248/0x450
kthread+0x118/0x11c
ret_from_fork+0x10/0x20
irq event stamp: 3599
hardirqs last enabled at (3599): [<ffff80000919053c>]
_raw_spin_unlock_irqrestore+0x98/0x9c
hardirqs last disabled at (3598): [<ffff800009190ba4>]
_raw_spin_lock_irqsave+0xc0/0xcc
softirqs last enabled at (3554): [<ffff800008010470>] _stext+0x470/0x5e8
softirqs last disabled at (3549): [<ffff8000080a4514>]
__irq_exit_rcu+0x180/0x1ac
---[ end trace 0000000000000000 ]---
I don't see any direct relation between the $subject and the above log,
but reverting the $subject on top of linux next-20220429 hides/fixes it.
Maybe there is a kind of memory trashing somewhere there and your change
only revealed it?
> ---
> 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;
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
@ 2022-04-29 12:29 ` Marek Szyprowski
0 siblings, 0 replies; 61+ messages in thread
From: Marek Szyprowski @ 2022-04-29 12:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko
Hi Krzysztof,
On 19.04.2022 13:34, 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: 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>
This patch landed recently in linux-next as commit 42cd402b8fd4 ("rpmsg:
Fix kfree() of static memory on setting driver_override"). In my tests I
found that it triggers the following issue during boot of the
DragonBoard410c SBC (arch/arm64/boot/dts/qcom/apq8016-sbc.dtb):
------------[ cut here ]------------
DEBUG_LOCKS_WARN_ON(lock->magic != lock)
WARNING: CPU: 1 PID: 8 at kernel/locking/mutex.c:582
__mutex_lock+0x1ec/0x430
Modules linked in:
CPU: 1 PID: 8 Comm: kworker/u8:0 Not tainted 5.18.0-rc4-next-20220429 #11815
Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __mutex_lock+0x1ec/0x430
lr : __mutex_lock+0x1ec/0x430
..
Call trace:
__mutex_lock+0x1ec/0x430
mutex_lock_nested+0x38/0x64
driver_set_override+0x124/0x150
qcom_smd_register_edge+0x2a8/0x4ec
qcom_smd_probe+0x54/0x80
platform_probe+0x68/0xe0
really_probe.part.0+0x9c/0x29c
__driver_probe_device+0x98/0x144
driver_probe_device+0xac/0x14c
__device_attach_driver+0xb8/0x120
bus_for_each_drv+0x78/0xd0
__device_attach+0xd8/0x180
device_initial_probe+0x14/0x20
bus_probe_device+0x9c/0xa4
deferred_probe_work_func+0x88/0xc4
process_one_work+0x288/0x6bc
worker_thread+0x248/0x450
kthread+0x118/0x11c
ret_from_fork+0x10/0x20
irq event stamp: 3599
hardirqs last enabled at (3599): [<ffff80000919053c>]
_raw_spin_unlock_irqrestore+0x98/0x9c
hardirqs last disabled at (3598): [<ffff800009190ba4>]
_raw_spin_lock_irqsave+0xc0/0xcc
softirqs last enabled at (3554): [<ffff800008010470>] _stext+0x470/0x5e8
softirqs last disabled at (3549): [<ffff8000080a4514>]
__irq_exit_rcu+0x180/0x1ac
---[ end trace 0000000000000000 ]---
I don't see any direct relation between the $subject and the above log,
but reverting the $subject on top of linux next-20220429 hides/fixes it.
Maybe there is a kind of memory trashing somewhere there and your change
only revealed it?
> ---
> 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;
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
2022-04-29 12:29 ` Marek Szyprowski
@ 2022-04-29 14:16 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-29 14:16 UTC (permalink / raw)
To: Marek Szyprowski, Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko
On 29/04/2022 14:29, Marek Szyprowski wrote:
> Hi Krzysztof,
>
> On 19.04.2022 13:34, 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: 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>
>
> This patch landed recently in linux-next as commit 42cd402b8fd4 ("rpmsg:
> Fix kfree() of static memory on setting driver_override"). In my tests I
> found that it triggers the following issue during boot of the
> DragonBoard410c SBC (arch/arm64/boot/dts/qcom/apq8016-sbc.dtb):
>
> ------------[ cut here ]------------
> DEBUG_LOCKS_WARN_ON(lock->magic != lock)
> WARNING: CPU: 1 PID: 8 at kernel/locking/mutex.c:582
> __mutex_lock+0x1ec/0x430
> Modules linked in:
> CPU: 1 PID: 8 Comm: kworker/u8:0 Not tainted 5.18.0-rc4-next-20220429 #11815
> Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
> Workqueue: events_unbound deferred_probe_work_func
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : __mutex_lock+0x1ec/0x430
> lr : __mutex_lock+0x1ec/0x430
> ..
> Call trace:
> __mutex_lock+0x1ec/0x430
> mutex_lock_nested+0x38/0x64
> driver_set_override+0x124/0x150
> qcom_smd_register_edge+0x2a8/0x4ec
> qcom_smd_probe+0x54/0x80
> platform_probe+0x68/0xe0
> really_probe.part.0+0x9c/0x29c
> __driver_probe_device+0x98/0x144
> driver_probe_device+0xac/0x14c
> __device_attach_driver+0xb8/0x120
> bus_for_each_drv+0x78/0xd0
> __device_attach+0xd8/0x180
> device_initial_probe+0x14/0x20
> bus_probe_device+0x9c/0xa4
> deferred_probe_work_func+0x88/0xc4
> process_one_work+0x288/0x6bc
> worker_thread+0x248/0x450
> kthread+0x118/0x11c
> ret_from_fork+0x10/0x20
> irq event stamp: 3599
> hardirqs last enabled at (3599): [<ffff80000919053c>]
> _raw_spin_unlock_irqrestore+0x98/0x9c
> hardirqs last disabled at (3598): [<ffff800009190ba4>]
> _raw_spin_lock_irqsave+0xc0/0xcc
> softirqs last enabled at (3554): [<ffff800008010470>] _stext+0x470/0x5e8
> softirqs last disabled at (3549): [<ffff8000080a4514>]
> __irq_exit_rcu+0x180/0x1ac
> ---[ end trace 0000000000000000 ]---
>
> I don't see any direct relation between the $subject and the above log,
> but reverting the $subject on top of linux next-20220429 hides/fixes it.
> Maybe there is a kind of memory trashing somewhere there and your change
> only revealed it?
Thanks for the report. I think the error path of my patch is wrong - I
should not kfree(rpdev->driver_override) from the rpmsg code. That's the
only thing I see now...
Could you test following patch and tell if it helps?
https://pastebin.ubuntu.com/p/rp3q9Z5fXj/
-----
diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 3e81642238d2..1e2ad944e2ec 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -102,11 +102,7 @@ static inline int
rpmsg_ctrldev_register_device(struct rpmsg_device *rpdev)
if (ret)
return ret;
- ret = rpmsg_register_device(rpdev);
- if (ret)
- kfree(rpdev->driver_override);
-
- return ret;
+ return rpmsg_register_device(rpdev);
}
#endif
diff --git a/drivers/rpmsg/rpmsg_ns.c b/drivers/rpmsg/rpmsg_ns.c
index 8eb8f328237e..f26078467899 100644
--- a/drivers/rpmsg/rpmsg_ns.c
+++ b/drivers/rpmsg/rpmsg_ns.c
@@ -31,11 +31,7 @@ int rpmsg_ns_register_device(struct rpmsg_device *rpdev)
rpdev->src = RPMSG_NS_ADDR;
rpdev->dst = RPMSG_NS_ADDR;
- ret = rpmsg_register_device(rpdev);
- if (ret)
- kfree(rpdev->driver_override);
-
- return ret;
+ return rpmsg_register_device(rpdev);
}
EXPORT_SYMBOL(rpmsg_ns_register_device);
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
@ 2022-04-29 14:16 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-29 14:16 UTC (permalink / raw)
To: Marek Szyprowski, Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko
On 29/04/2022 14:29, Marek Szyprowski wrote:
> Hi Krzysztof,
>
> On 19.04.2022 13:34, 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: 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>
>
> This patch landed recently in linux-next as commit 42cd402b8fd4 ("rpmsg:
> Fix kfree() of static memory on setting driver_override"). In my tests I
> found that it triggers the following issue during boot of the
> DragonBoard410c SBC (arch/arm64/boot/dts/qcom/apq8016-sbc.dtb):
>
> ------------[ cut here ]------------
> DEBUG_LOCKS_WARN_ON(lock->magic != lock)
> WARNING: CPU: 1 PID: 8 at kernel/locking/mutex.c:582
> __mutex_lock+0x1ec/0x430
> Modules linked in:
> CPU: 1 PID: 8 Comm: kworker/u8:0 Not tainted 5.18.0-rc4-next-20220429 #11815
> Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
> Workqueue: events_unbound deferred_probe_work_func
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : __mutex_lock+0x1ec/0x430
> lr : __mutex_lock+0x1ec/0x430
> ..
> Call trace:
> __mutex_lock+0x1ec/0x430
> mutex_lock_nested+0x38/0x64
> driver_set_override+0x124/0x150
> qcom_smd_register_edge+0x2a8/0x4ec
> qcom_smd_probe+0x54/0x80
> platform_probe+0x68/0xe0
> really_probe.part.0+0x9c/0x29c
> __driver_probe_device+0x98/0x144
> driver_probe_device+0xac/0x14c
> __device_attach_driver+0xb8/0x120
> bus_for_each_drv+0x78/0xd0
> __device_attach+0xd8/0x180
> device_initial_probe+0x14/0x20
> bus_probe_device+0x9c/0xa4
> deferred_probe_work_func+0x88/0xc4
> process_one_work+0x288/0x6bc
> worker_thread+0x248/0x450
> kthread+0x118/0x11c
> ret_from_fork+0x10/0x20
> irq event stamp: 3599
> hardirqs last enabled at (3599): [<ffff80000919053c>]
> _raw_spin_unlock_irqrestore+0x98/0x9c
> hardirqs last disabled at (3598): [<ffff800009190ba4>]
> _raw_spin_lock_irqsave+0xc0/0xcc
> softirqs last enabled at (3554): [<ffff800008010470>] _stext+0x470/0x5e8
> softirqs last disabled at (3549): [<ffff8000080a4514>]
> __irq_exit_rcu+0x180/0x1ac
> ---[ end trace 0000000000000000 ]---
>
> I don't see any direct relation between the $subject and the above log,
> but reverting the $subject on top of linux next-20220429 hides/fixes it.
> Maybe there is a kind of memory trashing somewhere there and your change
> only revealed it?
Thanks for the report. I think the error path of my patch is wrong - I
should not kfree(rpdev->driver_override) from the rpmsg code. That's the
only thing I see now...
Could you test following patch and tell if it helps?
https://pastebin.ubuntu.com/p/rp3q9Z5fXj/
-----
diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 3e81642238d2..1e2ad944e2ec 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -102,11 +102,7 @@ static inline int
rpmsg_ctrldev_register_device(struct rpmsg_device *rpdev)
if (ret)
return ret;
- ret = rpmsg_register_device(rpdev);
- if (ret)
- kfree(rpdev->driver_override);
-
- return ret;
+ return rpmsg_register_device(rpdev);
}
#endif
diff --git a/drivers/rpmsg/rpmsg_ns.c b/drivers/rpmsg/rpmsg_ns.c
index 8eb8f328237e..f26078467899 100644
--- a/drivers/rpmsg/rpmsg_ns.c
+++ b/drivers/rpmsg/rpmsg_ns.c
@@ -31,11 +31,7 @@ int rpmsg_ns_register_device(struct rpmsg_device *rpdev)
rpdev->src = RPMSG_NS_ADDR;
rpdev->dst = RPMSG_NS_ADDR;
- ret = rpmsg_register_device(rpdev);
- if (ret)
- kfree(rpdev->driver_override);
-
- return ret;
+ return rpmsg_register_device(rpdev);
}
EXPORT_SYMBOL(rpmsg_ns_register_device);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 61+ messages in thread* Re: [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
2022-04-29 14:16 ` Krzysztof Kozlowski
@ 2022-04-29 14:51 ` Marek Szyprowski
-1 siblings, 0 replies; 61+ messages in thread
From: Marek Szyprowski @ 2022-04-29 14:51 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko
On 29.04.2022 16:16, Krzysztof Kozlowski wrote:
> On 29/04/2022 14:29, Marek Szyprowski wrote:
>> On 19.04.2022 13:34, 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: 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>
>> This patch landed recently in linux-next as commit 42cd402b8fd4 ("rpmsg:
>> Fix kfree() of static memory on setting driver_override"). In my tests I
>> found that it triggers the following issue during boot of the
>> DragonBoard410c SBC (arch/arm64/boot/dts/qcom/apq8016-sbc.dtb):
>>
>> ------------[ cut here ]------------
>> DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>> WARNING: CPU: 1 PID: 8 at kernel/locking/mutex.c:582
>> __mutex_lock+0x1ec/0x430
>> Modules linked in:
>> CPU: 1 PID: 8 Comm: kworker/u8:0 Not tainted 5.18.0-rc4-next-20220429 #11815
>> Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
>> Workqueue: events_unbound deferred_probe_work_func
>> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : __mutex_lock+0x1ec/0x430
>> lr : __mutex_lock+0x1ec/0x430
>> ..
>> Call trace:
>> __mutex_lock+0x1ec/0x430
>> mutex_lock_nested+0x38/0x64
>> driver_set_override+0x124/0x150
>> qcom_smd_register_edge+0x2a8/0x4ec
>> qcom_smd_probe+0x54/0x80
>> platform_probe+0x68/0xe0
>> really_probe.part.0+0x9c/0x29c
>> __driver_probe_device+0x98/0x144
>> driver_probe_device+0xac/0x14c
>> __device_attach_driver+0xb8/0x120
>> bus_for_each_drv+0x78/0xd0
>> __device_attach+0xd8/0x180
>> device_initial_probe+0x14/0x20
>> bus_probe_device+0x9c/0xa4
>> deferred_probe_work_func+0x88/0xc4
>> process_one_work+0x288/0x6bc
>> worker_thread+0x248/0x450
>> kthread+0x118/0x11c
>> ret_from_fork+0x10/0x20
>> irq event stamp: 3599
>> hardirqs last enabled at (3599): [<ffff80000919053c>]
>> _raw_spin_unlock_irqrestore+0x98/0x9c
>> hardirqs last disabled at (3598): [<ffff800009190ba4>]
>> _raw_spin_lock_irqsave+0xc0/0xcc
>> softirqs last enabled at (3554): [<ffff800008010470>] _stext+0x470/0x5e8
>> softirqs last disabled at (3549): [<ffff8000080a4514>]
>> __irq_exit_rcu+0x180/0x1ac
>> ---[ end trace 0000000000000000 ]---
>>
>> I don't see any direct relation between the $subject and the above log,
>> but reverting the $subject on top of linux next-20220429 hides/fixes it.
>> Maybe there is a kind of memory trashing somewhere there and your change
>> only revealed it?
> Thanks for the report. I think the error path of my patch is wrong - I
> should not kfree(rpdev->driver_override) from the rpmsg code. That's the
> only thing I see now...
>
> Could you test following patch and tell if it helps?
> https://pastebin.ubuntu.com/p/rp3q9Z5fXj/
This doesn't help, the issue is still reported.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
@ 2022-04-29 14:51 ` Marek Szyprowski
0 siblings, 0 replies; 61+ messages in thread
From: Marek Szyprowski @ 2022-04-29 14:51 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko
On 29.04.2022 16:16, Krzysztof Kozlowski wrote:
> On 29/04/2022 14:29, Marek Szyprowski wrote:
>> On 19.04.2022 13:34, 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: 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>
>> This patch landed recently in linux-next as commit 42cd402b8fd4 ("rpmsg:
>> Fix kfree() of static memory on setting driver_override"). In my tests I
>> found that it triggers the following issue during boot of the
>> DragonBoard410c SBC (arch/arm64/boot/dts/qcom/apq8016-sbc.dtb):
>>
>> ------------[ cut here ]------------
>> DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>> WARNING: CPU: 1 PID: 8 at kernel/locking/mutex.c:582
>> __mutex_lock+0x1ec/0x430
>> Modules linked in:
>> CPU: 1 PID: 8 Comm: kworker/u8:0 Not tainted 5.18.0-rc4-next-20220429 #11815
>> Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
>> Workqueue: events_unbound deferred_probe_work_func
>> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : __mutex_lock+0x1ec/0x430
>> lr : __mutex_lock+0x1ec/0x430
>> ..
>> Call trace:
>> __mutex_lock+0x1ec/0x430
>> mutex_lock_nested+0x38/0x64
>> driver_set_override+0x124/0x150
>> qcom_smd_register_edge+0x2a8/0x4ec
>> qcom_smd_probe+0x54/0x80
>> platform_probe+0x68/0xe0
>> really_probe.part.0+0x9c/0x29c
>> __driver_probe_device+0x98/0x144
>> driver_probe_device+0xac/0x14c
>> __device_attach_driver+0xb8/0x120
>> bus_for_each_drv+0x78/0xd0
>> __device_attach+0xd8/0x180
>> device_initial_probe+0x14/0x20
>> bus_probe_device+0x9c/0xa4
>> deferred_probe_work_func+0x88/0xc4
>> process_one_work+0x288/0x6bc
>> worker_thread+0x248/0x450
>> kthread+0x118/0x11c
>> ret_from_fork+0x10/0x20
>> irq event stamp: 3599
>> hardirqs last enabled at (3599): [<ffff80000919053c>]
>> _raw_spin_unlock_irqrestore+0x98/0x9c
>> hardirqs last disabled at (3598): [<ffff800009190ba4>]
>> _raw_spin_lock_irqsave+0xc0/0xcc
>> softirqs last enabled at (3554): [<ffff800008010470>] _stext+0x470/0x5e8
>> softirqs last disabled at (3549): [<ffff8000080a4514>]
>> __irq_exit_rcu+0x180/0x1ac
>> ---[ end trace 0000000000000000 ]---
>>
>> I don't see any direct relation between the $subject and the above log,
>> but reverting the $subject on top of linux next-20220429 hides/fixes it.
>> Maybe there is a kind of memory trashing somewhere there and your change
>> only revealed it?
> Thanks for the report. I think the error path of my patch is wrong - I
> should not kfree(rpdev->driver_override) from the rpmsg code. That's the
> only thing I see now...
>
> Could you test following patch and tell if it helps?
> https://pastebin.ubuntu.com/p/rp3q9Z5fXj/
This doesn't help, the issue is still reported.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
2022-04-29 14:51 ` Marek Szyprowski
@ 2022-04-29 18:29 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-29 18:29 UTC (permalink / raw)
To: Marek Szyprowski, Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko
On 29/04/2022 16:51, Marek Szyprowski wrote:
> On 29.04.2022 16:16, Krzysztof Kozlowski wrote:
>> On 29/04/2022 14:29, Marek Szyprowski wrote:
>>> On 19.04.2022 13:34, 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: 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>
>>> This patch landed recently in linux-next as commit 42cd402b8fd4 ("rpmsg:
>>> Fix kfree() of static memory on setting driver_override"). In my tests I
>>> found that it triggers the following issue during boot of the
>>> DragonBoard410c SBC (arch/arm64/boot/dts/qcom/apq8016-sbc.dtb):
>>>
>>> ------------[ cut here ]------------
>>> DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>>> WARNING: CPU: 1 PID: 8 at kernel/locking/mutex.c:582
>>> __mutex_lock+0x1ec/0x430
>>> Modules linked in:
>>> CPU: 1 PID: 8 Comm: kworker/u8:0 Not tainted 5.18.0-rc4-next-20220429 #11815
>>> Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
>>> Workqueue: events_unbound deferred_probe_work_func
>>> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>>> pc : __mutex_lock+0x1ec/0x430
>>> lr : __mutex_lock+0x1ec/0x430
>>> ..
>>> Call trace:
>>> __mutex_lock+0x1ec/0x430
>>> mutex_lock_nested+0x38/0x64
>>> driver_set_override+0x124/0x150
>>> qcom_smd_register_edge+0x2a8/0x4ec
>>> qcom_smd_probe+0x54/0x80
>>> platform_probe+0x68/0xe0
>>> really_probe.part.0+0x9c/0x29c
>>> __driver_probe_device+0x98/0x144
>>> driver_probe_device+0xac/0x14c
>>> __device_attach_driver+0xb8/0x120
>>> bus_for_each_drv+0x78/0xd0
>>> __device_attach+0xd8/0x180
>>> device_initial_probe+0x14/0x20
>>> bus_probe_device+0x9c/0xa4
>>> deferred_probe_work_func+0x88/0xc4
>>> process_one_work+0x288/0x6bc
>>> worker_thread+0x248/0x450
>>> kthread+0x118/0x11c
>>> ret_from_fork+0x10/0x20
>>> irq event stamp: 3599
>>> hardirqs last enabled at (3599): [<ffff80000919053c>]
>>> _raw_spin_unlock_irqrestore+0x98/0x9c
>>> hardirqs last disabled at (3598): [<ffff800009190ba4>]
>>> _raw_spin_lock_irqsave+0xc0/0xcc
>>> softirqs last enabled at (3554): [<ffff800008010470>] _stext+0x470/0x5e8
>>> softirqs last disabled at (3549): [<ffff8000080a4514>]
>>> __irq_exit_rcu+0x180/0x1ac
>>> ---[ end trace 0000000000000000 ]---
>>>
>>> I don't see any direct relation between the $subject and the above log,
>>> but reverting the $subject on top of linux next-20220429 hides/fixes it.
>>> Maybe there is a kind of memory trashing somewhere there and your change
>>> only revealed it?
>> Thanks for the report. I think the error path of my patch is wrong - I
>> should not kfree(rpdev->driver_override) from the rpmsg code. That's the
>> only thing I see now...
>>
>> Could you test following patch and tell if it helps?
>> https://pastebin.ubuntu.com/p/rp3q9Z5fXj/
>
> This doesn't help, the issue is still reported.
I think I screwed this part of code. The new helper uses device_lock()
(the mutexes you see in backtrace) but in rpmsg it is called before
device_register() which initializes the device.
I don't have a device using qcom-smd rpmsg, so it's a bit tricky to
reproduce.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 61+ messages in thread* Re: [PATCH v7 12/12] rpmsg: Fix kfree() of static memory on setting driver_override
@ 2022-04-29 18:29 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-29 18:29 UTC (permalink / raw)
To: Marek Szyprowski, Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko
On 29/04/2022 16:51, Marek Szyprowski wrote:
> On 29.04.2022 16:16, Krzysztof Kozlowski wrote:
>> On 29/04/2022 14:29, Marek Szyprowski wrote:
>>> On 19.04.2022 13:34, 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: 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>
>>> This patch landed recently in linux-next as commit 42cd402b8fd4 ("rpmsg:
>>> Fix kfree() of static memory on setting driver_override"). In my tests I
>>> found that it triggers the following issue during boot of the
>>> DragonBoard410c SBC (arch/arm64/boot/dts/qcom/apq8016-sbc.dtb):
>>>
>>> ------------[ cut here ]------------
>>> DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>>> WARNING: CPU: 1 PID: 8 at kernel/locking/mutex.c:582
>>> __mutex_lock+0x1ec/0x430
>>> Modules linked in:
>>> CPU: 1 PID: 8 Comm: kworker/u8:0 Not tainted 5.18.0-rc4-next-20220429 #11815
>>> Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
>>> Workqueue: events_unbound deferred_probe_work_func
>>> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>>> pc : __mutex_lock+0x1ec/0x430
>>> lr : __mutex_lock+0x1ec/0x430
>>> ..
>>> Call trace:
>>> __mutex_lock+0x1ec/0x430
>>> mutex_lock_nested+0x38/0x64
>>> driver_set_override+0x124/0x150
>>> qcom_smd_register_edge+0x2a8/0x4ec
>>> qcom_smd_probe+0x54/0x80
>>> platform_probe+0x68/0xe0
>>> really_probe.part.0+0x9c/0x29c
>>> __driver_probe_device+0x98/0x144
>>> driver_probe_device+0xac/0x14c
>>> __device_attach_driver+0xb8/0x120
>>> bus_for_each_drv+0x78/0xd0
>>> __device_attach+0xd8/0x180
>>> device_initial_probe+0x14/0x20
>>> bus_probe_device+0x9c/0xa4
>>> deferred_probe_work_func+0x88/0xc4
>>> process_one_work+0x288/0x6bc
>>> worker_thread+0x248/0x450
>>> kthread+0x118/0x11c
>>> ret_from_fork+0x10/0x20
>>> irq event stamp: 3599
>>> hardirqs last enabled at (3599): [<ffff80000919053c>]
>>> _raw_spin_unlock_irqrestore+0x98/0x9c
>>> hardirqs last disabled at (3598): [<ffff800009190ba4>]
>>> _raw_spin_lock_irqsave+0xc0/0xcc
>>> softirqs last enabled at (3554): [<ffff800008010470>] _stext+0x470/0x5e8
>>> softirqs last disabled at (3549): [<ffff8000080a4514>]
>>> __irq_exit_rcu+0x180/0x1ac
>>> ---[ end trace 0000000000000000 ]---
>>>
>>> I don't see any direct relation between the $subject and the above log,
>>> but reverting the $subject on top of linux next-20220429 hides/fixes it.
>>> Maybe there is a kind of memory trashing somewhere there and your change
>>> only revealed it?
>> Thanks for the report. I think the error path of my patch is wrong - I
>> should not kfree(rpdev->driver_override) from the rpmsg code. That's the
>> only thing I see now...
>>
>> Could you test following patch and tell if it helps?
>> https://pastebin.ubuntu.com/p/rp3q9Z5fXj/
>
> This doesn't help, the issue is still reported.
I think I screwed this part of code. The new helper uses device_lock()
(the mutexes you see in backtrace) but in rpmsg it is called before
device_register() which initializes the device.
I don't have a device using qcom-smd rpmsg, so it's a bit tricky to
reproduce.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory)
2022-04-19 11:34 ` Krzysztof Kozlowski
(?)
@ 2022-04-20 9:20 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-20 9:20 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko
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] 61+ messages in thread
* Re: [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory)
@ 2022-04-20 9:20 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ 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] 61+ messages in thread
* Re: [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory)
@ 2022-04-20 9:20 ` Krzysztof Kozlowski
0 siblings, 0 replies; 61+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-20 9:20 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, Bjorn Helgaas, Bjorn Andersson,
Mathieu Poirier, Vineeth Vijayan, Peter Oberparleiter,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Andy Gross, linux-kernel,
linux-clk, NXP Linux Team, linux-arm-kernel, linux-hyperv,
linux-pci, linux-remoteproc, linux-s390, linux-arm-msm,
alsa-devel, linux-spi, virtualization, Linus Torvalds,
Rasmus Villemoes, Andy Shevchenko
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory)
2022-04-20 9:20 ` Krzysztof Kozlowski
(?)
(?)
@ 2022-04-22 14:54 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 61+ messages in thread
From: Greg Kroah-Hartman @ 2022-04-22 14:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rafael J. Wysocki, Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, Dexuan Cui, Bjorn Helgaas,
Bjorn Andersson, Mathieu Poirier, Vineeth Vijayan,
Peter Oberparleiter, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Andy Gross, linux-kernel, linux-clk, NXP Linux Team,
linux-arm-kernel, linux-hyperv, linux-pci, linux-remoteproc,
linux-s390, linux-arm-msm, alsa-devel, linux-spi, virtualization,
Linus Torvalds, Rasmus Villemoes, Andy Shevchenko
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] 61+ messages in thread
* Re: [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory)
@ 2022-04-22 14:54 ` Greg Kroah-Hartman
0 siblings, 0 replies; 61+ 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] 61+ messages in thread
* Re: [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory)
@ 2022-04-22 14:54 ` Greg Kroah-Hartman
0 siblings, 0 replies; 61+ messages in thread
From: Greg Kroah-Hartman @ 2022-04-22 14:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rafael J. Wysocki, Stuart Yoder, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, Dexuan Cui, Bjorn Helgaas,
Bjorn Andersson, Mathieu Poirier, Vineeth Vijayan,
Peter Oberparleiter, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Andy Gross, linux-kernel, linux-clk, NXP Linux Team,
linux-arm-kernel, linux-hyperv, linux-pci, linux-remoteproc,
linux-s390, linux-arm-msm, alsa-devel, linux-spi, virtualization,
Linus Torvalds, Rasmus Villemoes, Andy Shevchenko
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 61+ messages in thread
* Re: [PATCH v7 00/12] Fix broken usage of driver_override (and kfree of static memory)
@ 2022-04-22 14:54 ` Greg Kroah-Hartman
0 siblings, 0 replies; 61+ 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, linux-clk, linux-s390,
Wei Liu, Stephen Hemminger, Dexuan Cui, Andy Shevchenko,
Andy Gross, NXP Linux Team, 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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 61+ messages in thread