Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

media: mediatek: vcodec: Use spinlock for context list protection lock

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@mediatek.com>
Cc: stable@vger.kernel.org
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Fei Shao <fshao@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>

authored by

Chen-Yu Tsai and committed by
Hans Verkuil
a5844227 8652359f

+28 -20
+6 -4
drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
··· 47 47 { 48 48 struct mtk_vcodec_dec_dev *dev = priv; 49 49 struct mtk_vcodec_dec_ctx *ctx; 50 + unsigned long flags; 50 51 51 52 dev_err(&dev->plat_dev->dev, "Watchdog timeout!!"); 52 53 53 - mutex_lock(&dev->dev_ctx_lock); 54 + spin_lock_irqsave(&dev->dev_ctx_lock, flags); 54 55 list_for_each_entry(ctx, &dev->ctx_list, list) { 55 56 ctx->state = MTK_STATE_ABORT; 56 57 mtk_v4l2_vdec_dbg(0, ctx, "[%d] Change to state MTK_STATE_ABORT", ctx->id); 57 58 } 58 - mutex_unlock(&dev->dev_ctx_lock); 59 + spin_unlock_irqrestore(&dev->dev_ctx_lock, flags); 59 60 } 60 61 61 62 static void mtk_vcodec_vpu_reset_enc_handler(void *priv) 62 63 { 63 64 struct mtk_vcodec_enc_dev *dev = priv; 64 65 struct mtk_vcodec_enc_ctx *ctx; 66 + unsigned long flags; 65 67 66 68 dev_err(&dev->plat_dev->dev, "Watchdog timeout!!"); 67 69 68 - mutex_lock(&dev->dev_ctx_lock); 70 + spin_lock_irqsave(&dev->dev_ctx_lock, flags); 69 71 list_for_each_entry(ctx, &dev->ctx_list, list) { 70 72 ctx->state = MTK_STATE_ABORT; 71 73 mtk_v4l2_vdec_dbg(0, ctx, "[%d] Change to state MTK_STATE_ABORT", ctx->id); 72 74 } 73 - mutex_unlock(&dev->dev_ctx_lock); 75 + spin_unlock_irqrestore(&dev->dev_ctx_lock, flags); 74 76 } 75 77 76 78 static const struct mtk_vcodec_fw_ops mtk_vcodec_vpu_msg = {
+7 -5
drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
··· 198 198 struct mtk_vcodec_dec_ctx *ctx = NULL; 199 199 int ret = 0, i, hw_count; 200 200 struct vb2_queue *src_vq; 201 + unsigned long flags; 201 202 202 203 ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); 203 204 if (!ctx) ··· 268 267 269 268 ctx->dev->vdec_pdata->init_vdec_params(ctx); 270 269 271 - mutex_lock(&dev->dev_ctx_lock); 270 + spin_lock_irqsave(&dev->dev_ctx_lock, flags); 272 271 list_add(&ctx->list, &dev->ctx_list); 273 - mutex_unlock(&dev->dev_ctx_lock); 272 + spin_unlock_irqrestore(&dev->dev_ctx_lock, flags); 274 273 mtk_vcodec_dbgfs_create(ctx); 275 274 276 275 mutex_unlock(&dev->dev_mutex); ··· 295 294 { 296 295 struct mtk_vcodec_dec_dev *dev = video_drvdata(file); 297 296 struct mtk_vcodec_dec_ctx *ctx = file_to_dec_ctx(file); 297 + unsigned long flags; 298 298 299 299 mtk_v4l2_vdec_dbg(0, ctx, "[%d] decoder", ctx->id); 300 300 mutex_lock(&dev->dev_mutex); ··· 314 312 v4l2_ctrl_handler_free(&ctx->ctrl_hdl); 315 313 316 314 mtk_vcodec_dbgfs_remove(dev, ctx->id); 317 - mutex_lock(&dev->dev_ctx_lock); 315 + spin_lock_irqsave(&dev->dev_ctx_lock, flags); 318 316 list_del_init(&ctx->list); 319 - mutex_unlock(&dev->dev_ctx_lock); 317 + spin_unlock_irqrestore(&dev->dev_ctx_lock, flags); 320 318 kfree(ctx); 321 319 mutex_unlock(&dev->dev_mutex); 322 320 return 0; ··· 409 407 for (i = 0; i < MTK_VDEC_HW_MAX; i++) 410 408 mutex_init(&dev->dec_mutex[i]); 411 409 mutex_init(&dev->dev_mutex); 412 - mutex_init(&dev->dev_ctx_lock); 410 + spin_lock_init(&dev->dev_ctx_lock); 413 411 spin_lock_init(&dev->irqlock); 414 412 415 413 snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
+1 -1
drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
··· 285 285 /* decoder hardware mutex lock */ 286 286 struct mutex dec_mutex[MTK_VDEC_HW_MAX]; 287 287 struct mutex dev_mutex; 288 - struct mutex dev_ctx_lock; 288 + spinlock_t dev_ctx_lock; 289 289 struct workqueue_struct *decode_workqueue; 290 290 291 291 spinlock_t irqlock;
+3 -2
drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
··· 75 75 static bool vpu_dec_check_ap_inst(struct mtk_vcodec_dec_dev *dec_dev, struct vdec_vpu_inst *vpu) 76 76 { 77 77 struct mtk_vcodec_dec_ctx *ctx; 78 + unsigned long flags; 78 79 int ret = false; 79 80 80 - mutex_lock(&dec_dev->dev_ctx_lock); 81 + spin_lock_irqsave(&dec_dev->dev_ctx_lock, flags); 81 82 list_for_each_entry(ctx, &dec_dev->ctx_list, list) { 82 83 if (!IS_ERR_OR_NULL(ctx) && ctx->vpu_inst == vpu) { 83 84 ret = true; 84 85 break; 85 86 } 86 87 } 87 - mutex_unlock(&dec_dev->dev_ctx_lock); 88 + spin_unlock_irqrestore(&dec_dev->dev_ctx_lock, flags); 88 89 89 90 return ret; 90 91 }
+7 -5
drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
··· 117 117 struct mtk_vcodec_enc_ctx *ctx = NULL; 118 118 int ret = 0; 119 119 struct vb2_queue *src_vq; 120 + unsigned long flags; 120 121 121 122 ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); 122 123 if (!ctx) ··· 177 176 mtk_v4l2_venc_dbg(2, ctx, "Create instance [%d]@%p m2m_ctx=%p ", 178 177 ctx->id, ctx, ctx->m2m_ctx); 179 178 180 - mutex_lock(&dev->dev_ctx_lock); 179 + spin_lock_irqsave(&dev->dev_ctx_lock, flags); 181 180 list_add(&ctx->list, &dev->ctx_list); 182 - mutex_unlock(&dev->dev_ctx_lock); 181 + spin_unlock_irqrestore(&dev->dev_ctx_lock, flags); 183 182 184 183 mutex_unlock(&dev->dev_mutex); 185 184 mtk_v4l2_venc_dbg(0, ctx, "%s encoder [%d]", dev_name(&dev->plat_dev->dev), ··· 204 203 { 205 204 struct mtk_vcodec_enc_dev *dev = video_drvdata(file); 206 205 struct mtk_vcodec_enc_ctx *ctx = file_to_enc_ctx(file); 206 + unsigned long flags; 207 207 208 208 mtk_v4l2_venc_dbg(1, ctx, "[%d] encoder", ctx->id); 209 209 mutex_lock(&dev->dev_mutex); ··· 215 213 v4l2_fh_exit(&ctx->fh); 216 214 v4l2_ctrl_handler_free(&ctx->ctrl_hdl); 217 215 218 - mutex_lock(&dev->dev_ctx_lock); 216 + spin_lock_irqsave(&dev->dev_ctx_lock, flags); 219 217 list_del_init(&ctx->list); 220 - mutex_unlock(&dev->dev_ctx_lock); 218 + spin_unlock_irqrestore(&dev->dev_ctx_lock, flags); 221 219 kfree(ctx); 222 220 mutex_unlock(&dev->dev_mutex); 223 221 return 0; ··· 299 297 300 298 mutex_init(&dev->enc_mutex); 301 299 mutex_init(&dev->dev_mutex); 302 - mutex_init(&dev->dev_ctx_lock); 300 + spin_lock_init(&dev->dev_ctx_lock); 303 301 spin_lock_init(&dev->irqlock); 304 302 305 303 snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
+1 -1
drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
··· 206 206 /* encoder hardware mutex lock */ 207 207 struct mutex enc_mutex; 208 208 struct mutex dev_mutex; 209 - struct mutex dev_ctx_lock; 209 + spinlock_t dev_ctx_lock; 210 210 struct workqueue_struct *encode_workqueue; 211 211 212 212 int enc_irq;
+3 -2
drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
··· 45 45 static bool vpu_enc_check_ap_inst(struct mtk_vcodec_enc_dev *enc_dev, struct venc_vpu_inst *vpu) 46 46 { 47 47 struct mtk_vcodec_enc_ctx *ctx; 48 + unsigned long flags; 48 49 int ret = false; 49 50 50 - mutex_lock(&enc_dev->dev_ctx_lock); 51 + spin_lock_irqsave(&enc_dev->dev_ctx_lock, flags); 51 52 list_for_each_entry(ctx, &enc_dev->ctx_list, list) { 52 53 if (!IS_ERR_OR_NULL(ctx) && ctx->vpu_inst == vpu) { 53 54 ret = true; 54 55 break; 55 56 } 56 57 } 57 - mutex_unlock(&enc_dev->dev_ctx_lock); 58 + spin_unlock_irqrestore(&enc_dev->dev_ctx_lock, flags); 58 59 59 60 return ret; 60 61 }