mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] firmware/sysfb: Fix device reference count leak in sysfb_disable()
@ 2026-09-16  7:38 Wentao Liang
  2026-09-16 21:06 ` Deucher, Alexander
  0 siblings, 1 reply; 3+ messages in thread
From: Wentao Liang @ 2026-09-16  7:38 UTC (permalink / raw)
  To: alexander.deucher
  Cc: dri-devel, javierm, linux-kernel, tzimmermann, Wentao Liang, stable

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@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 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)) {
+		if (!dev || !parent || dev == parent) {
+			sysfb_unregister();
+			disabled = true;
+		}
+		put_device(parent);
 	}
 	mutex_unlock(&disable_lock);
 }
-- 
2.34.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH] firmware/sysfb: Fix device reference count leak in sysfb_disable()
  2026-09-16  7:38 [PATCH] firmware/sysfb: Fix device reference count leak in sysfb_disable() Wentao Liang
@ 2026-09-16 21:06 ` Deucher, Alexander
  2026-09-17  6:50   ` Thomas Zimmermann
  0 siblings, 1 reply; 3+ messages in thread
From: Deucher, Alexander @ 2026-09-16 21:06 UTC (permalink / raw)
  To: Wentao Liang; +Cc: dri-devel, javierm, linux-kernel, tzimmermann, stable

Public

> -----Original Message-----
> From: Wentao Liang <vulab@iscas.ac.cn>
> Sent: Wednesday, September 16, 2026 3:38 AM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>
> Cc: dri-devel@lists.freedesktop.org; javierm@redhat.com; linux-
> kernel@vger.kernel.org; tzimmermann@suse.de; Wentao Liang
> <vulab@iscas.ac.cn>; stable@vger.kernel.org
> 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@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  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?

Alex

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] firmware/sysfb: Fix device reference count leak in sysfb_disable()
  2026-09-16 21:06 ` Deucher, Alexander
@ 2026-09-17  6:50   ` Thomas Zimmermann
  0 siblings, 0 replies; 3+ messages in thread
From: Thomas Zimmermann @ 2026-09-17  6:50 UTC (permalink / raw)
  To: Deucher, Alexander, Wentao Liang; +Cc: dri-devel, javierm, linux-kernel, stable

Hi

Am 16.09.26 um 23:06 schrieb Deucher, Alexander:
> Public
>
>> -----Original Message-----
>> From: Wentao Liang <vulab@iscas.ac.cn>
>> Sent: Wednesday, September 16, 2026 3:38 AM
>> To: Deucher, Alexander <Alexander.Deucher@amd.com>
>> Cc: dri-devel@lists.freedesktop.org; javierm@redhat.com; linux-
>> kernel@vger.kernel.org; tzimmermann@suse.de; Wentao Liang
>> <vulab@iscas.ac.cn>; stable@vger.kernel.org
>> 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@vger.kernel.org
>> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
>> ---
>>   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)



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-17  6:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16  7:38 [PATCH] firmware/sysfb: Fix device reference count leak in sysfb_disable() Wentao Liang
2026-09-16 21:06 ` Deucher, Alexander
2026-09-17  6:50   ` Thomas Zimmermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®