Re: [PATCH] hwmon: (w83627hf) fix out of bounds read of PWM register array

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message inaccurately claims to fix a runtime out-of-bounds array access and hardware register read, but this vulnerability is a static analyzer false positive and did not actually exist at runtime.
--

--- Patch [1]: [PATCH] hwmon: (w83627hf) fix out of bounds read of PWM register array ---
commit 541d179df68e1d4e07bc936b67920fc0623c9326
Author: Mark Sercombe <[email protected]>

hwmon: (w83627hf) fix out of bounds read of PWM register array

This commit replaces a hardcoded loop bound with num_pwms in
w83627hf_update_device() to prevent an out-of-bounds array access reported
by a static analyzer. It also updates num_pwms to correctly account for the
W83627HF having two PWMs.

> w83627hf_update_device() reads PWM registers in a loop bounded by a
> hard coded "i <= 2", i.e. three iterations, for every chip type. For
> the W83627HF, W836X7HF_REG_PWM() indexes regpwm_627hf[], which only has
> two entries. The third iteration therefore reads regpwm_627hf[2], one
> element past the end of the array, and issues a read of a non existent
> PWM register.

[Severity: Medium]
Does the loop actually ever reach the third iteration for W83627HF at runtime?

Looking at the loop body in w83627hf_update_device():

drivers/hwmon/w83627hf.c:w83627hf_update_device() {
...
		for (i = 0; i <= 2; i++) {
			u8 tmp = w83627hf_read_value(data,
				W836X7HF_REG_PWM(data->type, i));
			/* bits 0-3 are reserved  in 627THF */
			if (data->type == w83627thf)
				tmp &= 0xf0;
			data->pwm[i] = tmp;
			if (i == 1 &&
			    (data->type == w83627hf || data->type == w83697hf))
				break;
		}
...
}

It appears there is already a conditional break that stops the loop when i is 1,
meaning the loop never proceeds to a third iteration where i would be 2.

Could the commit message be more precise that this is addressing a static
analyzer false positive rather than an actual runtime bug?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1