[PATCH 0/2] hwmon: (dell-smm) Validate pwm and fan_max values

[PATCH 0/2] hwmon: (dell-smm) Validate pwm and fan_max values

[Rosen Penev]

Two input validation fixes for dell-smm-hwmon: a huge value written to
pwmN overflowed the scaling and turned the fan off, and an out-of-range
fan_max module parameter could wrap an allocation size to zero.

These patches were developed with the help of an AI coding assistant
(Claude Opus 5.5): it reviewed the drivers, found the issues and drafted
the fixes and changelogs. I reviewed every change and I am responsible
for them.

All patches build cleanly with W=1.

Tested on a Dell Precision M4800 running v7.2.15, using the equivalent
backports built as out-of-tree modules:
fan_max=256 and fan_max=4294967295 are rejected with the new message,
fan_max=3 and the default still load. The pwm clamp could not be
observed: this machine's BIOS ignores manual pwm writes.


Rosen Penev (2):
  hwmon: (dell-smm) Clamp the pwm value before scaling it
  hwmon: (dell-smm) Reject a fan_max that the BIOS cannot take

 drivers/hwmon/dell-smm-hwmon.c | 7 +++++++
 1 file changed, 7 insertions(+)


base-commit: 2800868f63227c8237d70c41e70d441c919620d6
-- 
2.56.0

[PATCH 1/2] hwmon: (dell-smm) Clamp the pwm value before scaling it

[Rosen Penev]

dell_smm_write() scales the value written to pwmN with

	DIV_ROUND_CLOSEST(val, data->i8k_pwm_mult)

before clamping the result to the valid fan states. The rounding adds
i8k_pwm_mult / 2 to val first, which overflows for values near
LONG_MAX and wraps to a large negative number. That is then clamped to
0, so writing a huge value turns the fan off instead of running it at
the highest speed. With the default fan_max of 2, writing any value
from LONG_MAX - 63 up to LONG_MAX selects fan state 0.

Clamp val to the pwm range of 0 to 255 first.

Fixes: e7f5f275c62a ("i8k: Implement hwmon based fan speed control")
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/hwmon/dell-smm-hwmon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 5ec487843faa..e8b4e8a29e8b 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -1030,6 +1030,7 @@ static int dell_smm_write(struct device *dev, enum hwmon_sensor_types type, u32
 	case hwmon_pwm:
 		switch (attr) {
 		case hwmon_pwm_input:
+			val = clamp_val(val, 0, 255);
 			pwm = clamp_val(DIV_ROUND_CLOSEST(val, data->i8k_pwm_mult), 0,
 					data->i8k_fan_max);
 
-- 
2.56.0

[PATCH 2/2] hwmon: (dell-smm) Reject a fan_max that the BIOS cannot take

[Rosen Penev]

The fan_max module parameter is used without any range check, unlike
fan_mult. dell_smm_init_hwmon() allocates the nominal speed table with

	devm_kmalloc_array(dev, data->i8k_fan_max + 1, ...)

and fan_max=4294967295 wraps the count to 0. devm_kmalloc() then
returns ZERO_SIZE_PTR, which the first successful nominal speed call
writes through.

Values above 255 are also meaningless: the fan state is passed to the
BIOS as fan | (speed << 8), so the higher bits end up in other parts of
the request. Large values that do not wrap still cost one SMM call per
state and fan while probing.

Fail probing if the maximum fan state does not fit in a byte, like the
existing check for fan_mult.

Fixes: 81474fc2fae9 ("i8k: Add support for configurable maximum fan speed value")
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/hwmon/dell-smm-hwmon.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index e8b4e8a29e8b..24a9172b7fed 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -1249,6 +1249,12 @@ static int dell_smm_init_data(struct device *dev, const struct dell_smm_ops *ops
 		return -EINVAL;
 	}
 	data->i8k_fan_max = fan_max ? : I8K_FAN_HIGH;
+	/* The BIOS takes the fan state in a single byte */
+	if (data->i8k_fan_max > U8_MAX) {
+		dev_err(dev, "maximum fan state %u is too large (max %u)\n",
+			data->i8k_fan_max, U8_MAX);
+		return -EINVAL;
+	}
 	data->i8k_pwm_mult = DIV_ROUND_UP(255, data->i8k_fan_max);
 
 	return 0;
-- 
2.56.0

Re: [PATCH 1/2] hwmon: (dell-smm) Clamp the pwm value before scaling it

[sashiko-bot]

> dell_smm_write() scales the value written to pwmN with
> 
> 	DIV_ROUND_CLOSEST(val, data->i8k_pwm_mult)
> 
> before clamping the result to the valid fan states. The rounding adds
> i8k_pwm_mult / 2 to val first, which overflows for values near
> LONG_MAX and wraps to a large negative number. That is then clamped to
> 0, so writing a huge value turns the fan off instead of running it at
> the highest speed. With the default fan_max of 2, writing any value
> from LONG_MAX - 63 up to LONG_MAX selects fan state 0.
> 
> Clamp val to the pwm range of 0 to 255 first.
> 
> Fixes: e7f5f275c62a ("i8k: Implement hwmon based fan speed control")
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794633179.git.rosenp@gmail.com?part=1

Re: [PATCH 2/2] hwmon: (dell-smm) Reject a fan_max that the BIOS cannot take

[sashiko-bot]

> The fan_max module parameter is used without any range check, unlike
> fan_mult. dell_smm_init_hwmon() allocates the nominal speed table with
> 
> 	devm_kmalloc_array(dev, data->i8k_fan_max + 1, ...)
> 
> and fan_max=4294967295 wraps the count to 0. devm_kmalloc() then
> returns ZERO_SIZE_PTR, which the first successful nominal speed call
> writes through.
> 
> Values above 255 are also meaningless: the fan state is passed to the
> BIOS as fan | (speed << 8), so the higher bits end up in other parts of
> the request. Large values that do not wrap still cost one SMM call per
> state and fan while probing.
> 
> Fail probing if the maximum fan state does not fit in a byte, like the
> existing check for fan_mult.
> 
> Fixes: 81474fc2fae9 ("i8k: Add support for configurable maximum fan speed value")
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794633179.git.rosenp@gmail.com?part=2