Re: [PATCH v3 1/3] gpio: shared-proxy: Fix race between concurrent first users

[Shawn Guo <shengchao.guo@oss.qualcomm.com>]

On Sat, Oct 17, 2026 at 03:00:22AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] gpio: shared-proxy: unreset vote state in gpio_shared_proxy_direction_input()
> --
> 
> commit f02d19c15e193895e840a218407e46f54481d01c
> Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> 
> gpio: shared-proxy: Fix race between concurrent first users
> 
> This commit fixes a race between concurrent first users of a shared GPIO
> by tracking the number of proxies that have configured the line (via
> direction setting or voting) in cfgcnt rather than checking usecnt == 1.
> 
> > diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
> > index bc69b8729d196..0d1d724a6a376 100644
> > --- a/drivers/gpio/gpio-shared-proxy.c
> > +++ b/drivers/gpio/gpio-shared-proxy.c
> 
> [ ... ]
> 
> > @@ -155,15 +184,21 @@ static int gpio_shared_proxy_direction_input(struct gpio_chip *gc,
> >  	struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
> >  	struct gpio_shared_desc *shared_desc = proxy->shared_desc;
> >  	struct gpio_desc *desc = shared_desc->desc;
> > -	int dir;
> > +	int ret, dir;
> >  
> >  	guard(mutex)(&shared_desc->mutex);
> >  
> > -	if (shared_desc->usecnt == 1) {
> > +	if (gpio_shared_proxy_may_init(proxy)) {
> >  		dev_dbg(proxy->dev,
> > -			"Only one user of this shared GPIO, allowing to set direction to input\n");
> > +			"No other user configured this shared GPIO, allowing to set direction to input\n");
> > +
> > +		ret = gpiod_direction_input(desc);
> > +		if (ret)
> > +			return ret;
> > +
> > +		gpio_shared_proxy_mark_configured(proxy);
> >  
> > -		return gpiod_direction_input(desc);
> > +		return 0;
> >  	}
> 
> [Severity: Medium]
> Should gpio_shared_proxy_direction_input() also reset shared_desc->votecnt
> and proxy->voted_change here, just like gpio_shared_proxy_direction_output()
> does when gpio_shared_proxy_may_init(proxy) is true?
> 
> If a proxy becomes the sole configured user (shared_desc->cfgcnt == 1 and
> proxy->configured == true) while holding an active vote (proxy->voted_change
> == true and shared_desc->votecnt == 1) -- for example, after calling
> gpiod_direction_output() with 0 followed by gpiod_set_value_cansleep() with
> 1, after voting as a GPIOD_ASIS consumer, or after another consumer that
> initially set the direction to output-low releases its descriptor -- and
> then reconfigures the line as input via gpio_shared_proxy_direction_input(),
> gpiod_direction_input(desc) clears GPIOD_FLAG_IS_OUT on shared_desc->desc
> while leaving proxy->voted_change == true and shared_desc->votecnt == 1.
> 
> When the proxy is later released in gpio_shared_proxy_free():
> 
> drivers/gpio/gpio-shared-proxy.c:gpio_shared_proxy_free() {
> 	if (proxy->voted_change) {
> 		ret = gpio_shared_proxy_set_unlocked(proxy, shared_desc->def_val);
> 		if (ret)
> 			dev_err(proxy->dev,
> 				"Failed to unset the shared GPIO value on release: %d\n", ret);
> 	}
> }
> 
> gpio_shared_proxy_set_unlocked() -> gpiod_set_value_cansleep() ->
> gpiod_set_raw_value_commit() will see GPIOD_FLAG_IS_OUT cleared on the
> input descriptor, fail with -EPERM, and log an error (and at this commit,
> prior to commit "gpio: shared-proxy: Drop the vote of a released proxy on
> write failure", leave shared_desc->votecnt and proxy->voted_change leaked).

Agreed, but this predates the patch too. The original driver's
direction_input() also left voted_high/highcnt alone when it was the
sole user; this patch only changes the condition guarding that branch.
With patch 2 applied, the vote no longer leaks on the failed write in
free, so what remains is the spurious error on release.

I'll add a separate patch in v4 that clears the vote when the proxy
reinitializes the line as input, as the output path does. Resetting
votecnt to 0 there is safe: under gpio_shared_proxy_may_init() no other
proxy is configured, voting marks a proxy configured, and a proxy that
frees the line drops its vote.

Shawn