linux-kernel - Re: [PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler optimization induced 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: <3d81c0ff-bd80-4b06-9a59-545b5d94e6a4@roeck-us.net>
Date: Fri, 6 Feb 2026 21:31:00 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Gui-Dong Han <hanguidong02@...il.com>
Cc: linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
	baijiaju1990@...il.com, Ben Hutchings <ben@...adent.org.uk>,
	stable@...r.kernel.org
Subject: Re: [PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler
 optimization induced race

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@...adent.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@...r.kernel.org
> Signed-off-by: Gui-Dong Han <hanguidong02@...il.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.