diff mbox series

platform/x86: dell-wmi-sysman: Make populate_foo_data functions more robust

Message ID 20210427042233.5495-1-prasanth.ksr@dell.com (mailing list archive)
State Changes Requested, archived
Headers show
Series platform/x86: dell-wmi-sysman: Make populate_foo_data functions more robust | expand

Commit Message

Prasanth KSR April 27, 2021, 4:22 a.m. UTC
1. Check acpi type before assignment of each property value

2. Add boundary check for properties count

Co-developed-by: Divya Bharathi <divya.bharathi@dell.com>
Signed-off-by: Divya Bharathi <divya.bharathi@dell.com>
Signed-off-by: Prasanth KSR <prasanth.ksr@dell.com>
---
 .../dell/dell-wmi-sysman/dell-wmi-sysman.h    |  5 ++-
 .../dell/dell-wmi-sysman/enum-attributes.c    | 36 ++++++++++++++++---
 .../x86/dell/dell-wmi-sysman/int-attributes.c | 16 +++++++++
 .../dell/dell-wmi-sysman/passobj-attributes.c |  6 ++++
 .../dell/dell-wmi-sysman/string-attributes.c  | 16 ++++++++-
 .../x86/dell/dell-wmi-sysman/sysman.c         | 17 ++++++---
 6 files changed, 84 insertions(+), 12 deletions(-)

Comments

Hans de Goede May 10, 2021, 9:36 a.m. UTC | #1
Hi,

On 4/27/21 6:22 AM, Prasanth KSR wrote:
> 1. Check acpi type before assignment of each property value
> 
> 2. Add boundary check for properties count
> 
> Co-developed-by: Divya Bharathi <divya.bharathi@dell.com>
> Signed-off-by: Divya Bharathi <divya.bharathi@dell.com>
> Signed-off-by: Prasanth KSR <prasanth.ksr@dell.com>

Thanks this mostly looks good, 2 small remarks inline below.

Once those are fixed this is ready for merging.

