mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] staging: iio: adc: ad7816: Protect sysfs attributes with mutex
@ 2026-09-20  9:15 Abdelnasser Hussein
  2026-09-21  3:49 ` Jonathan Cameron
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Abdelnasser Hussein @ 2026-09-20  9:15 UTC (permalink / raw)
  To: jic23, gregkh, nuno.sa, Michael.Hennerich
  Cc: dlechner, andy, linux, linux-iio, linux-staging, linux-kernel,
	joshua.crofts1, error27, Abdelnasser Hussein

The sysfs store functions (ad7816_store_mode and ad7816_store_channel)
modify the device state and toggle GPIO pins. If accessed concurrently
by userspace during an ongoing SPI transfer, this could corrupt the
SPI transaction.

Add the existing chip->lock guard to these functions to serialize
access and prevent race conditions.

Fixes: 7924425db04a ("staging: iio: adc: new driver for AD7816 devices")
Suggested-by: Joshua Crofts <joshua.crofts1@gmail.com>
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Signed-off-by: Abdelnasser Hussein <abdelnasserhussein11@gmail.com>
---
Changes in v2:
- Added a blank line after guard(mutex) statements as requested by Joshua.
- Added Fixes tag as requested by Dan.
- Collected Reviewed-by tag from Joshua.

 drivers/staging/iio/adc/ad7816.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index f76f0215119a..2a15d7598abb 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -143,6 +143,8 @@ static ssize_t ad7816_store_mode(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
 
+	guard(mutex)(&chip->lock);
+
 	if (strcmp(buf, "full") == 0) {
 		gpiod_set_value(chip->rdwr_pin, 1);
 		chip->mode = AD7816_FULL;
@@ -207,6 +209,8 @@ static ssize_t ad7816_store_channel(struct device *dev,
 		return -EINVAL;
 	}
 
+	guard(mutex)(&chip->lock);
+
 	chip->channel_id = data;
 
 	return len;
-- 
2.54.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] staging: iio: adc: ad7816: Protect sysfs attributes with mutex
  2026-09-20  9:15 [PATCH v2] staging: iio: adc: ad7816: Protect sysfs attributes with mutex Abdelnasser Hussein
@ 2026-09-21  3:49 ` Jonathan Cameron
  2026-09-23  6:13 ` kernel test robot
  2026-09-23 12:44 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2026-09-21  3:49 UTC (permalink / raw)
  To: Abdelnasser Hussein
  Cc: gregkh, nuno.sa, Michael.Hennerich, dlechner, andy, linux,
	linux-iio, linux-staging, linux-kernel, joshua.crofts1, error27

On Sun, 20 Sep 2026 12:15:22 +0300
Abdelnasser Hussein <abdelnasserhussein11@gmail.com> wrote:

> The sysfs store functions (ad7816_store_mode and ad7816_store_channel)
> modify the device state and toggle GPIO pins. If accessed concurrently
> by userspace during an ongoing SPI transfer, this could corrupt the
> SPI transaction.

See below. One of them doesn't seem to have that possible affect.

Thanks,

Jonathan

> 
> Add the existing chip->lock guard to these functions to serialize
> access and prevent race conditions.
> 
> Fixes: 7924425db04a ("staging: iio: adc: new driver for AD7816 devices")
> Suggested-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Signed-off-by: Abdelnasser Hussein <abdelnasserhussein11@gmail.com>
> ---
> Changes in v2:
> - Added a blank line after guard(mutex) statements as requested by Joshua.
> - Added Fixes tag as requested by Dan.
> - Collected Reviewed-by tag from Joshua.
> 
>  drivers/staging/iio/adc/ad7816.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index f76f0215119a..2a15d7598abb 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -143,6 +143,8 @@ static ssize_t ad7816_store_mode(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct ad7816_chip_info *chip = iio_priv(indio_dev);
>  
> +	guard(mutex)(&chip->lock);
> +
>  	if (strcmp(buf, "full") == 0) {
>  		gpiod_set_value(chip->rdwr_pin, 1);
>  		chip->mode = AD7816_FULL;
> @@ -207,6 +209,8 @@ static ssize_t ad7816_store_channel(struct device *dev,
>  		return -EINVAL;
>  	}
>  
> +	guard(mutex)(&chip->lock);
This isn't toggling pins or modifying anything on the device state side
of things. So at very least the commit message needs to be expanded to
what this is doing that needs a lock.

> +
>  	chip->channel_id = data;
>  
>  	return len;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] staging: iio: adc: ad7816: Protect sysfs attributes with mutex
  2026-09-20  9:15 [PATCH v2] staging: iio: adc: ad7816: Protect sysfs attributes with mutex Abdelnasser Hussein
  2026-09-21  3:49 ` Jonathan Cameron
