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

[Heiko Stuebner <heiko@sntech.de>]

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@omp.ru>

Assuming that the DT is our friend, really is a bad assumption :-) .

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

Reviewed-by: Heiko Stuebner <heiko@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,
>