| Message ID | 20210422111645.24726-1-prasanth.ksr@dell.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Making populate_foo_data functions more robust | expand |
Hi, Thank you. Patch subject should be prefixed with: "platform/x86: dell-wmi-sysman: ", IOW it should be: "platform/x86: dell-wmi-sysman: Make populate_foo_data functions more robust" On 4/22/21 1:16 PM, Prasanth KSR wrote: > From: "Prasanth KSR" <prasanth.ksr@dell.com> > > 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 | 9 ++++- > .../dell/dell-wmi-sysman/enum-attributes.c | 26 ++++++++++--- > .../x86/dell/dell-wmi-sysman/int-attributes.c | 8 ++++ > .../dell/dell-wmi-sysman/passobj-attributes.c | 3 ++ > .../dell/dell-wmi-sysman/string-attributes.c | 9 ++++- > .../x86/dell/dell-wmi-sysman/sysman.c | 39 +++++++++++++++++-- > 6 files changed, 83 insertions(+), 11 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..5fd5fcd1976a 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,19 @@ static ssize_t curr_val##_store(struct kobject *kobj, \ > return ret ? ret : count; \ > } > > +#define check_property_type(attr, prop, valuetype) \ > +do { \ > + if (attr##_obj[prop].type != valuetype) { \ > + return -EINVAL; \ > + } \ > +} while (0) > + This is hiding a return inside a function-call which is actually a macro. So now someone reading the code will not know that this hides a possible return which may be an issue with releasing resources (e.g. a mutex) inside the hidden return path. Please drop this macro and just write out the check + return in the places where you need it. > 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); __u32 is reserved for userspace headers please just use plain u32. > 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..a5b0e2689e5f 100644 > --- a/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c > +++ b/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c > @@ -134,37 +134,53 @@ int alloc_enum_data(void) > * @attr_name_kobj: The parent kernel object > */ > 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; > + check_property_type(enumeration, ATTR_NAME, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.enumeration_data[instance_id].attribute_name, > enumeration_obj[ATTR_NAME].string.pointer); > + check_property_type(enumeration, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.enumeration_data[instance_id].display_name_language_code, > enumeration_obj[DISPL_NAME_LANG_CODE].string.pointer); > + check_property_type(enumeration, DISPLAY_NAME, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.enumeration_data[instance_id].display_name, > enumeration_obj[DISPLAY_NAME].string.pointer); > + check_property_type(enumeration, DEFAULT_VAL, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.enumeration_data[instance_id].default_value, > enumeration_obj[DEFAULT_VAL].string.pointer); > + check_property_type(enumeration, MODIFIER, ACPI_TYPE_STRING); > 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; > + > + check_property_type(enumeration, next_obj, ACPI_TYPE_INTEGER); > + 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; > + check_property_type(enumeration, next_obj, ACPI_TYPE_STRING); > 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; > + check_property_type(enumeration, next_obj, ACPI_TYPE_INTEGER); > + 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; > + check_property_type(enumeration, next_obj, ACPI_TYPE_STRING); > 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..12d6b99da43d 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,28 @@ 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; > + check_property_type(integer, ATTR_NAME, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.integer_data[instance_id].attribute_name, > integer_obj[ATTR_NAME].string.pointer); > + check_property_type(integer, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.integer_data[instance_id].display_name_language_code, > integer_obj[DISPL_NAME_LANG_CODE].string.pointer); > + check_property_type(integer, DISPLAY_NAME, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.integer_data[instance_id].display_name, > integer_obj[DISPLAY_NAME].string.pointer); > + check_property_type(integer, DEFAULT_VAL, ACPI_TYPE_INTEGER); > wmi_priv.integer_data[instance_id].default_value = > (uintptr_t)integer_obj[DEFAULT_VAL].string.pointer; > + check_property_type(integer, MODIFIER, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.integer_data[instance_id].dell_modifier, > integer_obj[MODIFIER].string.pointer); > + check_property_type(integer, MIN_VALUE, ACPI_TYPE_INTEGER); > wmi_priv.integer_data[instance_id].min_value = > (uintptr_t)integer_obj[MIN_VALUE].string.pointer; > + check_property_type(integer, MAX_VALUE, ACPI_TYPE_INTEGER); > wmi_priv.integer_data[instance_id].max_value = > (uintptr_t)integer_obj[MAX_VALUE].string.pointer; > + check_property_type(integer, SCALAR_INCR, ACPI_TYPE_INTEGER); > 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..866cf732cee2 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,13 @@ 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; > + check_property_type(po, ATTR_NAME, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.po_data[instance_id].attribute_name, > po_obj[ATTR_NAME].string.pointer); > + check_property_type(po, MIN_PASS_LEN, ACPI_TYPE_INTEGER); > wmi_priv.po_data[instance_id].min_password_length = > (uintptr_t)po_obj[MIN_PASS_LEN].string.pointer; > + check_property_type(po, MAX_PASS_LEN, ACPI_TYPE_INTEGER); > 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..557b9d4511af 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,31 @@ 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; > + check_property_type(str, ATTR_NAME, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.str_data[instance_id].attribute_name, > str_obj[ATTR_NAME].string.pointer); > + check_property_type(str, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.str_data[instance_id].display_name_language_code, > str_obj[DISPL_NAME_LANG_CODE].string.pointer); > + check_property_type(str, DISPLAY_NAME, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.str_data[instance_id].display_name, > str_obj[DISPLAY_NAME].string.pointer); > + check_property_type(str, DEFAULT_VAL, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.str_data[instance_id].default_value, > str_obj[DEFAULT_VAL].string.pointer); > + check_property_type(str, MODIFIER, ACPI_TYPE_STRING); > strlcpy_attr(wmi_priv.str_data[instance_id].dell_modifier, > str_obj[MODIFIER].string.pointer); > + check_property_type(str, MIN_LEN, ACPI_TYPE_INTEGER); > wmi_priv.str_data[instance_id].min_length = (uintptr_t)str_obj[MIN_LEN].string.pointer; > + check_property_type(str, MAX_LEN, ACPI_TYPE_INTEGER); > 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); Except for the issue of using a macro which hides the return the above changes look good, thank you. > diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c > index 7410ccae650c..fc45cdb5a5e8 100644 > --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c > +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c > @@ -399,6 +399,7 @@ static int init_bios_attributes(int attr_type, const char *guid) > union acpi_object *obj = NULL; > union acpi_object *elements; > struct kset *tmp_set; > + int min_elements; > > /* instance_id needs to be reset for each type GUID > * also, instance IDs are unique within GUID but not across > @@ -409,14 +410,43 @@ static int init_bios_attributes(int attr_type, const char *guid) > retval = alloc_attributes_data(attr_type); > if (retval) > return retval; > + > + switch (attr_type) { > + 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; > + } > + > /* need to use specific instance_id and guid combination to get right data */ > obj = get_wmiobj_pointer(instance_id, guid); > - if (!obj || obj->type != ACPI_TYPE_PACKAGE) > + if (!obj) > return -ENODEV; > - elements = obj->package.elements; > > mutex_lock(&wmi_priv.mutex); > - while (elements) { > + while (obj) { > + if (obj->type != ACPI_TYPE_PACKAGE) { > + pr_err("Error: Expected ACPI-package type, got: %d\n", obj->type); > + retval = -EIO; > + goto err_attr_init; > + } > + > + if (obj->package.count < min_elements) { > + pr_err("Error: ACPI-package does not have enough elements: %d < %d\n", > + obj->package.count, min_elements); > + goto nextobj; > + } > + > + elements = obj->package.elements; > /* sanity checking */ > if (elements[ATTR_NAME].type != ACPI_TYPE_STRING) { > pr_debug("incorrect element type\n"); The above changes to drivers/platform/x86/dell/dell-wmi-sysman/sysman.c are already present in for-next: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=for-next Please rebase on top of platform-drivers-x86.git/for-next. > @@ -456,7 +486,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 --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..5fd5fcd1976a 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,19 @@ static ssize_t curr_val##_store(struct kobject *kobj, \ return ret ? ret : count; \ } +#define check_property_type(attr, prop, valuetype) \ +do { \ + if (attr##_obj[prop].type != valuetype) { \ + return -EINVAL; \ + } \ +} while (0) + 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..a5b0e2689e5f 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/enum-attributes.c @@ -134,37 +134,53 @@ int alloc_enum_data(void) * @attr_name_kobj: The parent kernel object */ 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; + check_property_type(enumeration, ATTR_NAME, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.enumeration_data[instance_id].attribute_name, enumeration_obj[ATTR_NAME].string.pointer); + check_property_type(enumeration, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.enumeration_data[instance_id].display_name_language_code, enumeration_obj[DISPL_NAME_LANG_CODE].string.pointer); + check_property_type(enumeration, DISPLAY_NAME, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.enumeration_data[instance_id].display_name, enumeration_obj[DISPLAY_NAME].string.pointer); + check_property_type(enumeration, DEFAULT_VAL, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.enumeration_data[instance_id].default_value, enumeration_obj[DEFAULT_VAL].string.pointer); + check_property_type(enumeration, MODIFIER, ACPI_TYPE_STRING); 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; + + check_property_type(enumeration, next_obj, ACPI_TYPE_INTEGER); + 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; + check_property_type(enumeration, next_obj, ACPI_TYPE_STRING); 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; + check_property_type(enumeration, next_obj, ACPI_TYPE_INTEGER); + 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; + check_property_type(enumeration, next_obj, ACPI_TYPE_STRING); 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..12d6b99da43d 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,28 @@ 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; + check_property_type(integer, ATTR_NAME, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.integer_data[instance_id].attribute_name, integer_obj[ATTR_NAME].string.pointer); + check_property_type(integer, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.integer_data[instance_id].display_name_language_code, integer_obj[DISPL_NAME_LANG_CODE].string.pointer); + check_property_type(integer, DISPLAY_NAME, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.integer_data[instance_id].display_name, integer_obj[DISPLAY_NAME].string.pointer); + check_property_type(integer, DEFAULT_VAL, ACPI_TYPE_INTEGER); wmi_priv.integer_data[instance_id].default_value = (uintptr_t)integer_obj[DEFAULT_VAL].string.pointer; + check_property_type(integer, MODIFIER, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.integer_data[instance_id].dell_modifier, integer_obj[MODIFIER].string.pointer); + check_property_type(integer, MIN_VALUE, ACPI_TYPE_INTEGER); wmi_priv.integer_data[instance_id].min_value = (uintptr_t)integer_obj[MIN_VALUE].string.pointer; + check_property_type(integer, MAX_VALUE, ACPI_TYPE_INTEGER); wmi_priv.integer_data[instance_id].max_value = (uintptr_t)integer_obj[MAX_VALUE].string.pointer; + check_property_type(integer, SCALAR_INCR, ACPI_TYPE_INTEGER); 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..866cf732cee2 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,13 @@ 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; + check_property_type(po, ATTR_NAME, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.po_data[instance_id].attribute_name, po_obj[ATTR_NAME].string.pointer); + check_property_type(po, MIN_PASS_LEN, ACPI_TYPE_INTEGER); wmi_priv.po_data[instance_id].min_password_length = (uintptr_t)po_obj[MIN_PASS_LEN].string.pointer; + check_property_type(po, MAX_PASS_LEN, ACPI_TYPE_INTEGER); 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..557b9d4511af 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,31 @@ 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; + check_property_type(str, ATTR_NAME, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.str_data[instance_id].attribute_name, str_obj[ATTR_NAME].string.pointer); + check_property_type(str, DISPL_NAME_LANG_CODE, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.str_data[instance_id].display_name_language_code, str_obj[DISPL_NAME_LANG_CODE].string.pointer); + check_property_type(str, DISPLAY_NAME, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.str_data[instance_id].display_name, str_obj[DISPLAY_NAME].string.pointer); + check_property_type(str, DEFAULT_VAL, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.str_data[instance_id].default_value, str_obj[DEFAULT_VAL].string.pointer); + check_property_type(str, MODIFIER, ACPI_TYPE_STRING); strlcpy_attr(wmi_priv.str_data[instance_id].dell_modifier, str_obj[MODIFIER].string.pointer); + check_property_type(str, MIN_LEN, ACPI_TYPE_INTEGER); wmi_priv.str_data[instance_id].min_length = (uintptr_t)str_obj[MIN_LEN].string.pointer; + check_property_type(str, MAX_LEN, ACPI_TYPE_INTEGER); 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 7410ccae650c..fc45cdb5a5e8 100644 --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c @@ -399,6 +399,7 @@ static int init_bios_attributes(int attr_type, const char *guid) union acpi_object *obj = NULL; union acpi_object *elements; struct kset *tmp_set; + int min_elements; /* instance_id needs to be reset for each type GUID * also, instance IDs are unique within GUID but not across @@ -409,14 +410,43 @@ static int init_bios_attributes(int attr_type, const char *guid) retval = alloc_attributes_data(attr_type); if (retval) return retval; + + switch (attr_type) { + 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; + } + /* need to use specific instance_id and guid combination to get right data */ obj = get_wmiobj_pointer(instance_id, guid); - if (!obj || obj->type != ACPI_TYPE_PACKAGE) + if (!obj) return -ENODEV; - elements = obj->package.elements; mutex_lock(&wmi_priv.mutex); - while (elements) { + while (obj) { + if (obj->type != ACPI_TYPE_PACKAGE) { + pr_err("Error: Expected ACPI-package type, got: %d\n", obj->type); + retval = -EIO; + goto err_attr_init; + } + + if (obj->package.count < min_elements) { + pr_err("Error: ACPI-package does not have enough elements: %d < %d\n", + obj->package.count, min_elements); + goto nextobj; + } + + elements = obj->package.elements; /* sanity checking */ if (elements[ATTR_NAME].type != ACPI_TYPE_STRING) { pr_debug("incorrect element type\n"); @@ -456,7 +486,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);