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))
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
