[PATCH v4 0/5] platform/x86: dell_rbu: Packet data fixes

[PATCH v4 0/5] platform/x86: dell_rbu: Packet data fixes

[Stuart Hayes]

Make fixes related to packet data list and buffers, and fix a sparse
warning.

Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>

---
v4:
 -  Split patch relating to list into a fix patch and a refactor patch
 -  Add better explanation and "Fixes:" to buffer overwrite patch

v3:
 -  Cosmetic changes to commit messages
 -  Bump module version

v2:
 -  Add "__must_hold" to create_packet to remove sparse warning.
 -  Make struct packet_data_list static.

Stuart Hayes (5):
  platform/x86: dell_rbu: Fix lock context warning
  platform/x86: dell_rbu: Fix list usage
  platform/x86: dell_rbu: Remove unused struct
  platform/x86: dell_rbu: Stop overwriting data buffer
  platform/x86: dell_rbu: Bump version

 drivers/platform/x86/dell/dell_rbu.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

-- 
2.47.1

[PATCH v4 1/5] platform/x86: dell_rbu: Fix lock context warning

[Stuart Hayes]

Fix a sparse lock context warning.

Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
---
 drivers/platform/x86/dell/dell_rbu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell/dell_rbu.c b/drivers/platform/x86/dell/dell_rbu.c
index e30ca325938c..7b019fb72e86 100644
--- a/drivers/platform/x86/dell/dell_rbu.c
+++ b/drivers/platform/x86/dell/dell_rbu.c
@@ -91,7 +91,7 @@ static void init_packet_head(void)
 	rbu_data.imagesize = 0;
 }
 
