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 <[email protected]> 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/[email protected]?part=2
