]> git.proxmox.com Git - mirror_ubuntu-bionic-kernel.git/commitdiff
pinctrl: devicetree: Avoid taking direct reference to device name string
authorWill Deacon <will@kernel.org>
Wed, 2 Oct 2019 12:42:06 +0000 (13:42 +0100)
committerKhalid Elmously <khalid.elmously@canonical.com>
Wed, 29 Jan 2020 04:45:21 +0000 (23:45 -0500)
BugLink: https://bugs.launchpad.net/bugs/1859712
[ Upstream commit be4c60b563edee3712d392aaeb0943a768df7023 ]

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@linaro.org>
Reported-by: Elena Petrova <lenaptr@google.com>
Tested-by: Elena Petrova <lenaptr@google.com>
Signed-off-by: Will Deacon <will@kernel.org>
Link: https://lore.kernel.org/r/20191002124206.22928-1-will@kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>
drivers/pinctrl/devicetree.c

index c4aa411f5935b7b0275c004a3924ffda3613630a..3a7c2d6e4d5f6c406ced920a09e7127377d2098a 100644 (file)
@@ -40,6 +40,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)
@@ -74,7 +81,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);
@@ -82,10 +95,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;
@@ -93,6 +104,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)