driver: platform: Add helper for safer setting of driver_override
Subsystems (3)
PATCH 1/2 driver: platform: Add helper for safer setting of driver_override
From: Krzysztof Kozlowski <[email protected]>
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).
--- 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(-)
drivers/base/driver.c +69 -0
@@ -31,6 +31,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.
drivers/base/platform.c +4 -24
@@ -873,31 +873,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; }
include/linux/device.h +2 -0
@@ -319,6 +319,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,
include/linux/platform_device.h +5 -1
@@ -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;
Signature · 1 line
2.42.0.655.g421f12c284-goog
PATCH 2/2 rpmsg: Fix kfree() of static memory on setting driver_override
From: Krzysztof Kozlowski <[email protected]>
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.
--- drivers/rpmsg/rpmsg_internal.h | 13 +++++++++++-- include/linux/rpmsg.h | 6 ++++-- 2 files changed, 15 insertions(+), 4 deletions(-)
drivers/rpmsg/rpmsg_internal.h +11 -2
@@ -91,10 +91,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
include/linux/rpmsg.h +4 -2
@@ -66,7 +66,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
@@ -75,7 +77,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;
Signature · 1 line
2.42.0.655.g421f12c284-goog
Greg Kroah-Hartman
maintainerQuoted 3 lines · Lee Jones
From: Krzysztof Kozlowski <[email protected]>
commit
6c2f421174273de8f83cde4286d1c076d43a2d35upstream.
<snip>
Why only 2 4.14.y patches and not 3 like the 4.19.y and other series?
confused,
greg k-h
Quoted 10 lines · Greg Kroah-Hartman
Quoted 3 lines · Lee Jones
From: Krzysztof Kozlowski <[email protected]>
commit
6c2f421174273de8f83cde4286d1c076d43a2d35upstream.<snip>
Why only 2 4.14.y patches and not 3 like the 4.19.y and other series?
confused,
Because the function fixed by the other [PATCH 2/3] patches doesn't exist in to v4.14.
Signature · 1 line
Lee Jones [李琼斯]