[PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler optimization induced race

[PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler optimization induced race

[Gui-Dong Han]

Simply copying shared data to a local variable cannot prevent data
races. The compiler is allowed to optimize away the local copy and
re-read the shared memory, causing a Time-of-Check Time-of-Use (TOCTOU)
issue if the data changes between the check and the usage.

To enforce the use of the local variable, use READ_ONCE() when reading
the shared data and WRITE_ONCE() when updating it. Apply these macros to
the three identified locations (curr_sense, adc, and fault) where local
variables are used for error validation, ensuring the value remains
consistent.

Reported-by: Ben Hutchings <ben@decadent.org.uk>
Closes: https://lore.kernel.org/all/6fe17868327207e8b850cf9f88b7dc58b2021f73.camel@decadent.org.uk/
Fixes: f5bae2642e3d ("hwmon: Driver for MAX16065 System Manager and compatibles")
Fixes: b8d5acdcf525 ("hwmon: (max16065) Use local variable to avoid TOCTOU")
Cc: stable@vger.kernel.org
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
 drivers/hwmon/max16065.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/hwmon/max16065.c b/drivers/hwmon/max16065.c
index 4c9e7892a73c..43fbb9b26b10 100644
--- a/drivers/hwmon/max16065.c
+++ b/drivers/hwmon/max16065.c
@@ -151,27 +151,27 @@ static struct max16065_data *max16065_update_device(struct device *dev)
 		int i;
 
 		for (i = 0; i < data->num_adc; i++)
-			data->adc[i]
-			  = max16065_read_adc(client, MAX16065_ADC(i));
+			WRITE_ONCE(data->adc[i],
+				   max16065_read_adc(client, MAX16065_ADC(i)));
 
 		if (data->have_current) {
-			data->adc[MAX16065_NUM_ADC]
-			  = max16065_read_adc(client, MAX16065_CSP_ADC);
-			data->curr_sense
-			  = i2c_smbus_read_byte_data(client,
-						     MAX16065_CURR_SENSE);
+			WRITE_ONCE(data->adc[MAX16065_NUM_ADC],
+				   max16065_read_adc(client, MAX16065_CSP_ADC));
+			WRITE_ONCE(data->curr_sense,
+				   i2c_smbus_read_byte_data(client, MAX16065_CURR_SENSE));
 		}
 
 		for (i = 0; i < 2; i++)
-			data->fault[i]
-			  = i2c_smbus_read_byte_data(client, MAX16065_FAULT(i));
+			WRITE_ONCE(data->fault[i],
+				   i2c_smbus_read_byte_data(client, MAX16065_FAULT(i)));
 
 		/*
 		 * MAX16067 and MAX16068 have separate undervoltage and
 		 * overvoltage alarm bits. Squash them together.
 		 */
 		if (data->chip == max16067 || data->chip == max16068)
-			data->fault[0] |= data->fault[1];
+			WRITE_ONCE(data->fault[0],
+				   data->fault[0] | data->fault[1]);
 
 		data->last_updated = jiffies;
 		data->valid = true;
@@ -185,7 +185,7 @@ static ssize_t max16065_alarm_show(struct device *dev,
 {
 	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(da);
 	struct max16065_data *data = max16065_update_device(dev);
-	int val = data->fault[attr2->nr];
+	int val = READ_ONCE(data->fault[attr2->nr]);
 
 	if (val < 0)
 		return val;
@@ -203,7 +203,7 @@ static ssize_t max16065_input_show(struct device *dev,
 {
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
 	struct max16065_data *data = max16065_update_device(dev);
-	int adc = data->adc[attr->index];
+	int adc = READ_ONCE(data->adc[attr->index]);
 
 	if (unlikely(adc < 0))
 		return adc;
@@ -216,7 +216,7 @@ static ssize_t max16065_current_show(struct device *dev,
 				     struct device_attribute *da, char *buf)
 {
 	struct max16065_data *data = max16065_update_device(dev);
-	int curr_sense = data->curr_sense;
+	int curr_sense = READ_ONCE(data->curr_sense);
 
 	if (unlikely(curr_sense < 0))
 		return curr_sense;
-- 
2.43.0

Re: [PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler optimization induced race

[Guenter Roeck]

On Tue, Feb 03, 2026 at 08:14:43PM +0800, Gui-Dong Han wrote:
> Simply copying shared data to a local variable cannot prevent data
> races. The compiler is allowed to optimize away the local copy and
> re-read the shared memory, causing a Time-of-Check Time-of-Use (TOCTOU)
> issue if the data changes between the check and the usage.
> 
> To enforce the use of the local variable, use READ_ONCE() when reading
> the shared data and WRITE_ONCE() when updating it. Apply these macros to
> the three identified locations (curr_sense, adc, and fault) where local
> variables are used for error validation, ensuring the value remains
> consistent.
> 
> Reported-by: Ben Hutchings <ben@decadent.org.uk>
> Closes: https://lore.kernel.org/all/6fe17868327207e8b850cf9f88b7dc58b2021f73.camel@decadent.org.uk/
> Fixes: f5bae2642e3d ("hwmon: Driver for MAX16065 System Manager and compatibles")
> Fixes: b8d5acdcf525 ("hwmon: (max16065) Use local variable to avoid TOCTOU")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>

I don't claim to be an expert in this area, so I ran the patch through
an AI review and asked it if it is both necessary and complete. It claims
that this is the case, so I am going to apply this patch.

For those interested, I attached the AI review results below.

Thanks,
Guenter

---
[AI generated]
Summary:
  The patch addresses a theoretical data race and TOCTOU (Time-of-Check
  Time-of-Use) issue where compiler optimizations could reload shared data
  (data->adc, data->curr_sense, data->fault) after a validity check but
  before usage. By employing READ_ONCE in the reader functions
  (max16065_alarm_show, max16065_input_show, max16065_current_show) and
  WRITE_ONCE in the updater (max16065_update_device), the change ensures
  that the compiler treats these accesses as volatile, preventing reloads
  and ensuring that the value checked is the same as the value used.
  The logic correctly handles the "lockless reader / locked updater"
  pattern common in hwmon drivers.

  Verification:
   * Concurrency: The max16065_update_device function holds data->update_lock
     during updates, ensuring only one thread updates the cache. The
     READ_ONCE/WRITE_ONCE macros correctly protect against compiler
     tearing/reordering for the readers that run outside this lock.
   * Error Handling: Error codes from i2c_smbus_read_byte_data and
     max16065_read_adc are correctly stored in the data structure and
     checked in the show functions (via unlikely(val < 0)).
   * Code Style: The changes follow kernel coding style and best practices
     for concurrency annotations.
   * Regressions: No regressions were found.

  False Positives Eliminated:
   * Self-read in `max16065_update_device`: The line WRITE_ONCE(data->fault[0],
     data->fault[0] | data->fault[1]); reads data->fault[0] without READ_ONCE.
     This is safe because max16065_update_device holds the update_lock,
     so no other thread can be modifying data->fault concurrently.

Re: [PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler optimization induced race

[David Laight]

On Tue,  3 Feb 2026 20:14:43 +0800
Gui-Dong Han <hanguidong02@gmail.com> wrote:

> Simply copying shared data to a local variable cannot prevent data
> races. The compiler is allowed to optimize away the local copy and
> re-read the shared memory, causing a Time-of-Check Time-of-Use (TOCTOU)
> issue if the data changes between the check and the usage.

While the compiler is allowed to do this, is there any indication
that either gcc or clang have ever done it?
ISTR someone saying that they never did - although I thought that
was the original justification for adding ACCESS_ONCE().

READ_ONCE() also includes barriers to guarantee ordering between cpu.
These are empty on x86 but add code to architectures where the cpu
can (IIRC) re-order writes.
This is worst on alpha but affects arm and probably ppc.

For these cases is it enough to add the compile-time barrier() after
reading the variable to a local.
That will also generate better code on x86.

The WRITE_ONCE() aren't needed at all, the compilers definitely
guarantee to do a single memory access for aligned accesses that are
less than the size of a word.

This all stinks of being an AI generated patch.

	David

> 
> To enforce the use of the local variable, use READ_ONCE() when reading
> the shared data and WRITE_ONCE() when updating it. Apply these macros to
> the three identified locations (curr_sense, adc, and fault) where local
> variables are used for error validation, ensuring the value remains
> consistent.
> 
> Reported-by: Ben Hutchings <ben@decadent.org.uk>
> Closes: https://lore.kernel.org/all/6fe17868327207e8b850cf9f88b7dc58b2021f73.camel@decadent.org.uk/
> Fixes: f5bae2642e3d ("hwmon: Driver for MAX16065 System Manager and compatibles")
> Fixes: b8d5acdcf525 ("hwmon: (max16065) Use local variable to avoid TOCTOU")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
> ---
>  drivers/hwmon/max16065.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/hwmon/max16065.c b/drivers/hwmon/max16065.c
> index 4c9e7892a73c..43fbb9b26b10 100644
> --- a/drivers/hwmon/max16065.c
> +++ b/drivers/hwmon/max16065.c
> @@ -151,27 +151,27 @@ static struct max16065_data *max16065_update_device(struct device *dev)
>  		int i;
>  
>  		for (i = 0; i < data->num_adc; i++)
> -			data->adc[i]
> -			  = max16065_read_adc(client, MAX16065_ADC(i));
> +			WRITE_ONCE(data->adc[i],
> +				   max16065_read_adc(client, MAX16065_ADC(i)));
>  
>  		if (data->have_current) {
> -			data->adc[MAX16065_NUM_ADC]
> -			  = max16065_read_adc(client, MAX16065_CSP_ADC);
> -			data->curr_sense
> -			  = i2c_smbus_read_byte_data(client,
> -						     MAX16065_CURR_SENSE);
> +			WRITE_ONCE(data->adc[MAX16065_NUM_ADC],
> +				   max16065_read_adc(client, MAX16065_CSP_ADC));
> +			WRITE_ONCE(data->curr_sense,
> +				   i2c_smbus_read_byte_data(client, MAX16065_CURR_SENSE));
>  		}
>  
>  		for (i = 0; i < 2; i++)
> -			data->fault[i]
> -			  = i2c_smbus_read_byte_data(client, MAX16065_FAULT(i));
> +			WRITE_ONCE(data->fault[i],
> +				   i2c_smbus_read_byte_data(client, MAX16065_FAULT(i)));
>  
>  		/*
>  		 * MAX16067 and MAX16068 have separate undervoltage and
>  		 * overvoltage alarm bits. Squash them together.
>  		 */
>  		if (data->chip == max16067 || data->chip == max16068)
> -			data->fault[0] |= data->fault[1];
> +			WRITE_ONCE(data->fault[0],
> +				   data->fault[0] | data->fault[1]);
>  
>  		data->last_updated = jiffies;
>  		data->valid = true;
> @@ -185,7 +185,7 @@ static ssize_t max16065_alarm_show(struct device *dev,
>  {
>  	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(da);
>  	struct max16065_data *data = max16065_update_device(dev);
> -	int val = data->fault[attr2->nr];
> +	int val = READ_ONCE(data->fault[attr2->nr]);
>  
>  	if (val < 0)
>  		return val;
> @@ -203,7 +203,7 @@ static ssize_t max16065_input_show(struct device *dev,
>  {
>  	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
>  	struct max16065_data *data = max16065_update_device(dev);
> -	int adc = data->adc[attr->index];
> +	int adc = READ_ONCE(data->adc[attr->index]);
>  
>  	if (unlikely(adc < 0))
>  		return adc;
> @@ -216,7 +216,7 @@ static ssize_t max16065_current_show(struct device *dev,
>  				     struct device_attribute *da, char *buf)
>  {
>  	struct max16065_data *data = max16065_update_device(dev);
> -	int curr_sense = data->curr_sense;
> +	int curr_sense = READ_ONCE(data->curr_sense);
>  
>  	if (unlikely(curr_sense < 0))
>  		return curr_sense;

Re: [PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler optimization induced race

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2102 bytes --]

On Sat, 2026-02-07 at 10:43 +0000, David Laight wrote:
> On Tue,  3 Feb 2026 20:14:43 +0800
> Gui-Dong Han <hanguidong02@gmail.com> wrote:
> 
> > Simply copying shared data to a local variable cannot prevent data
> > races. The compiler is allowed to optimize away the local copy and
> > re-read the shared memory, causing a Time-of-Check Time-of-Use (TOCTOU)
> > issue if the data changes between the check and the usage.
> 
> While the compiler is allowed to do this, is there any indication
> that either gcc or clang have ever done it?
> ISTR someone saying that they never did - although I thought that
> was the original justification for adding ACCESS_ONCE().

They do it sometimes and it's precisely why these maros were added.  It
makes no sense to me to look at what these compilers currrently do (for
some particular versions, optimisation settings, and targets) and
extrapolate that to the assertion that they will never optimise away a
copy.

> READ_ONCE() also includes barriers to guarantee ordering between cpu.
> These are empty on x86 but add code to architectures where the cpu
> can (IIRC) re-order writes.
> This is worst on alpha but affects arm and probably ppc.

No, READ_ONCE() and WRITE_ONCE() don't include any CPU memory barriers.

> For these cases is it enough to add the compile-time barrier() after
> reading the variable to a local.
> That will also generate better code on x86.
> 
> The WRITE_ONCE() aren't needed at all, the compilers definitely
> guarantee to do a single memory access for aligned accesses that are
> less than the size of a word.

I think in this case WRITE_ONCE() might not be needed, but it's also
harmless and it's much easier to reason about {READ,WRITE}_ONCE() being
paired up.

> This all stinks of being an AI generated patch.

This is a follow-up to an earlier patch that claimed to fix the TOCTOU
issue.  I objected to that because in the absense of READ_ONCE() it was
not guaranteed to do so.

Ben.

-- 
Ben Hutchings
If more than one person is responsible for a bug, no one is at fault.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler optimization induced race

[David Laight]

On Sat, 07 Feb 2026 12:43:29 +0100
Ben Hutchings <ben@decadent.org.uk> wrote:

> On Sat, 2026-02-07 at 10:43 +0000, David Laight wrote:
> > On Tue,  3 Feb 2026 20:14:43 +0800
> > Gui-Dong Han <hanguidong02@gmail.com> wrote:
> >   
> > > Simply copying shared data to a local variable cannot prevent data
> > > races. The compiler is allowed to optimize away the local copy and
> > > re-read the shared memory, causing a Time-of-Check Time-of-Use (TOCTOU)
> > > issue if the data changes between the check and the usage.  
> > 
> > While the compiler is allowed to do this, is there any indication
> > that either gcc or clang have ever done it?
> > ISTR someone saying that they never did - although I thought that
> > was the original justification for adding ACCESS_ONCE().  
> 
> They do it sometimes and it's precisely why these maros were added.  It
> makes no sense to me to look at what these compilers currrently do (for
> some particular versions, optimisation settings, and targets) and
> extrapolate that to the assertion that they will never optimise away a
> copy.
> 
> > READ_ONCE() also includes barriers to guarantee ordering between cpu.
> > These are empty on x86 but add code to architectures where the cpu
> > can (IIRC) re-order writes.
> > This is worst on alpha but affects arm and probably ppc.  
> 
> No, READ_ONCE() and WRITE_ONCE() don't include any CPU memory barriers.

Look at the alpha version and the arm64 LTO code.
The latter changes the reads to have 'acquire' semantics to stop re-ordering.
Needed for LTO, but the thought is it might be needed in other cases.

	David 

> 
> > For these cases is it enough to add the compile-time barrier() after
> > reading the variable to a local.
> > That will also generate better code on x86.
> > 
> > The WRITE_ONCE() aren't needed at all, the compilers definitely
> > guarantee to do a single memory access for aligned accesses that are
> > less than the size of a word.  
> 
> I think in this case WRITE_ONCE() might not be needed, but it's also
> harmless and it's much easier to reason about {READ,WRITE}_ONCE() being
> paired up.
> 
> > This all stinks of being an AI generated patch.  
> 
> This is a follow-up to an earlier patch that claimed to fix the TOCTOU
> issue.  I objected to that because in the absense of READ_ONCE() it was
> not guaranteed to do so.
> 
> Ben.
> 

Re: [PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler optimization induced race

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1925 bytes --]

On Sun, 2026-02-08 at 11:48 +0000, David Laight wrote:
> On Sat, 07 Feb 2026 12:43:29 +0100
> Ben Hutchings <ben@decadent.org.uk> wrote:
> 
> > On Sat, 2026-02-07 at 10:43 +0000, David Laight wrote:
> > > On Tue,  3 Feb 2026 20:14:43 +0800
> > > Gui-Dong Han <hanguidong02@gmail.com> wrote:
> > >   
> > > > Simply copying shared data to a local variable cannot prevent data
> > > > races. The compiler is allowed to optimize away the local copy and
> > > > re-read the shared memory, causing a Time-of-Check Time-of-Use (TOCTOU)
> > > > issue if the data changes between the check and the usage.  
> > > 
> > > While the compiler is allowed to do this, is there any indication
> > > that either gcc or clang have ever done it?
> > > ISTR someone saying that they never did - although I thought that
> > > was the original justification for adding ACCESS_ONCE().  
> > 
> > They do it sometimes and it's precisely why these maros were added.  It
> > makes no sense to me to look at what these compilers currrently do (for
> > some particular versions, optimisation settings, and targets) and
> > extrapolate that to the assertion that they will never optimise away a
> > copy.
> > 
> > > READ_ONCE() also includes barriers to guarantee ordering between cpu.
> > > These are empty on x86 but add code to architectures where the cpu
> > > can (IIRC) re-order writes.
> > > This is worst on alpha but affects arm and probably ppc.  
> > 
> > No, READ_ONCE() and WRITE_ONCE() don't include any CPU memory barriers.
> 
> Look at the alpha version and the arm64 LTO code.
> The latter changes the reads to have 'acquire' semantics to stop re-ordering.
> Needed for LTO, but the thought is it might be needed in other cases.
[...]

Oh, so they do.  Sorry for "correcting" you based on my old information.

Ben.

-- 
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler optimization induced race

[David Laight]

On Sun, 08 Feb 2026 23:33:31 +0100
Ben Hutchings <ben@decadent.org.uk> wrote:

> On Sun, 2026-02-08 at 11:48 +0000, David Laight wrote:
> > On Sat, 07 Feb 2026 12:43:29 +0100
> > Ben Hutchings <ben@decadent.org.uk> wrote:
> >   
> > > On Sat, 2026-02-07 at 10:43 +0000, David Laight wrote:  
> > > > On Tue,  3 Feb 2026 20:14:43 +0800
> > > > Gui-Dong Han <hanguidong02@gmail.com> wrote:
> > > >     
> > > > > Simply copying shared data to a local variable cannot prevent data
> > > > > races. The compiler is allowed to optimize away the local copy and
> > > > > re-read the shared memory, causing a Time-of-Check Time-of-Use (TOCTOU)
> > > > > issue if the data changes between the check and the usage.    
> > > > 
> > > > While the compiler is allowed to do this, is there any indication
> > > > that either gcc or clang have ever done it?
> > > > ISTR someone saying that they never did - although I thought that
> > > > was the original justification for adding ACCESS_ONCE().    
> > > 
> > > They do it sometimes and it's precisely why these maros were added.  It
> > > makes no sense to me to look at what these compilers currrently do (for
> > > some particular versions, optimisation settings, and targets) and
> > > extrapolate that to the assertion that they will never optimise away a
> > > copy.
> > >   
> > > > READ_ONCE() also includes barriers to guarantee ordering between cpu.
> > > > These are empty on x86 but add code to architectures where the cpu
> > > > can (IIRC) re-order writes.
> > > > This is worst on alpha but affects arm and probably ppc.    
> > > 
> > > No, READ_ONCE() and WRITE_ONCE() don't include any CPU memory barriers.  
> > 
> > Look at the alpha version and the arm64 LTO code.
> > The latter changes the reads to have 'acquire' semantics to stop re-ordering.
> > Needed for LTO, but the thought is it might be needed in other cases.  
> [...]
> 
> Oh, so they do.  Sorry for "correcting" you based on my old information.

I'm not at all sure that the field which just need protection from TOCTOU
and load/store tearing shouldn't just be marked volatile.
ISTR that part of the original objection was that not all accesses needed
it - but the static check code seems to be enforcing that now.
Marking things volatile mostly stops the compiler doing CSE - which is
exactly what you want here.

	David


> 
> Ben.
> 

Re: [PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler optimization induced race

[Gui-Dong Han]

On Sat, Feb 7, 2026 at 6:43 PM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Tue,  3 Feb 2026 20:14:43 +0800
> Gui-Dong Han <hanguidong02@gmail.com> wrote:
>
> > Simply copying shared data to a local variable cannot prevent data
> > races. The compiler is allowed to optimize away the local copy and
> > re-read the shared memory, causing a Time-of-Check Time-of-Use (TOCTOU)
> > issue if the data changes between the check and the usage.
>
> While the compiler is allowed to do this, is there any indication
> that either gcc or clang have ever done it?
> ISTR someone saying that they never did - although I thought that
> was the original justification for adding ACCESS_ONCE().

This patch addresses an issue originally reported by Ben Hutchings
during his review of the 5.10 stable queue. Ben explicitly pointed out
the potential race and suggested using READ/WRITE_ONCE to enforce
local variable usage [1]. Many of his recent suggestions on stable
patches have been adopted by maintainers like Greg KH.

>
> READ_ONCE() also includes barriers to guarantee ordering between cpu.
> These are empty on x86 but add code to architectures where the cpu
> can (IIRC) re-order writes.
> This is worst on alpha but affects arm and probably ppc.
>
> For these cases is it enough to add the compile-time barrier() after
> reading the variable to a local.
> That will also generate better code on x86.
>
> The WRITE_ONCE() aren't needed at all, the compilers definitely
> guarantee to do a single memory access for aligned accesses that are
> less than the size of a word.

Following his report, I consulted the LKMM documentation. The access
pattern here fits the definition of a data race, and the documentation
recommends annotating these accesses to eliminate the data race [2].

>
> This all stinks of being an AI generated patch.

I assure you this patch was not generated by AI. It was created based
on feedback from an experienced developer and kernel documentation.

Thanks.

[1] https://lore.kernel.org/all/6fe17868327207e8b850cf9f88b7dc58b2021f73.camel@decadent.org.uk/
[2] https://elixir.bootlin.com/linux/v6.19-rc5/source/tools/memory-model/Documentation/explanation.txt#L2231