[RESEND PATCH] pinctrl: devicetree: Avoid taking direct reference to device name string
From: Will Deacon
Date: Wed Oct 02 2019 - 08:42:25 EST
When populating the pinctrl mapping table entries for a device, the
'dev_name' field for each entry is initialised to point directly at the
string returned by 'dev_name()' for the device and subsequently used by
'create_pinctrl()' when looking up the mappings for the device being
probed.
This is unreliable in the presence of calls to 'dev_set_name()', which may
reallocate the device name string leaving the pinctrl mappings with a
dangling reference. This then leads to a use-after-free every time the
name is dereferenced by a device probe:
| BUG: KASAN: invalid-access in strcmp+0x20/0x64
| Read of size 1 at addr 13ffffc153494b00 by task modprobe/590
| Pointer tag: [13], memory tag: [fe]
|
| Call trace:
| __kasan_report+0x16c/0x1dc
| kasan_report+0x10/0x18
| check_memory_region
| __hwasan_load1_noabort+0x4c/0x54
| strcmp+0x20/0x64
| create_pinctrl+0x18c/0x7f4
| pinctrl_get+0x90/0x114
| devm_pinctrl_get+0x44/0x98
| pinctrl_bind_pins+0x5c/0x450
| really_probe+0x1c8/0x9a4
| driver_probe_device+0x120/0x1d8
Follow the example of sysfs, and duplicate the device name string before
stashing it away in the pinctrl mapping entries.
Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
Reported-by: Elena Petrova <lenaptr@xxxxxxxxxx>
Tested-by: Elena Petrova <lenaptr@xxxxxxxxxx>
Signed-off-by: Will Deacon <will@xxxxxxxxxx>
---
drivers/pinctrl/devicetree.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index 88ddbb2e30de..9721ad30c541 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -29,6 +29,13 @@ struct pinctrl_dt_map {
static void dt_free_map(struct pinctrl_dev *pctldev,
struct pinctrl_map *map, unsigned num_maps)
{
+ int i;
+
+ for (i = 0; i < num_maps; ++i) {
+ kfree_const(map[i].dev_name);
+ map[i].dev_name = NULL;
+ }
+
if (pctldev) {
const struct pinctrl_ops *ops = pctldev->desc->pctlops;
if (ops->dt_free_map)
@@ -63,7 +70,13 @@ static int dt_remember_or_free_map(struct pinctrl *p, const char *statename,
/* Initialize common mapping table entry fields */
for (i = 0; i < num_maps; i++) {
- map[i].dev_name = dev_name(p->dev);
+ const char *devname;
+
+ devname = kstrdup_const(dev_name(p->dev), GFP_KERNEL);
+ if (!devname)
+ goto err_free_map;
+
+ map[i].dev_name = devname;
map[i].name = statename;
if (pctldev)
map[i].ctrl_dev_name = dev_name(pctldev->dev);
@@ -71,10 +84,8 @@ static int dt_remember_or_free_map(struct pinctrl *p, const char *statename,
/* Remember the converted mapping table entries */
dt_map = kzalloc(sizeof(*dt_map), GFP_KERNEL);
- if (!dt_map) {
- dt_free_map(pctldev, map, num_maps);
- return -ENOMEM;
- }
+ if (!dt_map)
+ goto err_free_map;
dt_map->pctldev = pctldev;
dt_map->map = map;
@@ -82,6 +93,10 @@ static int dt_remember_or_free_map(struct pinctrl *p, const char *statename,
list_add_tail(&dt_map->node, &p->dt_maps);
return pinctrl_register_map(map, num_maps, false);
+
+err_free_map:
+ dt_free_map(pctldev, map, num_maps);
+ return -ENOMEM;
}
struct pinctrl_dev *of_pinctrl_get(struct device_node *np)
--
2.23.0.444.g18eeb5a265-goog