[Public] Reviewed-by: Roman Li <[email protected]>
> -----Original Message----- > From: SHANMUGAM, SRINIVASAN <[email protected]> > Sent: Tuesday, February 10, 2026 9:59 PM > To: Hung, Alex <[email protected]>; Pillai, Aurabindo > <[email protected]> > Cc: [email protected]; SHANMUGAM, SRINIVASAN > <[email protected]>; Dan Carpenter > <[email protected]>; Wentland, Harry <[email protected]>; Mario > Limonciello <[email protected]>; Chung, ChiaHsuan (Tom) > <[email protected]>; Li, Roman <[email protected]> > Subject: [PATCH v3] drm/amd/display: Fix out-of-bounds stream encoder index v2 > > eng_id can be negative and that stream_enc_regs[] can be indexed out of > bounds. > > eng_id is used directly as an index into stream_enc_regs[], which has only 5 > entries. When eng_id is 5 (ENGINE_ID_DIGF) or negative, this can access > memory past the end of the array. > > Add a bounds check using ARRAY_SIZE() before using eng_id as an index. > The unsigned cast also rejects negative values. > > This avoids out-of-bounds access. > > Fixes the below smatch error: > dcn*_resource.c: stream_encoder_create() may index stream_enc_regs[eng_id] > out of bounds (size 5). > > drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn351/dcn351_resource.c > 1246 static struct stream_encoder *dcn35_stream_encoder_create( > 1247 enum engine_id eng_id, > 1248 struct dc_context *ctx) > 1249 { > > ... > > 1255 > 1256 /* Mapping of VPG, AFMT, DME register blocks to DIO block > instance > */ > 1257 if (eng_id <= ENGINE_ID_DIGF) { > > ENGINE_ID_DIGF is 5. should <= be <? > > Unrelated but, ugh, why is Smatch saying that "eng_id" can be negative? > end_id is type signed long, but there are checks in the caller which prevent > it from > being negative. > > 1258 vpg_inst = eng_id; > 1259 afmt_inst = eng_id; > 1260 } else > 1261 return NULL; > 1262 > > ... > > 1281 > 1282 dcn35_dio_stream_encoder_construct(enc1, ctx, ctx->dc_bios, > 1283 eng_id, vpg, afmt, > --> 1284 &stream_enc_regs[eng_id], > ^^^^^^^^^^^^^^^^^^^^^^^ > This stream_enc_regs[] > array has 5 elements so we are one element beyond the end of the array. > > ... > > 1287 return &enc1->base; > 1288 } > > v2: use explicit bounds check as suggested by Roman/Dan; avoid unsigned int > cast > > v3: The compiler already knows how to compare the two values, so the > cast (int) is not needed. (Roman) > > Fixes: 2728e9c7c842 ("drm/amd/display: add DC changes for DCN351") > Reported-by: Dan Carpenter <[email protected]> > Cc: Harry Wentland <[email protected]> > Cc: Mario Limonciello <[email protected]> > Cc: Alex Hung <[email protected]> > Cc: Aurabindo Pillai <[email protected]> > Cc: ChiaHsuan Chung <[email protected]> > Cc: Roman Li <[email protected]> > Signed-off-by: Srinivasan Shanmugam <[email protected]> > --- > .../drm/amd/display/dc/resource/dcn315/dcn315_resource.c | 8 ++++---- > .../drm/amd/display/dc/resource/dcn316/dcn316_resource.c | 8 ++++---- > .../drm/amd/display/dc/resource/dcn32/dcn32_resource.c | 8 ++++---- > .../drm/amd/display/dc/resource/dcn321/dcn321_resource.c | 8 ++++---- > .../drm/amd/display/dc/resource/dcn35/dcn35_resource.c | 8 ++++---- > .../drm/amd/display/dc/resource/dcn351/dcn351_resource.c | 8 ++++---- > 6 files changed, 24 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn315/dcn315_resource.c > b/drivers/gpu/drm/amd/display/dc/resource/dcn315/dcn315_resource.c > index a8283f21008e..3ae787a377b1 100644 > --- a/drivers/gpu/drm/amd/display/dc/resource/dcn315/dcn315_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn315/dcn315_resource.c > @@ -1230,12 +1230,12 @@ static struct stream_encoder > *dcn315_stream_encoder_create( > /*PHYB is wired off in HW, allow front end to remapping, otherwise needs > more changes*/ > > /* Mapping of VPG, AFMT, DME register blocks to DIO block instance */ > - if (eng_id <= ENGINE_ID_DIGF) { > - vpg_inst = eng_id; > - afmt_inst = eng_id; > - } else > + if (eng_id < 0 || eng_id >= ARRAY_SIZE(stream_enc_regs)) > return NULL; > > + vpg_inst = eng_id; > + afmt_inst = eng_id; > + > enc1 = kzalloc(sizeof(struct dcn10_stream_encoder), GFP_KERNEL); > vpg = dcn31_vpg_create(ctx, vpg_inst); > afmt = dcn31_afmt_create(ctx, afmt_inst); diff --git > a/drivers/gpu/drm/amd/display/dc/resource/dcn316/dcn316_resource.c > b/drivers/gpu/drm/amd/display/dc/resource/dcn316/dcn316_resource.c > index 4da0e9320159..4b8668458f03 100644 > --- a/drivers/gpu/drm/amd/display/dc/resource/dcn316/dcn316_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn316/dcn316_resource.c > @@ -1223,12 +1223,12 @@ static struct stream_encoder > *dcn316_stream_encoder_create( > int afmt_inst; > > /* Mapping of VPG, AFMT, DME register blocks to DIO block instance */ > - if (eng_id <= ENGINE_ID_DIGF) { > - vpg_inst = eng_id; > - afmt_inst = eng_id; > - } else > + if (eng_id < 0 || eng_id >= ARRAY_SIZE(stream_enc_regs)) > return NULL; > > + vpg_inst = eng_id; > + afmt_inst = eng_id; > + > enc1 = kzalloc(sizeof(struct dcn10_stream_encoder), GFP_KERNEL); > vpg = dcn31_vpg_create(ctx, vpg_inst); > afmt = dcn31_afmt_create(ctx, afmt_inst); diff --git > a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c > b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c > index d3b92c61b7ad..a55078458ba5 100644 > --- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c > @@ -1211,12 +1211,12 @@ static struct stream_encoder > *dcn32_stream_encoder_create( > int afmt_inst; > > /* Mapping of VPG, AFMT, DME register blocks to DIO block instance */ > - if (eng_id <= ENGINE_ID_DIGF) { > - vpg_inst = eng_id; > - afmt_inst = eng_id; > - } else > + if (eng_id < 0 || eng_id >= ARRAY_SIZE(stream_enc_regs)) > return NULL; > > + vpg_inst = eng_id; > + afmt_inst = eng_id; > + > enc1 = kzalloc(sizeof(struct dcn10_stream_encoder), GFP_KERNEL); > vpg = dcn32_vpg_create(ctx, vpg_inst); > afmt = dcn32_afmt_create(ctx, afmt_inst); diff --git > a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c > b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c > index d08691ea27ea..188c3f24f110 100644 > --- a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c > @@ -1192,12 +1192,12 @@ static struct stream_encoder > *dcn321_stream_encoder_create( > int afmt_inst; > > /* Mapping of VPG, AFMT, DME register blocks to DIO block instance */ > - if (eng_id <= ENGINE_ID_DIGF) { > - vpg_inst = eng_id; > - afmt_inst = eng_id; > - } else > + if (eng_id < 0 || eng_id >= ARRAY_SIZE(stream_enc_regs)) > return NULL; > > + vpg_inst = eng_id; > + afmt_inst = eng_id; > + > enc1 = kzalloc(sizeof(struct dcn10_stream_encoder), GFP_KERNEL); > vpg = dcn321_vpg_create(ctx, vpg_inst); > afmt = dcn321_afmt_create(ctx, afmt_inst); diff --git > a/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c > b/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c > index 945da8cffada..5ea805fcff48 100644 > --- a/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn35/dcn35_resource.c > @@ -1274,12 +1274,12 @@ static struct stream_encoder > *dcn35_stream_encoder_create( > int afmt_inst; > > /* Mapping of VPG, AFMT, DME register blocks to DIO block instance */ > - if (eng_id <= ENGINE_ID_DIGF) { > - vpg_inst = eng_id; > - afmt_inst = eng_id; > - } else > + if (eng_id < 0 || eng_id >= ARRAY_SIZE(stream_enc_regs)) > return NULL; > > + vpg_inst = eng_id; > + afmt_inst = eng_id; > + > enc1 = kzalloc(sizeof(struct dcn10_stream_encoder), GFP_KERNEL); > vpg = dcn31_vpg_create(ctx, vpg_inst); > afmt = dcn31_afmt_create(ctx, afmt_inst); diff --git > a/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c > b/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c > index e660889904a9..424b52e2dd7b 100644 > --- a/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/resource/dcn351/dcn351_resource.c > @@ -1254,12 +1254,12 @@ static struct stream_encoder > *dcn35_stream_encoder_create( > int afmt_inst; > > /* Mapping of VPG, AFMT, DME register blocks to DIO block instance */ > - if (eng_id <= ENGINE_ID_DIGF) { > - vpg_inst = eng_id; > - afmt_inst = eng_id; > - } else > + if (eng_id < 0 || eng_id >= ARRAY_SIZE(stream_enc_regs)) > return NULL; > > + vpg_inst = eng_id; > + afmt_inst = eng_id; > + > enc1 = kzalloc(sizeof(struct dcn10_stream_encoder), GFP_KERNEL); > vpg = dcn31_vpg_create(ctx, vpg_inst); > afmt = dcn31_afmt_create(ctx, afmt_inst); > -- > 2.34.1
