Re: [PATCH v2] drm/nouveau: validate legacy BIT table bounds

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Nested VBIOS table pointers parsed from BIT payloads lack bounds checks before dereference.
--

--- Patch [1]: [PATCH v2] drm/nouveau: validate legacy BIT table bounds ---
commit 3d3dd9aae1a602cb536b23bfafdac9694396793b
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

drm/nouveau: validate legacy BIT table bounds

The legacy BIT parser finds the BIT signature and then reads the
entry shape and entry array directly from the VBIOS image. The signature
does not prove that the complete header, entry array or referenced table
payload fits in the image.

Validate each extent before use. Preserve the legacy zero-offset meaning
for zero-length entries, but reject non-empty entries with a null or
out-of-range offset.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c b/drivers/gpu/drm/nouveau/nouveau_bios.c
> index 38032bb958262..a4ef62928ba3d 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bios.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
> @@ -972,24 +972,39 @@ bit_table(struct drm_device *dev, u8 id, struct bit_entry *bit)
[ ... ]
> +			offset = bit->offset;
> +			if ((bit->length && !offset) || offset > bios->length ||
> +			    bit->length > bios->length - offset)
> +				return -EINVAL;
> +

[Severity: High]
This isn't a bug introduced by this patch, but since we are hardening VBIOS
parsing here, are there other nested VBIOS table pointers parsed from BIT
payloads that lack bounds checks before dereference?

For example, in parse_bit_A_tbl_entry(), load_table_ptr is extracted and
used directly without checking if it exceeds bios->length:

drivers/gpu/drm/nouveau/nouveau_bios.c:parse_bit_A_tbl_entry() {
	...
	load_table_ptr = ROM16(bios->data[bitentry->offset]);

	if (load_table_ptr == 0x0) {
		NV_DEBUG(drm, "Pointer to BIT loadval table invalid\n");
		return -EINVAL;
	}

	version = bios->data[load_table_ptr];
	...
}

Similarly, in parse_fp_mode_table(), fptablepointer is used as an offset
without validating against bios->length:

drivers/gpu/drm/nouveau/nouveau_bios.c:parse_fp_mode_table() {
	...
	fptable = &bios->data[bios->fp.fptablepointer];
	fptable_ver = fptable[0];
	...
}

Could these out-of-bounds reads cause a kernel panic if they access unmapped
memory pages, especially if a malicious hardware device provides a crafted
VBIOS?

>  			bit->data = ROMPTR(dev, entry[4]);
>  			return 0;
>  		}
[ ... ]

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