-static int create_packet(void *data, size_t length)
+static int create_packet(void *data, size_t length) __must_hold(&rbu_data.lock)
 {
 	struct packet_data *newpacket;
 	int ordernum = 0;
-- 
2.47.1

[PATCH v4 2/5] platform/x86: dell_rbu: Fix list usage

[Stuart Hayes]

Pass the correct list head to list_for_each_entry*() when looping through
the packet list.

Without this patch, reading the packet data via sysfs will show the data
incorrectly (because it starts at the wrong packet), and clearing the
packet list will result in a NULL pointer dereference.

Fixes: d19f359fbdc6 ("platform/x86: dell_rbu: don't open code list_for_each_entry*()")
Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
---
 drivers/platform/x86/dell/dell_rbu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/dell/dell_rbu.c b/drivers/platform/x86/dell/dell_rbu.c
index 7b019fb72e86..722979b19e0e 100644
--- a/drivers/platform/x86/dell/dell_rbu.c
+++ b/drivers/platform/x86/dell/dell_rbu.c
@@ -292,7 +292,7 @@ static int packet_read_list(char *data, size_t * pread_length)
 	remaining_bytes = *pread_length;
 	bytes_read = rbu_data.packet_read_count;
 
-	list_for_each_entry(newpacket, (&packet_data_head.list)->next, list) {
+	list_for_each_entry(newpacket, &packet_data_head.list, list) {
 		bytes_copied = do_packet_read(pdest, newpacket,
 			remaining_bytes, bytes_read, &temp_count);
 		remaining_bytes -= bytes_copied;
@@ -315,7 +315,7 @@ static void packet_empty_list(void)
 {
 	struct packet_data *newpacket, *tmp;
 
-	list_for_each_entry_safe(newpacket, tmp, (&packet_data_head.list)->next, list) {
+	list_for_each_entry_safe(newpacket, tmp, &packet_data_head.list, list) {
 		list_del(&newpacket->list);
 
 		/*
-- 
2.47.1

[PATCH v4 3/5] platform/x86: dell_rbu: Remove unused struct

[Stuart Hayes]

Stop using an entire struct packet_data just for its embedded list_head.

Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
---
 drivers/platform/x86/dell/dell_rbu.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/dell/dell_rbu.c b/drivers/platform/x86/dell/dell_rbu.c
index 722979b19e0e..c03d4d55fcc1 100644
--- a/drivers/platform/x86/dell/dell_rbu.c
+++ b/drivers/platform/x86/dell/dell_rbu.c
@@ -77,14 +77,14 @@ struct packet_data {
 	int ordernum;
 };
 
-static struct packet_data packet_data_head;
+static struct list_head packet_data_list;
 
 static struct platform_device *rbu_device;
 static int context;
 
 static void init_packet_head(void)
 {
-	INIT_LIST_HEAD(&packet_data_head.list);
+	INIT_LIST_HEAD(&packet_data_list);
 	rbu_data.packet_read_count = 0;
 	rbu_data.num_packets = 0;
 	rbu_data.packetsize = 0;
@@ -183,7 +183,7 @@ static int create_packet(void *data, size_t length) __must_hold(&rbu_data.lock)
 
 	/* initialize the newly created packet headers */
 	INIT_LIST_HEAD(&newpacket->list);
-	list_add_tail(&newpacket->list, &packet_data_head.list);
+	list_add_tail(&newpacket->list, &packet_data_list);
 
 	memcpy(newpacket->data, data, length);
 
@@ -292,7 +292,7 @@ static int packet_read_list(char *data, size_t * pread_length)
 	remaining_bytes = *pread_length;
 	bytes_read = rbu_data.packet_read_count;
 
-	list_for_each_entry(newpacket, &packet_data_head.list, list) {
+	list_for_each_entry(newpacket, &packet_data_list, list) {
 		bytes_copied = do_packet_read(pdest, newpacket,
 			remaining_bytes, bytes_read, &temp_count);
 		remaining_bytes -= bytes_copied;
@@ -315,7 +315,7 @@ static void packet_empty_list(void)
 {
 	struct packet_data *newpacket, *tmp;
 
-	list_for_each_entry_safe(newpacket, tmp, &packet_data_head.list, list) {
+	list_for_each_entry_safe(newpacket, tmp, &packet_data_list, list) {
 		list_del(&newpacket->list);
 
 		/*
-- 
2.47.1

[PATCH v4 4/5] platform/x86: dell_rbu: Stop overwriting data buffer

[Stuart Hayes]

The dell_rbu driver will use memset() to clear the data held by each
packet when it is no longer needed (when the driver is unloaded, the
packet size is changed, etc).

The amount of memory that is cleared (before this patch) is the normal
packet size. However, the last packet in the list may be smaller.

Fix this to only clear the memory actually used by each packet, to prevent
it from writing past the end of data buffer.

Because the packet data buffers are allocated with __get_free_pages() (in
page-sized increments), this bug could only result in a buffer being
overwritten when a packet size larger than one page is used. The only user
of the dell_rbu module should be the Dell BIOS update program, which uses
a packet size of 4096, so no issues should be seen without the patch, it
just blocks the possiblity.

Fixes: 6c54c28e69f2 ("[PATCH] dell_rbu: new Dell BIOS update driver")
Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
---
 drivers/platform/x86/dell/dell_rbu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell/dell_rbu.c b/drivers/platform/x86/dell/dell_rbu.c
index c03d4d55fcc1..7d5b26735a20 100644
--- a/drivers/platform/x86/dell/dell_rbu.c
+++ b/drivers/platform/x86/dell/dell_rbu.c
@@ -322,7 +322,7 @@ static void packet_empty_list(void)
 		 * zero out the RBU packet memory before freeing
 		 * to make sure there are no stale RBU packets left in memory
 		 */
-		memset(newpacket->data, 0, rbu_data.packetsize);
+		memset(newpacket->data, 0, newpacket->length);
 		set_memory_wb((unsigned long)newpacket->data,
 			1 << newpacket->ordernum);
 		free_pages((unsigned long) newpacket->data,
-- 
2.47.1

[PATCH v4 5/5] platform/x86: dell_rbu: Bump version

[Stuart Hayes]

Bump the module version.

Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
---
 drivers/platform/x86/dell/dell_rbu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell/dell_rbu.c b/drivers/platform/x86/dell/dell_rbu.c
index 7d5b26735a20..45c0a72e494a 100644
--- a/drivers/platform/x86/dell/dell_rbu.c
+++ b/drivers/platform/x86/dell/dell_rbu.c
@@ -45,7 +45,7 @@
 MODULE_AUTHOR("Abhay Salunke <abhay_salunke@dell.com>");
 MODULE_DESCRIPTION("Driver for updating BIOS image on DELL systems");
 MODULE_LICENSE("GPL");
-MODULE_VERSION("3.2");
+MODULE_VERSION("3.3");
 
 #define BIOS_SCAN_LIMIT 0xffffffff
 #define MAX_IMAGE_LENGTH 16
-- 
2.47.1

Re: [PATCH v4 3/5] platform/x86: dell_rbu: Remove unused struct

[Ilpo Järvinen]

On Mon, 9 Jun 2025, Stuart Hayes wrote:

> Stop using an entire struct packet_data just for its embedded list_head.
> 
> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>

Thanks for the update.

I've applied all but this patch 3 now into the review-ilpo-fixes branch. 
As this patch is for-next material and I can only apply it once I've 
merged the fixes branch into the for-next branch somewhere later in the 
cycle (it'll sit in patchwork in the meantime, so I won't be forget
it :-)).

--
 i.

> ---
>  drivers/platform/x86/dell/dell_rbu.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell_rbu.c b/drivers/platform/x86/dell/dell_rbu.c
> index 722979b19e0e..c03d4d55fcc1 100644
> --- a/drivers/platform/x86/dell/dell_rbu.c
> +++ b/drivers/platform/x86/dell/dell_rbu.c
> @@ -77,14 +77,14 @@ struct packet_data {
>  	int ordernum;
>  };
>  
> -static struct packet_data packet_data_head;
> +static struct list_head packet_data_list;
>  
>  static struct platform_device *rbu_device;
>  static int context;
>  
>  static void init_packet_head(void)
>  {
> -	INIT_LIST_HEAD(&packet_data_head.list);
> +	INIT_LIST_HEAD(&packet_data_list);
>  	rbu_data.packet_read_count = 0;
>  	rbu_data.num_packets = 0;
>  	rbu_data.packetsize = 0;
> @@ -183,7 +183,7 @@ static int create_packet(void *data, size_t length) __must_hold(&rbu_data.lock)
>  
>  	/* initialize the newly created packet headers */
>  	INIT_LIST_HEAD(&newpacket->list);
> -	list_add_tail(&newpacket->list, &packet_data_head.list);
> +	list_add_tail(&newpacket->list, &packet_data_list);
>  
>  	memcpy(newpacket->data, data, length);
>  
> @@ -292,7 +292,7 @@ static int packet_read_list(char *data, size_t * pread_length)
>  	remaining_bytes = *pread_length;
>  	bytes_read = rbu_data.packet_read_count;
>  
> -	list_for_each_entry(newpacket, &packet_data_head.list, list) {
> +	list_for_each_entry(newpacket, &packet_data_list, list) {
>  		bytes_copied = do_packet_read(pdest, newpacket,
>  			remaining_bytes, bytes_read, &temp_count);
>  		remaining_bytes -= bytes_copied;
> @@ -315,7 +315,7 @@ static void packet_empty_list(void)
>  {
>  	struct packet_data *newpacket, *tmp;
>  
> -	list_for_each_entry_safe(newpacket, tmp, &packet_data_head.list, list) {
> +	list_for_each_entry_safe(newpacket, tmp, &packet_data_list, list) {
>  		list_del(&newpacket->list);
>  
>  		/*
> 

Re: [PATCH v4 0/5] platform/x86: dell_rbu: Packet data fixes

[Ilpo Järvinen]

On Mon, 09 Jun 2025 13:46:54 -0500, Stuart Hayes wrote:

> Make fixes related to packet data list and buffers, and fix a sparse
> warning.
> 
> 


Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/5] platform/x86: dell_rbu: Fix lock context warning
      (no commit info)
[2/5] platform/x86: dell_rbu: Fix list usage
      (no commit info)
[3/5] platform/x86: dell_rbu: Remove unused struct
      commit: 56036d6af41a473a8129fc960a5ab3673eda13d5
[4/5] platform/x86: dell_rbu: Stop overwriting data buffer
      (no commit info)
[5/5] platform/x86: dell_rbu: Bump version
      (no commit info)

--
 i.