mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] hwmon: Fix sensor_device_attribute{,_2} type confusion
@ 2026-11-15  8:02 Ziheng Wei
  2026-11-15  8:02 ` [PATCH 1/3] hwmon: (nct6775) Fix incorrect struct cast in beep callbacks Ziheng Wei
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ziheng Wei @ 2026-11-15  8:02 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Marc Hulsman, linux-hwmon, linux-kernel

These are the same kind of bug as the one fixed for nct6683 in commit
c4a01c2a80b2 ("hwmon: (nct6683) Fix incorrect struct cast in show_pwm()
and store_pwm()"): a sysfs callback downcasts its attribute to the
other variant of struct sensor_device_attribute{,_2}.

Both structs start with struct device_attribute, so container_of()
gives the right base address, but the index field is an int in one and
a u8 in the other. On little-endian machines the low byte happens to
be the same, which is why this has gone unnoticed. On big-endian
machines:

  - nct6775 (_1 attribute read as _2): every *_beep write lands on
    channel 0.
  - w83791d, w83792d (_2 attribute read as _1): temp1_max and
    temp1_max_hyst index data->temp1[] with index << 24, i.e. 16 MiB
    and 32 MiB out of bounds.

The w83791d and w83792d effect was observed on an s390x QEMU guest
(v7.3.1) with each driver probed against an emulated I2C chip:
temp1_max and temp1_max_hyst read 0 instead of the values in the chip
registers. The nct6775 platform driver cannot be built on s390x (no
HAS_IOPORT), so that one is based on code inspection only.

The three patches are independent of each other.

All three were found by LLM-assisted static analysis. The changelogs
and this cover letter were drafted with an AI assistant from my
analysis; I have reviewed the code, the fixes and the text.

Ziheng Wei (3):
  hwmon: (nct6775) Fix incorrect struct cast in beep callbacks
  hwmon: (w83791d) Fix incorrect struct cast in show_temp1() and
    store_temp1()
  hwmon: (w83792d) Fix incorrect struct cast in show_temp1() and
    store_temp1()

 drivers/hwmon/nct6775-core.c | 4 ++--
 drivers/hwmon/w83791d.c      | 4 ++--
 drivers/hwmon/w83792d.c      | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)


base-commit: 97d73c8251226cf27cc6656b4c542869bb8c69fa
-- 
2.43.0


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

* [PATCH 1/3] hwmon: (nct6775) Fix incorrect struct cast in beep callbacks
  2026-11-15  8:02 [PATCH 0/3] hwmon: Fix sensor_device_attribute{,_2} type confusion Ziheng Wei
@ 2026-11-15  8:02 ` Ziheng Wei
  2026-11-15 13:35   ` Guenter Roeck
  2026-11-15  8:02 ` [PATCH 2/3] hwmon: (w83791d) Fix incorrect struct cast in show_temp1() and store_temp1() Ziheng Wei
  2026-11-15  8:02 ` [PATCH 3/3] hwmon: (w83792d) " Ziheng Wei
  2 siblings, 1 reply; 7+ messages in thread
From: Ziheng Wei @ 2026-11-15  8:02 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Marc Hulsman, linux-hwmon, linux-kernel, stable

On big-endian architectures, writing the in[1-*]_beep, fan[1-*]_beep
and temp[1-*]_beep sysfs attributes in the nct6775 hardware monitoring
driver always changes channel 0 due to an incorrect struct downcast in
the beep store callbacks.

The driver defines these sysfs attributes using SENSOR_TEMPLATE(),
which initializes them as struct sensor_device_attribute (where the
index is a 32-bit int). However, the sysfs store callbacks
nct6775_store_beep() and store_temp_beep() incorrectly downcast the
attribute to a struct sensor_device_attribute_2 (where the index is
an 8-bit u8).

Because both structs start with struct device_attribute,
container_of() resolves to the same base address. On big-endian
architectures, reading the 8-bit index field from the memory layout
of a 32-bit int fetches the most significant byte (which is 0),
rather than the intended channel number.

This type mismatch causes all beep writes to change the beep setting
of channel 0, regardless of which channel the user targeted.

Fix the type casts in the two store callbacks.

Not runtime-tested: the only big-endian machine available to me (s390x
under QEMU) cannot build the platform driver (no HAS_IOPORT).

Fixes: 30846993047b ("hwmon: (nct6775) Add support for beep attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Ziheng Wei <zihengw03@163.com>
---

Notes:
    The changelog was drafted with an AI assistant from my analysis;
    I have reviewed the code, the fix and the text.

 drivers/hwmon/nct6775-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/nct6775-core.c b/drivers/hwmon/nct6775-core.c
index 6e97e4f08..d423f058f 100644
--- a/drivers/hwmon/nct6775-core.c
+++ b/drivers/hwmon/nct6775-core.c
@@ -1859,7 +1859,7 @@ EXPORT_SYMBOL_GPL(nct6775_show_beep);
 ssize_t
 nct6775_store_beep(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
 {
-	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+	struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
 	struct nct6775_data *data = dev_get_drvdata(dev);
 	int nr = data->BEEP_BITS[sattr->index];
 	int regindex = nr >> 3;
@@ -1913,7 +1913,7 @@ static ssize_t
 store_temp_beep(struct device *dev, struct device_attribute *attr,
 		const char *buf, size_t count)
 {
-	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+	struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
 	struct nct6775_data *data = dev_get_drvdata(dev);
 	int nr, bit, regindex;
 	unsigned long val;
-- 
2.43.0


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

* [PATCH 2/3] hwmon: (w83791d) Fix incorrect struct cast in show_temp1() and store_temp1()
  2026-11-15  8:02 [PATCH 0/3] hwmon: Fix sensor_device_attribute{,_2} type confusion Ziheng Wei
  2026-11-15  8:02 ` [PATCH 1/3] hwmon: (nct6775) Fix incorrect struct cast in beep callbacks Ziheng Wei
