[PATCH] mtd: spi: Add missing free in sf_get_bootflow

[Tom Rini]

On Fri, Feb 13, 2026 at 01:19:54PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 13 Feb 2026 at 11:01, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Fri, Feb 13, 2026 at 10:53:25AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Fri, 13 Feb 2026 at 09:57, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Tue, Dec 09, 2025 at 08:17:06AM -0600, Tom Rini wrote:
> > > > > On Tue, Dec 09, 2025 at 12:16:33PM +0200, Tudor Ambarus wrote:
> > > > > >
> > > > > >
> > > > > > On 12/9/25 11:56 AM, Francois Berder wrote:
> > > > > > > Free buf if spi_flash_read_dm fails.
> > > > > > >
> > > > > > > Signed-off-by: Francois Berder <fberder at outlook.fr>
> > > > > > > ---
> > > > > > >  drivers/mtd/spi/sf_bootdev.c | 4 +++-
> > > > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/mtd/spi/sf_bootdev.c b/drivers/mtd/spi/sf_bootdev.c
> > > > > > > index 017a74a3016..720faf51a1f 100644
> > > > > > > --- a/drivers/mtd/spi/sf_bootdev.c
> > > > > > > +++ b/drivers/mtd/spi/sf_bootdev.c
> > > > > > > @@ -35,8 +35,10 @@ static int sf_get_bootflow(struct udevice *dev, struct bootflow_iter *iter,
> > > > > > >
> > > > > > >   ret = spi_flash_read_dm(sf, env_get_hex("script_offset_f", 0),
> > > > > > >                           size, buf);
> > > > > > > - if (ret)
> > > > > > > + if (ret) {
> > > > > > > +         free(buf);
> > > > > > >           return log_msg_ret("cmd", -EINVAL);
> > > > > > > + }
> > > > > >
> > > > > >
> > > > > > can we use a devm alloc method to get rid of the extra free handling?
> > > > >
> > > > > No, because this isn't the kernel and devm alloc doesn't always free
> > > > > here. Making that no longer be the case (always freeing) is on my list
> > > > > to look at doing (drivers that use devm alloc always implys/select'ing
> > > > > DEVRES).
> > > >
> > > > We've since changed so that devm alloc does behave in U-Boot the same as
> > > > the kernel (except in xPL phases), so Tudor's suggestion is a good one.
> > >
> > > I still like the idea of having this patch so that others don't hit
> > > it. Does it show up as a leak on any code-analysis?
> >
> > I'm unsure what you're suggesting. If we can switch these to devm alloc
> > functions now that we match the expected API of the linux kernel
> > (outside of xPL where we don't care about leaks) we don't need to
> > manually track frees here.
> 
> Yes I understand that. But it still looks like a memory leak. Does
> coverity flag this as a memory leak?

What we have now, or what devm alloc does?

> Also, most boards do in fact leak, if this is correct:
> 
> ./tools/qconfig.py -f DEVRES DM_DEVICE_REMOVE
> 132 matches

That is not, no:
$ ./tools/qconfig.py -f DEVRES DM_DEVICE_REMOVE
1389 matches 

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20260213/da312121/attachment.sig>