Re: [PATCH] firmware/sysfb: Fix device reference count leak in sysfb_disable()

[Thomas Zimmermann]

Hi

Am 16.09.26 um 23:06 schrieb Deucher, Alexander:
> Public
>
> > -----Original Message-----
> > From: Wentao Liang <vulab@xxxxxxxxxxx>
> > Sent: Wednesday, September 16, 2026 3:38 AM
> > To: Deucher, Alexander <Alexander.Deucher@xxxxxxx>
> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; javierm@xxxxxxxxxx; linux-
> > kernel@xxxxxxxxxxxxxxx; tzimmermann@xxxxxxx; Wentao Liang
> > <vulab@xxxxxxxxxxx>; stable@xxxxxxxxxxxxxxx
> > Subject: [PATCH] firmware/sysfb: Fix device reference count leak in
> > sysfb_disable()
> >
> > In sysfb_disable(), parent is obtained via sysfb_parent_dev(si), which calls
> > screen_info_pci_dev(si) and returns a pointer to &pdev->dev with an acquired
> > reference count. However, sysfb_disable() does not release this reference
> > before returning, leading to a device reference leak.
> >
> > Fix this by checking if parent is not an ERR_PTR and calling
> > put_device(parent) before releasing the mutex.
> >
> > Fixes: b49420d6a1ae ("video/aperture: optionally match the device in
> > sysfb_disable()")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Wentao Liang <vulab@xxxxxxxxxxx>
> > ---
> >   drivers/firmware/sysfb.c | 9 ++++++---
> >   1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c index
> > 8833582c1883..11c5ce128a6c 100644
> > --- a/drivers/firmware/sysfb.c
> > +++ b/drivers/firmware/sysfb.c
> > @@ -71,9 +71,12 @@ void sysfb_disable(struct device *dev)
> >
> >        mutex_lock(&disable_lock);
> >        parent = sysfb_parent_dev(si);
> > -     if (!dev || !parent || dev == parent) {
> > -             sysfb_unregister();
> > -             disabled = true;
> > +     if (!IS_ERR(parent)) {
> Are there cases where sysfb_unregister() would need to be called even if parent returns an error?

It could be seen as a defensive measure. Usually we want to unregister 
the system framebuffer only if the native driver's device equals the 
parent. But if we fail to retrieve a parent, it might be better to 
unregister unconditionally.  Otherwise the sysfb driver might interfere 
with the native driver.

I think that would be a good idea.

Best regards
Thomas


> Alex
>
> > +             if (!dev || !parent || dev == parent) {
> > +                     sysfb_unregister();
> > +                     disabled = true;
> > +             }
> > +             put_device(parent);
> >        }
> >        mutex_unlock(&disable_lock);
> >   }
> > --
> > 2.34.1

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)