| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20250304062319.86270-1-marchese@hp.com>
Date: Tue, 4 Mar 2025 00:23:19 -0600
From: Tadeu Marchese <marchese.kdev@...il.com>
To: hdegoede@...hat.com,
ilpo.jarvinen@...ux.intel.com
Cc: platform-driver-x86@...r.kernel.org,
linux-kernel@...r.kernel.org,
Tadeu Marchese <marchese@...com>
Subject: platform/x86/hp-bioscfg: Fix buffer alignment and conversion
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@...com>
---
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
Powered by blists - more mailing lists