Get and store the AWCC system description in alienware_awcc_setup()
instead of awcc_platform_profile_probe() and add a check for integer
overflows to avoid misbehaviors.
While at it, replace set_bit() with it's non-atomic version __set_bit()
because `choices` belong to this thread only.
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
drivers/platform/x86/dell/alienware-wmi-wmax.c | 61 +++++++++++++++++++-------
1 file changed, 46 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index a8a13779793abbcc8d1263474cac6227d524a6f5..0530f25b956f73f47c0354f40dac2910448c894e 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -38,6 +38,9 @@
/* Some IDs have a BIT(8) flag that we ignore */
#define AWCC_RESOURCE_ID_MASK GENMASK(7, 0)
+/* Arbitrary limit based on supported models */
+#define AWCC_MAX_RES_COUNT 16
+
static bool force_platform_profile;
module_param_unsafe(force_platform_profile, bool, 0);
MODULE_PARM_DESC(force_platform_profile, "Forces auto-detecting thermal profiles without checking if WMI thermal backend is available");
@@ -212,6 +215,17 @@ struct wmax_u32_args {
struct awcc_priv {
struct wmi_device *wdev;
+ union {
+ u32 system_description;
+ struct {
+ u8 fan_count;
+ u8 temp_count;
+ u8 unknown_count;
+ u8 profile_count;
+ };
+ u8 res_count[4];
+ };
+
struct device *ppdev;
u8 supported_profiles[PLATFORM_PROFILE_LAST];
};
@@ -624,37 +638,40 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
enum platform_profile_option profile;
struct awcc_priv *priv = drvdata;
enum awcc_thermal_profile mode;
- u8 sys_desc[4];
- u32 first_mode;
+ u8 id, offset = 0;
u32 out_data;
int ret;
- u8 id;
- ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
- 0, (u32 *) &sys_desc);
- if (ret < 0)
- return ret;
-
- first_mode = sys_desc[0] + sys_desc[1];
-
- for (u32 i = 0; i < sys_desc[3]; i++) {
- ret = awcc_op_get_resource_id(priv->wdev, i + first_mode, &out_data);
+ /*
+ * Thermal profile IDs are listed last at offset
+ * fan_count + temp_count + unknown_count
+ */
+ for (unsigned int i = 0; i < ARRAY_SIZE(priv->res_count) - 1; i++)
+ offset += priv->res_count[i];
+ for (unsigned int i = 0; i < priv->profile_count; i++) {
+ ret = awcc_op_get_resource_id(priv->wdev, i + offset, &out_data);
if (ret == -EIO)
return ret;
+ /*
+ * Some devices report an incorrect number of thermal profiles
+ * so the resource ID list may end prematurely
+ */
if (ret == -EBADRQC)
break;
id = FIELD_GET(AWCC_RESOURCE_ID_MASK, out_data);
- if (!is_awcc_thermal_profile_id(id))
+ if (!is_awcc_thermal_profile_id(id)) {
+ dev_dbg(&priv->wdev->dev, "Unmapped thermal profile ID 0x%02x\n", id);
continue;
+ }
mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, id);
profile = awcc_mode_to_platform_profile[mode];
priv->supported_profiles[profile] = id;
- set_bit(profile, choices);
+ __set_bit(profile, choices);
}
if (bitmap_empty(choices, PLATFORM_PROFILE_LAST))
@@ -664,7 +681,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
priv->supported_profiles[PLATFORM_PROFILE_PERFORMANCE] =
AWCC_THERMAL_MODE_GMODE;
- set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
+ __set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
}
return 0;
@@ -695,6 +712,20 @@ static int alienware_awcc_setup(struct wmi_device *wdev)
if (!priv)
return -ENOMEM;
+ ret = awcc_thermal_information(wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
+ 0, &priv->system_description);
+ if (ret < 0)
+ return ret;
+
+ /* Sanity check */
+ for (unsigned int i = 0; i < ARRAY_SIZE(priv->res_count); i++) {
+ if (priv->res_count[i] > AWCC_MAX_RES_COUNT) {
+ dev_err(&wdev->dev, "Malformed system description: 0x%08x\n",
+ priv->system_description);
+ return -ENXIO;
+ }
+ }
+
priv->wdev = wdev;
dev_set_drvdata(&wdev->dev, priv);
--
2.48.1On Thu, 13 Mar 2025, Kurt Borja wrote:
> Get and store the AWCC system description in alienware_awcc_setup()
> instead of awcc_platform_profile_probe() and add a check for integer
> overflows to avoid misbehaviors.
>
> While at it, replace set_bit() with it's non-atomic version __set_bit()
> because `choices` belong to this thread only.
>
> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> drivers/platform/x86/dell/alienware-wmi-wmax.c | 61 +++++++++++++++++++-------
> 1 file changed, 46 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> index a8a13779793abbcc8d1263474cac6227d524a6f5..0530f25b956f73f47c0354f40dac2910448c894e 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> @@ -38,6 +38,9 @@
> /* Some IDs have a BIT(8) flag that we ignore */
> #define AWCC_RESOURCE_ID_MASK GENMASK(7, 0)
>
> +/* Arbitrary limit based on supported models */
> +#define AWCC_MAX_RES_COUNT 16
> +
> static bool force_platform_profile;
> module_param_unsafe(force_platform_profile, bool, 0);
> MODULE_PARM_DESC(force_platform_profile, "Forces auto-detecting thermal profiles without checking if WMI thermal backend is available");
> @@ -212,6 +215,17 @@ struct wmax_u32_args {
>
> struct awcc_priv {
> struct wmi_device *wdev;
> + union {
> + u32 system_description;
> + struct {
> + u8 fan_count;
> + u8 temp_count;
> + u8 unknown_count;
> + u8 profile_count;
> + };
> + u8 res_count[4];
> + };
> +
> struct device *ppdev;
> u8 supported_profiles[PLATFORM_PROFILE_LAST];
> };
> @@ -624,37 +638,40 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
> enum platform_profile_option profile;
> struct awcc_priv *priv = drvdata;
> enum awcc_thermal_profile mode;
> - u8 sys_desc[4];
> - u32 first_mode;
> + u8 id, offset = 0;
> u32 out_data;
> int ret;
> - u8 id;
>
> - ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
> - 0, (u32 *) &sys_desc);
> - if (ret < 0)
> - return ret;
> -
> - first_mode = sys_desc[0] + sys_desc[1];
> -
> - for (u32 i = 0; i < sys_desc[3]; i++) {
> - ret = awcc_op_get_resource_id(priv->wdev, i + first_mode, &out_data);
> + /*
> + * Thermal profile IDs are listed last at offset
> + * fan_count + temp_count + unknown_count
> + */
> + for (unsigned int i = 0; i < ARRAY_SIZE(priv->res_count) - 1; i++)
> + offset += priv->res_count[i];
Doesn't this change go slightly beyond what its changelog describes, could
you please amend the changelog to explain it.
>
> + for (unsigned int i = 0; i < priv->profile_count; i++) {
> + ret = awcc_op_get_resource_id(priv->wdev, i + offset, &out_data);
> if (ret == -EIO)
> return ret;
>
> + /*
> + * Some devices report an incorrect number of thermal profiles
> + * so the resource ID list may end prematurely
> + */
> if (ret == -EBADRQC)
> break;
>
> id = FIELD_GET(AWCC_RESOURCE_ID_MASK, out_data);
> - if (!is_awcc_thermal_profile_id(id))
> + if (!is_awcc_thermal_profile_id(id)) {
> + dev_dbg(&priv->wdev->dev, "Unmapped thermal profile ID 0x%02x\n", id);
> continue;
> + }
>
> mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, id);
> profile = awcc_mode_to_platform_profile[mode];
> priv->supported_profiles[profile] = id;
>
> - set_bit(profile, choices);
> + __set_bit(profile, choices);
> }
>
> if (bitmap_empty(choices, PLATFORM_PROFILE_LAST))
> @@ -664,7 +681,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
> priv->supported_profiles[PLATFORM_PROFILE_PERFORMANCE] =
> AWCC_THERMAL_MODE_GMODE;
>
> - set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
> + __set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
> }
>
> return 0;
> @@ -695,6 +712,20 @@ static int alienware_awcc_setup(struct wmi_device *wdev)
> if (!priv)
> return -ENOMEM;
>
> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
> + 0, &priv->system_description);
> + if (ret < 0)
> + return ret;
> +
> + /* Sanity check */
> + for (unsigned int i = 0; i < ARRAY_SIZE(priv->res_count); i++) {
> + if (priv->res_count[i] > AWCC_MAX_RES_COUNT) {
> + dev_err(&wdev->dev, "Malformed system description: 0x%08x\n",
> + priv->system_description);
> + return -ENXIO;
> + }
> + }
> +
> priv->wdev = wdev;
> dev_set_drvdata(&wdev->dev, priv);
>
>
>
--
i.
On Fri Mar 28, 2025 at 12:03 PM -03, Ilpo Järvinen wrote:
> On Thu, 13 Mar 2025, Kurt Borja wrote:
>
>> Get and store the AWCC system description in alienware_awcc_setup()
>> instead of awcc_platform_profile_probe() and add a check for integer
>> overflows to avoid misbehaviors.
>>
>> While at it, replace set_bit() with it's non-atomic version __set_bit()
>> because `choices` belong to this thread only.
>>
>> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>> drivers/platform/x86/dell/alienware-wmi-wmax.c | 61 +++++++++++++++++++-------
>> 1 file changed, 46 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> index a8a13779793abbcc8d1263474cac6227d524a6f5..0530f25b956f73f47c0354f40dac2910448c894e 100644
>> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> @@ -38,6 +38,9 @@
>> /* Some IDs have a BIT(8) flag that we ignore */
>> #define AWCC_RESOURCE_ID_MASK GENMASK(7, 0)
>>
>> +/* Arbitrary limit based on supported models */
>> +#define AWCC_MAX_RES_COUNT 16
>> +
>> static bool force_platform_profile;
>> module_param_unsafe(force_platform_profile, bool, 0);
>> MODULE_PARM_DESC(force_platform_profile, "Forces auto-detecting thermal profiles without checking if WMI thermal backend is available");
>> @@ -212,6 +215,17 @@ struct wmax_u32_args {
>>
>> struct awcc_priv {
>> struct wmi_device *wdev;
>> + union {
>> + u32 system_description;
>> + struct {
>> + u8 fan_count;
>> + u8 temp_count;
>> + u8 unknown_count;
>> + u8 profile_count;
>> + };
>> + u8 res_count[4];
>> + };
>> +
>> struct device *ppdev;
>> u8 supported_profiles[PLATFORM_PROFILE_LAST];
>> };
>> @@ -624,37 +638,40 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
>> enum platform_profile_option profile;
>> struct awcc_priv *priv = drvdata;
>> enum awcc_thermal_profile mode;
>> - u8 sys_desc[4];
>> - u32 first_mode;
>> + u8 id, offset = 0;
>> u32 out_data;
>> int ret;
>> - u8 id;
>>
>> - ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
>> - 0, (u32 *) &sys_desc);
>> - if (ret < 0)
>> - return ret;
>> -
>> - first_mode = sys_desc[0] + sys_desc[1];
>> -
>> - for (u32 i = 0; i < sys_desc[3]; i++) {
>> - ret = awcc_op_get_resource_id(priv->wdev, i + first_mode, &out_data);
>> + /*
>> + * Thermal profile IDs are listed last at offset
>> + * fan_count + temp_count + unknown_count
>> + */
>> + for (unsigned int i = 0; i < ARRAY_SIZE(priv->res_count) - 1; i++)
>> + offset += priv->res_count[i];
>
> Doesn't this change go slightly beyond what its changelog describes, could
> you please amend the changelog to explain it.
Sure! Only difference would be that now we also sum the unknown_ids.
I'll mention it.
>>
>> + for (unsigned int i = 0; i < priv->profile_count; i++) {
>> + ret = awcc_op_get_resource_id(priv->wdev, i + offset, &out_data);
>> if (ret == -EIO)
>> return ret;
>>
>> + /*
>> + * Some devices report an incorrect number of thermal profiles
>> + * so the resource ID list may end prematurely
>> + */
>> if (ret == -EBADRQC)
>> break;
>>
>> id = FIELD_GET(AWCC_RESOURCE_ID_MASK, out_data);
>> - if (!is_awcc_thermal_profile_id(id))
>> + if (!is_awcc_thermal_profile_id(id)) {
>> + dev_dbg(&priv->wdev->dev, "Unmapped thermal profile ID 0x%02x\n", id);
>> continue;
>> + }
>>
>> mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, id);
>> profile = awcc_mode_to_platform_profile[mode];
>> priv->supported_profiles[profile] = id;
>>
>> - set_bit(profile, choices);
>> + __set_bit(profile, choices);
>> }
>>
>> if (bitmap_empty(choices, PLATFORM_PROFILE_LAST))
>> @@ -664,7 +681,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
>> priv->supported_profiles[PLATFORM_PROFILE_PERFORMANCE] =
>> AWCC_THERMAL_MODE_GMODE;
>>
>> - set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
>> + __set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
>> }
>>
>> return 0;
>> @@ -695,6 +712,20 @@ static int alienware_awcc_setup(struct wmi_device *wdev)
>> if (!priv)
>> return -ENOMEM;
>>
>> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
>> + 0, &priv->system_description);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Sanity check */
>> + for (unsigned int i = 0; i < ARRAY_SIZE(priv->res_count); i++) {
>> + if (priv->res_count[i] > AWCC_MAX_RES_COUNT) {
>> + dev_err(&wdev->dev, "Malformed system description: 0x%08x\n",
>> + priv->system_description);
>> + return -ENXIO;
>> + }
>> + }
>> +
>> priv->wdev = wdev;
>> dev_set_drvdata(&wdev->dev, priv);
>>
>>
>>
© 2016 - 2025 Red Hat, Inc.