From: Tadeu Marchese <marchese.kdev@gmail.com>
To: hdegoede@redhat.com, ilpo.jarvinen@linux.intel.com
Cc: platform-driver-x86@vger.kernel.org,
linux-kernel@vger.kernel.org, Tadeu Marchese <marchese@hp.com>
Subject: platform/x86/hp-bioscfg: Fix buffer alignment and conversion
Date: Tue, 4 Mar 2025 00:23:19 -0600 [thread overview]
Message-ID: <20250304062319.86270-1-marchese@hp.com> (raw)
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@hp.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
next reply other threads:[~2025-03-04 6:23 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-04 6:23 Tadeu Marchese [this message]
2025-03-04 14:57 ` platform/x86/hp-bioscfg: Fix buffer alignment and conversion Ilpo Järvinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250304062319.86270-1-marchese@hp.com \
--to=marchese.kdev@gmail.com \
--cc=hdegoede@redhat.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=marchese@hp.com \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.