From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9676541C62; Mon, 1 Sep 2025 20:01:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756756893; cv=none; b=qLjQDs6W3nglpw9bkGvdXlqe6yUDWeft9XeXnNjth4XoqQe3JYQbSUwYZCFwmoDY2vfncJ0r788oxQ5gl8DRaZMraYAGz1df7/Gww6jDlHdEO236F5P6t1XCBhBlCblqbEEnnwirAvMPXjupUz/bb0/Q6uilMG/4ICtP+V6jxF8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756756893; c=relaxed/simple; bh=VRtuR5jwtMjmkf89hHU6v6T8VPdE+c8Qg/l+jnNwDSI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=T8XMop5+GwTNgbrLh/BFGxESt5FWv2lc6145RGgHGShZgby1lXrVrYo3IDIo/j65VRUsn4LRLhAs3kHTHEsCEG8N6/lUcJiwphbducaVEDfFU9oEwQ+xGjZYoiJRvK5tv4ioiljOsQy+AEqDXGEEkJ1TiQ/sL02t5y2vHxkQ5mk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=Fs/2yd/N; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="Fs/2yd/N" Received: by smtp.kernel.org (Postfix) with ESMTPSA id AA6E8C4CEF0; Mon, 1 Sep 2025 20:01:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1756756893; bh=VRtuR5jwtMjmkf89hHU6v6T8VPdE+c8Qg/l+jnNwDSI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Fs/2yd/NKzgpeL642FAW5Ispn7TMipLzdMwRt9Lu7hV5AbvkLqLIeWtmbzYd5mfgJ Pt2BmoI7OGwn+0bNtkdBhqeOIuqczogT6bkN+gIKwKUDQHhiP2gPFW86cPSnUmZuvp VL9KeXMA1rzHdrAmvZZILPC3fK5To7zpmFRY0pps= Date: Mon, 1 Sep 2025 22:01:29 +0200 From: Greg KH To: Mohammad Amin Hosseini Cc: linux-iio@vger.kernel.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org, jic23@kernel.org, lars@metafoo.de, Michael.Hennerich@analog.com, dlechner@baylibre.com, nuno.sa@analog.com, andy@kernel.org, sonic.zhang@analog.com, vapier@gentoo.org, dan.carpenter@linaro.org Subject: Re: [PATCH v4] staging: iio: adc: ad7816: fix race condition in SPI operations Message-ID: <2025090110-dipper-sulfate-36cc@gregkh> References: <20250901160310.399-1-moahmmad.hosseinii@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250901160310.399-1-moahmmad.hosseinii@gmail.com> On Mon, Sep 01, 2025 at 07:33:10PM +0330, Mohammad Amin Hosseini wrote: > The ad7816 driver lacks proper synchronization around SPI operations > and device state access. Concurrent access from multiple threads can > lead to data corruption and inconsistent device state. > > The driver performs sequences of GPIO pin manipulations followed by > SPI transactions without any locking. Device state variables (mode, > channel_id, oti_data) are also accessed without synchronization. > > This bug was found through manual code review using static analysis > techniques. The review focused on identifying unsynchronized access > patterns to shared resources. Key indicators were: > - GPIO pin state changes followed by SPI operations without atomicity > - Shared state variables accessed from multiple sysfs entry points > - No mutex or spinlock protection around sections > - Potential for interleaved execution in multi-threaded environments > > The review methodology involved tracing data flow paths and identifying > points where concurrent access could corrupt device state or SPI > communication sequences. > > Add io_lock mutex to protect: > - SPI transactions and GPIO sequences in read/write functions > - Device state variables in sysfs show/store functions > - Concurrent access to chip configuration > > This prevents race conditions when multiple processes access the device > simultaneously through sysfs attributes or device file operations. > > Fixes: 7924425db04a ("staging: iio: adc: new driver for AD7816 devices") > > Signed-off-by: Mohammad Amin Hosseini > > --- > Changes in v4: > - Added locking to reader functions (show_mode, show_channel, show_oti) > - Fixed incomplete reader/writer synchronization that could still race > - Ensured all device state access is properly synchronized > - Replace sprintf() with sysfs_emit() in all sysfs show functions > - Use sysfs_streq() instead of strcmp() for proper input parsing > - Implement locked/unlocked SPI function variants to prevent deadlock > - Use channel snapshot to ensure atomic read operations > - Fix sizeof() usage in spi_read to be more explicit (sizeof(buf)) > - Make oti write operations atomic (SPI write + shadow update under lock) > - Fix race condition in ad7816_set_oti() by taking channel_id snapshot under lock > - Fix return type consistency (ssize_t vs int) in show functions > - Use chip->id instead of string comparison for channel validation > - Add explicit cast for narrowing assignment > - Add default case for unknown chip ID validation > - Use cansleep GPIO variants in sleepable context > - Improve lock documentation for protected resources > --- > drivers/staging/iio/adc/ad7816.c | 210 ++++++++++++++++++++----------- > 1 file changed, 138 insertions(+), 72 deletions(-) Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch did many different things all at once, making it difficult to review. All Linux kernel patches need to only do one thing at a time. If you need to do multiple things (such as clean up all coding style issues in a file/driver), do it in a sequence of patches, each one doing only one thing. This will make it easier to review the patches to ensure that they are correct, and to help alleviate any merge issues that larger patches can cause. - You have marked a patch with a "Fixes:" tag for a commit that is in an older released kernel, yet you do not have a cc: stable line in the signed-off-by area at all, which means that the patch will not be applied to any older kernel releases. To properly fix this, please follow the documented rules in the Documentation/process/stable-kernel-rules.rst file for how to resolve this. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot