Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] A wildcard entry in vs_chip_identities shadows specific customer ID entries, making them unreachable. - [High] The function vs_fill_chip_identity() ignores the return values of regmap_read() calls, leading to potential use of uninitialized stack memory. -- commit a6a5d6f3773180ae44a10c5525c46667d81239fe Author: Joey Lu <[email protected]> drm/verisilicon: add DCUltraLite chip identity to HWDB Register the Nuvoton MA35D1 DCUltraLite chip identity in vs_chip_identities[]: model = 0x0 (DCUltraLite; Verisilicon uses 0 for this IP) [ ... ] > diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c > b/drivers/gpu/drm/verisilicon/vs_hwdb.c > index 91524d16f7785..7d630a667a3f0 100644 > --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c > +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c [ ... ] > @@ -129,6 +129,16 @@ static struct vs_chip_identity vs_chip_identities[] = { > .max_cursor_size = 64, > .formats = &vs_formats_no_yuv444, > }, > + { > + .model = 0x0, /* DCUltraLite */ > + .revision = 0x5560, > + .customer_id = 0x305, > + > + .generation = VSDC_GEN_DC8000, > + .display_count = 1, > + .max_cursor_size = 32, > + .formats = &vs_formats_no_yuv444, > + }, > }; > > int vs_fill_chip_identity(struct regmap *regs, > struct vs_chip_identity *ident) > { > u32 model; > u32 revision; > u32 customer_id; > int i; > > regmap_read(regs, VSDC_TOP_CHIP_MODEL, &model); > regmap_read(regs, VSDC_TOP_CHIP_REV, &revision); > regmap_read(regs, VSDC_TOP_CHIP_CUSTOMER_ID, &customer_id); [Severity: High] This isn't a bug introduced by this patch, but does vs_fill_chip_identity() ignore the return values of these regmap_read() calls? The regmap_read() API returns an integer error code on failure and leaves the output pointer's target unmodified. If any of these reads fail due to bus errors or unmapped I/O, will the function proceed to use uninitialized stack memory in the loop to compare against the vs_chip_identities array? > > for (i = 0; i < ARRAY_SIZE(vs_chip_identities); i++) { > if (vs_chip_identities[i].model == model && > vs_chip_identities[i].revision == revision && > (vs_chip_identities[i].customer_id == customer_id || > vs_chip_identities[i].customer_id == ~0U)) { > memcpy(ident, &vs_chip_identities[i], sizeof(*ident)); > ident->customer_id = customer_id; > return 0; > } [Severity: High] This is a pre-existing issue, but does the wildcard entry in vs_chip_identities[] shadow specific customer ID entries? Looking at vs_chip_identities[] in drivers/gpu/drm/verisilicon/vs_hwdb.c, there is a wildcard entry at the beginning of the array: { .model = 0x8200, .revision = 0x5720, .customer_id = ~0U, ... } Since vs_fill_chip_identity() searches sequentially and returns on the first match, will any hardware with model 0x8200 and revision 0x5720 instantly match this wildcard entry? If so, the specific configurations for customer IDs 0x310 and 0x311 defined later in the array might be completely shadowed and never reached. This would cause customer ID 0x310 to incorrectly use vs_formats_no_yuv444 instead of its intended vs_formats_with_yuv444. > } > > return -EINVAL; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
