[PATCH] drm/i915/bios: validate VBT vswing/preemphasis override table

[PATCH] drm/i915/bios: validate VBT vswing/preemphasis override table

[Noah Weinberger]

Hello,

Proposed-fix limitation: the patch below does not validate the full
VBT table extent against the section size. The malformed/short-table
out-of-bounds case described below remains unresolved by these
existence and minimum-count checks. This is an incomplete proposal for
maintainer review; a complete fix still needs section-extent bounds
validation.

parse_vswing_preemph_override() stores the raw pointer and count fields
from VBT block 57 without validating that the block actually contains
the announced table, and allocate_vswing_preemph_override() allocates
the consumer structures whenever a child device sets use_vbt_vswing -
even when the block is absent entirely. validate_vspeo() only checks
that devdata->vspeo is non-NULL and that the port has DP.

Two failure modes follow at DDI buffer-translation time:
 - block absent: tables == NULL, num_columns == 0, so the consumer
   dereferences tables[0] (NULL).
 - block short or malformed: the table walk reads up to
   (5+1)*num_rows*num_columns u32s from a block whose enforced minimum
   size is 2 bytes - with num_columns = 255 that is ~98 KB past a small
   heap allocation, and the values are programmed into PHY registers.

The patch gates the consumer on the table existing with sane counts.
A complete fix should also validate the table extent against the
section size, which is not currently plumbed through; happy to rework
if there is a preferred way to carry the section size.

Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), drivers/gpu/drm/i915/display/intel_bios.c:2216-2234 (parse), 2654-2676 (allocate), 3874-3923 (validate/consume).
Introduced in the v7.4 merge window.

This was found during an assisted (AI-supported) static source review of
mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
reporter against that tree. The patch below has NOT been compiled and has
NOT been runtime tested, and no reproducer was produced - the triggering
conditions are described above instead. Please treat this as a report for
verification rather than a battle-tested fix.