@ 2026-11-15  8:02 ` Ziheng Wei
  2026-11-15 13:36   ` Guenter Roeck
  2026-11-15  8:02 ` [PATCH 3/3] hwmon: (w83792d) " Ziheng Wei
  2 siblings, 1 reply; 7+ messages in thread
From: Ziheng Wei @ 2026-11-15  8:02 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Marc Hulsman, linux-hwmon, linux-kernel, stable

On big-endian architectures, accessing the temp1_max and temp1_max_hyst
sysfs attributes in the w83791d hardware monitoring driver uses a huge
out-of-bounds index, because show_temp1() and store_temp1() downcast the
attribute to the wrong struct.

The driver defines the temp1_* attributes with SENSOR_ATTR_2(), which
initializes them as struct sensor_device_attribute_2 (where the index is
an 8-bit u8). The temp1_input attribute has index 0, temp1_max has
index 1 and temp1_max_hyst has index 2. However, show_temp1() and
store_temp1() downcast the attribute to struct sensor_device_attribute
(where the index is a 32-bit int) and use it to index data->temp1[].

Because both structs start with struct device_attribute, container_of()
resolves to the same base address, but the field widths differ. On
big-endian architectures, reading the 32-bit int from the memory layout
of the two u8 fields yields index << 24: 0x01000000 for temp1_max and
0x02000000 for temp1_max_hyst, instead of 1 and 2. data->temp1[] has
only 3 elements, so the access runs off the end of the array by up to
32 MiB.

Fix the type casts in show_temp1() and store_temp1().

Confirmed on a big-endian s390x QEMU guest with the driver bound to an
emulated chip: temp1_max and temp1_max_hyst read 0 instead of the
register values. Not tested on real hardware.

Fixes: 9873964d6eb2 ("[PATCH] HWMON: w83791d: New hardware monitoring driver for the Winbond W83791D")
Cc: stable@vger.kernel.org
Signed-off-by: Ziheng Wei <zihengw03@163.com>
---

Notes:
    The changelog was drafted with an AI assistant from my analysis;
    I have reviewed the code, the fix and the text.

 drivers/hwmon/w83791d.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/w83791d.c b/drivers/hwmon/w83791d.c
index 8bb457326..530935a6d 100644
--- a/drivers/hwmon/w83791d.c
+++ b/drivers/hwmon/w83791d.c
@@ -924,7 +924,7 @@ static struct sensor_device_attribute sda_temp_tolerance[] = {
 static ssize_t show_temp1(struct device *dev, struct device_attribute *devattr,
 				char *buf)
 {
-	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
 	struct w83791d_data *data = w83791d_update_device(dev);
 	return sprintf(buf, "%d\n", TEMP1_FROM_REG(data->temp1[attr->index]));
 }
@@ -932,7 +932,7 @@ static ssize_t show_temp1(struct device *dev, struct device_attribute *devattr,
 static ssize_t store_temp1(struct device *dev, struct device_attribute *devattr,
 				const char *buf, size_t count)
 {
-	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
 	struct i2c_client *client = to_i2c_client(dev);
 	struct w83791d_data *data = i2c_get_clientdata(client);
 	int nr = attr->index;
-- 
2.43.0


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

* [PATCH 3/3] hwmon: (w83792d) Fix incorrect struct cast in show_temp1() and store_temp1()
  2026-11-15  8:02 [PATCH 0/3] hwmon: Fix sensor_device_attribute{,_2} type confusion Ziheng Wei
  2026-11-15  8:02 ` [PATCH 1/3] hwmon: (nct6775) Fix incorrect struct cast in beep callbacks Ziheng Wei
  2026-11-15  8:02 ` [PATCH 2/3] hwmon: (w83791d) Fix incorrect struct cast in show_temp1() and store_temp1() Ziheng Wei
@ 2026-11-15  8:02 ` Ziheng Wei
  2026-11-15 13:38   ` Guenter Roeck
  2 siblings, 1 reply; 7+ messages in thread
From: Ziheng Wei @ 2026-11-15  8:02 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Marc Hulsman, linux-hwmon, linux-kernel, stable

On big-endian architectures, accessing the temp1_max and temp1_max_hyst
sysfs attributes in the w83792d hardware monitoring driver uses a huge
out-of-bounds index, because show_temp1() and store_temp1() downcast the
attribute to the wrong struct.

The driver defines the temp1_* attributes with SENSOR_DEVICE_ATTR_2(),
which initializes them as struct sensor_device_attribute_2 (where the
index is an 8-bit u8). The temp1_input attribute has index 0, temp1_max
has index 1 and temp1_max_hyst has index 2. However, show_temp1() and
store_temp1() downcast the attribute to struct sensor_device_attribute
(where the index is a 32-bit int) and use it to index data->temp1[].

Because both structs start with struct device_attribute, container_of()
resolves to the same base address, but the field widths differ. On
big-endian architectures, reading the 32-bit int from the memory layout
of the two u8 fields yields index << 24: 0x01000000 for temp1_max and
0x02000000 for temp1_max_hyst, instead of 1 and 2. data->temp1[] has
only 3 elements, so the access runs off the end of the array by up to
32 MiB.

Fix the type casts in show_temp1() and store_temp1().

Confirmed on a big-endian s390x QEMU guest with the driver bound to an
emulated chip: temp1_max and temp1_max_hyst read 0 instead of the
register values. Not tested on real hardware.

Fixes: 1f8dae31fe8d ("[PATCH] hwmon: w83792d use arrays of attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Ziheng Wei <zihengw03@163.com>
---

Notes:
    The changelog was drafted with an AI assistant from my analysis;
    I have reviewed the code, the fix and the text.

 drivers/hwmon/w83792d.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/w83792d.c b/drivers/hwmon/w83792d.c
index 2de6d7b68..cba907713 100644
--- a/drivers/hwmon/w83792d.c
+++ b/drivers/hwmon/w83792d.c
@@ -487,7 +487,7 @@ store_fan_div(struct device *dev, struct device_attribute *attr,
 static ssize_t show_temp1(struct device *dev, struct device_attribute *attr,
 				char *buf)
 {
-	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+	struct sensor_device_attribute_2 *sensor_attr = to_sensor_dev_attr_2(attr);
 	int nr = sensor_attr->index;
 	struct w83792d_data *data = w83792d_update_device(dev);
 	return sprintf(buf, "%d\n", TEMP1_FROM_REG(data->temp1[nr]));
@@ -496,7 +496,7 @@ static ssize_t show_temp1(struct device *dev, struct device_attribute *attr,
 static ssize_t store_temp1(struct device *dev, struct device_attribute *attr,
 				const char *buf, size_t count)
 {
-	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+	struct sensor_device_attribute_2 *sensor_attr = to_sensor_dev_attr_2(attr);
 	int nr = sensor_attr->index;
 	struct i2c_client *client = to_i2c_client(dev);
 	struct w83792d_data *data = i2c_get_clientdata(client);
-- 
2.43.0


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

* Re: [PATCH 1/3] hwmon: (nct6775) Fix incorrect struct cast in beep callbacks
  2026-11-15  8:02 ` [PATCH 1/3] hwmon: (nct6775) Fix incorrect struct cast in beep callbacks Ziheng Wei
