Re: [PATCH v11 2/8] drm/i915/bios: store VBT #57's metadata in intel_vbt_data

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Out-of-bounds Read due to Missing VBT Block Size Validation and Hardware Assumptions
- [High] Undefined Behavior from Unaligned Pointer Cast of Packed Struct Member
--

--- Patch [2]: [PATCH v11 2/8] drm/i915/bios: store VBT #57's metadata in intel_vbt_data ---
commit ecc48f16388bfb4ef7b96b15e355044be1dd9fb7
Author: Michał Grzelak <michal.grzelak@intel.com>

drm/i915/bios: store VBT #57's metadata in intel_vbt_data

This commit stores VBT #57 (VS/PE-O) metadata inside a new vspeo struct
in intel_vbt_data. It also introduces a helper to calculate the number of
rows based on the display version, assigning 16 rows to MTL+ and 10 rows
to older platforms.

> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index ece4df4a9d62a..06a8dd7581bb8 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -2205,6 +2205,11 @@ parse_compression_parameters(struct intel_display *display)
>  	}
>  }
>  
> +static int vswing_preemph_num_rows(struct intel_display *display)
> +{
> +	return DISPLAY_VER(display) >= 14 ? 16 : 10;
> +}
> +
>  static void
>  parse_vswing_preemph_override(struct intel_display *display)
>  {
> @@ -2218,6 +2223,11 @@ parse_vswing_preemph_override(struct intel_display *display)
>  	/* pre-ICL GOPs don't have VBT #57 */
>  	if (!block)
>  		return;
> +
> +	display->vbt.vspeo.tables = block->tables;
> +	display->vbt.vspeo.num_tables = block->num_tables;
> +	display->vbt.vspeo.num_columns = block->num_columns;
> +	display->vbt.vspeo.num_rows = vswing_preemph_num_rows(display);

[Severity: High]
Does this code perform an out-of-bounds read if the VBT block is smaller
than expected?

The parser reads num_tables and num_columns from the untrusted VBT block,
and hardcodes num_rows to 16 for DISPLAY_VER >= 14 via
vswing_preemph_num_rows().

However, it doesn't validate whether get_blocksize(block) is large enough
to contain num_tables * num_columns * num_rows * sizeof(u32) bytes.

Additionally, if a platform with DISPLAY_VER >= 14 lacks a C20 PHY and
legitimately provides a smaller 10-row VBT block, this hardcoded 16-row
assumption will cause the driver to read past the end of the block.

>  }
>  
>  static u8 translate_iboost(struct intel_display *display, u8 val)
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 7e988b7b1fe7a..c80b4a2f74f9e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -244,6 +244,13 @@ struct intel_vbt_data {
>  	struct list_head display_devices;
>  	struct list_head bdb_blocks;
>  
> +	struct {
> +		const u32 *tables;

[Severity: High]
Will assigning the packed array to this pointer cause undefined behavior?

In struct bdb_vswing_preemph, the tables array is part of a __packed
structure, meaning it can be unaligned. When block->tables is assigned
to this standard const u32 * pointer in parse_vswing_preemph_override(),
the compiler's __packed annotations are dropped.

Any subsequent array accesses through vspeo.tables will assume standard
alignment, which can cause unaligned memory reads and UBSAN alignment faults.

> +		int num_tables;
> +		int num_columns;
> +		int num_rows;
> +	} vspeo;
> +
>  	struct sdvo_device_mapping {
>  		u8 initialized;
>  		u8 dvo_port;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813160918.918857-1-michal.grzelak@intel.com?part=2