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: <aLV0LBxD0KIHPSmo@stanley.mountain>
Date: Mon, 1 Sep 2025 13:23:40 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Mohammad Amin Hosseini <moahmmad.hosseinii@...il.com>
Cc: linux-iio@...r.kernel.org, linux-staging@...ts.linux.dev,
	linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org,
	jic23@...nel.org, lars@...afoo.de, Michael.Hennerich@...log.com,
	dlechner@...libre.com, nuno.sa@...log.com, andy@...nel.org
Subject: Re: [PATCH v2] staging: iio: adc: ad7816: add mutex to serialize
 SPI/GPIO operations

On Mon, Sep 01, 2025 at 10:24:45AM +0330, Mohammad Amin Hosseini wrote:
> From: mohammad amin hosseini <moahmmad.hosseinii@...il.com>
> 
> The ad7816 driver was accessing SPI and GPIO lines without
> synchronization, which could lead to race conditions when accessed
> concurrently from multiple contexts. This might result in corrupted
> readings or inconsistent GPIO states.
> 
> Introduce an io_lock mutex in the driver structure to serialize:
> - SPI transactions in ad7816_spi_read() and ad7816_spi_write()
> - GPIO pin toggling sequences
> - Updates to device state via sysfs store functions (mode, channel, oti)
> 
> The mutex ensures proper mutual exclusion and prevents race
> conditions under concurrent access.
> 

The commit message says "might result", which basically implies this
is from reviewing the code and that we don't have a stack trace?  Is
that correct?

Are you using some kind of static checker to find racy code?  What
kind of heuristic is it using to find buggy code?

> Changes in v2:
> - Fixed mismatch between From: and Signed-off-by lines
> 
> Signed-off-by: mohammad amin hosseini <moahmmad.hosseinii@...il.com>

Please add a Fixes tag.

> @@ -200,7 +204,9 @@ static ssize_t ad7816_store_channel(struct device *dev,
>  		return -EINVAL;
>  	}
>  
> +	mutex_lock(&chip->io_lock);
>  	chip->channel_id = data;
> +	mutex_unlock(&chip->io_lock);
>  
>  	return len;
>  }
> @@ -322,7 +328,9 @@ static inline ssize_t ad7816_set_oti(struct device *dev,
>  	if (ret)
>  		return -EIO;
>  
> +	mutex_lock(&chip->io_lock);
>  	chip->oti_data[chip->channel_id] = data;
> +	mutex_unlock(&chip->io_lock);
>  
>  	return len;
>  }

I'm not really knowledgeable to review the others, if they are
required or how the locking is supposed to work.  But these aren't
correct because we're only locking around the writers and not the
readers so it could still race.

regards,
dan carpenter

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