Re: [PATCH 2/2] pinctrl: meson: restore non-sleeping GPIO access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Setting `can_sleep = false` without configuring regmap to use a raw spinlock causes a sleep-in-atomic bug on PREEMPT_RT.

Pre-existing issues:
- [High] The GPIO chip is registered without devres management, resulting in a Use-After-Free if the driver is unbound.
- [High] Concurrently mutating the global static `meson_regmap_config` during probe causes a data race and potential Use-After-Free.
--

commit 6e7d1a2ca4b3fb51bef0c13ed0f7a05d0fff4365
Author: Viacheslav Bocharov <v@baodeep.com>

pinctrl: meson: restore non-sleeping GPIO access

This commit sets the meson gpio chip's can_sleep flag to false, restoring
non-sleeping access. This reverts a previous workaround since the underlying
gpio-shared-proxy locking issue has been resolved. Atomic consumers like
w1-gpio can now operate without triggering sleep warnings.

Link: https://lore.kernel.org/all/20260105150509.56537-1-bartosz.golaszewski@oss.qualcomm.com/

> diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
> index 4507dc8b5563c..18295b15ecd9d 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
> @@ -619,7 +619,7 @@ static int meson_gpiolib_register(struct meson_pinctrl *pc)
>  	pc->chip.set = meson_gpio_set;
>  	pc->chip.base = -1;
>  	pc->chip.ngpio = pc->data->num_pins;
> -	pc->chip.can_sleep = true;
> +	pc->chip.can_sleep = false;

[Severity: High]
Does this change safely support preempt-rt kernels?

Since this patch declares can_sleep = false, the gpio get/set functions
can be called from atomic contexts. However, the mmio regmap configuration
lacks .use_raw_spinlock = true:

drivers/pinctrl/meson/pinctrl-meson.c:
static struct regmap_config meson_regmap_config = {
	.reg_bits = 32,
	.val_bits = 32,
	.reg_stride = 4,
};

On preempt-rt, standard spinlocks become sleepable rt_mutexes. Without
.use_raw_spinlock = true, any atomic caller of these gpio functions
will attempt to acquire a sleepable lock with interrupts disabled, leading to
a lockdep warning and potential deadlock. Should .use_raw_spinlock = true be
added to meson_regmap_config to honor the non-sleeping contract?

>  
>  	ret = gpiochip_add_data(&pc->chip, pc);

[Severity: High]
This is a pre-existing issue, and was not introduced by the patch under
review, but could the use of gpiochip_add_data() lead to a use-after-free
when the driver unbinds?

The parent structure pc is allocated with devm_kzalloc(), but the gpio chip
is not registered using the managed devm_gpiochip_add_data(). Since there is
no .remove callback or devres action to unregister the chip, if the driver
is unbound, devres will free pc while the gpio chip remains registered
in gpiolib. Subsequent access to the gpio device would dereference the freed
memory.

[Severity: High]
This is also a pre-existing issue, and was not introduced by the patch
under review, but is there a data race when modifying the global
meson_regmap_config structure during probe?

In meson_map_resource(), the global static configuration is mutated directly:

drivers/pinctrl/meson/pinctrl-meson.c:meson_map_resource() {
...
	meson_regmap_config.max_register = resource_size(&res) - 4;
	meson_regmap_config.name = devm_kasprintf(pc->dev, GFP_KERNEL,
						  "%pOFn-%s", node,
						  name);
...
}

Since Amlogic SoCs have multiple pinctrl domains that can probe concurrently,
one thread could overwrite the .name pointer while another is using it.
Because .name is bound to the device's lifecycle via devm_kasprintf(),
if the first device unbinds, its devres frees the string, leaving the second
regmap with a dangling pointer for its name.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610153329.937833-1-v@baodeep.com?part=2

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic