From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f173.google.com (mail-pf1-f173.google.com [209.85.210.173]) (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 1C71B286419 for ; Thu, 30 Apr 2026 23:00:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.173 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777590026; cv=none; b=EjzyAjlYD4FBY4OUBcozkuD630n5Fw0t5XfDv1EiNP0MVHxU4HCYXQxz/zDV0StSt9LEZ9U+U/nyxJLBna/sgtN3TQmyaEWiZRExQP8MuRgyTxAKUjA/gaAtKRn6oeP0L8TC74q8HaB93+8dMVFrEB355hG99iLTaST0fK7zP60= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777590026; c=relaxed/simple; bh=ItXw1X4UVqDE0APuaEw/HlVoV6vthPIaAOO5Vqp9NXM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=oPzTGiaiYHiPeofUfkatLKFqVlBR1BFYIeLf1r7WjfUdErSQ7oDGzm6PXXJpPvRmrV3LbysIEKuIWIlDZRQ+dBNQ78GG/mqqoq9ukjTpc8ljt/fRQ9C61Mlskf3UCOPqrvZsySBprSDP8Y44XLenIiPjh8MGhEQKcRxIbywhIvI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=roeck-us.net; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=WE+C1iNK; arc=none smtp.client-ip=209.85.210.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=roeck-us.net 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="WE+C1iNK" Received: by mail-pf1-f173.google.com with SMTP id d2e1a72fcca58-82f9fdfc965so674720b3a.1 for ; Thu, 30 Apr 2026 16:00:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1777590024; x=1778194824; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:from:to:cc:subject:date:message-id :reply-to; bh=e21aQeKoqL29PG87k6OugvmzaO38ohD/iR9PEx/x6lc=; b=WE+C1iNKOClY1Qzu1ooCTXTevTj561pzEJdBxrGJWNVVb2aFoedGG1p4jw42oYGGjP NeT1uBYcSv+zYgKsoLqOi67Kov1UTWhzIn/jTkieUU000U95MvpMMdgtk5DIeyZGn67j wsfyiXAxhDTZuES2kV0IlJnMnLURDFyX3WTr3hh/5W7GTRbsjd3U0j6ek6fEa5JMDIwA qY+Q8JasM3CT5uL05NWwz3l72SeKKzvSVe4+Mx6aTYoDf1HKKTkOOXLb1IJdO5pnpzQI GHBG/O2QQTU9l28U92fzNxiFWNeZSY6n065pxCPTnhRj3aep5A+zfRLF68kmlDNRIW/Q ASag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777590024; x=1778194824; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=e21aQeKoqL29PG87k6OugvmzaO38ohD/iR9PEx/x6lc=; b=jqSe7CoNNLhbkpcE2MgzoqpiJM3tb7Z6N9ygqgbXrvxjSdtOHrH0/b/kIMmy8Wl/yY V59f+2maZEX4UfhZRLQ4Cd1yZNb4fIo4AOGJFlOrYZrj7jNHrbitFfitldIdOljHdLXV 9eeKs/X1dx0SDCk/DzLZSR3I5KYcJsU1pnS6TjRQKJLOjcqjXhnV1QKPIOCXpL3MW9dR k8xpWINWmpoY7MITnNW3wf0fTWuWWNjcvl6RsCUNs3+4zjYw2MJNr8MVNVPvCUHW9yb6 SzTTGoVXNbSVlitfUA0IrzAIec+k4rDJzuNXyBTlIhNGwvvD4ie2ZvQiMKS/RH0sfcHG +1XQ== X-Forwarded-Encrypted: i=1; AFNElJ/sMIKPuh69eHml2PRz51dQC6ZmqPh6hJe0v5uW/dyiEdq+MXwJGVuQHDybsRZakylKC6GJYq28Tqm1VLg=@vger.kernel.org X-Gm-Message-State: AOJu0Yze2g7KpJv47lln6OmdrlmsQRkGD3csWnmQjOnv0atNacYnBqDQ pmqoRZtArZegnCRKV9AWukCaZSjMU3w+Np1oskofEg/ISTH87uAKMxd/UfTG7A== X-Gm-Gg: AeBDieuS4+ze9WuHLLy+hicjQVtFO7frS7c0njNqYzOExB80Ussgy/pol0pLyNqFbxR JZ6JyYf077NDAz+tpTpBgGSk/ntCnok3nuaFtb2wjVbYpuJOExbKI7NZz6P9zeud3tAs7NyHq8y lTJFQ0cNrI+9ztC8Bxn1lyrXbxhViVhRlS8E1aJ1qIsJ7lo0eUDaEpfwZrcBP6I4patxnRv/d29 4JcPamyqrxVcTpfvhL/s0WkU5PdVefq97NgBLpDuMqOfiTnVd69PsO66zfYDQDDWvqFkOxq4op5 JcN8K+/0SJanB0tPZ4DvNgKp1VwS29vrFs6+/VOa9xa0ORQxKdJ2yGeDorayC7P7Jw75fpYI6Tc jfIhs/U4F6MUtcvkDr/UbdL7Y6nV+JW0iZbuL/RfmgS/xhCTUVnEU3EBwpJhab11N1/waAG2Ygx h+j8ffnR0Yg0LgEZUWPNeLJV5jOq3PaXwJ0KtNvRgC8skxOoQ= X-Received: by 2002:a05:6a00:3691:b0:81f:3afe:281e with SMTP id d2e1a72fcca58-834fe05fbb9mr5547003b3a.3.1777590024291; Thu, 30 Apr 2026 16:00:24 -0700 (PDT) Received: from server.roeck-us.net ([2600:1700:e321:62f0:da43:aeff:fecc:bfd5]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-83515b485eesm683535b3a.48.2026.04.30.16.00.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Apr 2026 16:00:23 -0700 (PDT) Sender: Guenter Roeck Date: Thu, 30 Apr 2026 16:00:22 -0700 From: Guenter Roeck To: Gui-Dong Han Cc: linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org, baijiaju1990@gmail.com Subject: Re: [PATCH v2] hwmon: (lm63) Add locking to avoid TOCTOU Message-ID: References: <20260416135703.53262-1-hanguidong02@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: <20260416135703.53262-1-hanguidong02@gmail.com> On Thu, Apr 16, 2026 at 09:57:03PM +0800, Gui-Dong Han wrote: > The functions show_fan(), show_pwm1(), show_temp11(), > temp2_crit_hyst_show(), and show_lut_temp_hyst() access shared cached > data without holding the update lock. This can cause TOCTOU races if > the cached values change between the checks and the later calculations. > > Those cached values are updated in lm63_update_device(). In the general > case, the affected functions combine multiple cached values without > locking and can therefore observe a mixed old/new snapshot. In > addition, show_fan() reads data->fan[nr] locklessly while > lm63_update_device() updates data->fan[0] in two steps, which can > expose an intermediate torn value and potentially trigger a > divide-by-zero error. This means that converting the macro to a > function is not sufficient to fix show_fan(). > > Hold the update lock across the whole read and calculation sequence so > that the values remain stable. > > Check the other functions in the driver as well. Keep them unchanged > because they either do not access shared cached values multiple times > or already do so under lock. > > Link: https://lore.kernel.org/linux-hwmon/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8B-g@mail.gmail.com/ > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Fixes: e872c91e726e ("hwmon: (lm63) Add support for unsigned upper temperature limits") > Fixes: d216f6809eb6 ("hwmon: (lm63) Expose automatic fan speed control lookup table") > Signed-off-by: Gui-Dong Han Applied. Thanks, Guenter > --- > v2: > - Sashiko-bot pointed out that show_fan(), temp2_crit_hyst_show(), and > show_lut_temp_hyst() also need locking. > > While learning the hwmon driver code, I found a few more potential > TOCTOU problems in drivers still using the older non-_with_info() APIs. > Fix them. > --- > drivers/hwmon/lm63.c | 37 +++++++++++++++++++++++++++++-------- > 1 file changed, 29 insertions(+), 8 deletions(-) > > diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c > index 035176a98ce9..b8b1de5fa90f 100644 > --- a/drivers/hwmon/lm63.c > +++ b/drivers/hwmon/lm63.c > @@ -333,7 +333,13 @@ static ssize_t show_fan(struct device *dev, struct device_attribute *devattr, > { > struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > struct lm63_data *data = lm63_update_device(dev); > - return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[attr->index])); > + int fan; > + > + mutex_lock(&data->update_lock); > + fan = FAN_FROM_REG(data->fan[attr->index]); > + mutex_unlock(&data->update_lock); > + > + return sprintf(buf, "%d\n", fan); > } > > static ssize_t set_fan(struct device *dev, struct device_attribute *dummy, > @@ -366,12 +372,14 @@ static ssize_t show_pwm1(struct device *dev, struct device_attribute *devattr, > int nr = attr->index; > int pwm; > > + mutex_lock(&data->update_lock); > if (data->pwm_highres) > pwm = data->pwm1[nr]; > else > pwm = data->pwm1[nr] >= 2 * data->pwm1_freq ? > 255 : (data->pwm1[nr] * 255 + data->pwm1_freq) / > (2 * data->pwm1_freq); > + mutex_unlock(&data->update_lock); > > return sprintf(buf, "%d\n", pwm); > } > @@ -529,6 +537,7 @@ static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr, > int nr = attr->index; > int temp; > > + mutex_lock(&data->update_lock); > if (!nr) { > /* > * Use unsigned temperature unless its value is zero. > @@ -544,7 +553,10 @@ static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr, > else > temp = TEMP11_FROM_REG(data->temp11[nr]); > } > - return sprintf(buf, "%d\n", temp + data->temp2_offset); > + temp += data->temp2_offset; > + mutex_unlock(&data->update_lock); > + > + return sprintf(buf, "%d\n", temp); > } > > static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, > @@ -592,9 +604,14 @@ static ssize_t temp2_crit_hyst_show(struct device *dev, > struct device_attribute *dummy, char *buf) > { > struct lm63_data *data = lm63_update_device(dev); > - return sprintf(buf, "%d\n", temp8_from_reg(data, 2) > - + data->temp2_offset > - - TEMP8_FROM_REG(data->temp2_crit_hyst)); > + int temp; > + > + mutex_lock(&data->update_lock); > + temp = temp8_from_reg(data, 2) + data->temp2_offset > + - TEMP8_FROM_REG(data->temp2_crit_hyst); > + mutex_unlock(&data->update_lock); > + > + return sprintf(buf, "%d\n", temp); > } > > static ssize_t show_lut_temp_hyst(struct device *dev, > @@ -602,10 +619,14 @@ static ssize_t show_lut_temp_hyst(struct device *dev, > { > struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > struct lm63_data *data = lm63_update_device(dev); > + int temp; > > - return sprintf(buf, "%d\n", lut_temp_from_reg(data, attr->index) > - + data->temp2_offset > - - TEMP8_FROM_REG(data->lut_temp_hyst)); > + mutex_lock(&data->update_lock); > + temp = lut_temp_from_reg(data, attr->index) + data->temp2_offset > + - TEMP8_FROM_REG(data->lut_temp_hyst); > + mutex_unlock(&data->update_lock); > + > + return sprintf(buf, "%d\n", temp); > } > > /*