[PATCH v2] platform/x86: lenovo: wmi-other: Fix uninitialized variable in lwmi_om_hwmon_write()

[PATCH v2] platform/x86: lenovo: wmi-other: Fix uninitialized variable in lwmi_om_hwmon_write()

[Yufei CHENG]

When the flag relax_fan_constraint is set, local variable 'raw'
is never assigned, and lwmi_om_hwmon_write() will pass uninitialized
value to lwmi_om_fan_get_set() resulting in undefined behavior.

This flag allows user to bypass minimum fan RPM divisor rounding,
but assignment to 'raw' only happens in the non-relaxed path.
Fix by defaulting 'raw' to user provided 'val' in the else branch.

Fixes: 51ed34282f63fab5b3996477cc56135eb4de5284
Signed-off-by: Yufei CHENG <cd345al@gmail.com>
---
v2: rewrite the commit title to be more specific

 drivers/platform/x86/lenovo/wmi-other.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index 6040f45aa..6c2febe1a 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -349,6 +349,8 @@ static int lwmi_om_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
 			 */
 			if (!relax_fan_constraint)
 				raw = val / LWMI_FAN_DIV * LWMI_FAN_DIV;
+			else
+				raw = val;
 
 			err = lwmi_om_fan_get_set(priv, channel, &raw, true);
 			if (err)
-- 
2.43.0

Re: [PATCH v2] platform/x86: lenovo: wmi-other: Fix uninitialized variable in lwmi_om_hwmon_write()

[Rong Zhang]

Hi Yufei,

On Sun, 2026-04-26 at 22:59 +0800, Yufei CHENG wrote:
> When the flag relax_fan_constraint is set, local variable 'raw'
> is never assigned, and lwmi_om_hwmon_write() will pass uninitialized
> value to lwmi_om_fan_get_set() resulting in undefined behavior.
> 
> This flag allows user to bypass minimum fan RPM divisor rounding,
> but assignment to 'raw' only happens in the non-relaxed path.
> Fix by defaulting 'raw' to user provided 'val' in the else branch.
> 
> Fixes: 51ed34282f63fab5b3996477cc56135eb4de5284

You didn't run scripts/checkpatch.pl before submitting patches, as I've
reminded in the previous reply to v1.