Reported-by: Noah Weinberger <noahweinberger2003@proton.me>
---
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 7b6a6f8..d99a675 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -2657,7 +2657,8 @@ static void allocate_vswing_preemph_override(struct intel_bios_encoder_data *dev
 	union intel_ddi_buf_trans_entry *entries;
 	struct intel_ddi_buf_trans *vspeo;
 
-	if (!intel_bios_encoder_requests_vspeo(devdata))
+	if (!intel_bios_encoder_requests_vspeo(devdata) ||
+	    !devdata->display->vbt.vspeo.tables)
 		return;
 
 	vspeo = kzalloc_obj(*vspeo);
@@ -3876,6 +3877,8 @@ validate_vspeo(const struct intel_bios_encoder_data *devdata, bool has_dp)
 {
 	struct intel_ddi_buf_trans *vspeo;
 
+	struct intel_display *display = devdata->display;
+
 	vspeo = devdata->vspeo;
 	if (!vspeo)
 		return false;
@@ -3883,5 +3886,10 @@ validate_vspeo(const struct intel_bios_encoder_data *devdata, bool has_dp)
 	if (!has_dp)
 		return false;
 
+	if (!display->vbt.vspeo.tables ||
+	    display->vbt.vspeo.num_columns < 3 ||
+	    display->vbt.vspeo.num_tables < 6)
+		return false;
+
 	return true;
 }

Re: [PATCH] drm/i915/bios: validate VBT vswing/preemphasis override table

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [Medium] drm/i915/bios: missing BDB block size check in
  parse_vswing_preemph_override()
- [Medium] drm/i915/bios: out-of-bounds read in combo PHY vspeo due to
  validate_vspeo()
- [Medium] drm/i915/bios: validate_vspeo() rejects valid C10 and combo
  PHY VBT tables
- [Low] drm/i915/bios: missing Fixes/Signed-off-by trailers and
  inaccurate changelog
--

commit e60dd76fbe3fb6893876e4ed2f372a4da1a69f33
Author: Noah Weinberger <NoahWeinberger2003@proton.me>

> drm/i915/bios: validate VBT vswing/preemphasis override table
> 
> Hello,
> 
> Proposed-fix limitation: the patch below does not validate the full
> VBT table extent against the section size. The malformed/short-table
> out-of-bounds case described below remains unresolved by these
> existence and minimum-count checks. This is an incomplete proposal for
> maintainer review; a complete fix still needs section-extent bounds
> validation.
> 
> parse_vswing_preemph_override() stores the raw pointer and count fields
> from VBT block 57 without validating that the block actually contains
> the announced table, and allocate_vswing_preemph_override() allocates
> the consumer structures whenever a child device sets use_vbt_vswing -
> even when the block is absent entirely. validate_vspeo() only checks
> that devdata->vspeo is non-NULL and that the port has DP.
> 
> Two failure modes follow at DDI buffer-translation time:
>  - block absent: tables == NULL, num_columns == 0, so the consumer
>    dereferences tables[0] (NULL).
>  - block short or malformed: the table walk reads up to
>    (5+1)*num_rows*num_columns u32s from a block whose enforced minimum
>    size is 2 bytes - with num_columns = 255 that is ~98 KB past a small
>    heap allocation, and the values are programmed into PHY registers.
> 
> The patch gates the consumer on the table existing with sane counts.
> A complete fix should also validate the table extent against the
> section size, which is not currently plumbed through; happy to rework
> if there is a preferred way to carry the section size.
> 
> Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), drivers/gpu/drm/i915/display/intel_bios.c:2216-2234 (parse), 2654-2676 (allocate), 3874-3923 (validate/consume).
> Introduced in the v7.4 merge window.
> 
> This was found during an assisted (AI-supported) static source review of
> mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
> reporter against that tree. The patch below has NOT been compiled and has
> NOT been runtime tested, and no reproducer was produced - the triggering
> conditions are described above instead. Please treat this as a report for
> verification rather than a battle-tested fix.
> 
> Reported-by: Noah Weinberger <noahweinberger2003@proton.me>

[Severity: Low]
This commit fixes bugs in VBT block 57 handling, so it needs a
Signed-off-by: trailer and Fixes: tags for the introducing commits:

  Fixes: a138a81d7805 ("drm/i915/bios: de/allocate VS/PE-O buffers for each port")
  Fixes: e8fe5008ae3d ("drm/i915: override Snps's VS/PE when requested")
  Fixes: 7fdf86e530c7 ("drm/i915: override Combo's VS/PE when requested")

The greeting and testing disclaimers also belong below the "---" separator
rather than in the changelog.

In addition, is the description of the absent-block failure mode accurate?
When block 57 is absent, parse_vswing_preemph_override() returns before
initializing display->vbt.vspeo.num_rows, so num_rows is 0; the loops in
intel_bios_get_*_vspeo() do not iterate or dereference tables[0], and
instead allocate_vswing_preemph_override() passes 0 to kzalloc_objs(),
leaving devdata->entries set to ZERO_SIZE_PTR for callers to dereference.

Finally, in validate_vspeo(), the blank line between the local variable
declarations of vspeo and display is unneeded, and the
!display->vbt.vspeo.tables check is redundant once
allocate_vswing_preemph_override() skips allocating devdata->vspeo when
tables is NULL.

> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 7b6a6f8c3cf19..d99a6756d68be 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c

[ ... ]

