[PATCH] fbdev: sstfb: use managed PCI device enable

[PATCH] fbdev: sstfb: use managed PCI device enable

[Myeonghun Pak]

sstfb_probe() enables the PCI device with pci_enable_device(), but
failure paths after that point return without disabling it. The successful
probe path has the same imbalance because sstfb_remove() releases the
framebuffer, mappings and memory regions, but never calls
pci_disable_device().

Use pcim_enable_device() so the PCI core disables the device automatically
after probe failure or driver removal. The driver still releases its
framebuffer, mappings and memory regions explicitly.

This issue was identified during our ongoing static-analysis research while
reviewing kernel code.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/video/fbdev/sstfb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/sstfb.c b/drivers/video/fbdev/sstfb.c
index 2ea947f57efb..0496b9d5ecc4 100644
--- a/drivers/video/fbdev/sstfb.c
+++ b/drivers/video/fbdev/sstfb.c
@@ -1332,7 +1332,8 @@ static int sstfb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return err;
 
 	/* Enable device in PCI config. */
-	if ((err=pci_enable_device(pdev))) {
+	err = pcim_enable_device(pdev);
+	if (err) {
 		printk(KERN_ERR "cannot enable device\n");
 		return err;
 	}
-- 
2.47.1

Re: [PATCH] fbdev: sstfb: use managed PCI device enable

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Unregistering the framebuffer after tearing down hardware mappings creates an exploitable use-after-free/unmap race condition.
- [High] Hardware is left in an active, initialized state on probe error paths because `sst_shutdown()` is missing.
- [High] `__sst_wait_idle()` contains an unbounded busy loop that will silently hang the CPU if the device becomes unresponsive.
--

commit 9516997f46dea14ef20c216ac89acefd0aaf2e36
Author: Myeonghun Pak <mhun512@gmail.com>

fbdev: sstfb: use managed PCI device enable

This commit updates sstfb_probe() to use pcim_enable_device() instead of
pci_enable_device(). This ensures that the PCI device is automatically
disabled by the PCI core after a probe failure or driver removal. The driver
still explicitly releases its other resources.

> diff --git a/drivers/video/fbdev/sstfb.c b/drivers/video/fbdev/sstfb.c
> index 2745557822f76..039279feb5bf4 100644
> --- a/drivers/video/fbdev/sstfb.c
> +++ b/drivers/video/fbdev/sstfb.c
> @@ -1332,7 +1332,8 @@ static int sstfb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		return err;
>  
>  	/* Enable device in PCI config. */
> -	if ((err=pci_enable_device(pdev))) {
> +	err = pcim_enable_device(pdev);
> +	if (err) {
>  		printk(KERN_ERR "cannot enable device\n");
>  		return err;
>  	}

[Severity: High]
This is a pre-existing issue, but hardware is left in an active, initialized
state on probe error paths because sst_shutdown() is missing.

During sstfb_probe(), sst_init() is called to configure the hardware. If a
subsequent initialization step fails, the error path unwinds memory but omits
calling sst_shutdown():

drivers/video/fbdev/sstfb.c:sstfb_probe() {
    ...
    if (!sst_init(info, par)) {
        printk(KERN_ERR "sstfb: Init failed\n");
        goto fail;
    }
    ...
fail:
    iounmap(info->screen_base);
    ...
}

Would this leave the PCI device fully active and configured, potentially
breaking system functionality like leaving VGA pass-through disabled?

[Severity: High]
This is a pre-existing issue, but unregistering the framebuffer after tearing
down hardware mappings creates an exploitable use-after-free/unmap race
condition.

If a user opens the framebuffer device, and then the driver is unbound,
sstfb_remove() unmaps the memory before unregistering the framebuffer:

drivers/video/fbdev/sstfb.c:sstfb_remove() {
    ...
    iounmap(info->screen_base);
    iounmap(par->mmio_vbase);
    ...
    unregister_framebuffer(info);
    ...
}

Could a user then issue an ioctl (like SSTFB_SET_VGAPASS), which calls
sstfb_ioctl() and accesses the now-unmapped memory?

[Severity: High]
This is a pre-existing issue, but __sst_wait_idle() contains an unbounded
busy loop that could silently hang the CPU if the device becomes unresponsive.

__sst_wait_idle() is called repeatedly during device operation and waits for
the busy status to clear:

drivers/video/fbdev/sstfb.c:__sst_wait_idle() {
    ...
    while(1) {
        if (__sst_read(vbase, STATUS) & STATUS_FBI_BUSY) {
    ...
}

If the PCI device is removed (e.g., via surprise unplug) or fails, the MMIO
read will return ~0. Since STATUS_FBI_BUSY is BIT(7), the condition would
remain true indefinitely. Should there be a timeout or fallback here to
prevent a permanent kernel hang?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911000835.88909-1-mhun512@gmail.com?part=1

Re: [PATCH] fbdev: sstfb: use managed PCI device enable

[Helge Deller]

On 9/11/26 02:08, Myeonghun Pak wrote:
> sstfb_probe() enables the PCI device with pci_enable_device(), but
> failure paths after that point return without disabling it. The successful
> probe path has the same imbalance because sstfb_remove() releases the
> framebuffer, mappings and memory regions, but never calls
> pci_disable_device().
> 
> Use pcim_enable_device() so the PCI core disables the device automatically
> after probe failure or driver removal. The driver still releases its
> framebuffer, mappings and memory regions explicitly.
> 
> This issue was identified during our ongoing static-analysis research while
> reviewing kernel code.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
>   drivers/video/fbdev/sstfb.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
applied.

Thanks!
Helge