linux-kernel - Re: [PATCH] platform/x86: alienware-wmi-wmax: Fix uninitialized variable due to bad error handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <14424f02-c800-a482-4d23-fd05f61cec82@linux.intel.com>
Date: Thu, 17 Apr 2025 13:57:19 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Kurt Borja <kuurtb@...il.com>
cc: Hans de Goede <hdegoede@...hat.com>, Armin Wolf <W_Armin@....de>, 
    Mario Limonciello <mario.limonciello@....com>, 
    platform-driver-x86@...r.kernel.org, Dell.Client.Kernel@...l.com, 
    LKML <linux-kernel@...r.kernel.org>, 
    Dan Carpenter <dan.carpenter@...aro.org>
Subject: Re: [PATCH] platform/x86: alienware-wmi-wmax: Fix uninitialized
 variable due to bad error handling

On Wed, 16 Apr 2025, Kurt Borja wrote:

> wmax_thermal_information() may also return -ENOMSG, which would leave
> `id` uninitialized in thermal_profile_probe.
> 
> Reorder and modify logic to catch all errors.
> 
> Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> Closes: https://lore.kernel.org/r/Z_-KVqNbD9ygvE2X@stanley.mountain
> Fixes: 27e9e6339896 ("platform/x86: alienware-wmi: Refactor thermal control methods")
> Signed-off-by: Kurt Borja <kuurtb@...il.com>
> ---
> Hi all,
> 
> @Ilpo: This will definitely conflict with the for-next branch when
> merging.

Okay, thanks for the heads up (I'll eventually merge fixes into for-next
once I merge this fix).

> Also, the fixes tag references a commit from before the split (same
> series though), but ofc this fix is meant to be applied on top of it
> (fixes branch). Is this ok or would it be better to change the fixes
> tag to the "split" commit?

Pointing to the correct commit is preferred.

It doesn't look very likely that the series would be "split" such that 
only a part of it appears in a specific stable kernel so the difference 
shouldn't matter anyway.

In general, stable people would just send you a notification if something 
cannot be backported to some stable version due to a conflict, and they'd 
depend on you to submit the amended backport anyway so it's not much extra 
"work" for them if something ends up conflicting. (And I don't think your 
inbox would be exactly filling from stable notifications unlike mine --- 
one of those joys of being a subsystem maintainer).

-- 
 i.

> ---
>  drivers/platform/x86/dell/alienware-wmi-wmax.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> index 3d3014b5adf046c94c1ebf39a0e28a92622b40d6..b8e71f06fdde347573bff5c27a9ba53a0efcacae 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> @@ -607,12 +607,10 @@ static int thermal_profile_probe(void *drvdata, unsigned long *choices)
>  	for (u32 i = 0; i < sys_desc[3]; i++) {
>  		ret = wmax_thermal_information(priv->wdev, WMAX_OPERATION_LIST_IDS,
>  					       i + first_mode, &out_data);
> -
> -		if (ret == -EIO)
> -			return ret;
> -
>  		if (ret == -EBADRQC)
>  			break;
> +		if (ret)
> +			return ret;
>  
>  		if (!is_wmax_thermal_code(out_data))
>  			continue;
> 
> ---
> base-commit: fcf27a6a926fd9eeba39e9c3fde43c9298fe284e
> change-id: 20250416-smatch-fix-d1191e2514f5
> 
> Best regards,
>