[PATCH] pinctrl: rockchip: fix NULL ptr deref in rockchip_pinctrl_parse_groups()

[Chen-Yu Tsai]

On Mon, Sep 8, 2025 at 1:32 AM Heiko Stuebner <heiko at sntech.de> wrote:
>
> Am Mittwoch, 3. September 2025, 21:48:54 Mitteleuropäische Sommerzeit schrieb Sergey Shtylyov:
> > In the Rockchip driver, rockchip_pinctrl_parse_groups() assumes that the
> > "rockchip,pins" property will always be present in the DT node it parses
> > and so doesn't check the result of of_get_property() for NULL. If the DT
> > passed to the kernel happens to have such property missing, then we will
> > get a kernel oops when the pointer is dereferenced in the *for* loop just
> > a few lines after the call.  I think it's better to play safe by checking
> > the list variable for NULL (and reporting error if so), like we check the
> > size variable for validity further down...
> >
> > Found by Linux Verification Center (linuxtesting.org) with the Svace static
> > analysis tool.
> >
> > Fixes: d3e5116119bd ("pinctrl: add pinctrl driver for Rockchip SoCs")
> > Signed-off-by: Sergey Shtylyov <s.shtylyov at omp.ru>
>
> Assuming that the DT is our friend, really is a bad assumption :-) .

If this is invalid, perhaps you should make the "rockchip,pins" property
required in the binding?

> While I can't imagine what 12-year-ago-me was thinking then, simply
> checking the return value really is the better way

I think some of us have thought that guarding against incorrect DTs
is not what the kernel is supposed to do.

ChenYu

> Reviewed-by: Heiko Stuebner <heiko at sntech.de>
>
>
> >
> > ---
> > The patch is against the master branch of Linus Torvalds' linux.git repo.
> >
> >  drivers/pinctrl/pinctrl-rockchip.c |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > Index: linux/drivers/pinctrl/pinctrl-rockchip.c
> > ===================================================================
> > --- linux.orig/drivers/pinctrl/pinctrl-rockchip.c
> > +++ linux/drivers/pinctrl/pinctrl-rockchip.c
> > @@ -3488,7 +3488,9 @@ static int rockchip_pinctrl_parse_groups
> >        * do sanity check and calculate pins number
> >        */
> >       list = of_get_property(np, "rockchip,pins", &size);
> > -     /* we do not check return since it's safe node passed down */
> > +     if (!list)
> > +             return dev_err_probe(dev, -EINVAL,
> > +                                  "%pOF: no rockchip,pins property\n", np);
> >       size /= sizeof(*list);
> >       if (!size || size % 4)
> >               return dev_err_probe(dev, -EINVAL,
> >
>
>
>
>
>