linux-kernel - Re: [PATCH v4 2/5] iio: light: al3000a: Fix an error handling path in al3000a

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: <20250405175755.4f5d57e3@jic23-huawei>
Date: Sat, 5 Apr 2025 17:57:55 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Heidelberg via B4 Relay <devnull+david.ixit.cz@...nel.org>
Cc: david@...t.cz, Lars-Peter Clausen <lars@...afoo.de>, Svyatoslav Ryhel
 <clamor95@...il.com>, Robert Eckelmann <longnoserob@...il.com>,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/5] iio: light: al3000a: Fix an error handling path
 in al3000a_probe()

On Wed, 02 Apr 2025 21:33:25 +0200
David Heidelberg via B4 Relay <devnull+david.ixit.cz@...nel.org> wrote:

> From: David Heidelberg <david@...t.cz>
> 
> If regmap_write() fails in al3000a_init(), al3000a_set_pwr_off is
> not called.
> 
> In order to avoid such a situation, move the devm_add_action_or_reset()
> which calls al3000a_set_pwr_off right after a successful
> al3000a_set_pwr_on.
> 
> Signed-off-by: David Heidelberg <david@...t.cz>
> ---
>  drivers/iio/light/al3000a.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/al3000a.c b/drivers/iio/light/al3000a.c
> index e2fbb1270040f43d9f0a97838861818a8eaef813..6d5115b2a06c5acce18d831b9c41d3d5121fba12 100644
> --- a/drivers/iio/light/al3000a.c
> +++ b/drivers/iio/light/al3000a.c
> @@ -85,12 +85,17 @@ static void al3000a_set_pwr_off(void *_data)
>  
>  static int al3000a_init(struct al3000a_data *data)
>  {
> +	struct device *dev = regmap_get_device(data->regmap);
>  	int ret;
>  
>  	ret = al3000a_set_pwr_on(data);
>  	if (ret)
>  		return ret;
>  
> +	ret = devm_add_action_or_reset(dev, al3000a_set_pwr_off, data);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to add action\n");
Whilst this is the same thing Christophe pointed out in patch 1, as it is simple code
movement I think it is fine to leave it here 'for now'. 

It probably makes sense to scrub IIO in general for cases of this if we decide
the general rule is no error messages for devm_add_action* failures (given they
are only possible if memory allocation fails).

Jonathan

> +
>  	ret = regmap_write(data->regmap, AL3000A_REG_SYSTEM, AL3000A_CONFIG_RESET);
>  	if (ret)
>  		return ret;
> @@ -157,10 +162,6 @@ static int al3000a_probe(struct i2c_client *client)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "failed to init ALS\n");
>  
> -	ret = devm_add_action_or_reset(dev, al3000a_set_pwr_off, data);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "failed to add action\n");
> -
>  	return devm_iio_device_register(dev, indio_dev);
>  }
>  
>