[PATCH 0/2] hwmon: (macsmc) Various fixes hwmon driver

[PATCH 0/2] hwmon: (macsmc) Various fixes hwmon driver

[Guenter Roeck]

Nathan reported a problem with FIELD_PREP in the macmsc driver.
Repeated analysis using Gemini 3 exposed a number of additional problems.
This patch series fixes all identified problems.

----------------------------------------------------------------
Guenter Roeck (2):
      hwmon: (macsmc) Fix regressions in Apple Silicon SMC hwmon driver
      hwmon: (macsmc) Fix overflows, underflows, and sign extension

 drivers/hwmon/macsmc-hwmon.c | 51 ++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

[PATCH 1/2] hwmon: (macsmc) Fix regressions in Apple Silicon SMC hwmon driver

[Guenter Roeck]

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: 785205fd8139 ("hwmon: Add Apple Silicon SMC hwmon driver")
Reported-by: Nathan 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: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/macsmc-hwmon.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/hwmon/macsmc-hwmon.c b/drivers/hwmon/macsmc-hwmon.c
index 1c0bbec7e8eb..40d25c81b443 100644
--- a/drivers/hwmon/macsmc-hwmon.c
+++ b/drivers/hwmon/macsmc-hwmon.c
@@ -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;
 
-- 
2.45.2

Re: [PATCH 1/2] hwmon: (macsmc) Fix regressions in Apple Silicon SMC hwmon driver

[Nathan Chancellor]

On Thu, Jan 29, 2026 at 09:51:10AM -0800, Guenter Roeck wrote:
> 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: 785205fd8139 ("hwmon: Add Apple Silicon SMC hwmon driver")
> Reported-by: Nathan 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: Guenter Roeck <linux@roeck-us.net>

Thanks, I build tested this with all affected clang versions and saw no
errors. I cannot say if it is correct from a hardware perspective
though.

Tested-by: Nathan Chancellor <nathan@kernel.org> # build only

> ---
>  drivers/hwmon/macsmc-hwmon.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/hwmon/macsmc-hwmon.c b/drivers/hwmon/macsmc-hwmon.c
> index 1c0bbec7e8eb..40d25c81b443 100644
> --- a/drivers/hwmon/macsmc-hwmon.c
> +++ b/drivers/hwmon/macsmc-hwmon.c
> @@ -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;
>  
> -- 
> 2.45.2
> 

Re: [PATCH 1/2] hwmon: (macsmc) Fix regressions in Apple Silicon SMC hwmon driver

[Guenter Roeck]

On 2/3/26 00:06, Nathan Chancellor wrote:
> On Thu, Jan 29, 2026 at 09:51:10AM -0800, Guenter Roeck wrote:
>> 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: 785205fd8139 ("hwmon: Add Apple Silicon SMC hwmon driver")
>> Reported-by: Nathan 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: Guenter Roeck <linux@roeck-us.net>
> 
> Thanks, I build tested this with all affected clang versions and saw no
> errors. I cannot say if it is correct from a hardware perspective
> though.
> 
> Tested-by: Nathan Chancellor <nathan@kernel.org> # build only
> 

I would have liked to get some feedback from the driver author,
but it looks like that won't happen. I will queue up the patches
for v7.0.

Guenter

Re: [PATCH 1/2] hwmon: (macsmc) Fix regressions in Apple Silicon SMC hwmon driver

[James Calligeros]

On Friday, 30 January 2026 3:51:10 am Australian Eastern Standard Time
Guenter Roeck wrote:
> 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: 785205fd8139 ("hwmon: Add Apple Silicon SMC hwmon driver")
> Reported-by: Nathan 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: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/hwmon/macsmc-hwmon.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 

Reviewed-by: James Calligeros <jcalligeros99@gmail.com> for the series.

It would be helpful to Cc the Asahi list as per MAINTAINERS in the
future; any one of us could have reviewed and tested these two patches while
I had been unable to[1].

Regards,
James

[1] https://lore.kernel.org/asahi/CAHgNfTynZHNt3=JY82-WPR1b_1JyoJ=hnazcPwJtSStOZsA1=g@mail.gmail.com/

[PATCH 2/2] hwmon: (macsmc) Fix overflows, underflows, and sign extension

[Guenter Roeck]

The macsmc-hwmon driver experienced several issues related to value
scaling and type conversion:

1. macsmc_hwmon_read_f32_scaled() clipped values to INT_MAX/INT_MIN.
   On 64-bit systems, hwmon supports long values, so clipping to
   32-bit range was premature and caused loss of range for high-power
   sensors. Changed it to use long and clip to LONG_MAX/LONG_MIN.