> @@ -3883,6 +3886,11 @@ validate_vspeo(const struct intel_bios_encoder_data *devdata, bool has_dp)
>  	if (!has_dp)
>  		return false;
>  
> +	if (!display->vbt.vspeo.tables ||

[Severity: Medium]
Can parse_vswing_preemph_override() validate the section size directly
with get_blocksize(block) before populating display->vbt.vspeo?

Whenever BDB_VSWING_PREEMPH is present, block->tables is a non-NULL
flexible array address inside the block allocation, whose enforced minimum
size in bdb_blocks[] is only 2 bytes.  If a malformed or truncated VBT has
num_columns >= 3 and num_tables >= 6 with a smaller payload,
intel_bios_get_*_vspeo() will still read up to 6 * 16 * 255 * sizeof(u32)
bytes past the heap buffer and program arbitrary slab memory into PHY
registers.

get_blocksize() is already available in intel_bios.c for any pointer
returned by bdb_find_section():

drivers/gpu/drm/i915/display/intel_bios.c:get_blocksize() {
static u32 get_blocksize(const void *block_data)
{
	return _get_blocksize(block_data - 3);
}
}

Checking get_blocksize(block) against struct_size(block, tables,
size_mul(num_rows, size_mul(block->num_tables, block->num_columns))) in
parse_vswing_preemph_override() would prevent out-of-bounds reads without
needing extra plumbing.

> +	    display->vbt.vspeo.num_columns < 3 ||

[Severity: Medium]
Does checking only num_columns < 3 allow out-of-bounds reads in
intel_bios_get_ehl_combo_vspeo() and intel_bios_get_jsl_combo_vspeo()?

While C10 and C20 PHYs read 3 columns per row, both combo PHY helpers read
5 columns per row after calling validate_vspeo():

drivers/gpu/drm/i915/display/intel_bios.c:intel_bios_get_ehl_combo_vspeo() {
	for (level = 0; level < num_rows; level++) {
		u32 dw2_swing_sel = tables[offset];
		u32 dw7_n_scalar = tables[offset + 1];
		u32 dw4_cursor_coeff = tables[offset + 2];
		u32 dw4_post_cursor_2 = tables[offset + 3];
		u32 dw4_post_cursor_1 = tables[offset + 4];
    ...
		offset += num_columns;
	}
}

If VBT block 57 specifies num_columns as 3 or 4 on Elkhart Lake or Jasper
Lake, validate_vspeo() succeeds, and the combo PHY loop reads across row
boundaries on every level and up to 2 u32 entries past the end of the
selected table on the last row.  Passing the required column count into
validate_vspeo() (5 for combo PHY, 3 for C10/C20) would catch this.

> +	    display->vbt.vspeo.num_tables < 6)
> +		return false;

[Severity: Medium]
Doesn't requiring num_tables >= 6 for all callers introduce a regression
on C10, EHL, and JSL platforms whose VBT block 57 has fewer than 6 tables?

Only intel_bios_get_c20_vspeo() indexes tables 4 and 5;
intel_bios_get_c10_vspeo() only uses table indices 0 through 3, and
intel_bios_get_ehl_combo_vspeo() and intel_bios_get_jsl_combo_vspeo() only
use indices 0 through 2.  Passing the selected table index idx into
validate_vspeo() and checking idx >= display->vbt.vspeo.num_tables would
avoid rejecting valid non-C20 VBT override tables.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/40IXMMqFQJJm26J3saK7hx09nF4FuE3ZJcU6gMIryuxuGC5VENrcgA9q4klryPNSLnMwE64szv7eg4wf__gU8OlgzlL8tw3CmV_78Kc_H14=@proton.me?part=1

✗ LGCI.VerificationFailed: failure for drm/i915/bios: validate VBT vswing/preemphasis override table

[Patchwork]

== Series Details ==

Series: drm/i915/bios: validate VBT vswing/preemphasis override table
URL   : https://patchwork.freedesktop.org/series/178112/
State : failure

== Summary ==

Series author address 'noahweinberger2003@proton.me' is not on the allowlist, which prevents CI from being automatically triggered.
If you want CI to run for this series, ask Patchwork project owners to click 'retest' on the series in Patchwork.
Exception occurred during validation, bailing out!
Build URL: http://gfx-ci.igk.intel.com:8080/job/CI_PW_kernel/192923/ (on built-in)