linux-kernel - Re: [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder context list

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]

Message-ID: <b0b32ef4fb6edb979840b49a3de0278089088f14.camel@mediatek.com>
Date: Tue, 30 Jan 2024 06:29:02 +0000
From: Yunfei Dong (董云飞) <Yunfei.Dong@...iatek.com>
To: "nhebert@...omium.org" <nhebert@...omium.org>,
	"benjamin.gaignard@...labora.com" <benjamin.gaignard@...labora.com>,
	"nfraprado@...labora.com" <nfraprado@...labora.com>,
	"angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>, "nicolas.dufresne@...labora.com"
	<nicolas.dufresne@...labora.com>, "hverkuil-cisco@...all.nl"
	<hverkuil-cisco@...all.nl>, Irui Wang (王瑞)
	<Irui.Wang@...iatek.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"frkoenig@...omium.org" <frkoenig@...omium.org>, "stevecho@...omium.org"
	<stevecho@...omium.org>, "linux-media@...r.kernel.org"
	<linux-media@...r.kernel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "daniel@...ll.ch" <daniel@...ll.ch>,
	Project_Global_Chrome_Upstream_Group
	<Project_Global_Chrome_Upstream_Group@...iatek.com>, "hsinyi@...omium.org"
	<hsinyi@...omium.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect
 decoder context list

Hi AngeloGioacchino,

Thanks for your reviewing.
On Mon, 2024-01-29 at 12:19 +0100, AngeloGioacchino Del Regno wrote:
> Il 29/01/24 03:31, Yunfei Dong ha scritto:
> > The ctx_list will be deleted when scp getting unexpected behavior,
> > then the
> > ctx_list->next will be NULL, the kernel driver maybe access NULL
> > pointer in
> > function vpu_dec_ipi_handler when going through each context, then
> > reboot.
> > 
> > Need to add lock to protect the ctx_list to make sure the ctx_list-
> > >next isn't
> > NULL pointer.
> > 
> > Hardware name: Google juniper sku16 board (DT)
> > pstate: 20400005 (nzCv daif +PAN -UAO -TCO BTYPE=--)
> > pc : vpu_dec_ipi_handler+0x58/0x1f8 [mtk_vcodec_dec]
> > lr : scp_ipi_handler+0xd0/0x194 [mtk_scp]
> > sp : ffffffc0131dbbd0
> > x29: ffffffc0131dbbd0 x28: 0000000000000000
> > x27: ffffff9bb277f348 x26: ffffff9bb242ad00
> > x25: ffffffd2d440d3b8 x24: ffffffd2a13ff1d4
> > x23: ffffff9bb7fe85a0 x22: ffffffc0133fbdb0
> > x21: 0000000000000010 x20: ffffff9b050ea328
> > x19: ffffffc0131dbc08 x18: 0000000000001000
> > x17: 0000000000000000 x16: ffffffd2d461c6e0
> > x15: 0000000000000242 x14: 000000000000018f
> > x13: 000000000000004d x12: 0000000000000000
> > x11: 0000000000000001 x10: fffffffffffffff0
> > x9 : ffffff9bb6e793a8 x8 : 0000000000000000
> > x7 : 0000000000000000 x6 : 000000000000003f
> > x5 : 0000000000000040 x4 : fffffffffffffff0
> > x3 : 0000000000000020 x2 : ffffff9bb6e79080
> > x1 : 0000000000000010 x0 : ffffffc0131dbc08
> > Call trace:
> > vpu_dec_ipi_handler+0x58/0x1f8 [mtk_vcodec_dec (HASH:6c3f 2)]
> > scp_ipi_handler+0xd0/0x194 [mtk_scp (HASH:7046 3)]
> > mt8183_scp_irq_handler+0x44/0x88 [mtk_scp (HASH:7046 3)]
> > scp_irq_handler+0x48/0x90 [mtk_scp (HASH:7046 3)]
> > irq_thread_fn+0x38/0x94
> > irq_thread+0x100/0x1c0
> > kthread+0x140/0x1fc
> > ret_from_fork+0x10/0x30
> > Code: 54000088 f94ca50a eb14015f 54000060 (f9400108)
> > ---[ end trace ace43ce36cbd5c93 ]---
> > Kernel panic - not syncing: Oops: Fatal exception
> > SMP: stopping secondary CPUs
> > Kernel Offset: 0x12c4000000 from 0xffffffc010000000
> > PHYS_OFFSET: 0xffffffe580000000
> > CPU features: 0x08240002,2188200c
> > Memory Limit: none
> > 
> > 'Fixes: 655b86e52eac ("media: mediatek: vcodec: Fix possible
> > invalid memory access for decoder")'
> 
> Hello Yunfei,
> 
> You've sent two patches as a v2, but:
>   - The two patches are identical (!) apart from the commit message?!
>   - It's Fixes: xxxx , not 'Fixes: xxxx' (please remove the quotes!)
>   - There's no changelog from v1, so, what changed in v2?!
> 
1> These two patch used to fix the same issue, just used to separate
encoder with decoder;
2> Will fix in next patch;
3> patch 1 are the same for v1 and v2, just the patch 2 (encoder)
change something.

Best Regards,
Yunfei Dong 
> Cheers,
> Angelo
> 
> > Signed-off-by: Yunfei Dong <yunfei.dong@...iatek.com>
> > ---
> >   .../platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c      | 4
> > ++--
> >   .../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c    | 5
> > +++++
> >   .../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h    | 2
> > ++
> >   drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c | 2
> > ++
> >   4 files changed, 11 insertions(+), 2 deletions(-)
> > 
> 
>