Re: [PATCH 1/3] media: mediatek: vcodec: fix h264 multi statless decoder smatch warning

From: Andrzej Pietrasiewicz
Date: Wed May 29 2024 - 09:14:36 EST


Hi,

W dniu 3.04.2024 o 05:45, Yunfei Dong (董云飞) pisze:
Hi AngeloGioacchino,

Thanks for your reviewing.
On Tue, 2024-04-02 at 11:50 +0200, AngeloGioacchino Del Regno wrote:
Il 29/02/24 10:56, Yunfei Dong ha scritto:
Fix smatch static checker warning for vdec_h264_req_multi_if.c.
Leading to kernel crash when fb is NULL.

Fixes: 397edc703a10 ("media: mediatek: vcodec: add h264 decoder")
Signed-off-by: Yunfei Dong <yunfei.dong@xxxxxxxxxxxx>
---
.../vcodec/decoder/vdec/vdec_h264_req_multi_if.c | 9
+++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git
a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req
_multi_if.c
b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req
_multi_if.c
index 0e741e0dc8ba..ab8e708e0df1 100644
---
a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req
_multi_if.c
+++
b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req
_multi_if.c
@@ -724,11 +724,16 @@ static int vdec_h264_slice_single_decode(void
*h_vdec, struct mtk_vcodec_mem *bs
return vpu_dec_reset(vpu);
fb = inst->ctx->dev->vdec_pdata->get_cap_buffer(inst->ctx);
+ if (!fb) {
+ mtk_vdec_err(inst->ctx, "fb buffer is NULL");
+ return -EBUSY;
+ }
+
src_buf_info = container_of(bs, struct mtk_video_dec_buf,
bs_buffer);
dst_buf_info = container_of(fb, struct mtk_video_dec_buf,
frame_buffer);
- y_fb_dma = fb ? (u64)fb->base_y.dma_addr : 0;
- c_fb_dma = fb ? (u64)fb->base_c.dma_addr : 0;

You're changing the behavior here, can you please explain why this
change is valid
into the commit description?

The driver already add the condition to check whether fb is NULL at the
front, no need these two lines again.


Maybe Angelo refers to the function never returning -EBUSY before?
While at it, if fb is a kind of a buffer, why not -ENOMEM
when get_cap_buffer() fails?

Regards,

Andrzej

Thanks,
Angelo

Best Regards,
Yunfei Dong
+ y_fb_dma = (u64)fb->base_y.dma_addr;
+ c_fb_dma = (u64)fb->base_c.dma_addr;
mtk_vdec_debug(inst->ctx, "[h264-dec] [%d] y_dma=%llx
c_dma=%llx",
inst->ctx->decoded_frame_cnt, y_fb_dma,
c_fb_dma);