All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: Huihui Huang <hhhuang@smu.edu.sg>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Bingbu Cao <bingbu.cao@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-media@vger.kernel.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] staging: media: ipu7: fix boot_config leak on queue_mem failure
Date: Fri, 17 Apr 2026 11:05:34 +0300	[thread overview]
Message-ID: <aeHpzkBvclNESSUv@stanley.mountain> (raw)
In-Reply-To: <aeHo2WOF1Rcp9zwf@stanley.mountain>

On Fri, Apr 17, 2026 at 11:01:29AM +0300, Dan Carpenter wrote:
> On Fri, Apr 17, 2026 at 03:39:39PM +0800, Huihui Huang wrote:
> > There is a memory leak in drivers/staging/media/ipu7/ipu7-boot.c.
> > 
> > In ipu7_boot_init_boot_config(), boot_config is allocated by
> > ipu7_dma_alloc(). If the second ipu7_dma_alloc() for queue_mem fails,
> > the function returns -ENOMEM without freeing the previously allocated
> > boot_config.
> > 
> > Add the missing ipu7_dma_free() call before returning on the error
> > path.
> > 
> > Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
> > ---
> > v2: Reword commit message in imperative mood. Remove unnecessary
> >     NULL assignment on the error path.
> > ---
> >  drivers/staging/media/ipu7/ipu7-boot.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/staging/media/ipu7/ipu7-boot.c b/drivers/staging/media/ipu7/ipu7-boot.c
> > index d7901ff78b38..495b3e05a9b1 100644
> > --- a/drivers/staging/media/ipu7/ipu7-boot.c
> > +++ b/drivers/staging/media/ipu7/ipu7-boot.c
> > @@ -263,6 +263,8 @@ int ipu7_boot_init_boot_config(struct ipu7_bus_device *adev,
> >  					   GFP_KERNEL, 0);
> >  	if (!syscom->queue_mem) {
> >  		dev_err(dev, "Failed to allocate queue memory.\n");
> > +		ipu7_dma_free(adev, adev->boot_config_size,
> > +			      adev->boot_config, adev->boot_config_dma_addr, 0);
> >  		return -ENOMEM;
> 
> Adding a free here leads to a double free.  It's the same issue.
> One magical cleanup function in the caller.
> 
> I haven't looked at this but I bet there are bugs in the error handling
> since magical cleanup functions are always buggy.

Btw, if you had kept the "adev->boot_config = NULL;" assignment that
you had in v1 then that would have prevented the double free since
ipu7_boot_release_boot_config() tests for that...  This information is
not useful to you at this point but I'm sure you will find it
frustrating.  :P

regards,
dan carpenter


  reply	other threads:[~2026-04-17  8:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-16  7:48 [PATCH] staging: media: ipu7: fix boot_config leak on queue_mem failure Huihui Huang
2026-04-17  7:39 ` [PATCH v2] " Huihui Huang
2026-04-17  8:01   ` Dan Carpenter
2026-04-17  8:05     ` Dan Carpenter [this message]
2026-04-17  8:46     ` Dan Carpenter
2026-04-17 14:22       ` [v2] " Huihui Huang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aeHpzkBvclNESSUv@stanley.mountain \
    --to=error27@gmail.com \
    --cc=bingbu.cao@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hhhuang@smu.edu.sg \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.