[PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup

[PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup

[Jorge Lopez]

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(-)

--
2.34.1

[PATCH 1/8] hp-bioscfg: Fix memory leaks in attribute packages

[Jorge Lopez]

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

[PATCH 2/8] hp-bioscfg: Fix uninitialized variable errors

[Jorge Lopez]

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

[PATCH 3/8] hp-bioscfg: Replace the word HACK from source code

[Jorge Lopez]

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

[PATCH 4/8] hp-bioscfg: Change how prerequisites size is evaluated

[Jorge Lopez]

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

[PATCH 5/8] hp-bioscfg: Change how order list size is evaluated

[Jorge Lopez]

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

[PATCH 6/8] hp-bioscfg: Change how enum possible values size is evaluated

[Jorge Lopez]

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

[PATCH 7/8] hp-bioscfg: Change how password encoding size is evaluated

[Jorge Lopez]

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

[PATCH 8/8] hp-bioscfg: Remove duplicate use of variable in inner loop

[Jorge Lopez]

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

Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup

[Dan Carpenter]

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;
 

Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup

[Jorge Lopez]

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;
>

Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup

[Dan Carpenter]

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

Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup

[Jorge Lopez]

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
>

Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup

[Dan Carpenter]

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

Re: [PATCH 0/8] hp-bioscfg: Overall fixes and code cleanup

[Hans de Goede]

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