linux-kernel - Re: [PATCH] gpio: it87: balance superio enter/exit calls in error path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <89a59ff8-b016-4bff-96da-92868aeee401@gibson.sh>
Date: Tue, 9 Dec 2025 15:42:38 +0100
From: Daniel Gibson <daniel@...son.sh>
To: Bartosz Golaszewski <bartosz.golaszewski@....qualcomm.com>,
 Linus Walleij <linusw@...nel.org>, Peng Fan <peng.fan@....com>,
 Bartosz Golaszewski <brgl@...nel.org>
Cc: linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] gpio: it87: balance superio enter/exit calls in error
 path

On 12/9/25 07:12, Bartosz Golaszewski wrote:
> We always call superio_enter() in it87_gpio_direction_out() but only
> call superio_exit() if the call to it87_gpio_set() succeeds. Move the
> label to balance the calls in error path as well.
> 
> Fixes: ef877a159072 ("gpio: it87: use new line value setter callbacks")
> Reported-by: Daniel Gibson <daniel@...son.sh>
> Closes: https://lore.kernel.org/all/bd0a00e3-9b8c-43e8-8772-e67b91f4c71e@gibson.sh/
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@....qualcomm.com>
> ---
>  drivers/gpio/gpio-it87.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-it87.c b/drivers/gpio/gpio-it87.c
> index 5d677bcfccf2..528ac1890613 100644
> --- a/drivers/gpio/gpio-it87.c
> +++ b/drivers/gpio/gpio-it87.c
> @@ -254,9 +254,8 @@ static int it87_gpio_direction_out(struct gpio_chip *chip,
>  	if (rc)
>  		goto exit;
>  
> -	superio_exit();
> -
>  exit:
> +	superio_exit();
>  	spin_unlock(&it87_gpio->lock);
>  	return rc;
>  }

Moving this after exit: means that superio_exit() is also called if
superio_enter() failed, a few lines above this patch:

	rc = superio_enter();
	if (rc)
		goto exit;

I don't know if this is really wrong, but it looks fishy and this code
behaved differently for years - in contrast to the change to skip
superio_exit() if it87_gpio_set() failed, which is very recent,
introduced when the return type of it87_gpio_set() was changed from void
to int.

Probably (again, I don't know if this is actually wrong, I'm not that
familiar with how this chip works) superio_exit() should remain above
`exit:` and `if(rc) goto exit;` after `rc = it87_gpio_set(chip,
gpio_num, val);` should be removed, so:
- if superio_enter() fails, its error code is returned,
  but superio_exit() is *not* called
- the it87_gpio_set(chip, gpio_num, val); return code is returned by
  it87_gpio_direction_out() no matter if it succeeds or fails,
  but in either case superio_exit() is called


Cheers,
Daniel