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

[Tom Rini]

On Fri, Feb 13, 2026 at 04:06:20PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 13 Feb 2026 at 13:29, Tom Rini <trini at konsulko.com> wrote:
> >
> > 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?
> 
> What we have now.
> 
> The code looks like it has a memory leak (without the free()) but I
> suspect you are saying that devres doesn't look like an allocation -
> that would be devm_kmalloc(), right? So I suspect coverity just
> ignores that since it isn't called malloc(), or it knows it is a
> special case.
> 
> >
> > > 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
> 
> OK, I wonder why so many boards enable these, given the code-size impact?

It was actually really minimal impact to gain expected functionality.
The question really then becomes can this code path here make use of
devm alloc functions, or not, and so needs to keep this path for free()
instead.

-- 
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/d7e8b9e6/attachment.sig>