On Fri, May 16, 2025 at 08:40:20PM +0300, [email protected] wrote: > From: Anton Moryakov <[email protected]> > > 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 <[email protected]>" > --- > 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! -- Tom
signature.asc
Description: PGP signature
