[v1] platform/x86: dell-wmi-sysman: Fix WMI data block retrieval in sysfs callbacks

[PATCH] platform/x86: dell-wmi-sysman: Fix WMI data block retrieval in sysfs callbacks

Posted by Kurt Borja 3 months, 1 week ago

After retrieving WMI data blocks in sysfs callbacks, check for the
validity of them before dereferencing their content.

Reported-by: Jan Graczyk <jangraczyk@yahoo.ca>
Closes: https://lore.kernel.org/r/CAHk-=wgMiSKXf7SvQrfEnxVtmT=QVQPjJdNjfm3aXS7wc=rzTw@mail.gmail.com/
Fixes: e8a60aa7404b ("platform/x86: Introduce support for Systems Management Driver over WMI for Dell Systems")
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h    | 7 +++++++
 drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c    | 5 +++--
 drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c     | 5 +++--
 drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c | 5 +++--
 drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c  | 5 +++--
 drivers/platform/x86/dell/dell-wmi-sysman/sysman.c             | 8 ++++----
 6 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h b/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
index 3ad33a094588c6a258786a02f952eaa6bf953234..792e7d865bfb1cfc13b59c90ddf7de47feff408f 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
@@ -89,6 +89,13 @@ extern struct wmi_sysman_priv wmi_priv;
 
 enum { ENUM, INT, STR, PO };
 
