Re: [PATCH v2 5/5] fbdev: tdfxfb: Program the initial video mode

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Unbounded memset_io() leads to out-of-bounds physical memory write

Pre-existing issues:
- [High] Infinite loop in banshee_wait_idle() causes kernel hang on probe
--

--- Patch [5]: [PATCH v2 5/5] fbdev: tdfxfb: Program the initial video mode ---
commit 99f06e3d4bb98c9560cb7c49215cddea173febfd
Author: Daniel Palmer <daniel@0x0f.com>

fbdev: tdfxfb: Program the initial video mode

If the card does not get bound to by fbcon set_par() never happens and the
initial video mode is not setup and the display detects no signal. This
commit programs the video mode and also clears the framebuffer memory so
random garbage isn't displayed.

> diff --git a/drivers/video/fbdev/tdfxfb.c b/drivers/video/fbdev/tdfxfb.c
> index f2b658a47135d..cb1c2e9f37dbc 100644
> --- a/drivers/video/fbdev/tdfxfb.c
> +++ b/drivers/video/fbdev/tdfxfb.c
> @@ -1686,6 +1686,14 @@ static int tdfxfb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		goto out_err_iobase;
>  	}
>  
> +	/*
> +	 * Program a video mode and clear the framebuffer now, this
> +	 * ensures the display comes up even if fbcon doesn't bind
> +	 * when the framebuffer is registered.
> +	 */
> +	tdfxfb_set_par(info);

[Severity: High]
This is a pre-existing issue, but can this lead to an infinite loop and
kernel hang during device probe?

When tdfxfb_set_par() is called, it invokes the code path leading to
banshee_wait_idle(), which contains a tight loop waiting for hardware
register status to clear:

drivers/video/fbdev/tdfxfb.c:banshee_wait_idle() {
    ...
	do {
		if ((tdfx_inl(par, STATUS) & STATUS_BUSY) == 0)
			i++;
	} while (i < 3);
    ...
}

If a malfunctioning or malicious PCI device returns a constant busy status,
would the driver loop indefinitely here since the loop is unbounded?

> +	memset_io(info->screen_base, 0, info->fix.smem_len);

[Severity: Critical]
Could this regression result in an out-of-bounds physical memory write?

The length info->fix.smem_len is derived from untrusted hardware registers
read during do_lfb_size(), rather than the validated PCI BAR length:

drivers/video/fbdev/tdfxfb.c:do_lfb_size() {
    ...
	return num_chips * chip_size * 1024l * 1024;
    ...
}

If a device exposes large values for these registers, can
info->fix.smem_len exceed the actual size remapped at info->screen_base,
causing memset_io() to overwrite adjacent memory?

> +
>  	if (register_framebuffer(info) < 0) {
>  		printk(KERN_ERR "tdfxfb: can't register framebuffer\n");
>  		fb_dealloc_cmap(&info->cmap);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731121915.3362232-1-daniel@0x0f.com?part=5