Re: [PATCH v14 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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing mutex lock in pinconf_groups_show() leading to potential Use-After-Free during debugfs access.
--

commit 3be3569fd20f9c647f76233cee1c7285d9ad8584
Author: Oleksij Rempel <o.rempel@pengutronix.de>

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

This patch relaxes the requirement for pin-only drivers to implement
.get_groups_count and .get_group_name callbacks by making them optional
when no muxing or group pin configuration support is provided. The core
and debugfs helpers are updated to check for these callbacks before
invoking them.

> 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);

[Severity: High]
This is a pre-existing issue, but does pinconf_groups_show() need to hold
pctldev->mutex while iterating through pin groups?

Drivers such as pinctrl-lpass-lpi dynamically remove groups via
pinctrl_generic_remove_group() during device unbind (e.g., in
lpi_pinctrl_remove()). This deletes the radix tree node and frees the
group memory via devm_kfree(). Because the controller is registered
with devm_pinctrl_register(), the debugfs entries remain fully accessible
during the driver's remove() callback until devres cleanup runs later.

Could a concurrent read of the pinconf-groups debugfs file race with
this group removal, causing pinctrl_generic_get_group_name() to perform
an unsafe radix_tree_lookup() and dereference the freed group struct?

Also, since pinconf_groups_show() eventually calls pin_config_group_get()
which acquires pctldev->mutex, would wrapping the loop in
mutex_lock(&pctldev->mutex) cause a self-deadlock, thus requiring
unlocked internal accessors to fix properly?

>  
>  	seq_puts(s, "Pin config settings per pin group\n");
>  	seq_puts(s, "Format: group (name): configs\n");

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