From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D4FCE33E346 for ; Sat, 7 Feb 2026 10:43:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770460992; cv=none; b=rCAED2VeKst58WxpCReCn/gZxJnvDYcjY2TDc2zouHfzM04yLFJPSGM8hUbcY/V3kXDCmSA8w4ydNm/tpEnPvWWVm7k32uC5b6yOA46P1U9tOutxbe5sOhNmlcDKAPRoNvxvhB9/wfUoBnmdEWjmXhylzoYb5n2Q7b77GI14pSQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770460992; c=relaxed/simple; bh=VpylFaXQ5R9Ce/kUeXCB7cheL20SuIrXZ7nduGMGXJw=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=BY1D1DwTdrFze2MDMHAD4PrqY9Bswu7YZBL5ymf/GHvVbdTgEc/PIZeT6h1j5SxdYPGZWYRbZSBoI3rwLZXeYxcsvJkUXKEIpv4YfHWHDlDk2op0aO5YE3GSSJDPcUoFK/90kmblTOnlE4Q7bPUOPG8V1dHTPi/z7nuQribcm2A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=RBmezeGz; arc=none smtp.client-ip=209.85.128.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="RBmezeGz" Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-480706554beso15966715e9.1 for ; Sat, 07 Feb 2026 02:43:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1770460990; x=1771065790; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=nip7PxraMmxt86GWPJ7mOALMTbQ7LbZ8aEjfsVnU+KA=; b=RBmezeGzM1CPVl4ZZOIomSaKBNoCtr/TMk9cmDAoGFvSZAexiZwA+CdYaQ/FqrfI5N cpL/AM30x1w+byQ0LJEvuajI4kXSGlDWHMf9VniV6EpP/r7POyKF1+9IVo0HQU5TrrhP PwFo91RHG6SGMcjA8L2GNn+ZXCjS2fjn+jOb9pOm1TtJP8MwXRryuIceQDkwcvXPnZ47 fXgCub9yF4lQzU2kHQtV/IUd1xbTgE3bLjZJWSpIXJenAXDHvahUSyR2+NmRbmSXjo4g QyXQ1EindK2jWotPCH+w/xZgp0vnw00qihd6NtM6brNP65ReUd6Km2nW6LfzhtiXgTiw +h3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770460990; x=1771065790; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=nip7PxraMmxt86GWPJ7mOALMTbQ7LbZ8aEjfsVnU+KA=; b=LbfkuaRLN/+6SbwNnFko4Sg+Yx/AXwiEVARD8lW6aZ0GfzOYXV4RwjZJpSrN85OrAo XBrHmrYFFPVdZF00i63StI6P1Atgq+D2Xzuj/gz4xJWS5ugtS3xnvQfbWs0DMm+kn7W6 4KwWkUDeNakwU2CNKjM0tbcNFzWJw1TITkXVNCAcnfPxu57DDMB2n3T3h0aoTENI9IgR 92fJsVerWRWWHCdI1qeUnSnpUF2rHSwm9xBkN82uYy2cnxSh4Q71Qp7p973W79qNJf3C 6R9Wman5fHi1+cltlPj0eXDmLUaJJCzD++6maaLBZm7h4GW+62lKBJl/dnVh2e9gFGyM OPtQ== X-Forwarded-Encrypted: i=1; AJvYcCXb3jIR55/y4e0qe84AZ8WURgVDBcn08osvLj6/7OvOgku1jMHdNQvj+zTjsGyKnYAWYa+7ncGsB7cJYJ4=@vger.kernel.org X-Gm-Message-State: AOJu0YyPifg0PMQ2qBNjA1c1fds6EcxenYMm6y+nobQVcAfZvZBmCOOE fFNEX8VQivvGk+d+yAO6c/dRp67piYpAFA8Ey3x9q2Zt6mYaEFkBKvKmERdb6A== X-Gm-Gg: AZuq6aJzRlQixUA78R7WPzzZuj8av7VxFnJ80Igo8DmIrzVo2Ff+/JqCXUt/ZJLmrWr 1kJcnwtzWWITmvGmQSmI2W8JWhsdg2Z8cqmOq8bwiMBs+b/K3NIiHvbtWaC1xl9XUf8s+hCLK8x Q3Ojh7EqPXUi41Z3rDj6Zz/lZD38rrj39EMpWnNFLkUKpBZdEIwA+hpUMy3NVz+qrah7CvLwhIE WK3GOJdO0DSl0AVbtDPu7Szqi3HXI+1g9hLc8KesjMRc8xO319zj7pRLtWyTCAWsVA3cih4m9mi Gz8nSMU1ZPpyiTV7Muzvs1YGgE7GeXO98tB6NdtL6yp85bvrJNAsjmwBAvmiZLba27cGoR3rGQg jr0LfvM+sVVeUs4i8VAHtVE7/XKs1BN+HCwGc1dgqEmd3K+S5HsFrKUacvCDt3AMjkfXjzHxoYZ CX65NMzqbxwzZqCfN12ykyZyesHLcoMbCfqkPk8LX+9fo73B3vXQgs X-Received: by 2002:a05:600c:8489:b0:47d:3ffa:5f03 with SMTP id 5b1f17b1804b1-483202161b8mr88947085e9.21.1770460990170; Sat, 07 Feb 2026 02:43:10 -0800 (PST) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-483203d606fsm42725245e9.2.2026.02.07.02.43.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 07 Feb 2026 02:43:09 -0800 (PST) Date: Sat, 7 Feb 2026 10:43:08 +0000 From: David Laight To: Gui-Dong Han Cc: linux@roeck-us.net, linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org, baijiaju1990@gmail.com, Ben Hutchings , stable@vger.kernel.org Subject: Re: [PATCH] hwmon: (max16065) Use READ/WRITE_ONCE to avoid compiler optimization induced race Message-ID: <20260207104308.1bc31102@pumpkin> In-Reply-To: <20260203121443.5482-1-hanguidong02@gmail.com> References: <20260203121443.5482-1-hanguidong02@gmail.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) 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-Transfer-Encoding: 7bit On Tue, 3 Feb 2026 20:14:43 +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. 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 > 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 > --- > 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;