linux-kernel - Re: [PATCH] media: chips-media: wave5: Fix Potential Probe Resource Leak

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fb603b6159766bbba606260d44080733786d290f.camel@collabora.com>
Date: Thu, 11 Dec 2025 10:04:25 -0500
From: Nicolas Dufresne <nicolas.dufresne@...labora.com>
To: "jackson.lee" <jackson.lee@...psnmedia.com>, Brandon Brnich	
 <b-brnich@...com>, Nas Chung <nas.chung@...psnmedia.com>, Mauro Carvalho
 Chehab	 <mchehab@...nel.org>, "linux-media@...r.kernel.org"	
 <linux-media@...r.kernel.org>, "linux-kernel@...r.kernel.org"	
 <linux-kernel@...r.kernel.org>
Cc: Darren Etheridge <detheridge@...com>
Subject: Re: [PATCH] media: chips-media: wave5: Fix Potential Probe Resource
 Leak

Hi,

Le mardi 02 décembre 2025 à 02:06 +0000, jackson.lee a écrit :
> Hi Brandon
> 
> 
> > -----Original Message-----
> > From: Brandon Brnich <b-brnich@...com>
> > Sent: Thursday, November 20, 2025 6:32 AM
> > To: Nas Chung <nas.chung@...psnmedia.com>; jackson.lee
> > <jackson.lee@...psnmedia.com>; Mauro Carvalho Chehab <mchehab@...nel.org>;
> > linux-media@...r.kernel.org; linux-kernel@...r.kernel.org; Nicolas
> > Dufresne <nicolas.dufresne@...labora.com>
> > Cc: Darren Etheridge <detheridge@...com>; Brandon Brnich <b-brnich@...com>
> > Subject: [PATCH] media: chips-media: wave5: Fix Potential Probe Resource
> > Leak
> > 
> > After kthread creation during probe sequence, a handful of other failures
> > could occur. If this were to happen, the kthread is never explicitly
> > deleted which results in a resource leak. Add explicit cleanup of this
> > resource.
> > 
> > Signed-off-by: Brandon Brnich <b-brnich@...com>
> > ---
> > 
> > I am aware that all the dev attributes would be freed since it is
> > allocated using the devm_* framework. But I did not believe that this
> > framework would recursively free the thread and stop the timer. These
> > would just be dangling resources unable to get killed unless deliberately
> > removed in the probe function.
> > 
> >  drivers/media/platform/chips-media/wave5/wave5-vpu.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > index e1715d3f43b0..f027b4ac775a 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > @@ -339,6 +339,11 @@ static int wave5_vpu_probe(struct platform_device
> > *pdev)
> >  	v4l2_device_unregister(&dev->v4l2_dev);
> >  err_vdi_release:
> >  	wave5_vdi_release(&pdev->dev);
> > +
> > +	if (dev->irq < 0) {
> > +		kthread_destroy_worker(dev->worker);
> > +		hrtimer_cancel(&dev->hrtimer);
> > +	}
> 
> I'd like to change the above to as below.
> I think we have to distinguish failure between registering IRQ handler and
> registering v4l2_device_register.
> 
> err_irq_release:
> 	if (dev->irq < 0) {
> 		kthread_destroy_worker(dev->worker);
> 		hrtimer_cancel(&dev->hrtimer);
> 	}
> err_vdi_release:

That's seems more then just a suggestion, I see that err_vdi_release: is reached
on worker creation failure. Checking the kthread code, this will cause a use
after free instead of a leak.

An additional question, aren't we are supposed to also cleanup irq_thread ? We
have this code being introduced in the remove function now:


		if (dev->irq_thread) {
			kthread_stop(dev->irq_thread);
			up(&dev->irq_sem);
			dev->irq_thread = NULL;
		}



regards,
Nicolas


> 
> thanks
> Jackson
> 
> 
> >  err_clk_dis:
> >  	clk_bulk_disable_unprepare(dev->num_clks, dev->clks);
> >  err_reset_assert:
> > --
> > 2.34.1
> 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)