Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Missing NULL check on `kzalloc` causing NULL pointer dereference. - [High] Unconditional memory leak of `bufs_table` and its elements. --
--- Patch [4]: [PATCH v1 04/16] drm/i915/bios: prepare for parsing VBT #57 --- commit a6241aaaf22342914933a8b58de0747a6e01cf2a Author: Michał Grzelak <[email protected]> drm/i915/bios: prepare for parsing VBT #57 This commit adds parsing code for VBT #57 and a structure for storing VS/PE data. It also allocates a matrix of intel_ddi_buf_trans_entry to store the deparsed values and handles its deallocation on driver removal. > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > b/drivers/gpu/drm/i915/display/intel_bios.c > index b6fe87c29aa7c..778584c59fc6f 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c [ ... ] > @@ -2183,6 +2184,38 @@ parse_compression_parameters(struct intel_display > *display) > } > } > > +static void > +parse_vswing_preemph_override(struct intel_display *display) > +{ > + union intel_ddi_buf_trans_entry **bufs_table; > + const struct bdb_vswing_preemph *block; > + u8 num_rows; > + > + if (display->vbt.version < 218) > + return; > + > + block = bdb_find_section(display, BDB_VSWING_PREEMPH); > + > + /* pre-ICL GOP don't have VBT #57 */ > + if (!block) > + return; > + > + num_rows = DISPLAY_VER(display) >= 14 ? 16 : 10; > + > + bufs_table = kzalloc(block->num_tables * sizeof(*bufs_table), > GFP_KERNEL); > + > + for (int idx = 0; idx < block->num_tables; idx++) > + bufs_table[idx] = kzalloc(num_rows * sizeof(**bufs_table), > GFP_KERNEL); [Severity: High] Is a NULL check missing here? If kzalloc() fails to allocate bufs_table, the loop will immediately attempt to assign to bufs_table[idx] and cause a NULL pointer dereference. Additionally, if the inner kzalloc() for individual rows fails, it leaves NULL pointers in the table without any error handling. > + > + drm_dbg_kms(display->drm, "Vswing / Preemph Override not yet supported > on the platform\n"); > + bufs_table = NULL; [Severity: High] Does this cause an unconditional memory leak? The memory just allocated for bufs_table and its elements is explicitly overwritten with NULL here. This causes display->vbt.vswing_preemph.bufs_table to receive NULL, and the original allocated pointers are completely lost, preventing them from being freed during intel_bios_driver_remove(). > + > + display->vbt.vswing_preemph.bufs_table = bufs_table; > + display->vbt.vswing_preemph.num_tables = block->num_tables; > + display->vbt.vswing_preemph.num_rows = num_rows; > + display->vbt.vswing_preemph.num_cols = block->num_columns; > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
