Use PTR_ALIGN to access buffer in hp_get_string_from_buffer(). Remove code that escapes characters with backslashes. Use kstrtouint() for unsigned string-to-integer conversion. Increase the string_data buffer size by defining MAX_STRING_BUFF_SIZE. Signed-off-by: Tadeu Marchese <marchese@xxxxxx> --- This patch ensures proper alignment when reading the string size from the buffer and simplifies Unicode-to-UTF-8 conversion by removing unnecessary character escaping. Issues fixed at the module initialization: [ 433.823905] hp_bioscfg: Prerequisites size value exceeded the maximum number of elements supported or data may be malformed [ 433.837747] Unable to convert string to integer: 4294967295 The buffer size was too small for string attributes such as 'HP_Disk0MapForUefiBootOrder'. drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 82 ++++++------------- drivers/platform/x86/hp/hp-bioscfg/bioscfg.h | 6 +- .../x86/hp/hp-bioscfg/int-attributes.c | 8 +- 3 files changed, 33 insertions(+), 63 deletions(-) diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c index 0b277b7e37dd..b537fbaac15e 100644 --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c @@ -51,19 +51,21 @@ int hp_get_integer_from_buffer(u8 **buffer, u32 *buffer_size, u32 *integer) int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_size) { - u16 *src = (u16 *)*buffer; + u16 *src = PTR_ALIGN((u16 *)*buffer, sizeof(u16)); u16 src_size; - u16 size; - int i; + u16 length; int conv_dst_size; + int result; if (*buffer_size < sizeof(u16)) return -EINVAL; + /* String size is in the first two bytes of the buffer */ src_size = *(src++); - /* size value in u16 chars */ - size = src_size / sizeof(u16); + + /* Get the string length considering it is u16 */ + length = src_size / sizeof(u16); /* Ensure there is enough space remaining to read and convert * the string @@ -71,54 +73,22 @@ int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_ if (*buffer_size < src_size) return -EINVAL; - for (i = 0; i < size; i++) - if (src[i] == '\\' || - src[i] == '\r' || - src[i] == '\n' || - src[i] == '\t') - size++; - - /* - * Conversion is limited to destination string max number of - * bytes. - */ - conv_dst_size = size; - if (size > dst_size) - conv_dst_size = dst_size - 1; + conv_dst_size = length; + if (dst_size < conv_dst_size) + return -EINVAL; /* - * convert from UTF-16 unicode to ASCII + * Convert from UTF-16 unicode to UTF-8 and ensure + * the string is null terminated */ - utf16s_to_utf8s(src, src_size, UTF16_HOST_ENDIAN, dst, conv_dst_size); - dst[conv_dst_size] = 0; - - for (i = 0; i < conv_dst_size; i++) { - if (*src == '\\' || - *src == '\r' || - *src == '\n' || - *src == '\t') { - dst[i++] = '\\'; - if (i == conv_dst_size) - break; - } - - if (*src == '\r') - dst[i] = 'r'; - else if (*src == '\n') - dst[i] = 'n'; - else if (*src == '\t') - dst[i] = 't'; - else if (*src == '"') - dst[i] = '\''; - else - dst[i] = *src; - src++; - } + result = utf16s_to_utf8s(src, src_size, UTF16_HOST_ENDIAN, dst, conv_dst_size); + dst[result] = 0; - *buffer = (u8 *)src; - *buffer_size -= size * sizeof(u16); + /* Update buffer to point to the next position */ + *buffer = (u8 *)src + src_size; + *buffer_size -= src_size; - return size; + return result; } int hp_get_common_data_from_buffer(u8 **buffer_ptr, u32 *buffer_size, @@ -999,37 +969,37 @@ static int __init hp_init(void) */ ret = create_attributes_level_sysfs_files(); if (ret) - pr_debug("Failed to create sysfs level attributes\n"); + pr_warn("Failed to create sysfs level attributes\n"); ret = hp_init_bios_attributes(HPWMI_STRING_TYPE, HP_WMI_BIOS_STRING_GUID); if (ret) - pr_debug("Failed to populate string type attributes\n"); + pr_warn("Failed to populate string type attributes\n"); ret = hp_init_bios_attributes(HPWMI_INTEGER_TYPE, HP_WMI_BIOS_INTEGER_GUID); if (ret) - pr_debug("Failed to populate integer type attributes\n"); + pr_warn("Failed to populate integer type attributes\n"); ret = hp_init_bios_attributes(HPWMI_ENUMERATION_TYPE, HP_WMI_BIOS_ENUMERATION_GUID); if (ret) - pr_debug("Failed to populate enumeration type attributes\n"); + pr_warn("Failed to populate enumeration type attributes\n"); ret = hp_init_bios_attributes(HPWMI_ORDERED_LIST_TYPE, HP_WMI_BIOS_ORDERED_LIST_GUID); if (ret) - pr_debug("Failed to populate ordered list object type attributes\n"); + pr_warn("Failed to populate ordered list object type attributes\n"); ret = hp_init_bios_attributes(HPWMI_PASSWORD_TYPE, HP_WMI_BIOS_PASSWORD_GUID); if (ret) - pr_debug("Failed to populate password object type attributes\n"); + pr_warn("Failed to populate password object type attributes\n"); bioscfg_drv.spm_data.attr_name_kobj = NULL; ret = hp_add_other_attributes(HPWMI_SECURE_PLATFORM_TYPE); if (ret) - pr_debug("Failed to populate secure platform object type attribute\n"); + pr_warn("Failed to populate secure platform object type attribute\n"); bioscfg_drv.sure_start_attr_kobj = NULL; ret = hp_add_other_attributes(HPWMI_SURE_START_TYPE); if (ret) - pr_debug("Failed to populate sure start object type attribute\n"); + pr_warn("Failed to populate sure start object type attribute\n"); return 0; diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h index 3166ef328eba..99a95c709061 100644 --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h @@ -17,11 +17,11 @@ #define DRIVER_NAME "hp-bioscfg" +#define MAX_STRING_BUFF_SIZE 1024 #define MAX_BUFF_SIZE 512 #define MAX_KEY_MOD_SIZE 256 #define MAX_PASSWD_SIZE 64 #define MAX_PREREQUISITES_SIZE 20 -#define MAX_REQ_ELEM_SIZE 128 #define MAX_VALUES_SIZE 16 #define MAX_ENCODINGS_SIZE 16 #define MAX_ELEMENTS_SIZE 16 @@ -131,8 +131,8 @@ struct common_data { struct string_data { struct common_data common; struct kobject *attr_name_kobj; - u8 current_value[MAX_BUFF_SIZE]; - u8 new_value[MAX_BUFF_SIZE]; + u8 current_value[MAX_STRING_BUFF_SIZE]; + u8 new_value[MAX_STRING_BUFF_SIZE]; u32 min_length; u32 max_length; }; diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c index 6c7f4d5fa9cb..3e8f99b4174d 100644 --- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c +++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c @@ -38,7 +38,7 @@ static int validate_integer_input(int instance_id, char *buf) if (integer_data->common.is_readonly) return -EIO; - ret = kstrtoint(buf, 10, &in_val); + ret = kstrtouint(buf, 10, &in_val); if (ret < 0) return ret; @@ -55,7 +55,7 @@ static void update_integer_value(int instance_id, char *attr_value) int ret; struct integer_data *integer_data = &bioscfg_drv.integer_data[instance_id]; - ret = kstrtoint(attr_value, 10, &in_val); + ret = kstrtouint(attr_value, 10, &in_val); if (ret == 0) integer_data->current_value = in_val; else @@ -185,7 +185,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_ /* Assign appropriate element value to corresponding field*/ switch (eloc) { case VALUE: - ret = kstrtoint(str_value, 10, &int_value); + ret = kstrtouint(str_value, 10, &int_value); if (ret) continue; @@ -328,7 +328,7 @@ static int hp_populate_integer_elements_from_buffer(u8 *buffer_ptr, u32 *buffer_ integer_data->current_value = 0; hp_get_string_from_buffer(&buffer_ptr, buffer_size, dst, dst_size); - ret = kstrtoint(dst, 10, &integer_data->current_value); + ret = kstrtouint(dst, 10, &integer_data->current_value); if (ret) pr_warn("Unable to convert string to integer: %s\n", dst); kfree(dst); -- 2.40.1