Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Calling `fb_destroy_modelist()` on a zero-initialized list head causes a kernel NULL pointer dereference during probe error cleanup. -- commit e7b75661a1b19e182f38b86092ac7adc397e31df Author: Myeonghun Pak <[email protected]> fbdev: tdfxfb: fix PCI enable cleanup with pcim_enable_device() tdfxfb_probe() enables the PCI device with pci_enable_device(), but several failure paths after that point return without disabling it. This patch uses pcim_enable_device() so the PCI device is disabled automatically on probe failure and driver detach. > diff --git a/drivers/video/fbdev/tdfxfb.c b/drivers/video/fbdev/tdfxfb.c > index cc6a074f3165b..9a06cef756994 100644 > --- a/drivers/video/fbdev/tdfxfb.c > +++ b/drivers/video/fbdev/tdfxfb.c > @@ -1385,7 +1385,7 @@ static int tdfxfb_probe(struct pci_dev *pdev, const > struct pci_device_id *id) > if (err) > return err; > > - err = pci_enable_device(pdev); > + err = pcim_enable_device(pdev); > if (err) { > printk(KERN_ERR "tdfxfb: Can't enable pdev: %d\n", err); > return err; [Severity: High] This is a pre-existing issue, but does the out_err_iobase error path cause a NULL pointer dereference during device probe? If CONFIG_FB_3DFX_I2C is enabled but the monitor's EDID does not provide a mode database (specs->modedb == NULL), fb_videomode_to_modelist() is skipped. This leaves the info->modelist list head zero-initialized (with next and prev pointers set to NULL) as originally allocated by framebuffer_alloc(). If a subsequent operation fails (e.g., info->var.yres_virtual < info->var.yres or fb_alloc_cmap() returning -ENOMEM), the driver jumps to out_err_iobase: drivers/video/fbdev/tdfxfb.c:tdfxfb_probe() { ... out_err_iobase: #ifdef CONFIG_FB_3DFX_I2C fb_destroy_modelist(&info->modelist); ... } This unconditionally calls fb_destroy_modelist(), which uses list_for_each_safe(): drivers/video/fbdev/core/modedb.c:fb_destroy_modelist() { struct list_head *pos, *n; list_for_each_safe(pos, n, head) { ... } Because head->next is NULL, pos evaluates to NULL when the macro evaluates n = pos->next, resulting in an immediate NULL pointer dereference. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
