linux-kernel - Re: [RESEND PATCH] pinctrl: devicetree: Avoid taking direct reference to device name string

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACRpkdYFzrUT9YE3VvRdWpx-n9szyvoOnEBM7GWLZAv8t1drww@mail.gmail.com>
Date:   Thu, 3 Oct 2019 14:54:21 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Will Deacon <will@...nel.org>
Cc:     "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH] pinctrl: devicetree: Avoid taking direct reference
 to device name string

On Wed, Oct 2, 2019 at 2:42 PM Will Deacon <will@...nel.org> wrote:

> 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@...aro.org>
> Reported-by: Elena Petrova <lenaptr@...gle.com>
> Tested-by: Elena Petrova <lenaptr@...gle.com>
> Signed-off-by: Will Deacon <will@...nel.org>

Patch applied, sorry for not getting back to you earlier.

The fact that dev_set_name() is reallocating the name is a bit
scary, it doesn't feel super-stable, but I suppose there is some
particularly good reason for it.

I guess the look-up table still refers to the struct device *
directly so pin control functionality will work, but the pin controller
device name down in /sys/kernel/debug/pinctrl
is going to be bogus, am I right? Like the name given there
will be whatever the name was before the call to dev_set_name().

Yours,
Lnus Walleij