[v1] media: vicodec: zero-initialize stateful decoder heap buffers

[PATCH] media: vicodec: zero-initialize stateful decoder heap buffers

Posted by Junrui Luo via B4 Relay 1 month, 2 weeks ago

From: Junrui Luo <moonafterrain@outlook.com>

vicodec_start_streaming() allocates state->ref_frame.buf and the
compressed_frame buffer with kvmalloc() for the stateful decoder and
leaves both uninitialized.

decode_plane() derives is_intra from the reference pointer being NULL
(is_intra = !ref) rather than from frame-sequence state, and the
stateful decoder always passes a valid ref, so a P-coded first frame
reaches add_deltas() over stale heap content that is then folded into
the decoded frame and returned via VIDIOC_DQBUF. The padding rows
between visible_height and coded_height leak on every P-frame as well,
since copy_cap_to_ref() writes only visible_height rows while
decode_plane() reads up to round_up(visible_height, 8). For
compressed_frame, only comp_size bytes are copied into the new
comp_max_size allocation, leaving the tail uninitialized for derlc() to
walk into.

Use kvzalloc() for both allocations.

Fixes: 256bf813ba39 ("media: vicodec: add the virtual codec driver")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Cc: stable@vger.kernel.org
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 drivers/media/test-drivers/vicodec/vicodec-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/test-drivers/vicodec/vicodec-core.c b/drivers/media/test-drivers/vicodec/vicodec-core.c
index 318e8330f16a..7ebde8f3fc37 100644
--- a/drivers/media/test-drivers/vicodec/vicodec-core.c
+++ b/drivers/media/test-drivers/vicodec/vicodec-core.c
@@ -1595,9 +1595,9 @@ static int vicodec_start_streaming(struct vb2_queue *q,
 	}
 	state->ref_stride = q_data->coded_width * info->luma_alpha_step;
 
-	state->ref_frame.buf = kvmalloc(total_planes_size, GFP_KERNEL);
+	state->ref_frame.buf = kvzalloc(total_planes_size, GFP_KERNEL);
 	state->ref_frame.luma = state->ref_frame.buf;
-	new_comp_frame = kvmalloc(ctx->comp_max_size, GFP_KERNEL);
+	new_comp_frame = kvzalloc(ctx->comp_max_size, GFP_KERNEL);
 
 	if (!state->ref_frame.luma || !new_comp_frame) {
 		kvfree(state->ref_frame.luma);

---
base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a
change-id: 20260813-vicodec-fixes-a91ec4217ce3

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

Re: [PATCH] media: vicodec: zero-initialize stateful decoder heap buffers

Posted by Nicolas Dufresne 1 month, 2 weeks ago

Hi,

Le jeudi 13 août 2026 à 13:51 +0800, Junrui Luo via B4 Relay a écrit :
> From: Junrui Luo <moonafterrain@outlook.com>
> 
> vicodec_start_streaming() allocates state->ref_frame.buf and the
> compressed_frame buffer with kvmalloc() for the stateful decoder and
> leaves both uninitialized.
> 
> decode_plane() derives is_intra from the reference pointer being NULL
> (is_intra = !ref) rather than from frame-sequence state, and the
> stateful decoder always passes a valid ref, so a P-coded first frame
> reaches add_deltas() over stale heap content that is then folded into
> the decoded frame and returned via VIDIOC_DQBUF. The padding rows
> between visible_height and coded_height leak on every P-frame as well,
> since copy_cap_to_ref() writes only visible_height rows while
> decode_plane() reads up to round_up(visible_height, 8). For
> compressed_frame, only comp_size bytes are copied into the new
> comp_max_size allocation, leaving the tail uninitialized for derlc() to
> walk into.

There is a lot of description of what the code does already, and what your
change does, but for some reason, it seem you have forgotten the essential,
which is the rationale for this patch. Please focus on what this patch fixes,
and then go into details. Looking forward a v2.

Nicolas

> 
> Use kvzalloc() for both allocations.
> 
> Fixes: 256bf813ba39 ("media: vicodec: add the virtual codec driver")
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Assisted-by: Claude:claude-opus-5
> Cc: stable@vger.kernel.org
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> ---
>  drivers/media/test-drivers/vicodec/vicodec-core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/test-drivers/vicodec/vicodec-core.c b/drivers/media/test-drivers/vicodec/vicodec-core.c
> index 318e8330f16a..7ebde8f3fc37 100644
> --- a/drivers/media/test-drivers/vicodec/vicodec-core.c
> +++ b/drivers/media/test-drivers/vicodec/vicodec-core.c
> @@ -1595,9 +1595,9 @@ static int vicodec_start_streaming(struct vb2_queue *q,
>  	}
>  	state->ref_stride = q_data->coded_width * info->luma_alpha_step;
>  
> -	state->ref_frame.buf = kvmalloc(total_planes_size, GFP_KERNEL);
> +	state->ref_frame.buf = kvzalloc(total_planes_size, GFP_KERNEL);
>  	state->ref_frame.luma = state->ref_frame.buf;
> -	new_comp_frame = kvmalloc(ctx->comp_max_size, GFP_KERNEL);
> +	new_comp_frame = kvzalloc(ctx->comp_max_size, GFP_KERNEL);
>  
>  	if (!state->ref_frame.luma || !new_comp_frame) {
>  		kvfree(state->ref_frame.luma);
> 
> ---
> base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a
> change-id: 20260813-vicodec-fixes-a91ec4217ce3
> 
> Best regards,