> ---
>  .../dell/dell-wmi-sysman/dell-wmi-sysman.h    |  5 ++-
>  .../dell/dell-wmi-sysman/enum-attributes.c    | 36 ++++++++++++++++---
>  .../x86/dell/dell-wmi-sysman/int-attributes.c | 16 +++++++++
>  .../dell/dell-wmi-sysman/passobj-attributes.c |  6 ++++
>  .../dell/dell-wmi-sysman/string-attributes.c  | 16 ++++++++-
>  .../x86/dell/dell-wmi-sysman/sysman.c         | 17 ++++++---
>  6 files changed, 84 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 b80f2a62ea3f..3ad33a094588 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
> @@ -152,12 +152,15 @@ static ssize_t curr_val##_store(struct kobject *kobj,				\
>  	return ret ? ret : count;						\
>  }
>  
> +#define check_property_type(attr, prop, valuetype)				\
> +	(attr##_obj[prop].type != valuetype)
> +
>  union acpi_object *get_wmiobj_pointer(int instance_id, const char *guid_string);
>  int get_instance_count(const char *guid_string);
>  void strlcpy_attr(char *dest, char *src);
>  
>  int populate_enum_data(union acpi_object *enumeration_obj, int instance_id,
> -			struct kobject *attr_name_kobj);
> +			struct kobject *attr_name_kobj, u32 enum_property_count);
>  int alloc_enum_data(void);
>  void exit_enum_attributes(void);
>  
> 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 091e48c217ed..70f4ace56ec3 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
> @@ -132,39 +132,65 @@ int alloc_enum_data(void)
>   * @enumeration_obj: ACPI object with enumeration data
>   * @instance_id: The instance to enumerate
>   * @attr_name_kobj: The parent kernel object
> + * @enum_property_count: Total properties count under enumeration type
>   */
>  int populate_enum_data(union acpi_object *enumeration_obj, int instance_id,
> -			struct kobject *attr_name_kobj)
> +			struct kobject *attr_name_kobj, u32 enum_property_count)
>  {
>  	int i, next_obj, value_modifier_count, possible_values_count;
>  
>  	wmi_priv.enumeration_data[instance_id].attr_name_kobj = attr_name_kobj;
> +	if (check_property_type(enumeration, ATTR_NAME, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.enumeration_data[instance_id].attribute_name,
>  		enumeration_obj[ATTR_NAME].string.pointer);
> +	if (check_property_type(enumeration, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.enumeration_data[instance_id].display_name_language_code,
>  		enumeration_obj[DISPL_NAME_LANG_CODE].string.pointer);
> +	if (check_property_type(enumeration, DISPLAY_NAME, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.enumeration_data[instance_id].display_name,
>  		enumeration_obj[DISPLAY_NAME].string.pointer);
> +	if (check_property_type(enumeration, DEFAULT_VAL, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.enumeration_data[instance_id].default_value,
>  		enumeration_obj[DEFAULT_VAL].string.pointer);
> +	if (check_property_type(enumeration, MODIFIER, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.enumeration_data[instance_id].dell_modifier,
>  		enumeration_obj[MODIFIER].string.pointer);
>  
>  	next_obj = MODIFIER + 1;
>  
> -	value_modifier_count = (uintptr_t)enumeration_obj[next_obj].string.pointer;
> +	if (next_obj >= enum_property_count)
> +		return -EINVAL;
> +
> +	if (check_property_type(enumeration, next_obj, ACPI_TYPE_INTEGER))
> +		return -EINVAL;
> +	value_modifier_count = (uintptr_t)enumeration_obj[next_obj++].string.pointer;
>  
>  	for (i = 0; i < value_modifier_count; i++) {
> +		if (next_obj >= enum_property_count)
> +			return -EINVAL;
> +		if (check_property_type(enumeration, next_obj, ACPI_TYPE_STRING))
> +			return -EINVAL;
>  		strcat(wmi_priv.enumeration_data[instance_id].dell_value_modifier,
> -			enumeration_obj[++next_obj].string.pointer);
> +			enumeration_obj[next_obj++].string.pointer);
>  		strcat(wmi_priv.enumeration_data[instance_id].dell_value_modifier, ";");
>  	}
>  
> -	possible_values_count = (uintptr_t) enumeration_obj[++next_obj].string.pointer;

You are missing a:

	if (next_obj >= enum_property_count)
		return -EINVAL;

Check here.


> +	if (check_property_type(enumeration, next_obj, ACPI_TYPE_INTEGER))
> +		return -EINVAL;
> +	possible_values_count = (uintptr_t) enumeration_obj[next_obj++].string.pointer;
>  
>  	for (i = 0; i < possible_values_count; i++) {
> +		if (next_obj >= enum_property_count)
> +			return -EINVAL;
> +		if (check_property_type(enumeration, next_obj, ACPI_TYPE_STRING))
> +			return -EINVAL;
>  		strcat(wmi_priv.enumeration_data[instance_id].possible_values,
> -			enumeration_obj[++next_obj].string.pointer);
> +			enumeration_obj[next_obj++].string.pointer);
>  		strcat(wmi_priv.enumeration_data[instance_id].possible_values, ";");
>  	}
>  
> 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 8a49ba6e44f9..951e75b538fa 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
> @@ -141,20 +141,36 @@ int populate_int_data(union acpi_object *integer_obj, int instance_id,
>  			struct kobject *attr_name_kobj)
>  {
>  	wmi_priv.integer_data[instance_id].attr_name_kobj = attr_name_kobj;
> +	if (check_property_type(integer, ATTR_NAME, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.integer_data[instance_id].attribute_name,
>  		integer_obj[ATTR_NAME].string.pointer);
> +	if (check_property_type(integer, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.integer_data[instance_id].display_name_language_code,
>  		integer_obj[DISPL_NAME_LANG_CODE].string.pointer);
> +	if (check_property_type(integer, DISPLAY_NAME, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.integer_data[instance_id].display_name,
>  		integer_obj[DISPLAY_NAME].string.pointer);
> +	if (check_property_type(integer, DEFAULT_VAL, ACPI_TYPE_INTEGER))
> +		return -EINVAL;
>  	wmi_priv.integer_data[instance_id].default_value =
>  		(uintptr_t)integer_obj[DEFAULT_VAL].string.pointer;
> +	if (check_property_type(integer, MODIFIER, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.integer_data[instance_id].dell_modifier,
>  		integer_obj[MODIFIER].string.pointer);
> +	if (check_property_type(integer, MIN_VALUE, ACPI_TYPE_INTEGER))
> +		return -EINVAL;
>  	wmi_priv.integer_data[instance_id].min_value =
>  		(uintptr_t)integer_obj[MIN_VALUE].string.pointer;
> +	if (check_property_type(integer, MAX_VALUE, ACPI_TYPE_INTEGER))
> +		return -EINVAL;
>  	wmi_priv.integer_data[instance_id].max_value =
>  		(uintptr_t)integer_obj[MAX_VALUE].string.pointer;
> +	if (check_property_type(integer, SCALAR_INCR, ACPI_TYPE_INTEGER))
> +		return -EINVAL;
>  	wmi_priv.integer_data[instance_id].scalar_increment =
>  		(uintptr_t)integer_obj[SCALAR_INCR].string.pointer;
>  
> 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 834b3e82ad9f..230e6ee96636 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
> @@ -159,10 +159,16 @@ int alloc_po_data(void)
>  int populate_po_data(union acpi_object *po_obj, int instance_id, struct kobject *attr_name_kobj)
>  {
>  	wmi_priv.po_data[instance_id].attr_name_kobj = attr_name_kobj;
> +	if (check_property_type(po, ATTR_NAME, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.po_data[instance_id].attribute_name,
>  		     po_obj[ATTR_NAME].string.pointer);
> +	if (check_property_type(po, MIN_PASS_LEN, ACPI_TYPE_INTEGER))
> +		return -EINVAL;
>  	wmi_priv.po_data[instance_id].min_password_length =
>  		(uintptr_t)po_obj[MIN_PASS_LEN].string.pointer;
> +	if (check_property_type(po, MAX_PASS_LEN, ACPI_TYPE_INTEGER))
> +		return -EINVAL;
>  	wmi_priv.po_data[instance_id].max_password_length =
>  		(uintptr_t) po_obj[MAX_PASS_LEN].string.pointer;
>  
> 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 552537852459..c392f0ecf8b5 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
> @@ -118,24 +118,38 @@ int alloc_str_data(void)
>  
>  /**
>   * populate_str_data() - Populate all properties of an instance under string attribute
> - * @str_obj: ACPI object with integer data
> + * @str_obj: ACPI object with string data
>   * @instance_id: The instance to enumerate
>   * @attr_name_kobj: The parent kernel object
>   */
>  int populate_str_data(union acpi_object *str_obj, int instance_id, struct kobject *attr_name_kobj)
>  {
>  	wmi_priv.str_data[instance_id].attr_name_kobj = attr_name_kobj;
> +	if (check_property_type(str, ATTR_NAME, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.str_data[instance_id].attribute_name,
>  		     str_obj[ATTR_NAME].string.pointer);
> +	if (check_property_type(str, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.str_data[instance_id].display_name_language_code,
>  		     str_obj[DISPL_NAME_LANG_CODE].string.pointer);
> +	if (check_property_type(str, DISPLAY_NAME, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.str_data[instance_id].display_name,
>  		     str_obj[DISPLAY_NAME].string.pointer);
> +	if (check_property_type(str, DEFAULT_VAL, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.str_data[instance_id].default_value,
>  		     str_obj[DEFAULT_VAL].string.pointer);
> +	if (check_property_type(str, MODIFIER, ACPI_TYPE_STRING))
> +		return -EINVAL;
>  	strlcpy_attr(wmi_priv.str_data[instance_id].dell_modifier,
>  		     str_obj[MODIFIER].string.pointer);
> +	if (check_property_type(str, MIN_LEN, ACPI_TYPE_INTEGER))
> +		return -EINVAL;
>  	wmi_priv.str_data[instance_id].min_length = (uintptr_t)str_obj[MIN_LEN].string.pointer;
> +	if (check_property_type(str, MAX_LEN, ACPI_TYPE_INTEGER))
> +		return -EINVAL;
>  	wmi_priv.str_data[instance_id].max_length = (uintptr_t) str_obj[MAX_LEN].string.pointer;
>  
>  	return sysfs_create_group(attr_name_kobj, &str_attr_group);
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> index c8d276d78e92..5276e8c5aad5 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> @@ -412,10 +412,16 @@ 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:
> +	case STR:
> +		min_elements = 8;
> +		break;
> +	case INT:
> +		min_elements = 9;
> +		break;
> +	case PO:
> +		min_elements = 4;
> +		break;
>  	default:
>  		pr_err("Error: Unknown attr_type: %d\n", attr_type);
>  		return -EINVAL;

Please drop these unrelated changes. If you want to change this please submit a
separate patch for it, but I would prefer to keep this as is.

> @@ -481,7 +487,8 @@ static int init_bios_attributes(int attr_type, const char *guid)
>  		/* enumerate all of this attribute */
>  		switch (attr_type) {
>  		case ENUM:
> -			retval = populate_enum_data(elements, instance_id, attr_name_kobj);
> +			retval = populate_enum_data(elements, instance_id, attr_name_kobj,
> +					obj->package.count);
>  			break;
>  		case INT:
>  			retval = populate_int_data(elements, instance_id, attr_name_kobj);
> 

Regards,

Hans
Ksr, Prasanth May 11, 2021, 4:20 p.m. UTC | #2
Hi,

> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: Monday, May 10, 2021 3:06 PM
> To: Prasanth KSR; dvhart@infradead.org
> Cc: LKML; platform-driver-x86@vger.kernel.org; Ksr, Prasanth; Bharathi,
> Divya
> Subject: Re: [PATCH] platform/x86: dell-wmi-sysman: Make
> populate_foo_data functions more robust
> 
> 
> [EXTERNAL EMAIL]
> 
> Hi,
> 
> On 4/27/21 6:22 AM, Prasanth KSR wrote:
> > 1. Check acpi type before assignment of each property value
> >
> > 2. Add boundary check for properties count
> >
> > Co-developed-by: Divya Bharathi <divya.bharathi@dell.com>
> > Signed-off-by: Divya Bharathi <divya.bharathi@dell.com>
> > Signed-off-by: Prasanth KSR <prasanth.ksr@dell.com>
> 
> Thanks this mostly looks good, 2 small remarks inline below.
> 
> Once those are fixed this is ready for merging.
> 
> > ---
> >  .../dell/dell-wmi-sysman/dell-wmi-sysman.h    |  5 ++-
> >  .../dell/dell-wmi-sysman/enum-attributes.c    | 36 ++++++++++++++++---
> >  .../x86/dell/dell-wmi-sysman/int-attributes.c | 16 +++++++++
> > .../dell/dell-wmi-sysman/passobj-attributes.c |  6 ++++
> > .../dell/dell-wmi-sysman/string-attributes.c  | 16 ++++++++-
> >  .../x86/dell/dell-wmi-sysman/sysman.c         | 17 ++++++---
> >  6 files changed, 84 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 b80f2a62ea3f..3ad33a094588 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
> > @@ -152,12 +152,15 @@ static ssize_t curr_val##_store(struct kobject
> *kobj,				\
> >  	return ret ? ret : count;						\
> >  }
> >
> > +#define check_property_type(attr, prop, valuetype)
> 		\
> > +	(attr##_obj[prop].type != valuetype)
> > +
> >  union acpi_object *get_wmiobj_pointer(int instance_id, const char
> > *guid_string);  int get_instance_count(const char *guid_string);  void
> > strlcpy_attr(char *dest, char *src);
> >
> >  int populate_enum_data(union acpi_object *enumeration_obj, int
> instance_id,
> > -			struct kobject *attr_name_kobj);
> > +			struct kobject *attr_name_kobj, u32
> enum_property_count);
> >  int alloc_enum_data(void);
> >  void exit_enum_attributes(void);
> >
> > 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 091e48c217ed..70f4ace56ec3 100644
> > --- a/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
> > +++ b/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
> > @@ -132,39 +132,65 @@ int alloc_enum_data(void)
> >   * @enumeration_obj: ACPI object with enumeration data
> >   * @instance_id: The instance to enumerate
> >   * @attr_name_kobj: The parent kernel object
> > + * @enum_property_count: Total properties count under enumeration
> > + type
> >   */
> >  int populate_enum_data(union acpi_object *enumeration_obj, int
> instance_id,
> > -			struct kobject *attr_name_kobj)
> > +			struct kobject *attr_name_kobj, u32
> enum_property_count)
> >  {
> >  	int i, next_obj, value_modifier_count, possible_values_count;
> >
> >  	wmi_priv.enumeration_data[instance_id].attr_name_kobj =
> > attr_name_kobj;
> > +	if (check_property_type(enumeration, ATTR_NAME,
> ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >
> 	strlcpy_attr(wmi_priv.enumeration_data[instance_id].attribute_na
> me,
> >  		enumeration_obj[ATTR_NAME].string.pointer);
> > +	if (check_property_type(enumeration, DISPL_NAME_LANG_CODE,
> ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >
> 	strlcpy_attr(wmi_priv.enumeration_data[instance_id].display_name
> _language_code,
> >
> 	enumeration_obj[DISPL_NAME_LANG_CODE].string.pointer);
> > +	if (check_property_type(enumeration, DISPLAY_NAME,
> ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >
> 	strlcpy_attr(wmi_priv.enumeration_data[instance_id].display_name
> ,
> >  		enumeration_obj[DISPLAY_NAME].string.pointer);
> > +	if (check_property_type(enumeration, DEFAULT_VAL,
> ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >
> 	strlcpy_attr(wmi_priv.enumeration_data[instance_id].default_value
> ,
> >  		enumeration_obj[DEFAULT_VAL].string.pointer);
> > +	if (check_property_type(enumeration, MODIFIER,
> ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >  	strlcpy_attr(wmi_priv.enumeration_data[instance_id].dell_modifier,
> >  		enumeration_obj[MODIFIER].string.pointer);
> >
> >  	next_obj = MODIFIER + 1;
> >
> > -	value_modifier_count =
> (uintptr_t)enumeration_obj[next_obj].string.pointer;
> > +	if (next_obj >= enum_property_count)
> > +		return -EINVAL;
> > +
> > +	if (check_property_type(enumeration, next_obj,
> ACPI_TYPE_INTEGER))
> > +		return -EINVAL;
> > +	value_modifier_count =
> > +(uintptr_t)enumeration_obj[next_obj++].string.pointer;
> >
> >  	for (i = 0; i < value_modifier_count; i++) {
> > +		if (next_obj >= enum_property_count)
> > +			return -EINVAL;
> > +		if (check_property_type(enumeration, next_obj,
> ACPI_TYPE_STRING))
> > +			return -EINVAL;
> >
> 	strcat(wmi_priv.enumeration_data[instance_id].dell_value_modifier
> ,
> > -			enumeration_obj[++next_obj].string.pointer);
> > +			enumeration_obj[next_obj++].string.pointer);
> >
> 	strcat(wmi_priv.enumeration_data[instance_id].dell_value_modifier
> , ";");
> >  	}
> >
> > -	possible_values_count = (uintptr_t)
> enumeration_obj[++next_obj].string.pointer;
> 
> You are missing a:
> 
> 	if (next_obj >= enum_property_count)
> 		return -EINVAL;
> 
> Check here.

Ack will address this.

> 
> 
> > +	if (check_property_type(enumeration, next_obj,
> ACPI_TYPE_INTEGER))
> > +		return -EINVAL;
> > +	possible_values_count = (uintptr_t)
> > +enumeration_obj[next_obj++].string.pointer;
> >
> >  	for (i = 0; i < possible_values_count; i++) {
> > +		if (next_obj >= enum_property_count)
> > +			return -EINVAL;
> > +		if (check_property_type(enumeration, next_obj,
> ACPI_TYPE_STRING))
> > +			return -EINVAL;
> >
> 	strcat(wmi_priv.enumeration_data[instance_id].possible_values,
> > -			enumeration_obj[++next_obj].string.pointer);
> > +			enumeration_obj[next_obj++].string.pointer);
> >
> 	strcat(wmi_priv.enumeration_data[instance_id].possible_values,
> ";");
> >  	}
> >
> > 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 8a49ba6e44f9..951e75b538fa 100644
> > --- a/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
> > +++ b/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
> > @@ -141,20 +141,36 @@ int populate_int_data(union acpi_object
> *integer_obj, int instance_id,
> >  			struct kobject *attr_name_kobj)
> >  {
> >  	wmi_priv.integer_data[instance_id].attr_name_kobj =
> attr_name_kobj;
> > +	if (check_property_type(integer, ATTR_NAME, ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >  	strlcpy_attr(wmi_priv.integer_data[instance_id].attribute_name,
> >  		integer_obj[ATTR_NAME].string.pointer);
> > +	if (check_property_type(integer, DISPL_NAME_LANG_CODE,
> ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >
> 	strlcpy_attr(wmi_priv.integer_data[instance_id].display_name_lang
> uage_code,
> >  		integer_obj[DISPL_NAME_LANG_CODE].string.pointer);
> > +	if (check_property_type(integer, DISPLAY_NAME,
> ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >  	strlcpy_attr(wmi_priv.integer_data[instance_id].display_name,
> >  		integer_obj[DISPLAY_NAME].string.pointer);
> > +	if (check_property_type(integer, DEFAULT_VAL,
> ACPI_TYPE_INTEGER))
> > +		return -EINVAL;
> >  	wmi_priv.integer_data[instance_id].default_value =
> >  		(uintptr_t)integer_obj[DEFAULT_VAL].string.pointer;
> > +	if (check_property_type(integer, MODIFIER, ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >  	strlcpy_attr(wmi_priv.integer_data[instance_id].dell_modifier,
> >  		integer_obj[MODIFIER].string.pointer);
> > +	if (check_property_type(integer, MIN_VALUE,
> ACPI_TYPE_INTEGER))
> > +		return -EINVAL;
> >  	wmi_priv.integer_data[instance_id].min_value =
> >  		(uintptr_t)integer_obj[MIN_VALUE].string.pointer;
> > +	if (check_property_type(integer, MAX_VALUE,
> ACPI_TYPE_INTEGER))
> > +		return -EINVAL;
> >  	wmi_priv.integer_data[instance_id].max_value =
> >  		(uintptr_t)integer_obj[MAX_VALUE].string.pointer;
> > +	if (check_property_type(integer, SCALAR_INCR,
> ACPI_TYPE_INTEGER))
> > +		return -EINVAL;
> >  	wmi_priv.integer_data[instance_id].scalar_increment =
> >  		(uintptr_t)integer_obj[SCALAR_INCR].string.pointer;
> >
> > 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 834b3e82ad9f..230e6ee96636 100644
> > --- a/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
> > +++ b/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
> > @@ -159,10 +159,16 @@ int alloc_po_data(void)  int
> > populate_po_data(union acpi_object *po_obj, int instance_id, struct
> > kobject *attr_name_kobj)  {
> >  	wmi_priv.po_data[instance_id].attr_name_kobj = attr_name_kobj;
> > +	if (check_property_type(po, ATTR_NAME, ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >  	strlcpy_attr(wmi_priv.po_data[instance_id].attribute_name,
> >  		     po_obj[ATTR_NAME].string.pointer);
> > +	if (check_property_type(po, MIN_PASS_LEN, ACPI_TYPE_INTEGER))
> > +		return -EINVAL;
> >  	wmi_priv.po_data[instance_id].min_password_length =
> >  		(uintptr_t)po_obj[MIN_PASS_LEN].string.pointer;
> > +	if (check_property_type(po, MAX_PASS_LEN, ACPI_TYPE_INTEGER))
> > +		return -EINVAL;
> >  	wmi_priv.po_data[instance_id].max_password_length =
> >  		(uintptr_t) po_obj[MAX_PASS_LEN].string.pointer;
> >
> > 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 552537852459..c392f0ecf8b5 100644
> > --- a/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
> > +++ b/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
> > @@ -118,24 +118,38 @@ int alloc_str_data(void)
> >
> >  /**
> >   * populate_str_data() - Populate all properties of an instance under
> > string attribute
> > - * @str_obj: ACPI object with integer data
> > + * @str_obj: ACPI object with string data
> >   * @instance_id: The instance to enumerate
> >   * @attr_name_kobj: The parent kernel object
> >   */
> >  int populate_str_data(union acpi_object *str_obj, int instance_id,
> > struct kobject *attr_name_kobj)  {
> >  	wmi_priv.str_data[instance_id].attr_name_kobj = attr_name_kobj;
> > +	if (check_property_type(str, ATTR_NAME, ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >  	strlcpy_attr(wmi_priv.str_data[instance_id].attribute_name,
> >  		     str_obj[ATTR_NAME].string.pointer);
> > +	if (check_property_type(str, DISPL_NAME_LANG_CODE,
> ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >
> 	strlcpy_attr(wmi_priv.str_data[instance_id].display_name_language
> _code,
> >  		     str_obj[DISPL_NAME_LANG_CODE].string.pointer);
> > +	if (check_property_type(str, DISPLAY_NAME, ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >  	strlcpy_attr(wmi_priv.str_data[instance_id].display_name,
> >  		     str_obj[DISPLAY_NAME].string.pointer);
> > +	if (check_property_type(str, DEFAULT_VAL, ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >  	strlcpy_attr(wmi_priv.str_data[instance_id].default_value,
> >  		     str_obj[DEFAULT_VAL].string.pointer);
> > +	if (check_property_type(str, MODIFIER, ACPI_TYPE_STRING))
> > +		return -EINVAL;
> >  	strlcpy_attr(wmi_priv.str_data[instance_id].dell_modifier,
> >  		     str_obj[MODIFIER].string.pointer);
> > +	if (check_property_type(str, MIN_LEN, ACPI_TYPE_INTEGER))
> > +		return -EINVAL;
> >  	wmi_priv.str_data[instance_id].min_length =
> > (uintptr_t)str_obj[MIN_LEN].string.pointer;
> > +	if (check_property_type(str, MAX_LEN, ACPI_TYPE_INTEGER))
> > +		return -EINVAL;
> >  	wmi_priv.str_data[instance_id].max_length = (uintptr_t)
> > str_obj[MAX_LEN].string.pointer;
> >
> >  	return sysfs_create_group(attr_name_kobj, &str_attr_group); diff
> > --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> > b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> > index c8d276d78e92..5276e8c5aad5 100644
> > --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> > +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> > @@ -412,10 +412,16 @@ 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:
> > +	case STR:
> > +		min_elements = 8;
> > +		break;
> > +	case INT:
> > +		min_elements = 9;
> > +		break;
> > +	case PO:
> > +		min_elements = 4;
> > +		break;
> >  	default:
> >  		pr_err("Error: Unknown attr_type: %d\n", attr_type);
> >  		return -EINVAL;
> 
> Please drop these unrelated changes. If you want to change this please
> submit a separate patch for it, but I would prefer to keep this as is.

Sure. Will do the same. 
We addressed as checkpatch was throwing error on these lines.

> 
> > @@ -481,7 +487,8 @@ static int init_bios_attributes(int attr_type, const
> char *guid)
> >  		/* enumerate all of this attribute */
> >  		switch (attr_type) {
> >  		case ENUM:
> > -			retval = populate_enum_data(elements,
> instance_id, attr_name_kobj);
> > +			retval = populate_enum_data(elements,
> instance_id, attr_name_kobj,
> > +					obj->package.count);
> >  			break;
> >  		case INT:
> >  			retval = populate_int_data(elements, instance_id,
> attr_name_kobj);
> >
> 
> Regards,
> 
> Hans
diff mbox series

Patch

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 b80f2a62ea3f..3ad33a094588 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
@@ -152,12 +152,15 @@  static ssize_t curr_val##_store(struct kobject *kobj,				\
 	return ret ? ret : count;						\
 }
 
+#define check_property_type(attr, prop, valuetype)				\
+	(attr##_obj[prop].type != valuetype)
+
 union acpi_object *get_wmiobj_pointer(int instance_id, const char *guid_string);
 int get_instance_count(const char *guid_string);
 void strlcpy_attr(char *dest, char *src);
 
 int populate_enum_data(union acpi_object *enumeration_obj, int instance_id,
-			struct kobject *attr_name_kobj);
+			struct kobject *attr_name_kobj, u32 enum_property_count);
 int alloc_enum_data(void);
 void exit_enum_attributes(void);
 
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 091e48c217ed..70f4ace56ec3 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c
@@ -132,39 +132,65 @@  int alloc_enum_data(void)
  * @enumeration_obj: ACPI object with enumeration data
  * @instance_id: The instance to enumerate
  * @attr_name_kobj: The parent kernel object
+ * @enum_property_count: Total properties count under enumeration type
  */
 int populate_enum_data(union acpi_object *enumeration_obj, int instance_id,
-			struct kobject *attr_name_kobj)
+			struct kobject *attr_name_kobj, u32 enum_property_count)
 {
 	int i, next_obj, value_modifier_count, possible_values_count;
 
 	wmi_priv.enumeration_data[instance_id].attr_name_kobj = attr_name_kobj;
+	if (check_property_type(enumeration, ATTR_NAME, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.enumeration_data[instance_id].attribute_name,
 		enumeration_obj[ATTR_NAME].string.pointer);
+	if (check_property_type(enumeration, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.enumeration_data[instance_id].display_name_language_code,
 		enumeration_obj[DISPL_NAME_LANG_CODE].string.pointer);
+	if (check_property_type(enumeration, DISPLAY_NAME, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.enumeration_data[instance_id].display_name,
 		enumeration_obj[DISPLAY_NAME].string.pointer);
+	if (check_property_type(enumeration, DEFAULT_VAL, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.enumeration_data[instance_id].default_value,
 		enumeration_obj[DEFAULT_VAL].string.pointer);
+	if (check_property_type(enumeration, MODIFIER, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.enumeration_data[instance_id].dell_modifier,
 		enumeration_obj[MODIFIER].string.pointer);
 
 	next_obj = MODIFIER + 1;
 
-	value_modifier_count = (uintptr_t)enumeration_obj[next_obj].string.pointer;
+	if (next_obj >= enum_property_count)
+		return -EINVAL;
+
+	if (check_property_type(enumeration, next_obj, ACPI_TYPE_INTEGER))
+		return -EINVAL;
+	value_modifier_count = (uintptr_t)enumeration_obj[next_obj++].string.pointer;
 
 	for (i = 0; i < value_modifier_count; i++) {
+		if (next_obj >= enum_property_count)
+			return -EINVAL;
+		if (check_property_type(enumeration, next_obj, ACPI_TYPE_STRING))
+			return -EINVAL;
 		strcat(wmi_priv.enumeration_data[instance_id].dell_value_modifier,
-			enumeration_obj[++next_obj].string.pointer);
+			enumeration_obj[next_obj++].string.pointer);
 		strcat(wmi_priv.enumeration_data[instance_id].dell_value_modifier, ";");
 	}
 
-	possible_values_count = (uintptr_t) enumeration_obj[++next_obj].string.pointer;
+	if (check_property_type(enumeration, next_obj, ACPI_TYPE_INTEGER))
+		return -EINVAL;
+	possible_values_count = (uintptr_t) enumeration_obj[next_obj++].string.pointer;
 
 	for (i = 0; i < possible_values_count; i++) {
+		if (next_obj >= enum_property_count)
+			return -EINVAL;
+		if (check_property_type(enumeration, next_obj, ACPI_TYPE_STRING))
+			return -EINVAL;
 		strcat(wmi_priv.enumeration_data[instance_id].possible_values,
-			enumeration_obj[++next_obj].string.pointer);
+			enumeration_obj[next_obj++].string.pointer);
 		strcat(wmi_priv.enumeration_data[instance_id].possible_values, ";");
 	}
 
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 8a49ba6e44f9..951e75b538fa 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/int-attributes.c
@@ -141,20 +141,36 @@  int populate_int_data(union acpi_object *integer_obj, int instance_id,
 			struct kobject *attr_name_kobj)
 {
 	wmi_priv.integer_data[instance_id].attr_name_kobj = attr_name_kobj;
+	if (check_property_type(integer, ATTR_NAME, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.integer_data[instance_id].attribute_name,
 		integer_obj[ATTR_NAME].string.pointer);
+	if (check_property_type(integer, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.integer_data[instance_id].display_name_language_code,
 		integer_obj[DISPL_NAME_LANG_CODE].string.pointer);
+	if (check_property_type(integer, DISPLAY_NAME, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.integer_data[instance_id].display_name,
 		integer_obj[DISPLAY_NAME].string.pointer);
+	if (check_property_type(integer, DEFAULT_VAL, ACPI_TYPE_INTEGER))
+		return -EINVAL;
 	wmi_priv.integer_data[instance_id].default_value =
 		(uintptr_t)integer_obj[DEFAULT_VAL].string.pointer;
+	if (check_property_type(integer, MODIFIER, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.integer_data[instance_id].dell_modifier,
 		integer_obj[MODIFIER].string.pointer);
+	if (check_property_type(integer, MIN_VALUE, ACPI_TYPE_INTEGER))
+		return -EINVAL;
 	wmi_priv.integer_data[instance_id].min_value =
 		(uintptr_t)integer_obj[MIN_VALUE].string.pointer;
+	if (check_property_type(integer, MAX_VALUE, ACPI_TYPE_INTEGER))
+		return -EINVAL;
 	wmi_priv.integer_data[instance_id].max_value =
 		(uintptr_t)integer_obj[MAX_VALUE].string.pointer;
+	if (check_property_type(integer, SCALAR_INCR, ACPI_TYPE_INTEGER))
+		return -EINVAL;
 	wmi_priv.integer_data[instance_id].scalar_increment =
 		(uintptr_t)integer_obj[SCALAR_INCR].string.pointer;
 
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 834b3e82ad9f..230e6ee96636 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c
@@ -159,10 +159,16 @@  int alloc_po_data(void)
 int populate_po_data(union acpi_object *po_obj, int instance_id, struct kobject *attr_name_kobj)
 {
 	wmi_priv.po_data[instance_id].attr_name_kobj = attr_name_kobj;
+	if (check_property_type(po, ATTR_NAME, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.po_data[instance_id].attribute_name,
 		     po_obj[ATTR_NAME].string.pointer);
+	if (check_property_type(po, MIN_PASS_LEN, ACPI_TYPE_INTEGER))
+		return -EINVAL;
 	wmi_priv.po_data[instance_id].min_password_length =
 		(uintptr_t)po_obj[MIN_PASS_LEN].string.pointer;
+	if (check_property_type(po, MAX_PASS_LEN, ACPI_TYPE_INTEGER))
+		return -EINVAL;
 	wmi_priv.po_data[instance_id].max_password_length =
 		(uintptr_t) po_obj[MAX_PASS_LEN].string.pointer;
 
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 552537852459..c392f0ecf8b5 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/string-attributes.c
@@ -118,24 +118,38 @@  int alloc_str_data(void)
 
 /**
  * populate_str_data() - Populate all properties of an instance under string attribute
- * @str_obj: ACPI object with integer data
+ * @str_obj: ACPI object with string data
  * @instance_id: The instance to enumerate
  * @attr_name_kobj: The parent kernel object
  */
 int populate_str_data(union acpi_object *str_obj, int instance_id, struct kobject *attr_name_kobj)
 {
 	wmi_priv.str_data[instance_id].attr_name_kobj = attr_name_kobj;
+	if (check_property_type(str, ATTR_NAME, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.str_data[instance_id].attribute_name,
 		     str_obj[ATTR_NAME].string.pointer);
+	if (check_property_type(str, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.str_data[instance_id].display_name_language_code,
 		     str_obj[DISPL_NAME_LANG_CODE].string.pointer);
+	if (check_property_type(str, DISPLAY_NAME, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.str_data[instance_id].display_name,
 		     str_obj[DISPLAY_NAME].string.pointer);
+	if (check_property_type(str, DEFAULT_VAL, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.str_data[instance_id].default_value,
 		     str_obj[DEFAULT_VAL].string.pointer);
+	if (check_property_type(str, MODIFIER, ACPI_TYPE_STRING))
+		return -EINVAL;
 	strlcpy_attr(wmi_priv.str_data[instance_id].dell_modifier,
 		     str_obj[MODIFIER].string.pointer);
+	if (check_property_type(str, MIN_LEN, ACPI_TYPE_INTEGER))
+		return -EINVAL;
 	wmi_priv.str_data[instance_id].min_length = (uintptr_t)str_obj[MIN_LEN].string.pointer;
+	if (check_property_type(str, MAX_LEN, ACPI_TYPE_INTEGER))
+		return -EINVAL;
 	wmi_priv.str_data[instance_id].max_length = (uintptr_t) str_obj[MAX_LEN].string.pointer;
 
 	return sysfs_create_group(attr_name_kobj, &str_attr_group);
diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
index c8d276d78e92..5276e8c5aad5 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
@@ -412,10 +412,16 @@  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:
+	case STR:
+		min_elements = 8;
+		break;
+	case INT:
+		min_elements = 9;
+		break;
+	case PO:
+		min_elements = 4;
+		break;
 	default:
 		pr_err("Error: Unknown attr_type: %d\n", attr_type);
 		return -EINVAL;
@@ -481,7 +487,8 @@  static int init_bios_attributes(int attr_type, const char *guid)
 		/* enumerate all of this attribute */
 		switch (attr_type) {
 		case ENUM:
-			retval = populate_enum_data(elements, instance_id, attr_name_kobj);
+			retval = populate_enum_data(elements, instance_id, attr_name_kobj,
+					obj->package.count);
 			break;
 		case INT:
 			retval = populate_int_data(elements, instance_id, attr_name_kobj);