* [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override
@ 2023-10-31 9:30 Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 2/6] rpmsg: Constify local variable in field store macro Lee Jones
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: Lee Jones @ 2023-10-31 9:30 UTC (permalink / raw)
To: lee; +Cc: stable, Krzysztof Kozlowski, Rafael J . Wysocki,
Greg Kroah-Hartman
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
commit 6c2f421174273de8f83cde4286d1c076d43a2d35 upstream.
Several core drivers and buses expect that driver_override is a
dynamically allocated memory thus later they can kfree() it.
However such assumption is not documented, there were in the past and
there are already users setting it to a string literal. This leads to
kfree() of static memory during device release (e.g. in error paths or
during unbind):
kernel BUG at ../mm/slub.c:3960!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
...
(kfree) from [<c058da50>] (platform_device_release+0x88/0xb4)
(platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90)
(device_release) from [<c0a69050>] (kobject_put+0xec/0x20c)
(kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c)
(exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4)
(platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414)
(really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4)
(driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8)
(bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c)
(__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90)
(bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c)
(device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc)
(of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc)
(of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc)
(of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118)
(of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8)
(of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404)
Provide a helper which clearly documents the usage of driver_override.
This will allow later to reuse the helper and reduce the amount of
duplicated code.
Convert the platform driver to use a new helper and make the
driver_override field const char (it is not modified by the core).
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Link: https://lore.kernel.org/r/20220419113435.246203-2-krzysztof.kozlowski@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Lee Jones <lee@kernel.org>
Change-Id: I2f59769cfb99d8359d14e2cb7345ce3428593afc
---
drivers/base/driver.c | 69 +++++++++++++++++++++++++++++++++
drivers/base/platform.c | 28 ++-----------
include/linux/device.h | 2 +
include/linux/platform_device.h | 6 ++-
4 files changed, 80 insertions(+), 25 deletions(-)
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 857c8f1b876e5..668c6c8c22f1f 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -29,6 +29,75 @@ static struct device *next_device(struct klist_iter *i)
return dev;
}
+/**
+ * driver_set_override() - Helper to set or clear driver override.
+ * @dev: Device to change
+ * @override: Address of string to change (e.g. &device->driver_override);
+ * The contents will be freed and hold newly allocated override.
+ * @s: NUL-terminated string, new driver name to force a match, pass empty
+ * string to clear it ("" or "\n", where the latter is only for sysfs
+ * interface).
+ * @len: length of @s
+ *
+ * Helper to set or clear driver override in a device, intended for the cases
+ * when the driver_override field is allocated by driver/bus code.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+int driver_set_override(struct device *dev, const char **override,
+ const char *s, size_t len)
+{
+ const char *new, *old;
+ char *cp;
+
+ if (!override || !s)
+ return -EINVAL;
+
+ /*
+ * The stored value will be used in sysfs show callback (sysfs_emit()),
+ * which has a length limit of PAGE_SIZE and adds a trailing newline.
+ * Thus we can store one character less to avoid truncation during sysfs
+ * show.
+ */
+ if (len >= (PAGE_SIZE - 1))
+ return -EINVAL;
+
+ if (!len) {
+ /* Empty string passed - clear override */
+ device_lock(dev);
+ old = *override;
+ *override = NULL;
+ device_unlock(dev);
+ kfree(old);
+
+ return 0;
+ }
+
+ cp = strnchr(s, len, '\n');
+ if (cp)
+ len = cp - s;
+
+ new = kstrndup(s, len, GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+
+ device_lock(dev);
+ old = *override;
+ if (cp != s) {
+ *override = new;
+ } else {
+ /* "\n" passed - clear override */
+ kfree(new);
+ *override = NULL;
+ }
+ device_unlock(dev);
+
+ kfree(old);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(driver_set_override);
+
/**
* driver_for_each_device - Iterator for devices bound to a driver.
* @drv: Driver we're iterating.
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 2f89e618b142c..a09e7a681f7a7 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -891,31 +891,11 @@ static ssize_t driver_override_store(struct device *dev,
const char *buf, size_t count)
{
struct platform_device *pdev = to_platform_device(dev);
- char *driver_override, *old, *cp;
-
- /* We need to keep extra room for a newline */
- if (count >= (PAGE_SIZE - 1))
- return -EINVAL;
-
- driver_override = kstrndup(buf, count, GFP_KERNEL);
- if (!driver_override)
- return -ENOMEM;
-
- cp = strchr(driver_override, '\n');
- if (cp)
- *cp = '\0';
-
- device_lock(dev);
- old = pdev->driver_override;
- if (strlen(driver_override)) {
- pdev->driver_override = driver_override;
- } else {
- kfree(driver_override);
- pdev->driver_override = NULL;
- }
- device_unlock(dev);
+ int ret;
- kfree(old);
+ ret = driver_set_override(dev, &pdev->driver_override, buf, count);
+ if (ret)
+ return ret;
return count;
}
diff --git a/include/linux/device.h b/include/linux/device.h
index 37e359d81a86f..bccd367c11de5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -330,6 +330,8 @@ extern int __must_check driver_create_file(struct device_driver *driver,
extern void driver_remove_file(struct device_driver *driver,
const struct driver_attribute *attr);
+int driver_set_override(struct device *dev, const char **override,
+ const char *s, size_t len);
extern int __must_check driver_for_each_device(struct device_driver *drv,
struct device *start,
void *data,
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 9e5c98fcea8c6..8268439975b21 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -29,7 +29,11 @@ struct platform_device {
struct resource *resource;
const struct platform_device_id *id_entry;
- char *driver_override; /* Driver name to force a match */
+ /*
+ * Driver name to force a match. Do not set directly, because core
+ * frees it. Use driver_set_override() to set or clear it.
+ */
+ const char *driver_override;
/* MFD cell pointer */
struct mfd_cell *mfd_cell;
--
2.42.0.820.g83a721a137-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4.19.y 2/6] rpmsg: Constify local variable in field store macro
2023-10-31 9:30 [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
@ 2023-10-31 9:30 ` Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 3/6] rpmsg: Fix kfree() of static memory on setting driver_override Lee Jones
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2023-10-31 9:30 UTC (permalink / raw)
To: lee; +Cc: stable, Krzysztof Kozlowski, Greg Kroah-Hartman
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
commit e5f89131a06142e91073b6959d91cea73861d40e upstream.
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>
Link: https://lore.kernel.org/r/20220419113435.246203-12-krzysztof.kozlowski@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Lee Jones <lee@kernel.org>
Change-Id: I0dd497d7a6eb51e9d285b4a256a711d7c9627fec
---
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 65834153ba977..19c7df92c63e0 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -332,7 +332,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.42.0.820.g83a721a137-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4.19.y 3/6] rpmsg: Fix kfree() of static memory on setting driver_override
2023-10-31 9:30 [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 2/6] rpmsg: Constify local variable in field store macro Lee Jones
@ 2023-10-31 9:30 ` Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 4/6] rpmsg: Fix calling device_lock() on non-initialized device Lee Jones
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2023-10-31 9:30 UTC (permalink / raw)
To: lee; +Cc: stable, Krzysztof Kozlowski, Bjorn Andersson, Greg Kroah-Hartman
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
commit 42cd402b8fd4672b692400fe5f9eecd55d2794ac upstream.
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")
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Link: https://lore.kernel.org/r/20220419113435.246203-13-krzysztof.kozlowski@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Lee Jones <lee@kernel.org>
Change-Id: I76741cb55a11b57a8eb2caf6e4470d0a04a570a9
---
drivers/rpmsg/rpmsg_internal.h | 13 +++++++++++--
include/linux/rpmsg.h | 6 ++++--
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 0d791c30b7ea6..0b5085ecb815d 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -83,10 +83,19 @@ struct device *rpmsg_find_device(struct device *parent,
*/
static inline int rpmsg_chrdev_register_device(struct rpmsg_device *rpdev)
{
+ int ret;
+
strcpy(rpdev->id.name, "rpmsg_chrdev");
- rpdev->driver_override = "rpmsg_chrdev";
+ 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/include/linux/rpmsg.h b/include/linux/rpmsg.h
index a68972b097b72..6e7690e20dc51 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
@@ -50,7 +52,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.42.0.820.g83a721a137-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4.19.y 4/6] rpmsg: Fix calling device_lock() on non-initialized device
2023-10-31 9:30 [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 2/6] rpmsg: Constify local variable in field store macro Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 3/6] rpmsg: Fix kfree() of static memory on setting driver_override Lee Jones
@ 2023-10-31 9:30 ` Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 5/6] rpmsg: glink: Release driver_override Lee Jones
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2023-10-31 9:30 UTC (permalink / raw)
To: lee; +Cc: stable, Krzysztof Kozlowski, Marek Szyprowski, Greg Kroah-Hartman
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
commit bb17d110cbf270d5247a6e261c5ad50e362d1675 upstream.
driver_set_override() helper uses device_lock() so it should not be
called before rpmsg_register_device() (which calls device_register()).
Effect can be seen with CONFIG_DEBUG_MUTEXES:
DEBUG_LOCKS_WARN_ON(lock->magic != lock)
WARNING: CPU: 3 PID: 57 at kernel/locking/mutex.c:582 __mutex_lock+0x1ec/0x430
...
Call trace:
__mutex_lock+0x1ec/0x430
mutex_lock_nested+0x44/0x50
driver_set_override+0x124/0x150
qcom_glink_native_probe+0x30c/0x3b0
glink_rpm_probe+0x274/0x350
platform_probe+0x6c/0xe0
really_probe+0x17c/0x3d0
__driver_probe_device+0x114/0x190
driver_probe_device+0x3c/0xf0
...
Refactor the rpmsg_register_device() function to use two-step device
registering (initialization + add) and call driver_set_override() in
proper moment.
This moves the code around, so while at it also NULL-ify the
rpdev->driver_override in error path to be sure it won't be kfree()
second time.
Fixes: 42cd402b8fd4 ("rpmsg: Fix kfree() of static memory on setting driver_override")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Link: https://lore.kernel.org/r/20220429195946.1061725-2-krzysztof.kozlowski@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Lee Jones <lee@kernel.org>
Change-Id: I37a8db647cfe57fa14deae16bd09cfaa525c897b
---
drivers/rpmsg/rpmsg_core.c | 33 ++++++++++++++++++++++++++++++---
drivers/rpmsg/rpmsg_internal.h | 14 +-------------
include/linux/rpmsg.h | 8 ++++++++
3 files changed, 39 insertions(+), 16 deletions(-)
diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 19c7df92c63e0..3d6427b0edc41 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -525,24 +525,51 @@ static struct bus_type rpmsg_bus = {
.remove = rpmsg_dev_remove,
};
-int rpmsg_register_device(struct rpmsg_device *rpdev)
+/*
+ * A helper for registering rpmsg device with driver override and name.
+ * Drivers should not be using it, but instead rpmsg_register_device().
+ */
+int rpmsg_register_device_override(struct rpmsg_device *rpdev,
+ const char *driver_override)
{
struct device *dev = &rpdev->dev;
int ret;
+ if (driver_override)
+ strcpy(rpdev->id.name, driver_override);
+
dev_set_name(&rpdev->dev, "%s.%s.%d.%d", dev_name(dev->parent),
rpdev->id.name, rpdev->src, rpdev->dst);
rpdev->dev.bus = &rpmsg_bus;
- ret = device_register(&rpdev->dev);
+ device_initialize(dev);
+ if (driver_override) {
+ ret = driver_set_override(dev, &rpdev->driver_override,
+ driver_override,
+ strlen(driver_override));
+ if (ret) {
+ dev_err(dev, "device_set_override failed: %d\n", ret);
+ return ret;
+ }
+ }
+
+ ret = device_add(dev);
if (ret) {
- dev_err(dev, "device_register failed: %d\n", ret);
+ dev_err(dev, "device_add failed: %d\n", ret);
+ kfree(rpdev->driver_override);
+ rpdev->driver_override = NULL;
put_device(&rpdev->dev);
}
return ret;
}
+EXPORT_SYMBOL(rpmsg_register_device_override);
+
+int rpmsg_register_device(struct rpmsg_device *rpdev)
+{
+ return rpmsg_register_device_override(rpdev, NULL);
+}
EXPORT_SYMBOL(rpmsg_register_device);
/*
diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 0b5085ecb815d..ebd53616ef5dc 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -83,19 +83,7 @@ struct device *rpmsg_find_device(struct device *parent,
*/
static inline int rpmsg_chrdev_register_device(struct rpmsg_device *rpdev)
{
- int ret;
-
- strcpy(rpdev->id.name, "rpmsg_chrdev");
- 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 ret;
+ return rpmsg_register_device_override(rpdev, "rpmsg_ctrl");
}
#endif
diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
index 6e7690e20dc51..267533fecbdd9 100644
--- a/include/linux/rpmsg.h
+++ b/include/linux/rpmsg.h
@@ -115,6 +115,8 @@ struct rpmsg_driver {
#if IS_ENABLED(CONFIG_RPMSG)
+int rpmsg_register_device_override(struct rpmsg_device *rpdev,
+ const char *driver_override);
int register_rpmsg_device(struct rpmsg_device *dev);
void unregister_rpmsg_device(struct rpmsg_device *dev);
int __register_rpmsg_driver(struct rpmsg_driver *drv, struct module *owner);
@@ -139,6 +141,12 @@ __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,
#else
+static inline int rpmsg_register_device_override(struct rpmsg_device *rpdev,
+ const char *driver_override)
+{
+ return -ENXIO;
+}
+
static inline int register_rpmsg_device(struct rpmsg_device *dev)
{
return -ENXIO;
--
2.42.0.820.g83a721a137-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4.19.y 5/6] rpmsg: glink: Release driver_override
2023-10-31 9:30 [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
` (2 preceding siblings ...)
2023-10-31 9:30 ` [PATCH v4.19.y 4/6] rpmsg: Fix calling device_lock() on non-initialized device Lee Jones
@ 2023-10-31 9:30 ` Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 6/6] rpmsg: Fix possible refcount leak in rpmsg_register_device_override() Lee Jones
2023-10-31 9:36 ` [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
5 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2023-10-31 9:30 UTC (permalink / raw)
To: lee; +Cc: stable, Bjorn Andersson, Chris Lew, Bjorn Andersson
From: Bjorn Andersson <quic_bjorande@quicinc.com>
commit fb80ef67e8ff6a00d3faad4cb348dafdb8eccfd8 upstream.
Upon termination of the rpmsg_device, driver_override needs to be freed
to avoid leaking the potentially assigned string.
Fixes: 42cd402b8fd4 ("rpmsg: Fix kfree() of static memory on setting driver_override")
Fixes: 39e47767ec9b ("rpmsg: Add driver_override device attribute for rpmsg_device")
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230109223931.1706429-1-quic_bjorande@quicinc.com
Signed-off-by: Lee Jones <lee@kernel.org>
Change-Id: Ifc377cb3847accb80b52bb00e8ed769208fc6870
---
drivers/rpmsg/qcom_glink_native.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 02e39778d3c6b..48d2fb187a1bf 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1379,6 +1379,7 @@ static void qcom_glink_rpdev_release(struct device *dev)
struct glink_channel *channel = to_glink_channel(rpdev->ept);
channel->rpdev = NULL;
+ kfree(rpdev->driver_override);
kfree(rpdev);
}
--
2.42.0.820.g83a721a137-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4.19.y 6/6] rpmsg: Fix possible refcount leak in rpmsg_register_device_override()
2023-10-31 9:30 [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
` (3 preceding siblings ...)
2023-10-31 9:30 ` [PATCH v4.19.y 5/6] rpmsg: glink: Release driver_override Lee Jones
@ 2023-10-31 9:30 ` Lee Jones
2023-10-31 9:36 ` [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
5 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2023-10-31 9:30 UTC (permalink / raw)
To: lee; +Cc: stable, Hangyu Hua, Krzysztof Kozlowski, Mathieu Poirier
From: Hangyu Hua <hbh25y@gmail.com>
commit d7bd416d35121c95fe47330e09a5c04adbc5f928 upstream.
rpmsg_register_device_override need to call put_device to free vch when
driver_set_override fails.
Fix this by adding a put_device() to the error path.
Fixes: bb17d110cbf2 ("rpmsg: Fix calling device_lock() on non-initialized device")
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
Link: https://lore.kernel.org/r/20220624024120.11576-1-hbh25y@gmail.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Lee Jones <lee@kernel.org>
Change-Id: I38b3e338ed57bc27d8818be9c166f52762973db3
---
drivers/rpmsg/rpmsg_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 3d6427b0edc41..880c7c4deec30 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -550,6 +550,7 @@ int rpmsg_register_device_override(struct rpmsg_device *rpdev,
strlen(driver_override));
if (ret) {
dev_err(dev, "device_set_override failed: %d\n", ret);
+ put_device(dev);
return ret;
}
}
--
2.42.0.820.g83a721a137-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override
2023-10-31 9:30 [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
` (4 preceding siblings ...)
2023-10-31 9:30 ` [PATCH v4.19.y 6/6] rpmsg: Fix possible refcount leak in rpmsg_register_device_override() Lee Jones
@ 2023-10-31 9:36 ` Lee Jones
5 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2023-10-31 9:36 UTC (permalink / raw)
To: stable, Krzysztof Kozlowski, Rafael J . Wysocki,
Greg Kroah-Hartman
On Tue, 31 Oct 2023, Lee Jones wrote:
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> commit 6c2f421174273de8f83cde4286d1c076d43a2d35 upstream.
>
> Several core drivers and buses expect that driver_override is a
> dynamically allocated memory thus later they can kfree() it.
>
> However such assumption is not documented, there were in the past and
> there are already users setting it to a string literal. This leads to
> kfree() of static memory during device release (e.g. in error paths or
> during unbind):
>
> kernel BUG at ../mm/slub.c:3960!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> ...
> (kfree) from [<c058da50>] (platform_device_release+0x88/0xb4)
> (platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90)
> (device_release) from [<c0a69050>] (kobject_put+0xec/0x20c)
> (kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c)
> (exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4)
> (platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414)
> (really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4)
> (driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8)
> (bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c)
> (__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90)
> (bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c)
> (device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc)
> (of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc)
> (of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc)
> (of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118)
> (of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8)
> (of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404)
>
> Provide a helper which clearly documents the usage of driver_override.
> This will allow later to reuse the helper and reduce the amount of
> duplicated code.
>
> Convert the platform driver to use a new helper and make the
> driver_override field const char (it is not modified by the core).
>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Link: https://lore.kernel.org/r/20220419113435.246203-2-krzysztof.kozlowski@linaro.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Lee Jones <lee@kernel.org>
> Change-Id: I2f59769cfb99d8359d14e2cb7345ce3428593afc
Disregard this set please!
I'm pre-coffee and operated in the wrong kernel directory with
the incorrect `commit-msg` enabled.
> ---
> drivers/base/driver.c | 69 +++++++++++++++++++++++++++++++++
> drivers/base/platform.c | 28 ++-----------
> include/linux/device.h | 2 +
> include/linux/platform_device.h | 6 ++-
> 4 files changed, 80 insertions(+), 25 deletions(-)
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4.19.y 2/6] rpmsg: Constify local variable in field store macro
2023-10-31 11:39 Lee Jones
@ 2023-10-31 11:39 ` Lee Jones
0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2023-10-31 11:39 UTC (permalink / raw)
To: lee; +Cc: stable, Krzysztof Kozlowski, Greg Kroah-Hartman
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
commit e5f89131a06142e91073b6959d91cea73861d40e upstream.
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>
Link: https://lore.kernel.org/r/20220419113435.246203-12-krzysztof.kozlowski@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Lee Jones <lee@kernel.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 65834153ba977..19c7df92c63e0 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -332,7 +332,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.42.0.820.g83a721a137-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-10-31 11:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-31 9:30 [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 2/6] rpmsg: Constify local variable in field store macro Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 3/6] rpmsg: Fix kfree() of static memory on setting driver_override Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 4/6] rpmsg: Fix calling device_lock() on non-initialized device Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 5/6] rpmsg: glink: Release driver_override Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 6/6] rpmsg: Fix possible refcount leak in rpmsg_register_device_override() Lee Jones
2023-10-31 9:36 ` [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
-- strict thread matches above, loose matches on Subject: below --
2023-10-31 11:39 Lee Jones
2023-10-31 11:39 ` [PATCH v4.19.y 2/6] rpmsg: Constify local variable in field store macro Lee Jones
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.