@ 2026-09-23  6:13 ` kernel test robot
  2026-09-23 12:44 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2026-09-23  6:13 UTC (permalink / raw)
  To: Abdelnasser Hussein, jic23, gregkh, nuno.sa, Michael.Hennerich
  Cc: oe-kbuild-all, dlechner, andy, linux, linux-iio, linux-staging,
	linux-kernel, joshua.crofts1, error27, Abdelnasser Hussein

Hi Abdelnasser,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/Abdelnasser-Hussein/staging-iio-adc-ad7816-Protect-sysfs-attributes-with-mutex/20260920-121522
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20260920091522.15507-1-abdelnasserhussein11%40gmail.com
patch subject: [PATCH v2] staging: iio: adc: ad7816: Protect sysfs attributes with mutex
config: sparc-randconfig-2001-20260923 (https://download.01.org/0day-ci/archive/20260923/202609230855.IEzAf41u-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 14.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260923/202609230855.IEzAf41u-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202609230855.IEzAf41u-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/irqflags.h:17,
                    from include/asm-generic/cmpxchg-local.h:6,
                    from arch/sparc/include/asm/cmpxchg_32.h:67,
                    from arch/sparc/include/asm/cmpxchg.h:7,
                    from arch/sparc/include/asm/atomic_32.h:17,
                    from arch/sparc/include/asm/atomic.h:7,
                    from include/linux/atomic.h:7,
                    from include/asm-generic/bitops/lock.h:5,
                    from arch/sparc/include/asm/bitops_32.h:102,
                    from arch/sparc/include/asm/bitops.h:7,
                    from include/linux/bitops.h:67,
                    from include/linux/kernel.h:23,
                    from include/linux/interrupt.h:6,
                    from drivers/staging/iio/adc/ad7816.c:8:
   drivers/staging/iio/adc/ad7816.c: In function 'ad7816_store_mode':
>> drivers/staging/iio/adc/ad7816.c:139:27: error: 'struct ad7816_chip_info' has no member named 'lock'
     139 |         guard(mutex)(&chip->lock);
         |                           ^~
   include/linux/cleanup.h:558:37: note: in definition of macro 'WITH_LOCK_GUARD_1_ATTRS'
     558 |         class_##_name##_constructor(_T),                                \
         |                                     ^~
   include/linux/cleanup.h:303:17: note: in expansion of macro 'class_mutex_constructor'
     303 |                 class_##_name##_constructor
         |                 ^~~~~~
   include/linux/cleanup.h:423:9: note: in expansion of macro 'CLASS'
     423 |         CLASS(_name, __UNIQUE_ID(guard))
         |         ^~~~~
   drivers/staging/iio/adc/ad7816.c:139:9: note: in expansion of macro 'guard'
     139 |         guard(mutex)(&chip->lock);
         |         ^~~~~
>> drivers/staging/iio/adc/ad7816.c:139:27: error: 'struct ad7816_chip_info' has no member named 'lock'
     139 |         guard(mutex)(&chip->lock);
         |                           ^~
   include/linux/cleanup.h:559:97: note: in definition of macro 'WITH_LOCK_GUARD_1_ATTRS'
     559 |         *__UNIQUE_ID(unlock) __cleanup(__class_##_name##_cleanup_ctx) = (void *)(unsigned long)(_T)
         |                                                                                                 ^~
   include/linux/cleanup.h:303:17: note: in expansion of macro 'class_mutex_constructor'
     303 |                 class_##_name##_constructor
         |                 ^~~~~~
   include/linux/cleanup.h:423:9: note: in expansion of macro 'CLASS'
     423 |         CLASS(_name, __UNIQUE_ID(guard))
         |         ^~~~~
   drivers/staging/iio/adc/ad7816.c:139:9: note: in expansion of macro 'guard'
     139 |         guard(mutex)(&chip->lock);
         |         ^~~~~
   drivers/staging/iio/adc/ad7816.c: In function 'ad7816_store_channel':
   drivers/staging/iio/adc/ad7816.c:205:27: error: 'struct ad7816_chip_info' has no member named 'lock'
     205 |         guard(mutex)(&chip->lock);
         |                           ^~
   include/linux/cleanup.h:558:37: note: in definition of macro 'WITH_LOCK_GUARD_1_ATTRS'
     558 |         class_##_name##_constructor(_T),                                \
         |                                     ^~
   include/linux/cleanup.h:303:17: note: in expansion of macro 'class_mutex_constructor'
     303 |                 class_##_name##_constructor
         |                 ^~~~~~
   include/linux/cleanup.h:423:9: note: in expansion of macro 'CLASS'
     423 |         CLASS(_name, __UNIQUE_ID(guard))
         |         ^~~~~
   drivers/staging/iio/adc/ad7816.c:205:9: note: in expansion of macro 'guard'
     205 |         guard(mutex)(&chip->lock);
         |         ^~~~~
   drivers/staging/iio/adc/ad7816.c:205:27: error: 'struct ad7816_chip_info' has no member named 'lock'
     205 |         guard(mutex)(&chip->lock);
         |                           ^~
   include/linux/cleanup.h:559:97: note: in definition of macro 'WITH_LOCK_GUARD_1_ATTRS'
     559 |         *__UNIQUE_ID(unlock) __cleanup(__class_##_name##_cleanup_ctx) = (void *)(unsigned long)(_T)
         |                                                                                                 ^~
   include/linux/cleanup.h:303:17: note: in expansion of macro 'class_mutex_constructor'
     303 |                 class_##_name##_constructor
         |                 ^~~~~~
   include/linux/cleanup.h:423:9: note: in expansion of macro 'CLASS'
     423 |         CLASS(_name, __UNIQUE_ID(guard))
         |         ^~~~~
   drivers/staging/iio/adc/ad7816.c:205:9: note: in expansion of macro 'guard'
     205 |         guard(mutex)(&chip->lock);
         |         ^~~~~


vim +139 drivers/staging/iio/adc/ad7816.c

   > 8	#include <linux/interrupt.h>
     9	#include <linux/gpio/consumer.h>
    10	#include <linux/device.h>
    11	#include <linux/kernel.h>
    12	#include <linux/slab.h>
    13	#include <linux/sysfs.h>
    14	#include <linux/list.h>
    15	#include <linux/spi/spi.h>
    16	#include <linux/module.h>
    17	
    18	#include <linux/iio/iio.h>
    19	#include <linux/iio/sysfs.h>
    20	#include <linux/iio/events.h>
    21	
    22	/*
    23	 * AD7816 config masks
    24	 */
    25	#define AD7816_FULL			0x1
    26	#define AD7816_PD			0x2
    27	#define AD7816_CS_MASK			0x7
    28	#define AD7816_CS_MAX			0x4
    29	
    30	/*
    31	 * AD7816 temperature masks
    32	 */
    33	#define AD7816_VALUE_OFFSET		6
    34	#define AD7816_BOUND_VALUE_BASE		0x8
    35	#define AD7816_BOUND_VALUE_MIN		-95
    36	#define AD7816_BOUND_VALUE_MAX		152
    37	#define AD7816_TEMP_FLOAT_OFFSET	2
    38	#define AD7816_TEMP_FLOAT_MASK		0x3
    39	
    40	/*
    41	 * struct ad7816_chip_info - chip specific information
    42	 */
    43	
    44	struct ad7816_chip_info {
    45		kernel_ulong_t id;
    46		struct spi_device *spi_dev;
    47		struct gpio_desc *rdwr_pin;
    48		struct gpio_desc *convert_pin;
    49		struct gpio_desc *busy_pin;
    50		u8  oti_data[AD7816_CS_MAX + 1];
    51		u8  channel_id;	/* 0 always be temperature */
    52		u8  mode;
    53	};
    54	
    55	enum ad7816_type {
    56		ID_AD7816,
    57		ID_AD7817,
    58		ID_AD7818,
    59	};
    60	
    61	/*
    62	 * ad7816 data access by SPI
    63	 */
    64	static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
    65	{
    66		struct spi_device *spi_dev = chip->spi_dev;
    67		int ret;
    68		__be16 buf;
    69	
    70		gpiod_set_value(chip->rdwr_pin, 1);
    71		gpiod_set_value(chip->rdwr_pin, 0);
    72		ret = spi_write(spi_dev, &chip->channel_id, sizeof(chip->channel_id));
    73		if (ret < 0) {
    74			dev_err(&spi_dev->dev, "SPI channel setting error\n");
    75			return ret;
    76		}
    77		gpiod_set_value(chip->rdwr_pin, 1);
    78	
    79		if (chip->mode == AD7816_PD) { /* operating mode 2 */
    80			gpiod_set_value(chip->convert_pin, 1);
    81			gpiod_set_value(chip->convert_pin, 0);
    82		} else { /* operating mode 1 */
    83			gpiod_set_value(chip->convert_pin, 0);
    84			gpiod_set_value(chip->convert_pin, 1);
    85		}
    86	
    87		if (chip->id == ID_AD7816 || chip->id == ID_AD7817) {
    88			while (gpiod_get_value(chip->busy_pin))
    89				cpu_relax();
    90		}
    91	
    92		gpiod_set_value(chip->rdwr_pin, 0);
    93		gpiod_set_value(chip->rdwr_pin, 1);
    94		ret = spi_read(spi_dev, &buf, sizeof(*data));
    95		if (ret < 0) {
    96			dev_err(&spi_dev->dev, "SPI data read error\n");
    97			return ret;
    98		}
    99	
   100		*data = be16_to_cpu(buf);
   101	
   102		return ret;
   103	}
   104	
   105	static int ad7816_spi_write(struct ad7816_chip_info *chip, u8 data)
   106	{
   107		struct spi_device *spi_dev = chip->spi_dev;
   108		int ret;
   109	
   110		gpiod_set_value(chip->rdwr_pin, 1);
   111		gpiod_set_value(chip->rdwr_pin, 0);
   112		ret = spi_write(spi_dev, &data, sizeof(data));
   113		if (ret < 0)
   114			dev_err(&spi_dev->dev, "SPI oti data write error\n");
   115	
   116		return ret;
   117	}
   118	
   119	static ssize_t ad7816_show_mode(struct device *dev,
   120					struct device_attribute *attr,
   121					char *buf)
   122	{
   123		struct iio_dev *indio_dev = dev_to_iio_dev(dev);
   124		struct ad7816_chip_info *chip = iio_priv(indio_dev);
   125	
   126		if (chip->mode)
   127			return sysfs_emit(buf, "power-save\n");
   128		return sysfs_emit(buf, "full\n");
   129	}
   130	
   131	static ssize_t ad7816_store_mode(struct device *dev,
   132					 struct device_attribute *attr,
   133					 const char *buf,
   134					 size_t len)
   135	{
   136		struct iio_dev *indio_dev = dev_to_iio_dev(dev);
   137		struct ad7816_chip_info *chip = iio_priv(indio_dev);
   138	
 > 139		guard(mutex)(&chip->lock);
   140	
   141		if (strcmp(buf, "full") == 0) {
   142			gpiod_set_value(chip->rdwr_pin, 1);
   143			chip->mode = AD7816_FULL;
   144		} else {
   145			gpiod_set_value(chip->rdwr_pin, 0);
   146			chip->mode = AD7816_PD;
   147		}
   148	
   149		return len;
   150	}
   151	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] staging: iio: adc: ad7816: Protect sysfs attributes with mutex
  2026-09-20  9:15 [PATCH v2] staging: iio: adc: ad7816: Protect sysfs attributes with mutex Abdelnasser Hussein
  2026-09-21  3:49 ` Jonathan Cameron
  2026-09-23  6:13 ` kernel test robot
@ 2026-09-23 12:44 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2026-09-23 12:44 UTC (permalink / raw)
  To: Abdelnasser Hussein, jic23, gregkh, nuno.sa, Michael.Hennerich
  Cc: oe-kbuild-all, dlechner, andy, linux, linux-iio, linux-staging,
	linux-kernel, joshua.crofts1, error27, Abdelnasser Hussein

Hi Abdelnasser,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/Abdelnasser-Hussein/staging-iio-adc-ad7816-Protect-sysfs-attributes-with-mutex/20260920-121522
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20260920091522.15507-1-abdelnasserhussein11%40gmail.com
patch subject: [PATCH v2] staging: iio: adc: ad7816: Protect sysfs attributes with mutex
config: i386-randconfig-2102-20260923 (https://download.01.org/0day-ci/archive/20260923/202609231453.RPvUxbGl-lkp@intel.com/config)
compiler: clang version 22.1.8 (https://github.com/llvm/llvm-project ca7933e47d3a3451d81e72ac174dcb5aa28b59d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260923/202609231453.RPvUxbGl-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202609231453.RPvUxbGl-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/staging/iio/adc/ad7816.c:139:22: error: no member named 'lock' in 'struct ad7816_chip_info'
     139 |         guard(mutex)(&chip->lock);
         |                       ~~~~  ^
   include/linux/cleanup.h:423:2: note: expanded from macro 'guard'
     423 |         CLASS(_name, __UNIQUE_ID(guard))
         |         ^
   include/linux/cleanup.h:303:3: note: expanded from macro 'CLASS'
     303 |                 class_##_name##_constructor
         |                 ^
   <scratch space>:67:1: note: expanded from here
      67 | class_mutex_constructor
         | ^
>> drivers/staging/iio/adc/ad7816.c:139:22: error: no member named 'lock' in 'struct ad7816_chip_info'
     139 |         guard(mutex)(&chip->lock);
         |                       ~~~~  ^
   include/linux/cleanup.h:423:2: note: expanded from macro 'guard'
     423 |         CLASS(_name, __UNIQUE_ID(guard))
         |         ^
   include/linux/cleanup.h:303:3: note: expanded from macro 'CLASS'
     303 |                 class_##_name##_constructor
         |                 ^
   <scratch space>:67:1: note: expanded from here
      67 | class_mutex_constructor
         | ^
   drivers/staging/iio/adc/ad7816.c:205:22: error: no member named 'lock' in 'struct ad7816_chip_info'
     205 |         guard(mutex)(&chip->lock);
         |                       ~~~~  ^
   include/linux/cleanup.h:423:2: note: expanded from macro 'guard'
     423 |         CLASS(_name, __UNIQUE_ID(guard))
         |         ^
   include/linux/cleanup.h:303:3: note: expanded from macro 'CLASS'
     303 |                 class_##_name##_constructor
         |                 ^
   <scratch space>:107:1: note: expanded from here
     107 | class_mutex_constructor
         | ^
   drivers/staging/iio/adc/ad7816.c:205:22: error: no member named 'lock' in 'struct ad7816_chip_info'
     205 |         guard(mutex)(&chip->lock);
         |                       ~~~~  ^
   include/linux/cleanup.h:423:2: note: expanded from macro 'guard'
     423 |         CLASS(_name, __UNIQUE_ID(guard))
         |         ^
   include/linux/cleanup.h:303:3: note: expanded from macro 'CLASS'
     303 |                 class_##_name##_constructor
         |                 ^
   <scratch space>:107:1: note: expanded from here
     107 | class_mutex_constructor
         | ^
   4 errors generated.


vim +139 drivers/staging/iio/adc/ad7816.c

   130	
   131	static ssize_t ad7816_store_mode(struct device *dev,
   132					 struct device_attribute *attr,
   133					 const char *buf,
   134					 size_t len)
   135	{
   136		struct iio_dev *indio_dev = dev_to_iio_dev(dev);
   137		struct ad7816_chip_info *chip = iio_priv(indio_dev);
   138	
 > 139		guard(mutex)(&chip->lock);
   140	
   141		if (strcmp(buf, "full") == 0) {
   142			gpiod_set_value(chip->rdwr_pin, 1);
   143			chip->mode = AD7816_FULL;
   144		} else {
   145			gpiod_set_value(chip->rdwr_pin, 0);
   146			chip->mode = AD7816_PD;
   147		}
   148	
   149		return len;
   150	}
   151	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-23 12:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-20  9:15 [PATCH v2] staging: iio: adc: ad7816: Protect sysfs attributes with mutex Abdelnasser Hussein
2026-09-21  3:49 ` Jonathan Cameron
2026-09-23  6:13 ` kernel test robot
2026-09-23 12:44 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®