linux-kernel - Re: [PATCH 1/7] iio: dac: ad5592r: destroy mutexes in detach paths

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: <20250407200625.734c02d9@jic23-huawei>
Date: Mon, 7 Apr 2025 20:06:25 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Linus Walleij <linus.walleij@...aro.org>,
 Cosmin Tanislav <cosmin.tanislav@...log.com>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org, Bartosz
 Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH 1/7] iio: dac: ad5592r: destroy mutexes in detach paths

On Mon, 07 Apr 2025 09:18:09 +0200
Bartosz Golaszewski <brgl@...ev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> 
> The locks used here are initialized but never released which causes
> resource leaks with mutex debugging enabled. Add missing calls to
> mutex_destroy() or use devres if applicable.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Hi Bartosz,

> ---
>  drivers/iio/dac/ad5592r-base.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
> index 50d19304bacb..fe4c35689d4d 100644
> --- a/drivers/iio/dac/ad5592r-base.c
> +++ b/drivers/iio/dac/ad5592r-base.c
> @@ -155,6 +155,8 @@ static void ad5592r_gpio_cleanup(struct ad5592r_state *st)
>  {
>  	if (st->gpio_map)
>  		gpiochip_remove(&st->gpiochip);
> +
> +	mutex_destroy(&st->gpio_lock);
>  }
>  
>  static int ad5592r_reset(struct ad5592r_state *st)
> @@ -622,7 +624,9 @@ int ad5592r_probe(struct device *dev, const char *name,
>  	iio_dev->info = &ad5592r_info;
>  	iio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	mutex_init(&st->lock);
> +	ret = devm_mutex_init(dev, &st->lock);
> +	if (ret)
> +		goto error_disable_reg;

Please don't mix devm and gotos.  That tends to make for complex
logic to review for limited gain.   Devm then gotos is fine though
(i.e. a transition from all devm to gotos only mid way through probe).

Easiest solution would be to move this mutex init before the regulator
is enabled (so up about 10 lines).

Then we finish the devm stuff before starting the non devm part.

A more complex cleanup would be to drop the support for dynamic vref
(as that is almost certainly copied and pasted from elsewhere rather than
a real thing) and then use devm to manage the vref.  That wouldn't
be related to this series though so I'd just move that mutex init up.


>  
>  	ad5592r_init_scales(st, ad5592r_get_vref(st));
>  
>