+enum {
+	ENUM_MIN_ELEMENTS	= 8,
+	INT_MIN_ELEMENTS	= 9,
+	STR_MIN_ELEMENTS	= 8,
+	PO_MIN_ELEMENTS		= 4,
+};
+
 enum {
 	ATTR_NAME,
 	DISPL_NAME_LANG_CODE,
diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
index 8cc212c852668312096f756bc1fb1e3054a1f5c0..fc2f58b4cbc6eff863f2c3293cb4322d28048bb8 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
@@ -23,9 +23,10 @@ static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *a
 	obj = get_wmiobj_pointer(instance_id, DELL_WMI_BIOS_ENUMERATION_ATTRIBUTE_GUID);
 	if (!obj)
 		return -EIO;
-	if (obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_STRING) {
+	if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count < ENUM_MIN_ELEMENTS ||
+	    obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_STRING) {
 		kfree(obj);
-		return -EINVAL;
+		return -EIO;
 	}
 	ret = snprintf(buf, PAGE_SIZE, "%s\n", obj->package.elements[CURRENT_VAL].string.pointer);
 	kfree(obj);
diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
index 951e75b538fad42509614c2ebf2ef77aa05b614f..73524806423914bf210b9b5f78c0b5b4f6a7984c 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
@@ -25,9 +25,10 @@ static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *a
 	obj = get_wmiobj_pointer(instance_id, DELL_WMI_BIOS_INTEGER_ATTRIBUTE_GUID);
 	if (!obj)
 		return -EIO;
-	if (obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_INTEGER) {
+	if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count < INT_MIN_ELEMENTS ||
+	    obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_INTEGER) {
 		kfree(obj);
-		return -EINVAL;
+		return -EIO;
 	}
 	ret = snprintf(buf, PAGE_SIZE, "%lld\n", obj->package.elements[CURRENT_VAL].integer.value);
 	kfree(obj);
diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
index d8f1bf5e58a0f441cfd6c21f299c5426b2e28ce9..3167e06d416ede61cda5ad4c860dcb41b05cd5fa 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
@@ -26,9 +26,10 @@ static ssize_t is_enabled_show(struct kobject *kobj, struct kobj_attribute *attr
 	obj = get_wmiobj_pointer(instance_id, DELL_WMI_BIOS_PASSOBJ_ATTRIBUTE_GUID);
 	if (!obj)
 		return -EIO;
-	if (obj->package.elements[IS_PASS_SET].type != ACPI_TYPE_INTEGER) {
+	if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count < PO_MIN_ELEMENTS ||
+	    obj->package.elements[IS_PASS_SET].type != ACPI_TYPE_INTEGER) {
 		kfree(obj);
-		return -EINVAL;
+		return -EIO;
 	}
 	ret = snprintf(buf, PAGE_SIZE, "%lld\n", obj->package.elements[IS_PASS_SET].integer.value);
 	kfree(obj);
diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
index c392f0ecf8b55ba722246d67ba0073772a4f0094..0d2c74f8d1aad7843effcd7b600dd42e6947dc15 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
@@ -25,9 +25,10 @@ static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *a
 	obj = get_wmiobj_pointer(instance_id, DELL_WMI_BIOS_STRING_ATTRIBUTE_GUID);
 	if (!obj)
 		return -EIO;
-	if (obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_STRING) {
+	if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count < STR_MIN_ELEMENTS ||
+	    obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_STRING) {
 		kfree(obj);
-		return -EINVAL;
+		return -EIO;
 	}
 	ret = snprintf(buf, PAGE_SIZE, "%s\n", obj->package.elements[CURRENT_VAL].string.pointer);
 	kfree(obj);
diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
index d00389b860e4ea0655c740c78bc3751f323b6370..3c74d5e8350a413a55739ca5e9647be30bac50d4 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
@@ -407,10 +407,10 @@ static int init_bios_attributes(int attr_type, const char *guid)
 		return retval;
 
 	switch (attr_type) {
-	case ENUM:	min_elements = 8;	break;
-	case INT:	min_elements = 9;	break;
-	case STR:	min_elements = 8;	break;
-	case PO:	min_elements = 4;	break;
+	case ENUM:	min_elements = ENUM_MIN_ELEMENTS;	break;
+	case INT:	min_elements = INT_MIN_ELEMENTS;	break;
+	case STR:	min_elements = STR_MIN_ELEMENTS;	break;
+	case PO:	min_elements = PO_MIN_ELEMENTS;		break;
 	default:
 		pr_err("Error: Unknown attr_type: %d\n", attr_type);
 		return -EINVAL;

---
base-commit: 173bbec6693f3f3f00dac144f3aa0cd62fb60d33
change-id: 20250629-sysman-fix-9c527a28f1dd
-- 
 ~ Kurt

Re: [PATCH] platform/x86: dell-wmi-sysman: Fix WMI data block retrieval in sysfs callbacks

Posted by Armin Wolf 3 months, 1 week ago

Am 29.06.25 um 20:33 schrieb Kurt Borja:

> After retrieving WMI data blocks in sysfs callbacks, check for the
> validity of them before dereferencing their content.
>
> Reported-by: Jan Graczyk <jangraczyk@yahoo.ca>
> Closes: https://lore.kernel.org/r/CAHk-=wgMiSKXf7SvQrfEnxVtmT=QVQPjJdNjfm3aXS7wc=rzTw@mail.gmail.com/
> Fixes: e8a60aa7404b ("platform/x86: Introduce support for Systems Management Driver over WMI for Dell Systems")
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
>   drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h    | 7 +++++++
>   drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c    | 5 +++--
>   drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c     | 5 +++--
>   drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c | 5 +++--
>   drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c  | 5 +++--
>   drivers/platform/x86/dell/dell-wmi-sysman/sysman.c             | 8 ++++----
>   6 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h b/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
> index 3ad33a094588c6a258786a02f952eaa6bf953234..792e7d865bfb1cfc13b59c90ddf7de47feff408f 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
> @@ -89,6 +89,13 @@ extern struct wmi_sysman_priv wmi_priv;
>   
>   enum { ENUM, INT, STR, PO };
>   
> +enum {
> +	ENUM_MIN_ELEMENTS	= 8,
> +	INT_MIN_ELEMENTS	= 9,
> +	STR_MIN_ELEMENTS	= 8,
> +	PO_MIN_ELEMENTS		= 4,
> +};

Hi,

are you sure that this works? I suggest we use defines instead as ENUM_MIN_ELEMENTS has the same value as STR_MIN_ELEMENTS.

For the rest:

Reviewed-by: Armin Wolf <W_Armin@gmx.de>

> +
>   enum {
>   	ATTR_NAME,
>   	DISPL_NAME_LANG_CODE,
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
> index 8cc212c852668312096f756bc1fb1e3054a1f5c0..fc2f58b4cbc6eff863f2c3293cb4322d28048bb8 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
> @@ -23,9 +23,10 @@ static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *a
>   	obj = get_wmiobj_pointer(instance_id, DELL_WMI_BIOS_ENUMERATION_ATTRIBUTE_GUID);
>   	if (!obj)
>   		return -EIO;
> -	if (obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_STRING) {
> +	if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count < ENUM_MIN_ELEMENTS ||
> +	    obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_STRING) {
>   		kfree(obj);
> -		return -EINVAL;
> +		return -EIO;
>   	}
>   	ret = snprintf(buf, PAGE_SIZE, "%s\n", obj->package.elements[CURRENT_VAL].string.pointer);
>   	kfree(obj);
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
> index 951e75b538fad42509614c2ebf2ef77aa05b614f..73524806423914bf210b9b5f78c0b5b4f6a7984c 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
> @@ -25,9 +25,10 @@ static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *a
>   	obj = get_wmiobj_pointer(instance_id, DELL_WMI_BIOS_INTEGER_ATTRIBUTE_GUID);
>   	if (!obj)
>   		return -EIO;
> -	if (obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_INTEGER) {
> +	if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count < INT_MIN_ELEMENTS ||
> +	    obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_INTEGER) {
>   		kfree(obj);
> -		return -EINVAL;
> +		return -EIO;
>   	}
>   	ret = snprintf(buf, PAGE_SIZE, "%lld\n", obj->package.elements[CURRENT_VAL].integer.value);
>   	kfree(obj);
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
> index d8f1bf5e58a0f441cfd6c21f299c5426b2e28ce9..3167e06d416ede61cda5ad4c860dcb41b05cd5fa 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
> @@ -26,9 +26,10 @@ static ssize_t is_enabled_show(struct kobject *kobj, struct kobj_attribute *attr
>   	obj = get_wmiobj_pointer(instance_id, DELL_WMI_BIOS_PASSOBJ_ATTRIBUTE_GUID);
>   	if (!obj)
>   		return -EIO;
> -	if (obj->package.elements[IS_PASS_SET].type != ACPI_TYPE_INTEGER) {
> +	if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count < PO_MIN_ELEMENTS ||
> +	    obj->package.elements[IS_PASS_SET].type != ACPI_TYPE_INTEGER) {
>   		kfree(obj);
> -		return -EINVAL;
> +		return -EIO;
>   	}
>   	ret = snprintf(buf, PAGE_SIZE, "%lld\n", obj->package.elements[IS_PASS_SET].integer.value);
>   	kfree(obj);
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c b/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
> index c392f0ecf8b55ba722246d67ba0073772a4f0094..0d2c74f8d1aad7843effcd7b600dd42e6947dc15 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
> @@ -25,9 +25,10 @@ static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *a
>   	obj = get_wmiobj_pointer(instance_id, DELL_WMI_BIOS_STRING_ATTRIBUTE_GUID);
>   	if (!obj)
>   		return -EIO;
> -	if (obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_STRING) {
> +	if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count < STR_MIN_ELEMENTS ||
> +	    obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_STRING) {
>   		kfree(obj);
> -		return -EINVAL;
> +		return -EIO;
>   	}
>   	ret = snprintf(buf, PAGE_SIZE, "%s\n", obj->package.elements[CURRENT_VAL].string.pointer);
>   	kfree(obj);
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> index d00389b860e4ea0655c740c78bc3751f323b6370..3c74d5e8350a413a55739ca5e9647be30bac50d4 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> @@ -407,10 +407,10 @@ static int init_bios_attributes(int attr_type, const char *guid)
>   		return retval;
>   
>   	switch (attr_type) {
> -	case ENUM:	min_elements = 8;	break;
> -	case INT:	min_elements = 9;	break;
> -	case STR:	min_elements = 8;	break;
> -	case PO:	min_elements = 4;	break;
> +	case ENUM:	min_elements = ENUM_MIN_ELEMENTS;	break;
> +	case INT:	min_elements = INT_MIN_ELEMENTS;	break;
> +	case STR:	min_elements = STR_MIN_ELEMENTS;	break;
> +	case PO:	min_elements = PO_MIN_ELEMENTS;		break;
>   	default:
>   		pr_err("Error: Unknown attr_type: %d\n", attr_type);
>   		return -EINVAL;
>
> ---
> base-commit: 173bbec6693f3f3f00dac144f3aa0cd62fb60d33
> change-id: 20250629-sysman-fix-9c527a28f1dd

Re: [PATCH] platform/x86: dell-wmi-sysman: Fix WMI data block retrieval in sysfs callbacks

Posted by Kurt Borja 3 months, 1 week ago

On Sun Jun 29, 2025 at 8:37 PM -03, Armin Wolf wrote:
> Am 29.06.25 um 20:33 schrieb Kurt Borja:
>
>> After retrieving WMI data blocks in sysfs callbacks, check for the
>> validity of them before dereferencing their content.
>>
>> Reported-by: Jan Graczyk <jangraczyk@yahoo.ca>
>> Closes: https://lore.kernel.org/r/CAHk-=wgMiSKXf7SvQrfEnxVtmT=QVQPjJdNjfm3aXS7wc=rzTw@mail.gmail.com/
>> Fixes: e8a60aa7404b ("platform/x86: Introduce support for Systems Management Driver over WMI for Dell Systems")
>> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>>   drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h    | 7 +++++++
>>   drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c    | 5 +++--
>>   drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c     | 5 +++--
>>   drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c | 5 +++--
>>   drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c  | 5 +++--
>>   drivers/platform/x86/dell/dell-wmi-sysman/sysman.c             | 8 ++++----
>>   6 files changed, 23 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h b/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
>> index 3ad33a094588c6a258786a02f952eaa6bf953234..792e7d865bfb1cfc13b59c90ddf7de47feff408f 100644
>> --- a/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
>> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
>> @@ -89,6 +89,13 @@ extern struct wmi_sysman_priv wmi_priv;
>>   
>>   enum { ENUM, INT, STR, PO };
>>   
>> +enum {
>> +	ENUM_MIN_ELEMENTS	= 8,
>> +	INT_MIN_ELEMENTS	= 9,
>> +	STR_MIN_ELEMENTS	= 8,
>> +	PO_MIN_ELEMENTS		= 4,
>> +};
>
> Hi,
>
> are you sure that this works? I suggest we use defines instead as ENUM_MIN_ELEMENTS has the same value as STR_MIN_ELEMENTS.

Oh - I wrote that without thinking...

I'm pretty sure it works, but I'll go with defines anyway.

>
> For the rest:
>
> Reviewed-by: Armin Wolf <W_Armin@gmx.de>

Thanks!


-- 
 ~ Kurt