Re: [PATCH v3 3/4] platform/x86: dell_rbu: Stop overwriting data buffer

[stuart hayes <stuart.w.hayes@gmail.com>]

On 5/30/2025 3:03 AM, Ilpo Järvinen wrote:
> On Thu, 29 May 2025, Stuart Hayes wrote:
> 
>> 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.
>>
>> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> 
> This still doesn't have Fixes tag? If it writes part the buffer, there
> certainly should be one in this one. Did you perhaps add it to a wrong
> patch?
> 

This fixes a bug that's existed for the life of the driver. Should I use 
the patch that introduced the driver for "Fixes:"?

I don't think this has ever caused a problem, because, as far as I know, 
the Dell BIOS update program is the only user of this module, and it 
uses 4096 as the packet size. Because the packet data buffers are 
allocated with...

	ordernum = get_order(size);
	image_update_buffer =
		(unsigned char *)__get_free_pages(GRP_DMA32, ordernum);

...this will always allocate one full page for every packet data buffer.

I just happened to notice that the driver could overwrite the end of the 
buffer for the last packet if a packet size of more than 4096 was used, 
so I thought I'd fix that.

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