[Public]
> -----Original Message-----
> From: SHANMUGAM, SRINIVASAN <[email protected]>
> Sent: Friday, February 6, 2026 10:19 AM
> 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] drm/amd/display: Fix out-of-bounds stream encoder index
>
> 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 }
>
> 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 4e962f522f1b..d8c1f1911c37 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 ((unsigned int)eng_id >= ARRAY_SIZE(stream_enc_regs))
Could you replace the unsigned cast with an explicit bounds check, i.e.
if (eng_id < 0 || eng_id >= ARRAY_SIZE(stream_enc_regs)) return NULL;
It would be clearer about what values are being rejected.
> 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 5a95dd54cb42..732f7bfb9103 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 ((unsigned int)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 b276fec3e479..9c4a31c0224b 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 ((unsigned int)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 3466ca34c93f..3e760a9a8812 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 ((unsigned int)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 45454a097264..09ed0d5e50bc 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 ((unsigned int)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 e3c587165807..39ec7d5e6803 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 ((unsigned int)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