2. The overflow check in macsmc_hwmon_read_f32_scaled() used 1UL,
   which is 32-bit on some platforms. Switched to 1ULL.
3. macsmc_hwmon_read_key() used a u32 temporary variable for f32
   values. When assigned to a 64-bit long, negative values were
   zero-extended instead of sign-extended, resulting in large
   positive numbers.
4. macsmc_hwmon_read_ioft_scaled() used mult_frac() which could
   overflow during intermediate multiplication. Switched to
   mul_u64_u32_div() to handle the 64-bit multiplication safely.
5. ioft values (unsigned 48.16) could overflow long when scaled
   by 1,000,000. Added explicit clipping to LONG_MAX in the caller.
6. macsmc_hwmon_write_f32() truncated its long argument to int,
   potentially causing issues for large values.

Fix these issues by using appropriate types and helper functions.

Fixes: 785205fd8139 ("hwmon: Add Apple Silicon SMC hwmon driver")
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: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/macsmc-hwmon.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/hwmon/macsmc-hwmon.c b/drivers/hwmon/macsmc-hwmon.c
index 40d25c81b443..1500ec2cc9f8 100644
--- a/drivers/hwmon/macsmc-hwmon.c
+++ b/drivers/hwmon/macsmc-hwmon.c
@@ -22,6 +22,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/hwmon.h>
+#include <linux/math64.h>
 #include <linux/mfd/macsmc.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -130,7 +131,7 @@ static int macsmc_hwmon_read_ioft_scaled(struct apple_smc *smc, smc_key key,
 	if (ret < 0)
 		return ret;
 
-	*p = mult_frac(val, scale, 65536);
+	*p = mul_u64_u32_div(val, scale, 65536);
 
 	return 0;
 }
@@ -140,7 +141,7 @@ static int macsmc_hwmon_read_ioft_scaled(struct apple_smc *smc, smc_key key,
  * them.
  */
 static int macsmc_hwmon_read_f32_scaled(struct apple_smc *smc, smc_key key,
-					int *p, int scale)
+					long *p, int scale)
 {
 	u32 fval;
 	u64 val;
@@ -162,21 +163,21 @@ static int macsmc_hwmon_read_f32_scaled(struct apple_smc *smc, smc_key key,
 		val = 0;
 	else if (exp < 0)
 		val >>= -exp;
-	else if (exp != 0 && (val & ~((1UL << (64 - exp)) - 1))) /* overflow */
+	else if (exp != 0 && (val & ~((1ULL << (64 - exp)) - 1))) /* overflow */
 		val = U64_MAX;
 	else
 		val <<= exp;
 
 	if (fval & FLT_SIGN_MASK) {
-		if (val > (-(s64)INT_MIN))
-			*p = INT_MIN;
+		if (val > (u64)LONG_MAX + 1)
+			*p = LONG_MIN;
 		else
-			*p = -val;
+			*p = -(long)val;
 	} else {
-		if (val > INT_MAX)
-			*p = INT_MAX;
+		if (val > (u64)LONG_MAX)
+			*p = LONG_MAX;
 		else
-			*p = val;
+			*p = (long)val;
 	}
 
 	return 0;
@@ -195,7 +196,7 @@ static int macsmc_hwmon_read_key(struct apple_smc *smc,
 	switch (sensor->info.type_code) {
 	/* 32-bit IEEE 754 float */
 	case __SMC_KEY('f', 'l', 't', ' '): {
-		u32 flt_ = 0;
+		long flt_ = 0;
 
 		ret = macsmc_hwmon_read_f32_scaled(smc, sensor->macsmc_key,
 						   &flt_, scale);
@@ -214,7 +215,10 @@ static int macsmc_hwmon_read_key(struct apple_smc *smc,
 		if (ret)
 			return ret;
 
-		*val = (long)ioft;
+		if (ioft > LONG_MAX)
+			*val = LONG_MAX;
+		else
+			*val = (long)ioft;
 		break;
 	}
 	default:
@@ -224,7 +228,7 @@ static int macsmc_hwmon_read_key(struct apple_smc *smc,
 	return 0;
 }
 
-static int macsmc_hwmon_write_f32(struct apple_smc *smc, smc_key key, int value)
+static int macsmc_hwmon_write_f32(struct apple_smc *smc, smc_key key, long value)
 {
 	u64 val;
 	u32 fval = 0;
-- 
2.45.2