* [PATCH 1/8] hp-bioscfg: Fix memory leaks in attribute packages
2023-07-31 20:31 [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Jorge Lopez
@ 2023-07-31 20:31 ` Jorge Lopez
2023-07-31 20:31 ` [PATCH 2/8] hp-bioscfg: Fix uninitialized variable errors Jorge Lopez
` (8 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Jorge Lopez @ 2023-07-31 20:31 UTC (permalink / raw)
To: hdegoede, platform-driver-x86, linux-kernel, thomas,
ilpo.jarvinen, dan.carpenter
Address memory leaks while handling elements in packages.
Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
---
Based on the latest platform-drivers-x86.git/for-next
---
drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c | 6 ++++++
drivers/platform/x86/hp/hp-bioscfg/int-attributes.c | 5 +++++
.../platform/x86/hp/hp-bioscfg/order-list-attributes.c | 6 ++++++
drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c | 8 ++++++++
drivers/platform/x86/hp/hp-bioscfg/string-attributes.c | 3 +++
5 files changed, 28 insertions(+)
diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
index b1b241f0205a..8e615ccfc9b5 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
@@ -164,6 +164,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
if (expected_enum_types[eloc] != enum_obj[elem].type) {
pr_err("Error expected type %d for elem %d, but got type %d instead\n",
expected_enum_types[eloc], elem, enum_obj[elem].type);
+ kfree(str_value);
return -EIO;
}
@@ -224,6 +225,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
sizeof(enum_data->common.prerequisites[reqs]));
kfree(str_value);
+ str_value = NULL;
}
break;
@@ -275,6 +277,9 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
strscpy(enum_data->possible_values[pos_values],
str_value,
sizeof(enum_data->possible_values[pos_values]));
+
+ kfree(str_value);
+ str_value = NULL;
}
break;
default:
@@ -283,6 +288,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
}
kfree(str_value);
+ str_value = NULL;
}
exit_enumeration_package:
diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
index 7364c5ef9ef8..6db2c8ba02a9 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
@@ -179,6 +179,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
if (expected_integer_types[eloc] != integer_obj[elem].type) {
pr_err("Error expected type %d for elem %d, but got type %d instead\n",
expected_integer_types[eloc], elem, integer_obj[elem].type);
+ kfree(str_value);
return -EIO;
}
/* Assign appropriate element value to corresponding field*/
@@ -239,6 +240,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
str_value,
sizeof(integer_data->common.prerequisites[reqs]));
kfree(str_value);
+ str_value = NULL;
}
break;
@@ -258,6 +260,9 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
pr_warn("Invalid element: %d found in Integer attribute or data may be malformed\n", elem);
break;
}
+
+ kfree(str_value);
+ str_value = NULL;
}
exit_integer_package:
kfree(str_value);
diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index 7e49a8427c06..739998682874 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -174,6 +174,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
if (expected_order_types[eloc] != order_obj[elem].type) {
pr_err("Error expected type %d for elem %d, but got type %d instead\n",
expected_order_types[eloc], elem, order_obj[elem].type);
+ kfree(str_value);
return -EIO;
}
@@ -231,6 +232,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
sizeof(ordered_list_data->common.prerequisites[reqs]));
kfree(str_value);
+ str_value = NULL;
}
break;
@@ -277,13 +279,17 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
part = strsep(&part_tmp, SEMICOLON_SEP);
}
+ kfree(str_value);
+ str_value = NULL;
break;
default:
pr_warn("Invalid element: %d found in Ordered_List attribute or data may be malformed\n", elem);
break;
}
kfree(tmpstr);
+ tmpstr = NULL;
kfree(str_value);
+ str_value = NULL;
}
exit_list:
diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index 52e4d346b529..3f7b9fe857f1 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -264,6 +264,7 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
if (expected_password_types[eloc] != password_obj[elem].type) {
pr_err("Error expected type %d for elem %d, but got type %d instead\n",
expected_password_types[eloc], elem, password_obj[elem].type);
+ kfree(str_value);
return -EIO;
}
@@ -318,6 +319,8 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
sizeof(password_data->common.prerequisites[reqs]));
kfree(str_value);
+ str_value = NULL;
+
}
break;
case SECURITY_LEVEL:
@@ -356,6 +359,8 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
str_value,
sizeof(password_data->encodings[pos_values]));
kfree(str_value);
+ str_value = NULL;
+
}
break;
case PSWD_IS_SET:
@@ -365,6 +370,9 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
pr_warn("Invalid element: %d found in Password attribute or data may be malformed\n", elem);
break;
}
+
+ kfree(str_value);
+ str_value = NULL;
}
exit_package:
diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
index 1b62e372fb9e..c9e124af170e 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
@@ -171,6 +171,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
if (expected_string_types[eloc] != string_obj[elem].type) {
pr_err("Error expected type %d for elem %d, but got type %d instead\n",
expected_string_types[eloc], elem, string_obj[elem].type);
+ kfree(str_value);
return -EIO;
}
@@ -232,6 +233,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
str_value,
sizeof(string_data->common.prerequisites[reqs]));
kfree(str_value);
+ str_value = NULL;
}
break;
@@ -250,6 +252,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
}
kfree(str_value);
+ str_value = NULL;
}
exit_string_package:
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 2/8] hp-bioscfg: Fix uninitialized variable errors
2023-07-31 20:31 [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Jorge Lopez
2023-07-31 20:31 ` [PATCH 1/8] hp-bioscfg: Fix memory leaks in attribute packages Jorge Lopez
@ 2023-07-31 20:31 ` Jorge Lopez
2023-07-31 20:31 ` [PATCH 3/8] hp-bioscfg: Replace the word HACK from source code Jorge Lopez
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Jorge Lopez @ 2023-07-31 20:31 UTC (permalink / raw)
To: hdegoede, platform-driver-x86, linux-kernel, thomas,
ilpo.jarvinen, dan.carpenter
Fix uninitialized variable errors.
Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
---
Based on the latest platform-drivers-x86.git/for-next
---
drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c | 2 +-
drivers/platform/x86/hp/hp-bioscfg/int-attributes.c | 2 +-
drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c | 4 ++--
drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c | 2 +-
drivers/platform/x86/hp/hp-bioscfg/string-attributes.c | 2 +-
5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
index 8e615ccfc9b5..7f77963cd7fa 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
@@ -129,7 +129,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
char *str_value = NULL;
int value_len;
u32 size = 0;
- u32 int_value;
+ u32 int_value = 0;
int elem = 0;
int reqs;
int pos_values;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
index 6db2c8ba02a9..97f88e0ef0cc 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
@@ -143,7 +143,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
char *str_value = NULL;
int value_len;
int ret;
- u32 int_value;
+ u32 int_value = 0;
int elem;
int reqs;
int eloc;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index 739998682874..89e67db733eb 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -131,10 +131,10 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
int instance_id)
{
char *str_value = NULL;
- int value_len;
+ int value_len = 0;
int ret;
u32 size;
- u32 int_value;
+ u32 int_value = 0;
int elem;
int reqs;
int eloc;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index 3f7b9fe857f1..5e833ea0c5e3 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -227,7 +227,7 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
int value_len;
int ret;
u32 size;
- u32 int_value;
+ u32 int_value = 0;
int elem;
int reqs;
int eloc;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
index c9e124af170e..e0ecdfca4def 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
@@ -133,7 +133,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
char *str_value = NULL;
int value_len;
int ret = 0;
- u32 int_value;
+ u32 int_value = 0;
int elem;
int reqs;
int eloc;
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 3/8] hp-bioscfg: Replace the word HACK from source code
2023-07-31 20:31 [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Jorge Lopez
2023-07-31 20:31 ` [PATCH 1/8] hp-bioscfg: Fix memory leaks in attribute packages Jorge Lopez
2023-07-31 20:31 ` [PATCH 2/8] hp-bioscfg: Fix uninitialized variable errors Jorge Lopez
@ 2023-07-31 20:31 ` Jorge Lopez
2023-07-31 20:31 ` [PATCH 4/8] hp-bioscfg: Change how prerequisites size is evaluated Jorge Lopez
` (6 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Jorge Lopez @ 2023-07-31 20:31 UTC (permalink / raw)
To: hdegoede, platform-driver-x86, linux-kernel, thomas,
ilpo.jarvinen, dan.carpenter
Replace the word 'HACK' with 'step' from source code
Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
---
Based on the latest platform-drivers-x86.git/for-next
---
drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c | 4 ++--
drivers/platform/x86/hp/hp-bioscfg/int-attributes.c | 2 +-
drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c | 4 ++--
drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c | 4 ++--
drivers/platform/x86/hp/hp-bioscfg/string-attributes.c | 2 +-
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
index 7f77963cd7fa..50855ff48926 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
@@ -195,7 +195,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
/*
- * This HACK is needed to keep the expected
+ * This step is needed to keep the expected
* element list pointing to the right obj[elem].type
* when the size is zero. PREREQUISITES
* object is omitted by BIOS when the size is
@@ -243,7 +243,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
pr_warn("Possible number values size value exceeded the maximum number of elements supported or data may be malformed\n");
/*
- * This HACK is needed to keep the expected
+ * This step is needed to keep the expected
* element list pointing to the right obj[elem].type
* when the size is zero. POSSIBLE_VALUES
* object is omitted by BIOS when the size is zero.
diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
index 97f88e0ef0cc..a45919616fa8 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
@@ -211,7 +211,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
if (integer_data->common.prerequisites_size > MAX_PREREQUISITES_SIZE)
pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
/*
- * This HACK is needed to keep the expected
+ * This step is needed to keep the expected
* element list pointing to the right obj[elem].type
* when the size is zero. PREREQUISITES
* object is omitted by BIOS when the size is
diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index 89e67db733eb..aba7d26b54b2 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -207,7 +207,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
/*
- * This HACK is needed to keep the expected
+ * This step is needed to keep the expected
* element list pointing to the right obj[elem].type
* when the size is zero. PREREQUISITES
* object is omitted by BIOS when the size is
@@ -245,7 +245,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
if (int_value > MAX_ELEMENTS_SIZE)
pr_warn("Ordered List size value exceeded the maximum number of elements supported or data may be malformed\n");
/*
- * This HACK is needed to keep the expected
+ * This step is needed to keep the expected
* element list pointing to the right obj[elem].type
* when the size is zero. ORD_LIST_ELEMENTS
* object is omitted by BIOS when the size is
diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index 5e833ea0c5e3..3840380a5aee 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -293,7 +293,7 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
if (int_value > MAX_PREREQUISITES_SIZE)
pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
- /* This HACK is needed to keep the expected
+ /* This step is needed to keep the expected
* element list pointing to the right obj[elem].type
* when the size is zero. PREREQUISITES
* object is omitted by BIOS when the size is
@@ -337,7 +337,7 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
if (int_value > MAX_ENCODINGS_SIZE)
pr_warn("Password Encoding size value exceeded the maximum number of elements supported or data may be malformed\n");
- /* This HACK is needed to keep the expected
+ /* This step is needed to keep the expected
* element list pointing to the right obj[elem].type
* when the size is zero. PSWD_ENCODINGS
* object is omitted by BIOS when the size is
diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
index e0ecdfca4def..378cedb502ba 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
@@ -203,7 +203,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
if (string_data->common.prerequisites_size > MAX_PREREQUISITES_SIZE)
pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
/*
- * This HACK is needed to keep the expected
+ * This step is needed to keep the expected
* element list pointing to the right obj[elem].type
* when the size is zero. PREREQUISITES
* object is omitted by BIOS when the size is
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 4/8] hp-bioscfg: Change how prerequisites size is evaluated
2023-07-31 20:31 [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Jorge Lopez
` (2 preceding siblings ...)
2023-07-31 20:31 ` [PATCH 3/8] hp-bioscfg: Replace the word HACK from source code Jorge Lopez
@ 2023-07-31 20:31 ` Jorge Lopez
2023-07-31 20:31 ` [PATCH 5/8] hp-bioscfg: Change how order list " Jorge Lopez
` (5 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Jorge Lopez @ 2023-07-31 20:31 UTC (permalink / raw)
To: hdegoede, platform-driver-x86, linux-kernel, thomas,
ilpo.jarvinen, dan.carpenter
Update steps taken to evaluate prerequisites size value
Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
---
Based on the latest platform-drivers-x86.git/for-next
---
drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c | 6 ++++--
drivers/platform/x86/hp/hp-bioscfg/int-attributes.c | 6 +++++-
drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c | 6 ++++--
drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c | 6 ++++--
drivers/platform/x86/hp/hp-bioscfg/string-attributes.c | 6 ++++--
5 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
index 50855ff48926..89bb039a8a3c 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
@@ -190,9 +190,11 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
enum_data->common.sequence = int_value;
break;
case PREREQUISITES_SIZE:
- enum_data->common.prerequisites_size = int_value;
- if (int_value > MAX_PREREQUISITES_SIZE)
+ if (int_value > MAX_PREREQUISITES_SIZE) {
pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
+ int_value = MAX_PREREQUISITES_SIZE;
+ }
+ enum_data->common.prerequisites_size = int_value;
/*
* This step is needed to keep the expected
diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
index a45919616fa8..86b7ac63fec2 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
@@ -208,8 +208,12 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
integer_data->common.sequence = int_value;
break;
case PREREQUISITES_SIZE:
- if (integer_data->common.prerequisites_size > MAX_PREREQUISITES_SIZE)
+ if (int_value > MAX_PREREQUISITES_SIZE) {
pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
+ int_value = MAX_PREREQUISITES_SIZE;
+ }
+ integer_data->common.prerequisites_size = int_value;
+
/*
* This step is needed to keep the expected
* element list pointing to the right obj[elem].type
diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index aba7d26b54b2..71f588cbdf88 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -202,9 +202,11 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
ordered_list_data->common.sequence = int_value;
break;
case PREREQUISITES_SIZE:
- ordered_list_data->common.prerequisites_size = int_value;
- if (int_value > MAX_PREREQUISITES_SIZE)
+ if (int_value > MAX_PREREQUISITES_SIZE) {
pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
+ int_value = MAX_PREREQUISITES_SIZE;
+ }
+ ordered_list_data->common.prerequisites_size = int_value;
/*
* This step is needed to keep the expected
diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index 3840380a5aee..afb5190afc03 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -289,9 +289,11 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
password_data->common.sequence = int_value;
break;
case PREREQUISITES_SIZE:
- password_data->common.prerequisites_size = int_value;
- if (int_value > MAX_PREREQUISITES_SIZE)
+ if (int_value > MAX_PREREQUISITES_SIZE) {
pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
+ int_value = MAX_PREREQUISITES_SIZE;
+ }
+ password_data->common.prerequisites_size = int_value;
/* This step is needed to keep the expected
* element list pointing to the right obj[elem].type
diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
index 378cedb502ba..f0c20070094d 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
@@ -198,10 +198,12 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
string_data->common.sequence = int_value;
break;
case PREREQUISITES_SIZE:
+ if (int_value > MAX_PREREQUISITES_SIZE) {
+ pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
+ int_value = MAX_PREREQUISITES_SIZE;
+ }
string_data->common.prerequisites_size = int_value;
- if (string_data->common.prerequisites_size > MAX_PREREQUISITES_SIZE)
- pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
/*
* This step is needed to keep the expected
* element list pointing to the right obj[elem].type
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 5/8] hp-bioscfg: Change how order list size is evaluated
2023-07-31 20:31 [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Jorge Lopez
` (3 preceding siblings ...)
2023-07-31 20:31 ` [PATCH 4/8] hp-bioscfg: Change how prerequisites size is evaluated Jorge Lopez
@ 2023-07-31 20:31 ` Jorge Lopez
2023-07-31 20:31 ` [PATCH 6/8] hp-bioscfg: Change how enum possible values " Jorge Lopez
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Jorge Lopez @ 2023-07-31 20:31 UTC (permalink / raw)
To: hdegoede, platform-driver-x86, linux-kernel, thomas,
ilpo.jarvinen, dan.carpenter
Update steps how order list size is evaluated
Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
---
Based on the latest platform-drivers-x86.git/for-next
---
drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index 71f588cbdf88..3b073910b430 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -243,9 +243,12 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
break;
case ORD_LIST_SIZE:
+ if (int_value > MAX_ELEMENTS_SIZE) {
+ pr_warn("Order List size value exceeded the maximum number of elements supported or data may be malformed\n");
+ int_value = MAX_ELEMENTS_SIZE;
+ }
ordered_list_data->elements_size = int_value;
- if (int_value > MAX_ELEMENTS_SIZE)
- pr_warn("Ordered List size value exceeded the maximum number of elements supported or data may be malformed\n");
+
/*
* This step is needed to keep the expected
* element list pointing to the right obj[elem].type
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 6/8] hp-bioscfg: Change how enum possible values size is evaluated
2023-07-31 20:31 [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Jorge Lopez
` (4 preceding siblings ...)
2023-07-31 20:31 ` [PATCH 5/8] hp-bioscfg: Change how order list " Jorge Lopez
@ 2023-07-31 20:31 ` Jorge Lopez
2023-07-31 20:31 ` [PATCH 7/8] hp-bioscfg: Change how password encoding " Jorge Lopez
` (3 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Jorge Lopez @ 2023-07-31 20:31 UTC (permalink / raw)
To: hdegoede, platform-driver-x86, linux-kernel, thomas,
ilpo.jarvinen, dan.carpenter
Updates steps how enum possible values size is evaluated
Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
---
Based on the latest platform-drivers-x86.git/for-next
---
drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
index 89bb039a8a3c..a2402d31c146 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
@@ -240,9 +240,11 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
str_value, sizeof(enum_data->current_value));
break;
case ENUM_SIZE:
- enum_data->possible_values_size = int_value;
- if (int_value > MAX_VALUES_SIZE)
+ if (int_value > MAX_VALUES_SIZE) {
pr_warn("Possible number values size value exceeded the maximum number of elements supported or data may be malformed\n");
+ int_value = MAX_VALUES_SIZE;
+ }
+ enum_data->possible_values_size = int_value;
/*
* This step is needed to keep the expected
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 7/8] hp-bioscfg: Change how password encoding size is evaluated
2023-07-31 20:31 [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Jorge Lopez
` (5 preceding siblings ...)
2023-07-31 20:31 ` [PATCH 6/8] hp-bioscfg: Change how enum possible values " Jorge Lopez
@ 2023-07-31 20:31 ` Jorge Lopez
2023-07-31 20:31 ` [PATCH 8/8] hp-bioscfg: Remove duplicate use of variable in inner loop Jorge Lopez
` (2 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Jorge Lopez @ 2023-07-31 20:31 UTC (permalink / raw)
To: hdegoede, platform-driver-x86, linux-kernel, thomas,
ilpo.jarvinen, dan.carpenter
Update steps how password encoding size is evaluated
Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
---
Based on the latest platform-drivers-x86.git/for-next
---
drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index afb5190afc03..03d0188804ba 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -335,9 +335,12 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
password_data->max_password_length = int_value;
break;
case PSWD_SIZE:
- password_data->encodings_size = int_value;
- if (int_value > MAX_ENCODINGS_SIZE)
+
+ if (int_value > MAX_ENCODINGS_SIZE) {
pr_warn("Password Encoding size value exceeded the maximum number of elements supported or data may be malformed\n");
+ int_value = MAX_ENCODINGS_SIZE;
+ }
+ password_data->encodings_size = int_value;
/* This step is needed to keep the expected
* element list pointing to the right obj[elem].type
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 8/8] hp-bioscfg: Remove duplicate use of variable in inner loop
2023-07-31 20:31 [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Jorge Lopez
` (6 preceding siblings ...)
2023-07-31 20:31 ` [PATCH 7/8] hp-bioscfg: Change how password encoding " Jorge Lopez
@ 2023-07-31 20:31 ` Jorge Lopez
2023-08-01 13:35 ` [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Dan Carpenter
2023-08-07 11:38 ` Hans de Goede
9 siblings, 0 replies; 15+ messages in thread
From: Jorge Lopez @ 2023-07-31 20:31 UTC (permalink / raw)
To: hdegoede, platform-driver-x86, linux-kernel, thomas,
ilpo.jarvinen, dan.carpenter
Replace use of same variable in inner loop.
Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
---
Based on the latest platform-drivers-x86.git/for-next
---
.../x86/hp/hp-bioscfg/order-list-attributes.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index 3b073910b430..cffc1c9ba3e7 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -136,6 +136,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
u32 size;
u32 int_value = 0;
int elem;
+ int olist_elem;
int reqs;
int eloc;
char *tmpstr = NULL;
@@ -147,10 +148,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
if (!order_obj)
return -EINVAL;
- for (elem = 1, eloc = 1; elem < order_obj_count; elem++, eloc++) {
- /* ONLY look at the first ORDERED_ELEM_CNT elements */
- if (eloc == ORD_ELEM_CNT)
- goto exit_list;
+ for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT; elem++, eloc++) {
switch (order_obj[elem].type) {
case ACPI_TYPE_STRING:
@@ -277,10 +275,10 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
tmpstr,
sizeof(ordered_list_data->elements[0]));
- for (elem = 1; elem < MAX_ELEMENTS_SIZE && part; elem++) {
- strscpy(ordered_list_data->elements[elem],
+ for (olist_elem = 1; olist_elem < MAX_ELEMENTS_SIZE && part; olist_elem++) {
+ strscpy(ordered_list_data->elements[olist_elem],
part,
- sizeof(ordered_list_data->elements[elem]));
+ sizeof(ordered_list_data->elements[olist_elem]));
part = strsep(&part_tmp, SEMICOLON_SEP);
}
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup
2023-07-31 20:31 [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Jorge Lopez
` (7 preceding siblings ...)
2023-07-31 20:31 ` [PATCH 8/8] hp-bioscfg: Remove duplicate use of variable in inner loop Jorge Lopez
@ 2023-08-01 13:35 ` Dan Carpenter
2023-08-01 14:52 ` Jorge Lopez
2023-08-07 11:38 ` Hans de Goede
9 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2023-08-01 13:35 UTC (permalink / raw)
To: Jorge Lopez
Cc: hdegoede, platform-driver-x86, linux-kernel, thomas, ilpo.jarvinen
These are fine. We still need to do something like this. Also we could
just get rid of value_len completely. Nothing uses it.
diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index cffc1c9ba3e77..6ba0e49e787ec 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -264,7 +264,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
* Ordered list data is stored in hex and comma separated format
* Convert the data and split it to show each element
*/
- ret = hp_convert_hexstr_to_str(str_value, value_len, &tmpstr, &tmp_len);
+ ret = hp_convert_hexstr_to_str(str_value, size, &tmpstr, &tmp_len);
if (ret)
goto exit_list;
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup
2023-08-01 13:35 ` [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Dan Carpenter
@ 2023-08-01 14:52 ` Jorge Lopez
2023-08-01 15:04 ` Dan Carpenter
0 siblings, 1 reply; 15+ messages in thread
From: Jorge Lopez @ 2023-08-01 14:52 UTC (permalink / raw)
To: Dan Carpenter
Cc: hdegoede, platform-driver-x86, linux-kernel, thomas, ilpo.jarvinen
I will submit a new patch replacing 'value_len' for 'size' in line 267
as indicated.
'value_len' is utilized earlier in the code so we cannot remove it
completely from the function.
On Tue, Aug 1, 2023 at 8:35 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> These are fine. We still need to do something like this. Also we could
> just get rid of value_len completely. Nothing uses it.
>
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> index cffc1c9ba3e77..6ba0e49e787ec 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> @@ -264,7 +264,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
> * Ordered list data is stored in hex and comma separated format
> * Convert the data and split it to show each element
> */
> - ret = hp_convert_hexstr_to_str(str_value, value_len, &tmpstr, &tmp_len);
> + ret = hp_convert_hexstr_to_str(str_value, size, &tmpstr, &tmp_len);
> if (ret)
> goto exit_list;
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup
2023-08-01 14:52 ` Jorge Lopez
@ 2023-08-01 15:04 ` Dan Carpenter
2023-08-01 15:10 ` Jorge Lopez
0 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2023-08-01 15:04 UTC (permalink / raw)
To: Jorge Lopez
Cc: hdegoede, platform-driver-x86, linux-kernel, thomas, ilpo.jarvinen
On Tue, Aug 01, 2023 at 09:52:05AM -0500, Jorge Lopez wrote:
> I will submit a new patch replacing 'value_len' for 'size' in line 267
> as indicated.
> 'value_len' is utilized earlier in the code so we cannot remove it
> completely from the function.
>
After replacing size then it looks like this.
$ grep value_len drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
int value_len = 0;
&str_value, &value_len);
&str_value, &value_len);
It's a write only variable.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup
2023-08-01 15:04 ` Dan Carpenter
@ 2023-08-01 15:10 ` Jorge Lopez
2023-08-01 15:36 ` Dan Carpenter
0 siblings, 1 reply; 15+ messages in thread
From: Jorge Lopez @ 2023-08-01 15:10 UTC (permalink / raw)
To: Dan Carpenter
Cc: hdegoede, platform-driver-x86, linux-kernel, thomas, ilpo.jarvinen
Ok. Thanks for the clarification. I will remove 'value_len' and
replace all its references with 'size'.
On Tue, Aug 1, 2023 at 10:04 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Tue, Aug 01, 2023 at 09:52:05AM -0500, Jorge Lopez wrote:
> > I will submit a new patch replacing 'value_len' for 'size' in line 267
> > as indicated.
> > 'value_len' is utilized earlier in the code so we cannot remove it
> > completely from the function.
> >
>
> After replacing size then it looks like this.
>
> $ grep value_len drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> int value_len = 0;
> &str_value, &value_len);
> &str_value, &value_len);
>
> It's a write only variable.
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup
2023-08-01 15:10 ` Jorge Lopez
@ 2023-08-01 15:36 ` Dan Carpenter
0 siblings, 0 replies; 15+ messages in thread
From: Dan Carpenter @ 2023-08-01 15:36 UTC (permalink / raw)
To: Jorge Lopez
Cc: hdegoede, platform-driver-x86, linux-kernel, thomas, ilpo.jarvinen
On Tue, Aug 01, 2023 at 10:10:05AM -0500, Jorge Lopez wrote:
> Ok. Thanks for the clarification. I will remove 'value_len' and
> replace all its references with 'size'.
Ugh... No, that's worse than the original. Just leave value_len as is
in that case. :P
regards,
dan carpenter
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup
2023-07-31 20:31 [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Jorge Lopez
` (8 preceding siblings ...)
2023-08-01 13:35 ` [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup Dan Carpenter
@ 2023-08-07 11:38 ` Hans de Goede
9 siblings, 0 replies; 15+ messages in thread
From: Hans de Goede @ 2023-08-07 11:38 UTC (permalink / raw)
To: Jorge Lopez, platform-driver-x86, linux-kernel, thomas,
ilpo.jarvinen, dan.carpenter
Hi,
On 7/31/23 22:31, Jorge Lopez wrote:
> Submit individual patches to address memory leaks and uninitialized
> variable errors.
> Addressed several review comments making the source code more readable.
> Removed duplicate use of variable in inner loop.
>
> Changes were tested with a HP EliteBook x360 1030 G3
>
> Jorge Lopez (8):
> hp-bioscfg: Fix memory leaks in attribute packages
> hp-bioscfg: Fix uninitialized variable errors
> hp-bioscfg: Replace the word HACK from source code
> hp-bioscfg: Change how prerequisites size is evaluated
> hp-bioscfg: Change how order list size is evaluated
> hp-bioscfg: Change how enum possible values size is evaluated
> hp-bioscfg: Change how password encoding size is evaluated
> hp-bioscfg: Remove duplicate use of variable in inner loop
>
> .../x86/hp/hp-bioscfg/enum-attributes.c | 24 ++++++++----
> .../x86/hp/hp-bioscfg/int-attributes.c | 15 +++++--
> .../x86/hp/hp-bioscfg/order-list-attributes.c | 39 ++++++++++++-------
> .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 27 +++++++++----
> .../x86/hp/hp-bioscfg/string-attributes.c | 13 +++++--
> 5 files changed, 82 insertions(+), 36 deletions(-)
Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
^ permalink raw reply [flat|nested] 15+ messages in thread