From: Nathan Chancellor <nathan@kernel.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Hardware Monitoring <linux-hwmon@vger.kernel.org>,
James Calligeros <jcalligeros99@gmail.com>,
Neal Gompa <neal@gompa.dev>, Janne Grunau <j@jannau.net>
Subject: Re: [PATCH 1/2] hwmon: (macsmc) Fix regressions in Apple Silicon SMC hwmon driver
Date: Tue, 3 Feb 2026 01:06:19 -0700 [thread overview]
Message-ID: <20260203080619.GA1329615@ax162> (raw)
In-Reply-To: <20260129175112.3751907-2-linux@roeck-us.net>
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
>
next prev parent reply other threads:[~2026-02-03 8:06 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-29 17:51 [PATCH 0/2] hwmon: (macsmc) Various fixes hwmon driver Guenter Roeck
2026-01-29 17:51 ` [PATCH 1/2] hwmon: (macsmc) Fix regressions in Apple Silicon SMC " Guenter Roeck
2026-02-03 8:06 ` Nathan Chancellor [this message]
2026-02-12 6:18 ` Guenter Roeck
2026-02-20 10:20 ` James Calligeros
2026-01-29 17:51 ` [PATCH 2/2] hwmon: (macsmc) Fix overflows, underflows, and sign extension Guenter Roeck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260203080619.GA1329615@ax162 \
--to=nathan@kernel.org \
--cc=j@jannau.net \
--cc=jcalligeros99@gmail.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=neal@gompa.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a
Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.