Had you run it, you would see:

   WARNING: Please use correct Fixes: style 'Fixes: <12+ chars of sha1>
   ("<title line>")' - ie: 'Fixes: 51ed34282f63 ("platform/x86: lenovo-wmi-
   other: Add HWMON for fan reporting/tuning")'

> Signed-off-by: Yufei CHENG <cd345al@gmail.com>

My Reviewed-by was dropped. You should've included it in v2.

Please read https://docs.kernel.org/process/submitting-patches.html

Thanks,
Rong

> ---
> v2: rewrite the commit title to be more specific
> 
>  drivers/platform/x86/lenovo/wmi-other.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index 6040f45aa..6c2febe1a 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -349,6 +349,8 @@ static int lwmi_om_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
>  			 */
>  			if (!relax_fan_constraint)
>  				raw = val / LWMI_FAN_DIV * LWMI_FAN_DIV;
> +			else
> +				raw = val;
>  
>  			err = lwmi_om_fan_get_set(priv, channel, &raw, true);
>  			if (err)

Re: [PATCH v2] platform/x86: lenovo: wmi-other: Fix uninitialized

[Yufei CHENG]

Hi Rong,

Thanks for your feedback, I've corrected the format and resend the patch.
However I think I have to point out some other issues in this file.

First, we should not complexify simple things. You use lwmi_om_fan_get_set() to
combine get and set into a single function with a boolean flag to determine the
action.
This makes the control flow harder to follow and audit. Furthermore, this layer
of abstraction is not applied constantly throughout the whole file. In some
places, values are set through lwmi_om_fan_get_set() and in other places,
they're called manually through lwmi_dev_evaluate_int()
This inconsistency makes the abstraction pointless and is the direct cause of a
second uninitialized bug

In line 855, args.arg0 is set to attribute_id, but arg1 is never set. Though
arg1 is omitted in read action, you're still passing stack garbage to the
lwmi_dev_evaluate_int(). And I think it's better to zero it out if not used.

Additionally, I doubt whether bypassing the minimum RPM divisor has any
practical use at all, not to mention it's not guranteed to work and purely
depends on whether EC accepts that sub-division value.

I may do some refactor recently, and I'm happy to hear your suggestions.

Thanks,
Yufei

Re: [PATCH v2] platform/x86: lenovo: wmi-other: Fix uninitialized

[Derek J. Clark]

On April 26, 2026 10:43:32 AM PDT, Yufei CHENG <cd345al@gmail.com> wrote:
>Hi Rong,
>
>Thanks for your feedback, I've corrected the format and resend the patch.
>However I think I have to point out some other issues in this file.
>
>First, we should not complexify simple things. You use lwmi_om_fan_get_set() to
>combine get and set into a single function with a boolean flag to determine the
>action.
>This makes the control flow harder to follow and audit. Furthermore, this layer
>of abstraction is not applied constantly throughout the whole file. In some
>places, values are set through lwmi_om_fan_get_set() and in other places,
>they're called manually through lwmi_dev_evaluate_int()
>This inconsistency makes the abstraction pointless and is the direct cause of a
>second uninitialized bug
>
>In line 855, args.arg0 is set to attribute_id, but arg1 is never set. Though
>arg1 is omitted in read action, you're still passing stack garbage to the
>lwmi_dev_evaluate_int(). And I think it's better to zero it out if not used.

Hi Yufei,

This bug is already addressed in a patch series that has been submitted and is awaiting review from the subsystem maintainers. 

>Additionally, I doubt whether bypassing the minimum RPM divisor has any
>practical use at all, not to mention it's not guranteed to work and purely
>depends on whether EC accepts that sub-division value.
>
>I may do some refactor recently, and I'm happy to hear your suggestions.

Rong has a large refactor series queued up to simplify much of the driver already, pending the acceptance of the previously mentioned series. It would be best to avoid doing any additional large refactoring until that series has landed.

Thanks,
Derek

>Thanks,
>Yufei
>

Re: [PATCH v2] platform/x86: lenovo: wmi-other: Fix uninitialized

[no name]

Hi, Derek,
Thanks for your reply!

On Mon, Apr 27, 2026 at 2:22 AM Derek J. Clark
<derekjohn.clark@gmail.com> wrote:
> Hi Yufei,
>
> This bug is already addressed in a patch series that has been submitted and is awaiting review from the subsystem maintainers.
>
Which bug do you mean specifically? The args.arg1 uninitialized bug,
or the original uninitialized 'raw' variable, or the inappropriate
encapsulation?

> >I may do some refactor recently, and I'm happy to hear your suggestions.
>
> Rong has a large refactor series queued up to simplify much of the driver already, pending the acceptance of the previously mentioned series. It would be best to avoid doing any additional large refactoring until that series has landed.
>
OK, that sounds good. I'll wait until the refactor is done. I hope my
suggestions can be adopted or we can discuss further about the
appropriate architecture. I'm happy to hear your suggestions.

Thanks,
Yufei

Re: [PATCH v2] platform/x86: lenovo: wmi-other: Fix uninitialized

[Derek J. Clark]

On April 26, 2026 11:38:35 AM PDT, no name <cd345al@gmail.com> wrote:
>Hi, Derek,
>Thanks for your reply!
>
>On Mon, Apr 27, 2026 at 2:22 AM Derek J. Clark
><derekjohn.clark@gmail.com> wrote:
>> Hi Yufei,
>>
>> This bug is already addressed in a patch series that has been submitted and is awaiting review from the subsystem maintainers.
>>
>Which bug do you mean specifically? The args.arg1 uninitialized bug,
>or the original uninitialized 'raw' variable, or the inappropriate
>encapsulation?

I was replying to that specific part of your email,  regarding the args initializing. 

See <https://lore.kernel.org/platform-driver-x86/20260412211121.2220556-1-derekjohn.clark@gmail.com/T/#mc7f4ab5f74cd2fa21eedcb6d91488b76d3ba3f31>

Thanks,
Derek

>> >I may do some refactor recently, and I'm happy to hear your suggestions.
>>
>> Rong has a large refactor series queued up to simplify much of the driver already, pending the acceptance of the previously mentioned series. It would be best to avoid doing any additional large refactoring until that series has landed.
>>
>OK, that sounds good. I'll wait until the refactor is done. I hope my
>suggestions can be adopted or we can discuss further about the
>appropriate architecture. I'm happy to hear your suggestions.
>
>Thanks,
>Yufei

Re: [PATCH v2] platform/x86: lenovo: wmi-other: Fix uninitialized

[no name]

Thanks for your reply! I appreciate your work on the patch series. If
you or Rong have any suggestions, I'm happy to hear.
Thanks,
Yufei

[PATCH v3] platform/x86: lenovo: wmi-other: Fix uninitialized variable in lwmi_om_hwmon_write()

[Yufei CHENG]

When the flag relax_fan_constraint is set, local variable 'raw'
is never assigned, and lwmi_om_hwmon_write() will pass uninitialized
value to lwmi_om_fan_get_set() resulting in undefined behavior.

This flag allows user to bypass minimum fan RPM divisor rounding,
but assignment to 'raw' only happens in the non-relaxed path.
Fix by defaulting 'raw' to user provided 'val' in the else branch.

Fixes: 51ed34282f63 ("platform/x86: lenovo-wmi-other: Add HWMON for fan reporting/tuning")
Reviewed-by: Rong Zhang <i@rong.moe>
Signed-off-by: Yufei CHENG <cd345al@gmail.com>
---
v3: fix broken format
v2: rewrite the title to be more specific

 drivers/platform/x86/lenovo/wmi-other.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index 6040f45aa2b..6c2febe1a59 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -349,6 +349,8 @@ static int lwmi_om_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
 			 */
 			if (!relax_fan_constraint)
 				raw = val / LWMI_FAN_DIV * LWMI_FAN_DIV;
+			else
+				raw = val;
 
 			err = lwmi_om_fan_get_set(priv, channel, &raw, true);
 			if (err)
-- 
2.43.0