linux-kernel - Re: [PATCH v2] iio: adc: ad9467: Fix the "don't allow reading vref if not available" case

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] [day] [month] [year] [list]

Message-ID: <20241207175829.31a32667@jic23-huawei>
Date: Sat, 7 Dec 2024 17:58:29 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: Michael Hennerich <Michael.Hennerich@...log.com>, Nuno Sa
 <nuno.sa@...log.com>, Lars-Peter Clausen <lars@...afoo.de>,
 linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org, Jonathan
 Cameron <Jonathan.Cameron@...wei.com>, linux-iio@...r.kernel.org
Subject: Re: [PATCH v2] iio: adc: ad9467: Fix the "don't allow reading vref
 if not available" case

On Fri,  6 Dec 2024 17:39:28 +0100
Christophe JAILLET <christophe.jaillet@...adoo.fr> wrote:

> The commit in Fixes adds a special case when only one possible scale is
> available.
> If several scales are available, it sets the .read_avail field of the
> struct iio_info to ad9467_read_avail().
> 
> However, this field already holds this function pointer, so the code is a
> no-op.
> 
> Use another struct iio_info instead to actually reflect the intent
> described in the commit message. This way, the structure to use is selected
> at runtime and they can be kept as const.
> 
> This is safer because modifying static structs that are shared between all
> instances like this, based on the properties of a single instance, is
> asking for trouble down the road.
> 
> Fixes: b92f94f74826 ("iio: adc: ad9467: don't allow reading vref if not available")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> ---
> This patch is compile tested only and is completely speculative.
> 
> Changes in v2:
>   - use another struct iio_info to keep the structure const
Agree entirely with David that this is the way to go.

Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan

> 
> v1: https://lore.kernel.org/linux-kernel/556f87c8931d7d7cdf56ebc79f974f8bef045b0d.1733431628.git.christophe.jaillet@wanadoo.fr/
> ---
>  drivers/iio/adc/ad9467.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> index d358958ab310..f30119b42ba0 100644
> --- a/drivers/iio/adc/ad9467.c
> +++ b/drivers/iio/adc/ad9467.c
> @@ -895,7 +895,7 @@ static int ad9467_update_scan_mode(struct iio_dev *indio_dev,
>  	return 0;
>  }
>  
> -static struct iio_info ad9467_info = {
> +static const struct iio_info ad9467_info = {
>  	.read_raw = ad9467_read_raw,
>  	.write_raw = ad9467_write_raw,
>  	.update_scan_mode = ad9467_update_scan_mode,
> @@ -903,6 +903,14 @@ static struct iio_info ad9467_info = {
>  	.read_avail = ad9467_read_avail,
>  };
>  
> +/* Same as above, but without .read_avail */
> +static const struct iio_info ad9467_info_no_read_avail = {
> +	.read_raw = ad9467_read_raw,
> +	.write_raw = ad9467_write_raw,
> +	.update_scan_mode = ad9467_update_scan_mode,
> +	.debugfs_reg_access = ad9467_reg_access,
> +};
> +
>  static int ad9467_scale_fill(struct ad9467_state *st)
>  {
>  	const struct ad9467_chip_info *info = st->info;
> @@ -1214,11 +1222,12 @@ static int ad9467_probe(struct spi_device *spi)
>  	}
>  
>  	if (st->info->num_scales > 1)
> -		ad9467_info.read_avail = ad9467_read_avail;
> +		indio_dev->info = &ad9467_info;
> +	else
> +		indio_dev->info = &ad9467_info_no_read_avail;
>  	indio_dev->name = st->info->name;
>  	indio_dev->channels = st->info->channels;
>  	indio_dev->num_channels = st->info->num_channels;
> -	indio_dev->info = &ad9467_info;
>  
>  	ret = ad9467_iio_backend_get(st);
>  	if (ret)