Am 4. Juni 2025 20:24:01 MESZ schrieb Tom Rini <tr...@konsulko.com>:
>On Fri, May 16, 2025 at 08:40:20PM +0300, ant.v.morya...@gmail.com wrote:
>
>> From: Anton Moryakov <ant.v.morya...@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.morya...@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
