linux-kernel - Re: [PATCH v2 4/7] iio: as73211: copy/release available integration times to fix race

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: <2287601.vFx2qVVIhK@n9w6sw14>
Date: Mon, 7 Oct 2024 17:44:17 +0200
From: Christian Eggers <ceggers@...i.de>
To: Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
	Michael Hennerich <Michael.Hennerich@...log.com>, Alisa-Dariana Roman
	<alisa.roman@...log.com>, Peter Rosin <peda@...ntia.se>, Paul Cercueil
	<paul@...pouillou.net>, Sebastian Reichel <sre@...nel.org>, Matteo Martelli
	<matteomartelli3@...il.com>
CC: <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-mips@...r.kernel.org>, <linux-pm@...r.kernel.org>, Matteo Martelli
	<matteomartelli3@...il.com>
Subject: Re: [PATCH v2 4/7] iio: as73211: copy/release available integration times to
 fix race

Hi Matteo,

originally the `mutex` member of `struct as73211_data` was only intended for
protecting the cached device registers. So can you please update the
documentation of this member?

I can perform some testing für as73211, but I would like to wait until
the "copy data twice" question has been solved. IMHO a solution like Nuno
proposed may be easier to understand.

regards,
Christian

On Monday, 7 October 2024, 10:37:13 CEST, Matteo Martelli wrote:
> While available integration times are being printed to sysfs by iio core
> (iio_read_channel_info_avail), the sampling frequency might be changed.
> This could cause the buffer shared with iio core to be corrupted. To
> prevent it, make a copy of the integration times buffer and free it in
> the read_avail_release_resource callback.
> 
> Signed-off-by: Matteo Martelli <matteomartelli3@...il.com>
> ---
>  drivers/iio/light/as73211.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
> index be0068081ebbbb37fdfb252b67a77b302ff725f6..27bc8cb791039944662a74fc72f09e2c3642cfa6 100644
> --- a/drivers/iio/light/as73211.c
> +++ b/drivers/iio/light/as73211.c
> @@ -493,17 +493,32 @@ static int as73211_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec co
>  		*type = IIO_VAL_INT;
>  		return IIO_AVAIL_LIST;
>  
> -	case IIO_CHAN_INFO_INT_TIME:
> +	case IIO_CHAN_INFO_INT_TIME: {
>  		*length = ARRAY_SIZE(data->int_time_avail);
> -		*vals = data->int_time_avail;
>  		*type = IIO_VAL_INT_PLUS_MICRO;
> -		return IIO_AVAIL_LIST;
>  
> +		guard(mutex)(&data->mutex);
> +
> +		*vals = kmemdup_array(data->int_time_avail, *length,
> +				      sizeof(int), GFP_KERNEL);
> +		if (!*vals)
> +			return -ENOMEM;
> +
> +		return IIO_AVAIL_LIST;
> +	}
>  	default:
>  		return -EINVAL;
>  	}
>  }
>  
> +static void as73211_read_avail_release_res(struct iio_dev *indio_dev,
> +					   struct iio_chan_spec const *chan,
> +					   const int *vals, long mask)
> +{
> +	if (mask == IIO_CHAN_INFO_INT_TIME)
> +		kfree(vals);
> +}
> +
>  static int _as73211_write_raw(struct iio_dev *indio_dev,
>  			       struct iio_chan_spec const *chan __always_unused,
>  			       int val, int val2, long mask)
> @@ -699,6 +714,7 @@ static irqreturn_t as73211_trigger_handler(int irq __always_unused, void *p)
>  static const struct iio_info as73211_info = {
>  	.read_raw = as73211_read_raw,
>  	.read_avail = as73211_read_avail,
> +	.read_avail_release_resource = as73211_read_avail_release_res,
>  	.write_raw = as73211_write_raw,
>  };
>  
> 
>