@ 2026-11-15 13:35   ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2026-11-15 13:35 UTC (permalink / raw)
  To: Ziheng Wei; +Cc: Marc Hulsman, linux-hwmon, linux-kernel, stable

On Sun, Nov 15, 2026 at 04:02:01PM +0800, Ziheng Wei wrote:
> On big-endian architectures, writing the in[1-*]_beep, fan[1-*]_beep
> and temp[1-*]_beep sysfs attributes in the nct6775 hardware monitoring
> driver always changes channel 0 due to an incorrect struct downcast in
> the beep store callbacks.
> 
> The driver defines these sysfs attributes using SENSOR_TEMPLATE(),
> which initializes them as struct sensor_device_attribute (where the
> index is a 32-bit int). However, the sysfs store callbacks
> nct6775_store_beep() and store_temp_beep() incorrectly downcast the
> attribute to a struct sensor_device_attribute_2 (where the index is
> an 8-bit u8).
> 
> Because both structs start with struct device_attribute,
> container_of() resolves to the same base address. On big-endian
> architectures, reading the 8-bit index field from the memory layout
> of a 32-bit int fetches the most significant byte (which is 0),
> rather than the intended channel number.
> 
> This type mismatch causes all beep writes to change the beep setting
> of channel 0, regardless of which channel the user targeted.
> 
> Fix the type casts in the two store callbacks.
> 
> Not runtime-tested: the only big-endian machine available to me (s390x
> under QEMU) cannot build the platform driver (no HAS_IOPORT).
> 
> Fixes: 30846993047b ("hwmon: (nct6775) Add support for beep attributes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ziheng Wei <zihengw03@163.com>

Applied.

Thanks,
Guenter

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

* Re: [PATCH 2/3] hwmon: (w83791d) Fix incorrect struct cast in show_temp1() and store_temp1()
  2026-11-15  8:02 ` [PATCH 2/3] hwmon: (w83791d) Fix incorrect struct cast in show_temp1() and store_temp1() Ziheng Wei
@ 2026-11-15 13:36   ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2026-11-15 13:36 UTC (permalink / raw)
  To: Ziheng Wei; +Cc: Marc Hulsman, linux-hwmon, linux-kernel, stable

On Sun, Nov 15, 2026 at 04:02:02PM +0800, Ziheng Wei wrote:
> On big-endian architectures, accessing the temp1_max and temp1_max_hyst
> sysfs attributes in the w83791d hardware monitoring driver uses a huge
> out-of-bounds index, because show_temp1() and store_temp1() downcast the
> attribute to the wrong struct.
> 
> The driver defines the temp1_* attributes with SENSOR_ATTR_2(), which
> initializes them as struct sensor_device_attribute_2 (where the index is
> an 8-bit u8). The temp1_input attribute has index 0, temp1_max has
> index 1 and temp1_max_hyst has index 2. However, show_temp1() and
> store_temp1() downcast the attribute to struct sensor_device_attribute
> (where the index is a 32-bit int) and use it to index data->temp1[].
> 
> Because both structs start with struct device_attribute, container_of()
> resolves to the same base address, but the field widths differ. On
> big-endian architectures, reading the 32-bit int from the memory layout
> of the two u8 fields yields index << 24: 0x01000000 for temp1_max and
> 0x02000000 for temp1_max_hyst, instead of 1 and 2. data->temp1[] has
> only 3 elements, so the access runs off the end of the array by up to
> 32 MiB.
> 
> Fix the type casts in show_temp1() and store_temp1().
> 
> Confirmed on a big-endian s390x QEMU guest with the driver bound to an
> emulated chip: temp1_max and temp1_max_hyst read 0 instead of the
> register values. Not tested on real hardware.
> 
> Fixes: 9873964d6eb2 ("[PATCH] HWMON: w83791d: New hardware monitoring driver for the Winbond W83791D")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ziheng Wei <zihengw03@163.com>

Applied.

Thanks,
Guenter

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

* Re: [PATCH 3/3] hwmon: (w83792d) Fix incorrect struct cast in show_temp1() and store_temp1()
  2026-11-15  8:02 ` [PATCH 3/3] hwmon: (w83792d) " Ziheng Wei
@ 2026-11-15 13:38   ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2026-11-15 13:38 UTC (permalink / raw)
  To: Ziheng Wei; +Cc: Marc Hulsman, linux-hwmon, linux-kernel, stable

On Sun, Nov 15, 2026 at 04:02:03PM +0800, Ziheng Wei wrote:
> On big-endian architectures, accessing the temp1_max and temp1_max_hyst
> sysfs attributes in the w83792d hardware monitoring driver uses a huge
> out-of-bounds index, because show_temp1() and store_temp1() downcast the
> attribute to the wrong struct.
> 
> The driver defines the temp1_* attributes with SENSOR_DEVICE_ATTR_2(),
> which initializes them as struct sensor_device_attribute_2 (where the
> index is an 8-bit u8). The temp1_input attribute has index 0, temp1_max
> has index 1 and temp1_max_hyst has index 2. However, show_temp1() and
> store_temp1() downcast the attribute to struct sensor_device_attribute
> (where the index is a 32-bit int) and use it to index data->temp1[].
> 
> Because both structs start with struct device_attribute, container_of()
> resolves to the same base address, but the field widths differ. On
> big-endian architectures, reading the 32-bit int from the memory layout
> of the two u8 fields yields index << 24: 0x01000000 for temp1_max and
> 0x02000000 for temp1_max_hyst, instead of 1 and 2. data->temp1[] has
> only 3 elements, so the access runs off the end of the array by up to
> 32 MiB.
> 
> Fix the type casts in show_temp1() and store_temp1().
> 
> Confirmed on a big-endian s390x QEMU guest with the driver bound to an
> emulated chip: temp1_max and temp1_max_hyst read 0 instead of the
> register values. Not tested on real hardware.
> 
> Fixes: 1f8dae31fe8d ("[PATCH] hwmon: w83792d use arrays of attributes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ziheng Wei <zihengw03@163.com>

Applied.

Thanks,
Guenter

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

end of thread, other threads:[~2026-11-15 13:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-15  8:02 [PATCH 0/3] hwmon: Fix sensor_device_attribute{,_2} type confusion Ziheng Wei
2026-11-15  8:02 ` [PATCH 1/3] hwmon: (nct6775) Fix incorrect struct cast in beep callbacks Ziheng Wei
2026-11-15 13:35   ` Guenter Roeck
2026-11-15  8:02 ` [PATCH 2/3] hwmon: (w83791d) Fix incorrect struct cast in show_temp1() and store_temp1() Ziheng Wei
2026-11-15 13:36   ` Guenter Roeck
2026-11-15  8:02 ` [PATCH 3/3] hwmon: (w83792d) " Ziheng Wei
2026-11-15 13:38   ` Guenter Roeck

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®