[PATCH 0/3] hwmon: Fix sensor_device_attribute{,_2} type confusion

[PATCH 0/3] hwmon: Fix sensor_device_attribute{,_2} type confusion

[Ziheng Wei]

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

[PATCH 1/3] hwmon: (nct6775) Fix incorrect struct cast in beep callbacks

[Ziheng Wei]

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

Re: [PATCH 1/3] hwmon: (nct6775) Fix incorrect struct cast in beep callbacks

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115080203.910033-1-zihengw03@163.com?part=1

[PATCH 2/3] hwmon: (w83791d) Fix incorrect struct cast in show_temp1() and store_temp1()

[Ziheng Wei]

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

Re: [PATCH 2/3] hwmon: (w83791d) Fix incorrect struct cast in show_temp1() and store_temp1()

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115080203.910033-1-zihengw03@163.com?part=2

[PATCH 3/3] hwmon: (w83792d) Fix incorrect struct cast in show_temp1() and store_temp1()

[Ziheng Wei]

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

Re: [PATCH 3/3] hwmon: (w83792d) Fix incorrect struct cast in show_temp1() and store_temp1()

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115080203.910033-1-zihengw03@163.com?part=3