[PATCH] drivers: scsi: fix: memory leak in do_scsi_scan_one()

[Heinrich Schuchardt]

Am 4. Juni 2025 20:24:01 MESZ schrieb Tom Rini <trini at konsulko.com>:
>On Fri, May 16, 2025 at 08:40:20PM +0300, ant.v.moryakov at gmail.com wrote:
>
>> From: Anton Moryakov <ant.v.moryakov at gmail.com>
>> 
>> Free allocated name buffer when blk_create_devicef() fails to prevent
>> memory leak. After successful device creation, the name ownership is
>> transferred to the device structure and should not be freed manually.
>> 
>> Signed-off-by: Anton Moryakov <ant.v.moryakov at gmail.com>"
>> ---
>>  drivers/scsi/scsi.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
>> index cd0b84c0622..a9e364d3fdb 100644
>> --- a/drivers/scsi/scsi.c
>> +++ b/drivers/scsi/scsi.c
>> @@ -522,8 +522,10 @@ static int do_scsi_scan_one(struct udevice *dev, int id, int lun, bool verbose)
>>  		return log_msg_ret("pro", ret);
>>  
>>  	ret = bootdev_setup_for_sibling_blk(bdev, "scsi_bootdev");
>> -	if (ret)
>> +	if (ret) {
>> +		free(name); 
>>  		return log_msg_ret("bd", ret);
>> +	}
>>  
>>  	if (verbose) {
>>  		printf("  Device %d: ", bdesc->devnum);
>
>Your commit message and code changes do not match up. The free() here is
>at the end and not by the call to blk_create_devicef(). That said,
>looking at blk_create_devicef() and all of the callers, what we really
>need I think is to audit all of the callers and update/correct them. The
>third parameter, "name" is only used to print to the next string that's
>created, so an on-stack str[X], snprintf(...) is fine and what's usually
>done. The places calling sprintf(...) not snprintf should be updated for
>safety. The scsi case of allocating on stack and then strdup'ing that
>should be changed to just on stack. I would check with Heinrich and
>Ilias about the
>lib/efi_driver/efi_block_device.c::efi_bl_create_block_device() case to
>be clear that there's a good reason it's not on-stack. Thanks!
>

We should start with a proper documentation of blk_create_devicef() in blk.h describing the intended content of name and how it is used in the function. 

Afterwards we should fix
efi_driver: use blk_create_devicef()
<https://github.com/trini/u-boot/commit/640c6c6cbaafa1b049118d431cf218d9dce3cdd8>

Best regards 

Heinrich