linux-kernel - Re: [PATCH] media: mediatek: vcodec: Use spinlock for context list protection lock

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: <CAC=S1nhz_ESY4VrgWs=dVinLtdOamh6to3EgD1w1Kx=4YBOD9A@mail.gmail.com>
Date: Thu, 14 Aug 2025 16:58:21 +0800
From: Fei Shao <fshao@...omium.org>
To: Chen-Yu Tsai <wenst@...omium.org>
Cc: Yunfei Dong <yunfei.dong@...iatek.com>, Mauro Carvalho Chehab <mchehab@...nel.org>, 
	Hans Verkuil <hverkuil@...all.nl>, Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, linux-media@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH] media: mediatek: vcodec: Use spinlock for context list
 protection lock

On Thu, Aug 14, 2025 at 4:38 PM Chen-Yu Tsai <wenst@...omium.org> wrote:
>
> Previously a mutex was added to protect the encoder and decoder context
> lists from unexpected changes originating from the SCP IP block, causing
> the context pointer to go invalid, resulting in a NULL pointer
> dereference in the IPI handler.
>
> Turns out on the MT8173, the VPU IPI handler is called from hard IRQ
> context. This causes a big warning from the scheduler. This was first
> reported downstream on the ChromeOS kernels, but is also reproducible
> on mainline using Fluster with the FFmpeg v4l2m2m decoders. Even though
> the actual capture format is not supported, the affected code paths
> are triggered.
>
> Since this lock just protects the context list and operations on it are
> very fast, it should be OK to switch to a spinlock.
>
> Fixes: 6467cda18c9f ("media: mediatek: vcodec: adding lock to protect decoder context list")
> Fixes: afaaf3a0f647 ("media: mediatek: vcodec: adding lock to protect encoder context list")
> Cc: Yunfei Dong <yunfei.dong@...iatek.com>
> Signed-off-by: Chen-Yu Tsai <wenst@...omium.org>
> ---
>  .../mediatek/vcodec/common/mtk_vcodec_fw_vpu.c       | 10 ++++++----
>  .../mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c     | 12 +++++++-----
>  .../mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h     |  2 +-
>  .../platform/mediatek/vcodec/decoder/vdec_vpu_if.c   |  4 ++--
>  .../mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c     | 12 +++++++-----
>  .../mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h     |  2 +-
>  .../platform/mediatek/vcodec/encoder/venc_vpu_if.c   |  4 ++--
>  7 files changed, 26 insertions(+), 20 deletions(-)
>

[...]

> 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 145958206e38..e9b5cac9c63b 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
> @@ -77,14 +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);
> +       spin_lock(&dec_dev->dev_ctx_lock);

Do you mean spin_lock_irqsave()?

>         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);
> +       spin_unlock(&dec_dev->dev_ctx_lock);
>
>         return ret;
>  }

[...]

> diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
> index 51bb7ee141b9..79a91283da78 100644
> --- a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
> @@ -47,14 +47,14 @@ static bool vpu_enc_check_ap_inst(struct mtk_vcodec_enc_dev *enc_dev, struct ven
>         struct mtk_vcodec_enc_ctx *ctx;
>         int ret = false;
>
> -       mutex_lock(&enc_dev->dev_ctx_lock);
> +       spin_lock(&enc_dev->dev_ctx_lock);

Also here.

Regards,
Fei

>         list_for_each_entry(ctx, &enc_dev->ctx_list, list) {
>                 if (!IS_ERR_OR_NULL(ctx) && ctx->vpu_inst == vpu) {
>                         ret = true;
>                         break;
>                 }
>         }
> -       mutex_unlock(&enc_dev->dev_ctx_lock);
> +       spin_unlock(&enc_dev->dev_ctx_lock);
>
>         return ret;
>  }
> --
> 2.51.0.rc1.163.g2494970778-goog
>
>