Commit 5dd69b86 authored by Guenter Roeck's avatar Guenter Roeck
Browse files

hwmon: (macsmc) Fix regressions in Apple Silicon SMC hwmon driver 



The recently added macsmc-hwmon driver contained several critical
bugs in its sensor population logic and float conversion routines.

Specifically:
- The voltage sensor population loop used the wrong prefix ("volt-"
  instead of "voltage-") and incorrectly assigned sensors to the
  temperature sensor array (hwmon->temp.sensors) instead of the
  voltage sensor array (hwmon->volt.sensors). This would lead to
  out-of-bounds memory access or data corruption when both temperature
  and voltage sensors were present.
- The float conversion in macsmc_hwmon_write_f32() had flawed exponent
  logic for values >= 2^24 and lacked masking for the mantissa, which
  could lead to incorrect values being written to the SMC.

Fix these issues to ensure correct sensor registration and reliable
manual fan control.

Confirm that the reported overflow in FIELD_PREP is fixed by declaring
macsmc_hwmon_write_f32() as __always_inline for a compile test.

Fixes: 785205fd ("hwmon: Add Apple Silicon SMC hwmon driver")
Reported-by: default avatarNathan Chancellor <nathan@kernel.org>
Closes: https://lore.kernel.org/linux-hwmon/20260119195817.GA1035354@ax162/


Cc: James Calligeros <jcalligeros99@gmail.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Neal Gompa <neal@gompa.dev>
Cc: Janne Grunau <j@jannau.net>
Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
Tested-by: Nathan Chancellor <nathan@kernel.org> # build only
Link: https://lore.kernel.org/r/20260129175112.3751907-2-linux@roeck-us.net


Reviewed-by: default avatarJames Calligeros <jcalligeros99@gmail.com>
Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
parent 6de23f81

drivers/hwmon/macsmc-hwmon.c

+11 −14
Original line number Diff line number Diff line
@@ -228,25 +228,22 @@ static int macsmc_hwmon_write_f32(struct apple_smc *smc, smc_key key, int value) 
{

	u64 val;

	u32 fval = 0;

	int exp = 0, neg;

	int exp, neg;



	neg = value < 0;

	val = abs(value);

	neg = val != value;



	if (val) {

		int msb = __fls(val) - exp;

		exp = __fls(val);



		if (msb > 23) {

			val >>= msb - FLT_MANT_BIAS;

			exp -= msb - FLT_MANT_BIAS;

		} else if (msb < 23) {

			val <<= FLT_MANT_BIAS - msb;

			exp += msb;

		}

		if (exp > 23)

			val >>= exp - 23;

		else

			val <<= 23 - exp;



		fval = FIELD_PREP(FLT_SIGN_MASK, neg) |

		       FIELD_PREP(FLT_EXP_MASK, exp + FLT_EXP_BIAS) |

		       FIELD_PREP(FLT_MANT_MASK, val);

		       FIELD_PREP(FLT_MANT_MASK, val & FLT_MANT_MASK);

	}



	return apple_smc_write_u32(smc, key, fval);
@@ -663,8 +660,8 @@ static int macsmc_hwmon_populate_sensors(struct macsmc_hwmon *hwmon, 
		if (!hwmon->volt.sensors)

			return -ENOMEM;



		for_each_child_of_node_with_prefix(hwmon_node, key_node, "volt-") {

			sensor = &hwmon->temp.sensors[hwmon->temp.count];

		for_each_child_of_node_with_prefix(hwmon_node, key_node, "voltage-") {

			sensor = &hwmon->volt.sensors[hwmon->volt.count];

			if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node, sensor)) {

				sensor->attrs = HWMON_I_INPUT;