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

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

[Wentao Liang]

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] sysfb: kill-switch bypass when parent device query returns an error
--

commit cea5d6870e1a5b4277264d29fe9056b6df3310b7
Author: Wentao Liang <vulab@iscas.ac.cn>

firmware/sysfb: Fix device reference count leak in sysfb_disable()

This commit addresses a device reference count leak in sysfb_disable(). It
adds a check to ensure the parent device is not an error pointer and releases
the reference using put_device() before returning.

> diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
> --- 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)) {

[Severity: High]
Will wrapping the unregistration logic in !IS_ERR(parent) bypass the global
kill-switch when the parent device query returns an error?

Graphics drivers use sysfb_disable(NULL) as a global kill-switch to
unconditionally disable the generic firmware framebuffer before native DRM
drivers take over.

sysfb_parent_dev() can return an ERR_PTR (like -ENODEV) if the PCI device
exists but its memory decoding is not enabled. If this happens during early
boot or driver probe, this new check skips the block entirely, silently
bypassing the kill-switch and leaving the disabled flag as false.

If a built-in native DRM driver disables sysfb and subsequently enables PCI
memory decoding, could a later execution of sysfb_init() see disabled == false
and incorrectly register the generic framebuffer on top of the already-running
native driver? This concurrent access could lead to resource conflicts or kernel
panics.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916073815.1972045-1-vulab@iscas.ac.cn?part=1

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

[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?

Alex

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

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@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)