From: Yunfei Dong <yunfei.dong@mediatek.com> To: "Nícolas F . R . A . Prado" <nfraprado@collabora.com>, "Nicolas Dufresne" <nicolas.dufresne@collabora.com>, "Hans Verkuil" <hverkuil-cisco@xs4all.nl>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>, "Benjamin Gaignard" <benjamin.gaignard@collabora.com>, "Nathan Hebert" <nhebert@chromium.org>, "Sebastian Fricke" <sebastian.fricke@collabora.com>, "Irui Wang" <irui.wang@mediatek.com> Cc: Hsin-Yi Wang <hsinyi@chromium.org>, Fritz Koenig <frkoenig@chromium.org>, Daniel Vetter <daniel@ffwll.ch>, Steve Cho <stevecho@chromium.org>, "Yunfei Dong" <yunfei.dong@mediatek.com>, <linux-media@vger.kernel.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-mediatek@lists.infradead.org>, <Project_Global_Chrome_Upstream_Group@mediatek.com> Subject: [PATCH v4,1/2] media: mediatek: vcodec: adding lock to protect decoder context list Date: Thu, 14 Mar 2024 20:30:08 +0800 [thread overview] Message-ID: <20240314123009.20205-2-yunfei.dong@mediatek.com> (raw) In-Reply-To: <20240314123009.20205-1-yunfei.dong@mediatek.com> Add a lock for the ctx_list, to avoid accessing a NULL pointer within the 'vpu_dec_ipi_handler' fucntion when the ctx_list has been deleted due to an unexpected behavior on the SCP IP block. 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") Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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(-) diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c index 9f6e4b59455d..9a11a2c24804 100644 --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c @@ -58,12 +58,12 @@ static void mtk_vcodec_vpu_reset_dec_handler(void *priv) dev_err(&dev->plat_dev->dev, "Watchdog timeout!!"); - mutex_lock(&dev->dev_mutex); + mutex_lock(&dev->dev_ctx_lock); list_for_each_entry(ctx, &dev->ctx_list, list) { ctx->state = MTK_STATE_ABORT; mtk_v4l2_vdec_dbg(0, ctx, "[%d] Change to state MTK_STATE_ABORT", ctx->id); } - mutex_unlock(&dev->dev_mutex); + mutex_unlock(&dev->dev_ctx_lock); } static void mtk_vcodec_vpu_reset_enc_handler(void *priv) diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c index f47c98faf068..2073781ccadb 100644 --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c @@ -268,7 +268,9 @@ static int fops_vcodec_open(struct file *file) ctx->dev->vdec_pdata->init_vdec_params(ctx); + mutex_lock(&dev->dev_ctx_lock); list_add(&ctx->list, &dev->ctx_list); + mutex_unlock(&dev->dev_ctx_lock); mtk_vcodec_dbgfs_create(ctx); mutex_unlock(&dev->dev_mutex); @@ -311,7 +313,9 @@ static int fops_vcodec_release(struct file *file) v4l2_ctrl_handler_free(&ctx->ctrl_hdl); mtk_vcodec_dbgfs_remove(dev, ctx->id); + mutex_lock(&dev->dev_ctx_lock); list_del_init(&ctx->list); + mutex_unlock(&dev->dev_ctx_lock); kfree(ctx); mutex_unlock(&dev->dev_mutex); return 0; @@ -404,6 +408,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev) for (i = 0; i < MTK_VDEC_HW_MAX; i++) mutex_init(&dev->dec_mutex[i]); mutex_init(&dev->dev_mutex); + mutex_init(&dev->dev_ctx_lock); spin_lock_init(&dev->irqlock); snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s", diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h index 849b89dd205c..85b2c0d3d8bc 100644 --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h @@ -241,6 +241,7 @@ struct mtk_vcodec_dec_ctx { * * @dec_mutex: decoder hardware lock * @dev_mutex: video_device lock + * @dev_ctx_lock: the lock of context list * @decode_workqueue: decode work queue * * @irqlock: protect data access by irq handler and work thread @@ -282,6 +283,7 @@ struct mtk_vcodec_dec_dev { /* decoder hardware mutex lock */ struct mutex dec_mutex[MTK_VDEC_HW_MAX]; struct mutex dev_mutex; + struct mutex dev_ctx_lock; struct workqueue_struct *decode_workqueue; spinlock_t irqlock; diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c index 82e57ae983d5..da6be556727b 100644 --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c @@ -77,12 +77,14 @@ static bool vpu_dec_check_ap_inst(struct mtk_vcodec_dec_dev *dec_dev, struct vde struct mtk_vcodec_dec_ctx *ctx; int ret = false; + mutex_lock(&dec_dev->dev_ctx_lock); list_for_each_entry(ctx, &dec_dev->ctx_list, list) { if (!IS_ERR_OR_NULL(ctx) && ctx->vpu_inst == vpu) { ret = true; break; } } + mutex_unlock(&dec_dev->dev_ctx_lock); return ret; } -- 2.18.0
WARNING: multiple messages have this Message-ID (diff)
From: Yunfei Dong <yunfei.dong@mediatek.com> To: "Nícolas F . R . A . Prado" <nfraprado@collabora.com>, "Nicolas Dufresne" <nicolas.dufresne@collabora.com>, "Hans Verkuil" <hverkuil-cisco@xs4all.nl>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>, "Benjamin Gaignard" <benjamin.gaignard@collabora.com>, "Nathan Hebert" <nhebert@chromium.org>, "Sebastian Fricke" <sebastian.fricke@collabora.com>, "Irui Wang" <irui.wang@mediatek.com> Cc: Hsin-Yi Wang <hsinyi@chromium.org>, Fritz Koenig <frkoenig@chromium.org>, Daniel Vetter <daniel@ffwll.ch>, Steve Cho <stevecho@chromium.org>, "Yunfei Dong" <yunfei.dong@mediatek.com>, <linux-media@vger.kernel.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-mediatek@lists.infradead.org>, <Project_Global_Chrome_Upstream_Group@mediatek.com> Subject: [PATCH v4,1/2] media: mediatek: vcodec: adding lock to protect decoder context list Date: Thu, 14 Mar 2024 20:30:08 +0800 [thread overview] Message-ID: <20240314123009.20205-2-yunfei.dong@mediatek.com> (raw) In-Reply-To: <20240314123009.20205-1-yunfei.dong@mediatek.com> Add a lock for the ctx_list, to avoid accessing a NULL pointer within the 'vpu_dec_ipi_handler' fucntion when the ctx_list has been deleted due to an unexpected behavior on the SCP IP block. 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") Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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(-) diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c index 9f6e4b59455d..9a11a2c24804 100644 --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c @@ -58,12 +58,12 @@ static void mtk_vcodec_vpu_reset_dec_handler(void *priv) dev_err(&dev->plat_dev->dev, "Watchdog timeout!!"); - mutex_lock(&dev->dev_mutex); + mutex_lock(&dev->dev_ctx_lock); list_for_each_entry(ctx, &dev->ctx_list, list) { ctx->state = MTK_STATE_ABORT; mtk_v4l2_vdec_dbg(0, ctx, "[%d] Change to state MTK_STATE_ABORT", ctx->id); } - mutex_unlock(&dev->dev_mutex); + mutex_unlock(&dev->dev_ctx_lock); } static void mtk_vcodec_vpu_reset_enc_handler(void *priv) diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c index f47c98faf068..2073781ccadb 100644 --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c @@ -268,7 +268,9 @@ static int fops_vcodec_open(struct file *file) ctx->dev->vdec_pdata->init_vdec_params(ctx); + mutex_lock(&dev->dev_ctx_lock); list_add(&ctx->list, &dev->ctx_list); + mutex_unlock(&dev->dev_ctx_lock); mtk_vcodec_dbgfs_create(ctx); mutex_unlock(&dev->dev_mutex); @@ -311,7 +313,9 @@ static int fops_vcodec_release(struct file *file) v4l2_ctrl_handler_free(&ctx->ctrl_hdl); mtk_vcodec_dbgfs_remove(dev, ctx->id); + mutex_lock(&dev->dev_ctx_lock); list_del_init(&ctx->list); + mutex_unlock(&dev->dev_ctx_lock); kfree(ctx); mutex_unlock(&dev->dev_mutex); return 0; @@ -404,6 +408,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev) for (i = 0; i < MTK_VDEC_HW_MAX; i++) mutex_init(&dev->dec_mutex[i]); mutex_init(&dev->dev_mutex); + mutex_init(&dev->dev_ctx_lock); spin_lock_init(&dev->irqlock); snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s", diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h index 849b89dd205c..85b2c0d3d8bc 100644 --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h @@ -241,6 +241,7 @@ struct mtk_vcodec_dec_ctx { * * @dec_mutex: decoder hardware lock * @dev_mutex: video_device lock + * @dev_ctx_lock: the lock of context list * @decode_workqueue: decode work queue * * @irqlock: protect data access by irq handler and work thread @@ -282,6 +283,7 @@ struct mtk_vcodec_dec_dev { /* decoder hardware mutex lock */ struct mutex dec_mutex[MTK_VDEC_HW_MAX]; struct mutex dev_mutex; + struct mutex dev_ctx_lock; struct workqueue_struct *decode_workqueue; spinlock_t irqlock; diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c index 82e57ae983d5..da6be556727b 100644 --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c @@ -77,12 +77,14 @@ static bool vpu_dec_check_ap_inst(struct mtk_vcodec_dec_dev *dec_dev, struct vde struct mtk_vcodec_dec_ctx *ctx; int ret = false; + mutex_lock(&dec_dev->dev_ctx_lock); list_for_each_entry(ctx, &dec_dev->ctx_list, list) { if (!IS_ERR_OR_NULL(ctx) && ctx->vpu_inst == vpu) { ret = true; break; } } + mutex_unlock(&dec_dev->dev_ctx_lock); return ret; } -- 2.18.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-03-14 12:30 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-03-14 12:30 [PATCH v4,0/2] media: adding lock to protect the context list Yunfei Dong 2024-03-14 12:30 ` Yunfei Dong 2024-03-14 12:30 ` Yunfei Dong [this message] 2024-03-14 12:30 ` [PATCH v4,1/2] media: mediatek: vcodec: adding lock to protect decoder " Yunfei Dong 2024-03-14 12:30 ` [PATCH v4,2/2] media: mediatek: vcodec: adding lock to protect encoder " Yunfei Dong 2024-03-14 12:30 ` Yunfei Dong
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20240314123009.20205-2-yunfei.dong@mediatek.com \ --to=yunfei.dong@mediatek.com \ --cc=Project_Global_Chrome_Upstream_Group@mediatek.com \ --cc=angelogioacchino.delregno@collabora.com \ --cc=benjamin.gaignard@collabora.com \ --cc=daniel@ffwll.ch \ --cc=devicetree@vger.kernel.org \ --cc=frkoenig@chromium.org \ --cc=hsinyi@chromium.org \ --cc=hverkuil-cisco@xs4all.nl \ --cc=irui.wang@mediatek.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=linux-mediatek@lists.infradead.org \ --cc=nfraprado@collabora.com \ --cc=nhebert@chromium.org \ --cc=nicolas.dufresne@collabora.com \ --cc=sebastian.fricke@collabora.com \ --cc=stevecho@chromium.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.