linux-kernel - Re: [PATCH 5.0 100/101] leds: pca9532: fix a potential NULL pointer dereference

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: <CAMuHMdVQWLfBOCyXza0bwG5-4FJ5z2gLjpsPEj8FaQepd5_nMA@mail.gmail.com>
Date:   Mon, 6 May 2019 10:21:28 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>, Kangjie Lu <kjlu@....edu>,
        Jacek Anaszewski <jacek.anaszewski@...il.com>,
        "Sasha Levin (Microsoft)" <sashal@...nel.org>
Subject: Re: [PATCH 5.0 100/101] leds: pca9532: fix a potential NULL pointer dereference

Hi Greg,

On Thu, May 2, 2019 at 5:34 PM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
> [ Upstream commit 0aab8e4df4702b31314a27ec4b0631dfad0fae0a ]
>
> In case of_match_device cannot find a match, return -EINVAL to avoid
> NULL pointer dereference.
>
> Fixes: fa4191a609f2 ("leds: pca9532: Add device tree support")
> Signed-off-by: Kangjie Lu <kjlu@....edu>
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@...il.com>
> Signed-off-by: Sasha Levin (Microsoft) <sashal@...nel.org>

> --- a/drivers/leds/leds-pca9532.c
> +++ b/drivers/leds/leds-pca9532.c
> @@ -513,6 +513,7 @@ static int pca9532_probe(struct i2c_client *client,
>         const struct i2c_device_id *id)
>  {
>         int devid;
> +       const struct of_device_id *of_id;
>         struct pca9532_data *data = i2c_get_clientdata(client);
>         struct pca9532_platform_data *pca9532_pdata =
>                         dev_get_platdata(&client->dev);
> @@ -528,8 +529,11 @@ static int pca9532_probe(struct i2c_client *client,
>                         dev_err(&client->dev, "no platform data\n");
>                         return -EINVAL;
>                 }
> -               devid = (int)(uintptr_t)of_match_device(
> -                       of_pca9532_leds_match, &client->dev)->data;
> +               of_id = of_match_device(of_pca9532_leds_match,
> +                               &client->dev);
> +               if (unlikely(!of_id))

This condition (1) can never be true, as of_pca9532_leds_match[]
populates the .data field of all entries, and (2) is already checked for
in pca9532_of_populate_pdata(), so pca9532_probe() would already
have aborted with -ENODEV before.

https://lore.kernel.org/lkml/CAMuHMdXELu2tcSB5C1yKUGft6sDGPAy997ApPzy17n0MssfyWA@mail.gmail.com/

So please stop backporting this to even more stable trees.
Thanks!

> +                       return -EINVAL;
> +               devid = (int)(uintptr_t) of_id->data;
>         } else {
>                 devid = id->driver_data;
>         }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds