* [PATCH v2 2/4] drm/amdgpu/vcn3: Prevent OOB reads when parsing dec msg
2026-03-26 12:29 [PATCH v2 1/4] drm/amdgpu: Add bounds checking to ib_{get,set}_value Benjamin Cheng
@ 2026-03-26 12:29 ` Benjamin Cheng
2026-04-09 18:09 ` SDL
2026-03-26 12:29 ` [PATCH v2 3/4] drm/amdgpu/vcn4: " Benjamin Cheng
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Benjamin Cheng @ 2026-03-26 12:29 UTC (permalink / raw)
To: Alex Deucher, Christian König, amd-gfx
Cc: Leo Liu, Ruijing Dong, David (Ming Qiang) Wu, Benjamin Cheng
Check bounds against the end of the BO whenever we access the msg.
Signed-off-by: Benjamin Cheng <benjamin.cheng@amd.com>
---
v2: Moved 4 DWORD check earlier and added more DRM_ERROR logs.
drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
index 02d5c5af65f2..6fb4fcdbba4f 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
@@ -1909,7 +1909,7 @@ static int vcn_v3_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
struct ttm_operation_ctx ctx = { false, false };
struct amdgpu_device *adev = p->adev;
struct amdgpu_bo_va_mapping *map;
- uint32_t *msg, num_buffers;
+ uint32_t *msg, num_buffers, len_dw;
struct amdgpu_bo *bo;
uint64_t start, end;
unsigned int i;
@@ -1930,6 +1930,11 @@ static int vcn_v3_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
return -EINVAL;
}
+ if (end - addr < 16) {
+ DRM_ERROR("VCN messages must be at least 4 DWORDs!\n");
+ return -EINVAL;
+ }
+
bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
amdgpu_bo_placement_from_domain(bo, bo->allowed_domains);
r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
@@ -1946,8 +1951,8 @@ static int vcn_v3_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
msg = ptr + addr - start;
- /* Check length */
if (msg[1] > end - addr) {
+ DRM_ERROR("VCN message header does not fit in BO!\n");
r = -EINVAL;
goto out;
}
@@ -1955,7 +1960,16 @@ static int vcn_v3_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
if (msg[3] != RDECODE_MSG_CREATE)
goto out;
+ len_dw = msg[1] / 4;
num_buffers = msg[2];
+
+ /* Verify that all indices fit within the claimed length. Each index is 4 DWORDs */
+ if (num_buffers > len_dw || 6 + num_buffers * 4 > len_dw) {
+ DRM_ERROR("VCN message has too many buffers!\n");
+ r = -EINVAL;
+ goto out;
+ }
+
for (i = 0, msg = &msg[6]; i < num_buffers; ++i, msg += 4) {
uint32_t offset, size, *create;
@@ -1965,14 +1979,15 @@ static int vcn_v3_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
offset = msg[1];
size = msg[2];
- if (offset + size > end) {
+ if (size < 4 || offset + size > end - addr) {
+ DRM_ERROR("VCN message buffer exceeds BO bounds!\n");
r = -EINVAL;
goto out;
}
create = ptr + addr + offset - start;
- /* H246, HEVC and VP9 can run on any instance */
+ /* H264, HEVC and VP9 can run on any instance */
if (create[0] == 0x7 || create[0] == 0x10 || create[0] == 0x11)
continue;
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/4] drm/amdgpu/vcn3: Prevent OOB reads when parsing dec msg
2026-03-26 12:29 ` [PATCH v2 2/4] drm/amdgpu/vcn3: Prevent OOB reads when parsing dec msg Benjamin Cheng
@ 2026-04-09 18:09 ` SDL
0 siblings, 0 replies; 6+ messages in thread
From: SDL @ 2026-04-09 18:09 UTC (permalink / raw)
To: benjamin.cheng
Cc: David.Wu3, alexander.deucher, amd-gfx, christian.koenig, leo.liu,
ruijing.dong, lvc-project
Hi, Benjamin,
I'm working on a related fix for 'vcn_v3_0_dec_msg()' that you might
want to be aware of. In you update bounds check:
if (size < 4 || offset + size > end - addr)
the 'offset' and 'size' fields in the VCN decode message buffer are
both u32 values sourced from userspace. The check performs 32-bit
arithmetic before comparison against the 64-bit 'end' value, allowing
a crafted message with offset=0xffffffff and size=1 to wrap around
to 0 and bypass the bounds check.
Fix this by cast 'offset' to u64 before the addition so the
comparison is done in 64-bit arithmetic.
Best regards,
Alexey Nepomnyashih
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/4] drm/amdgpu/vcn4: Prevent OOB reads when parsing dec msg
2026-03-26 12:29 [PATCH v2 1/4] drm/amdgpu: Add bounds checking to ib_{get,set}_value Benjamin Cheng
2026-03-26 12:29 ` [PATCH v2 2/4] drm/amdgpu/vcn3: Prevent OOB reads when parsing dec msg Benjamin Cheng
@ 2026-03-26 12:29 ` Benjamin Cheng
2026-03-26 12:29 ` [PATCH v2 4/4] drm/amdgpu/vcn4: Prevent OOB reads when parsing IB Benjamin Cheng
2026-03-26 12:54 ` [PATCH v2 1/4] drm/amdgpu: Add bounds checking to ib_{get,set}_value Christian König
3 siblings, 0 replies; 6+ messages in thread
From: Benjamin Cheng @ 2026-03-26 12:29 UTC (permalink / raw)
To: Alex Deucher, Christian König, amd-gfx
Cc: Leo Liu, Ruijing Dong, David (Ming Qiang) Wu, Benjamin Cheng
Check bounds against the end of the BO whenever we access the msg.
Signed-off-by: Benjamin Cheng <benjamin.cheng@amd.com>
---
v2: Moved 4 DWORD check earlier and added more DRM_ERROR logs.
drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index d17219be50f3..1a1cdc14841a 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -1826,7 +1826,7 @@ static int vcn_v4_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
struct ttm_operation_ctx ctx = { false, false };
struct amdgpu_device *adev = p->adev;
struct amdgpu_bo_va_mapping *map;
- uint32_t *msg, num_buffers;
+ uint32_t *msg, num_buffers, len_dw;
struct amdgpu_bo *bo;
uint64_t start, end;
unsigned int i;
@@ -1847,6 +1847,11 @@ static int vcn_v4_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
return -EINVAL;
}
+ if (end - addr < 16) {
+ DRM_ERROR("VCN messages must be at least 4 DWORDs!\n");
+ return -EINVAL;
+ }
+
bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
amdgpu_bo_placement_from_domain(bo, bo->allowed_domains);
r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
@@ -1863,8 +1868,8 @@ static int vcn_v4_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
msg = ptr + addr - start;
- /* Check length */
if (msg[1] > end - addr) {
+ DRM_ERROR("VCN message header does not fit in BO!\n");
r = -EINVAL;
goto out;
}
@@ -1872,7 +1877,16 @@ static int vcn_v4_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
if (msg[3] != RDECODE_MSG_CREATE)
goto out;
+ len_dw = msg[1] / 4;
num_buffers = msg[2];
+
+ /* Verify that all indices fit within the claimed length. Each index is 4 DWORDs */
+ if (num_buffers > len_dw || 6 + num_buffers * 4 > len_dw) {
+ DRM_ERROR("VCN message has too many buffers!\n");
+ r = -EINVAL;
+ goto out;
+ }
+
for (i = 0, msg = &msg[6]; i < num_buffers; ++i, msg += 4) {
uint32_t offset, size, *create;
@@ -1882,7 +1896,8 @@ static int vcn_v4_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
offset = msg[1];
size = msg[2];
- if (offset + size > end) {
+ if (size < 4 || offset + size > end - addr) {
+ DRM_ERROR("VCN message buffer exceeds BO bounds!\n");
r = -EINVAL;
goto out;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 4/4] drm/amdgpu/vcn4: Prevent OOB reads when parsing IB
2026-03-26 12:29 [PATCH v2 1/4] drm/amdgpu: Add bounds checking to ib_{get,set}_value Benjamin Cheng
2026-03-26 12:29 ` [PATCH v2 2/4] drm/amdgpu/vcn3: Prevent OOB reads when parsing dec msg Benjamin Cheng
2026-03-26 12:29 ` [PATCH v2 3/4] drm/amdgpu/vcn4: " Benjamin Cheng
@ 2026-03-26 12:29 ` Benjamin Cheng
2026-03-26 12:54 ` [PATCH v2 1/4] drm/amdgpu: Add bounds checking to ib_{get,set}_value Christian König
3 siblings, 0 replies; 6+ messages in thread
From: Benjamin Cheng @ 2026-03-26 12:29 UTC (permalink / raw)
To: Alex Deucher, Christian König, amd-gfx
Cc: Leo Liu, Ruijing Dong, David (Ming Qiang) Wu, Benjamin Cheng
Rewrite the IB parsing to use amdgpu_ib_get_value() which handles the
bounds checks.
Signed-off-by: Benjamin Cheng <benjamin.cheng@amd.com>
---
v2: Rewrote using ib_get as suggested by Christian.
drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index 1a1cdc14841a..35c014861df2 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -1928,9 +1928,10 @@ static int vcn_v4_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
static int vcn_v4_0_enc_find_ib_param(struct amdgpu_ib *ib, uint32_t id, int start)
{
int i;
+ uint32_t len;
- for (i = start; i < ib->length_dw && ib->ptr[i] >= 8; i += ib->ptr[i] / 4) {
- if (ib->ptr[i + 1] == id)
+ for (i = start; (len = amdgpu_ib_get_value(ib, i)) >= 8; i += len / 4) {
+ if (amdgpu_ib_get_value(ib, i + 1) == id)
return i;
}
return -1;
@@ -1941,7 +1942,6 @@ static int vcn_v4_0_ring_patch_cs_in_place(struct amdgpu_cs_parser *p,
struct amdgpu_ib *ib)
{
struct amdgpu_ring *ring = amdgpu_job_ring(job);
- struct amdgpu_vcn_decode_buffer *decode_buffer;
uint64_t addr;
uint32_t val;
int idx = 0, sidx;
@@ -1953,20 +1953,19 @@ static int vcn_v4_0_ring_patch_cs_in_place(struct amdgpu_cs_parser *p,
while ((idx = vcn_v4_0_enc_find_ib_param(ib, RADEON_VCN_ENGINE_INFO, idx)) >= 0) {
val = amdgpu_ib_get_value(ib, idx + 2); /* RADEON_VCN_ENGINE_TYPE */
if (val == RADEON_VCN_ENGINE_TYPE_DECODE) {
- decode_buffer = (struct amdgpu_vcn_decode_buffer *)&ib->ptr[idx + 6];
-
- if (!(decode_buffer->valid_buf_flag & 0x1))
+ if (!(amdgpu_ib_get_value(ib, idx + 6) & 0x1))
return 0;
- addr = ((u64)decode_buffer->msg_buffer_address_hi) << 32 |
- decode_buffer->msg_buffer_address_lo;
+ addr = ((u64)amdgpu_ib_get_value(ib, idx + 7)) << 32 |
+ amdgpu_ib_get_value(ib, idx + 8);
return vcn_v4_0_dec_msg(p, job, addr);
} else if (val == RADEON_VCN_ENGINE_TYPE_ENCODE) {
sidx = vcn_v4_0_enc_find_ib_param(ib, RENCODE_IB_PARAM_SESSION_INIT, idx);
- if (sidx >= 0 && ib->ptr[sidx + 2] == RENCODE_ENCODE_STANDARD_AV1)
+ if (sidx >= 0 &&
+ amdgpu_ib_get_value(ib, sidx + 2) == RENCODE_ENCODE_STANDARD_AV1)
return vcn_v4_0_limit_sched(p, job);
}
- idx += ib->ptr[idx] / 4;
+ idx += amdgpu_ib_get_value(ib, idx) / 4;
}
return 0;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/4] drm/amdgpu: Add bounds checking to ib_{get,set}_value
2026-03-26 12:29 [PATCH v2 1/4] drm/amdgpu: Add bounds checking to ib_{get,set}_value Benjamin Cheng
` (2 preceding siblings ...)
2026-03-26 12:29 ` [PATCH v2 4/4] drm/amdgpu/vcn4: Prevent OOB reads when parsing IB Benjamin Cheng
@ 2026-03-26 12:54 ` Christian König
3 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2026-03-26 12:54 UTC (permalink / raw)
To: Benjamin Cheng, Alex Deucher, amd-gfx
Cc: Leo Liu, Ruijing Dong, David (Ming Qiang) Wu
On 3/26/26 13:29, Benjamin Cheng wrote:
> The uvd/vce/vcn code accesses the IB at predefined offsets without
> checking that the IB is large enough. Check the bounds here. The caller
> is responsible for making sure it can handle arbitrary return values.
>
> Also make the idx a uint32_t to prevent overflows causing the condition
> to fail.
>
> Signed-off-by: Benjamin Cheng <benjamin.cheng@amd.com>
Patches #1-#3 are Reviewed-by: Christian König <christian.koenig@amd.com>
Patch #4 is Acked-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index ce5af137ee40..715c9e43e13a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -559,15 +559,18 @@ void amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
>
> int amdgpu_ring_init_mqd(struct amdgpu_ring *ring);
>
> -static inline u32 amdgpu_ib_get_value(struct amdgpu_ib *ib, int idx)
> +static inline u32 amdgpu_ib_get_value(struct amdgpu_ib *ib, uint32_t idx)
> {
> - return ib->ptr[idx];
> + if (idx < ib->length_dw)
> + return ib->ptr[idx];
> + return 0;
> }
>
> -static inline void amdgpu_ib_set_value(struct amdgpu_ib *ib, int idx,
> +static inline void amdgpu_ib_set_value(struct amdgpu_ib *ib, uint32_t idx,
> uint32_t value)
> {
> - ib->ptr[idx] = value;
> + if (idx < ib->length_dw)
> + ib->ptr[idx] = value;
> }
>
> int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
^ permalink raw reply [flat|nested] 6+ messages in thread