Skip to content

Commit 6b3a7a0

Browse files
krzklag-google
authored andcommitted
UPSTREAM: driver: platform: Add helper for safer setting of driver_override
commit 6c2f421 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). Bug: 95334746 Reviewed-by: Rafael J. Wysocki <[email protected]> Acked-by: Rafael J. Wysocki <[email protected]> Signed-off-by: Krzysztof Kozlowski <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Lee Jones <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> (cherry picked from commit 389190b) Signed-off-by: Lee Jones <[email protected]> Change-Id: I1ef85ee1f3828e947adec4a45363d8434876aade
1 parent 34d95b5 commit 6b3a7a0

File tree

4 files changed

+80
-25
lines changed

4 files changed

+80
-25
lines changed

drivers/base/driver.c

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,75 @@ static struct device *next_device(struct klist_iter *i)
3030
return dev;
3131
}
3232

33+
/**
34+
* driver_set_override() - Helper to set or clear driver override.
35+
* @dev: Device to change
36+
* @override: Address of string to change (e.g. &device->driver_override);
37+
* The contents will be freed and hold newly allocated override.
38+
* @s: NUL-terminated string, new driver name to force a match, pass empty
39+
* string to clear it ("" or "\n", where the latter is only for sysfs
40+
* interface).
41+
* @len: length of @s
42+
*
43+
* Helper to set or clear driver override in a device, intended for the cases
44+
* when the driver_override field is allocated by driver/bus code.
45+
*
46+
* Returns: 0 on success or a negative error code on failure.
47+
*/
48+
int driver_set_override(struct device *dev, const char **override,
49+
const char *s, size_t len)
50+
{
51+
const char *new, *old;
52+
char *cp;
53+
54+
if (!override || !s)
55+
return -EINVAL;
56+
57+
/*
58+
* The stored value will be used in sysfs show callback (sysfs_emit()),
59+
* which has a length limit of PAGE_SIZE and adds a trailing newline.
60+
* Thus we can store one character less to avoid truncation during sysfs
61+
* show.
62+
*/
63+
if (len >= (PAGE_SIZE - 1))
64+
return -EINVAL;
65+
66+
if (!len) {
67+
/* Empty string passed - clear override */
68+
device_lock(dev);
69+
old = *override;
70+
*override = NULL;
71+
device_unlock(dev);
72+
kfree(old);
73+
74+
return 0;
75+
}
76+
77+
cp = strnchr(s, len, '\n');
78+
if (cp)
79+
len = cp - s;
80+
81+
new = kstrndup(s, len, GFP_KERNEL);
82+
if (!new)
83+
return -ENOMEM;
84+
85+
device_lock(dev);
86+
old = *override;
87+
if (cp != s) {
88+
*override = new;
89+
} else {
90+
/* "\n" passed - clear override */
91+
kfree(new);
92+
*override = NULL;
93+
}
94+
device_unlock(dev);
95+
96+
kfree(old);
97+
98+
return 0;
99+
}
100+
EXPORT_SYMBOL_GPL(driver_set_override);
101+
33102
/**
34103
* driver_for_each_device - Iterator for devices bound to a driver.
35104
* @drv: Driver we're iterating.

drivers/base/platform.c

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,31 +1270,11 @@ static ssize_t driver_override_store(struct device *dev,
12701270
const char *buf, size_t count)
12711271
{
12721272
struct platform_device *pdev = to_platform_device(dev);
1273-
char *driver_override, *old, *cp;
1274-
1275-
/* We need to keep extra room for a newline */
1276-
if (count >= (PAGE_SIZE - 1))
1277-
return -EINVAL;
1278-
1279-
driver_override = kstrndup(buf, count, GFP_KERNEL);
1280-
if (!driver_override)
1281-
return -ENOMEM;
1282-
1283-
cp = strchr(driver_override, '\n');
1284-
if (cp)
1285-
*cp = '\0';
1286-
1287-
device_lock(dev);
1288-
old = pdev->driver_override;
1289-
if (strlen(driver_override)) {
1290-
pdev->driver_override = driver_override;
1291-
} else {
1292-
kfree(driver_override);
1293-
pdev->driver_override = NULL;
1294-
}
1295-
device_unlock(dev);
1273+
int ret;
12961274

1297-
kfree(old);
1275+
ret = driver_set_override(dev, &pdev->driver_override, buf, count);
1276+
if (ret)
1277+
return ret;
12981278

12991279
return count;
13001280
}

include/linux/device/driver.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ extern int __must_check driver_create_file(struct device_driver *driver,
155155
extern void driver_remove_file(struct device_driver *driver,
156156
const struct driver_attribute *attr);
157157

158+
int driver_set_override(struct device *dev, const char **override,
159+
const char *s, size_t len);
158160
extern int __must_check driver_for_each_device(struct device_driver *drv,
159161
struct device *start,
160162
void *data,

include/linux/platform_device.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ struct platform_device {
3232
struct resource *resource;
3333

3434
const struct platform_device_id *id_entry;
35-
char *driver_override; /* Driver name to force a match */
35+
/*
36+
* Driver name to force a match. Do not set directly, because core
37+
* frees it. Use driver_set_override() to set or clear it.
38+
*/
39+
const char *driver_override;
3640

3741
/* MFD cell pointer */
3842
struct mfd_cell *mfd_cell;

0 commit comments

Comments
 (0)