Re: [PATCH v13 3/6] pinctrl: core: Make pin group callbacks optional for pin-only drivers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The function `pinconf_groups_show` iterates locklessly over pin groups using `pctlops->get_group_name` without taking `pctldev->mutex`, leading to a Use-After-Free (UAF) race condition.
- [Critical] The function `pinctrl_get_group_selector` iterates over `pctlops->get_group_name` locklessly, creating a Use-After-Free (UAF) race condition during map resolution.
--

commit 02ec67dd43dc0fad7f2e694d9ac1bbd0d9a91c1a
Author: Oleksij Rempel <o.rempel@pengutronix.de>

pinctrl: core: Make pin group callbacks optional for pin-only drivers

Relax the requirement for pin-only drivers by making the group callbacks
optional when no muxing or group pin configuration support is provided.

> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
> index 6cbcaa6709da1..3fcb7e584a937 100644
> --- a/drivers/pinctrl/core.c
> +++ b/drivers/pinctrl/core.c

[ ... ]

> @@ -738,8 +743,15 @@ int pinctrl_get_group_selector(struct pinctrl_dev *pctldev,
>  			       const char *pin_group)
>  {
>  	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
> -	unsigned int ngroups = pctlops->get_groups_count(pctldev);
>  	unsigned int group_selector = 0;
> +	unsigned int ngroups;
> +
> +	if (!pctlops->get_groups_count || !pctlops->get_group_name) {
> +		dev_err(pctldev->dev, "does not support pin groups\n");
> +		return -EINVAL;
> +	}
> +
> +	ngroups = pctlops->get_groups_count(pctldev);
>  
>  	while (group_selector < ngroups) {
>  		const char *gname = pctlops->get_group_name(pctldev,

This isn't a new regression introduced by this commit, but since the
surrounding logic is being modified, does this code have a use-after-free
race condition?

pinctrl_get_group_selector() iterates over pctlops->get_group_name
locklessly. This is called from map resolution paths
(e.g. pinmux_map_to_setting()) holding pinctrl_maps_mutex but not
pctldev->mutex. 

If there is a concurrent dynamic modification to the group list (such as a
provider driver unbinding and calling pinctrl_generic_remove_group() which
frees the descriptor), could it race against the lockless radix tree lookup
and lead to a use-after-free when accessing the returned group name?

Note that simply adding mutex_lock(&pctldev->mutex) inside
pinctrl_get_group_selector() would deadlock callers like
pin_config_group_get() which already hold the lock, so the locking might
need to be addressed correctly in the call stack.

[ ... ]

> diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c
> index dca963633b5d1..81686844dfa5f 100644
> --- a/drivers/pinctrl/pinconf.c
> +++ b/drivers/pinctrl/pinconf.c

[ ... ]

> @@ -348,8 +348,13 @@ static int pinconf_groups_show(struct seq_file *s, void *what)
>  {
>  	struct pinctrl_dev *pctldev = s->private;
>  	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
> -	unsigned int ngroups = pctlops->get_groups_count(pctldev);
>  	unsigned int selector = 0;
> +	unsigned int ngroups;
> +
> +	if (!pctlops->get_groups_count || !pctlops->get_group_name)
> +		return 0;
> +
> +	ngroups = pctlops->get_groups_count(pctldev);
>  
>  	seq_puts(s, "Pin config settings per pin group\n");
>  	seq_puts(s, "Format: group (name): configs\n");

Similarly, this isn't a new regression introduced by this patch, but does
pinconf_groups_show() have a use-after-free race condition?

It iterates locklessly over pin groups using pctlops->get_group_name without
taking pctldev->mutex (unlike its sibling pinctrl_groups_show).

Since pinconf_groups_show() is a debugfs callback, it can run concurrently
with a pinctrl driver's unbind or dynamic group removal. A driver unbinding
calls pinctrl_unregister() or pinctrl_generic_remove_group(), which frees
the group descriptor.

Because pinconf_groups_show() traverses the radix tree locklessly and does
not hold pctldev->mutex, can it access a freed group name and lead to memory
corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518100054.3552143-1-o.rempel@pengutronix.de?part=3