linux-kernel - Re: [PATCH v7,0/11] media: mediatek: vcodec: separate encoder and decoder

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 28 Jul 2023 11:25:47 +0200
From:   Hans Verkuil <hverkuil-cisco@...all.nl>
To:     Yunfei Dong <yunfei.dong@...iatek.com>,
        Nícolas F . R . A . Prado 
        <nfraprado@...labora.com>,
        Nicolas Dufresne <nicolas.dufresne@...labora.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        Benjamin Gaignard <benjamin.gaignard@...labora.com>,
        Nathan Hebert <nhebert@...omium.org>
Cc:     Chen-Yu Tsai <wenst@...omium.org>,
        Hsin-Yi Wang <hsinyi@...omium.org>,
        Fritz Koenig <frkoenig@...omium.org>,
        Daniel Vetter <daniel@...ll.ch>,
        Steve Cho <stevecho@...omium.org>, linux-media@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org,
        Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v7,0/11] media: mediatek: vcodec: separate encoder and
 decoder

On 25/07/2023 11:58, Hans Verkuil wrote:
> On 22/07/2023 09:42, Yunfei Dong wrote:
>> From: Yunfei Dong <yunfei.dong@...iatek.corp-partner.google.com>
>>
>> With the driver more and more complex, encoder and decoder need to add more parameter
>> in shared struct 'mtk_vcodec_ctx' and 'mtk_vcodec_dev'. Encoder use about 40% and
>> decoder use 60% parameter. Need to allocate extra unused memory when encoder and decoder
>> working.
>>
>> Separate encoder and decoder in different folder and use independent data struct.
>>
>> patch 1 remove unused parameter.
>> patch 2~3 align fw and interrupt related interface.
>> patch 4~6 remove the dependency of debug log
>> patch 7~8 separate mtk_vcodec_ctx and mtk_vcodec_dev
>> patch 9 fix unreasonable parameter
>> patch 10 removed unused header files
>> patch 11 separate encoder and decoder in different folder
>> ---
>> Changed from v6:
>> - rebase to: https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=fo-v6.6g.
>> Changed from v5:
>> - fix some words error for patch 3/6/11.
>> - rename mtk_vcodec_comm_drv.h to mtk_vcodec_cmn_drv.h for patch 7.
>> Changed from v4:
>> - add one parameter to record register base for reg_base for patch 3.
>> - add debug string for non ctx log for patch 6.
>> - change the comment of struct mtk_vcodec_dec_ctx and struct mtk_vcodec_enc_ctx for patch 7.
>> - prefer to use struct mtk_vcodec_dec_dev an current period, will re-construct in the future for patch 8.
>> Changed from v3:
>> - re-write commit message for patch 3.
>> Changed from v2:
>> - This patch main changed:
>>   1: add different macro mtk_dec_debug and mtk_enc_debug calling common
>>      macro mtk_vcodec_debug in order to use dev_dbg instead of pr_debug.
>>   2: add different macro mtk_v4l2_venc_dbg and mtk_v4l2_vdec_dbg calling common
>>      macro  in order to use dev_dbg instead of pr_debug.
>> Changed from v1:
>> - Change pr_dbg to dev_dbg for mtk_v4l2_level and mtk_vcodec_dbg for patch 4.
>> - Change pr_err to dev_err for mtk_v4l2_err and mtk_vcodec_err for patch 5.
>> - Fix unreasonable parameter for patch 8.
>> ---
>> Yunfei Dong (11):
>>   media: mediatek: vcodec: remove unused parameter
>>   media: mediatek: vcodec: align fw interface
>>   media: mediatek: vcodec: Removing struct 'mtk_vcodec_ctx/dev' for
>>     shared interface
>>   media: mediatek: vcodec: Removing useless debug log
>>   media: mediatek: vcodec: remove the dependency of vcodec debug log
>>   media: mediatek: vcodec: replace pr_* with dev_* for v4l2 debug
>>     message
>>   media: mediatek: vcodec: separate struct 'mtk_vcodec_ctx'
>>   media: mediatek: vcodec: separate struct mtk_vcodec_dev
>>   media: mediatek: vcodec: fix unreasonable parameter definition and
>>     style
>>   media: mediatek: vcodec: remove unused include header
>>   media: mediatek: vcodec: separate decoder and encoder
> 
> Besides the missing argument in patch 6/11 I also get this compiler warning:
> 
> drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c: In function 'vpu_enc_ipi_handler':
> drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c:64:31: warning: 'vpu' may be used uninitialized [-Wmaybe-uninitialized]
>    64 |         struct venc_vpu_inst *vpu;
>       |                               ^~~
> 
> and this smatch error:
> 
> drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:138 mtk_vcodec_get_reg_bases() error: buffer overflow 'mtk_dec_reg_names' 11 <= 11
> 
> However, I believe that was introduced by Nicolas' patch series.
> 
> I'll try to pinpoint the precise patch.

That smatch error is now found and fixed in the staging tree.

Can you post a v8 fixing the other issues?

Thanks!

Regards,

	Hans

> 
> Regards,
> 
> 	Hans