Skip to content

Commit b92c196

Browse files
wensgregkh
authored andcommitted
media: mediatek: vcodec: Use spinlock for context list protection lock
[ Upstream commit a584422 ] 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: 6467cda ("media: mediatek: vcodec: adding lock to protect decoder context list") Fixes: afaaf3a ("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> [ adapted file_to_dec_ctx() and file_to_enc_ctx() helper calls to equivalent fh_to_dec_ctx(file->private_data) and fh_to_enc_ctx(file->private_data) pattern ] Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent a219c54 commit b92c196

7 files changed

Lines changed: 28 additions & 20 deletions

File tree

drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,30 +47,32 @@ static void mtk_vcodec_vpu_reset_dec_handler(void *priv)
4747
{
4848
struct mtk_vcodec_dec_dev *dev = priv;
4949
struct mtk_vcodec_dec_ctx *ctx;
50+
unsigned long flags;
5051

5152
dev_err(&dev->plat_dev->dev, "Watchdog timeout!!");
5253

53-
mutex_lock(&dev->dev_ctx_lock);
54+
spin_lock_irqsave(&dev->dev_ctx_lock, flags);
5455
list_for_each_entry(ctx, &dev->ctx_list, list) {
5556
ctx->state = MTK_STATE_ABORT;
5657
mtk_v4l2_vdec_dbg(0, ctx, "[%d] Change to state MTK_STATE_ABORT", ctx->id);
5758
}
58-
mutex_unlock(&dev->dev_ctx_lock);
59+
spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
5960
}
6061

6162
static void mtk_vcodec_vpu_reset_enc_handler(void *priv)
6263
{
6364
struct mtk_vcodec_enc_dev *dev = priv;
6465
struct mtk_vcodec_enc_ctx *ctx;
66+
unsigned long flags;
6567

6668
dev_err(&dev->plat_dev->dev, "Watchdog timeout!!");
6769

68-
mutex_lock(&dev->dev_ctx_lock);
70+
spin_lock_irqsave(&dev->dev_ctx_lock, flags);
6971
list_for_each_entry(ctx, &dev->ctx_list, list) {
7072
ctx->state = MTK_STATE_ABORT;
7173
mtk_v4l2_vdec_dbg(0, ctx, "[%d] Change to state MTK_STATE_ABORT", ctx->id);
7274
}
73-
mutex_unlock(&dev->dev_ctx_lock);
75+
spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
7476
}
7577

7678
static const struct mtk_vcodec_fw_ops mtk_vcodec_vpu_msg = {

drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ static int fops_vcodec_open(struct file *file)
198198
struct mtk_vcodec_dec_ctx *ctx = NULL;
199199
int ret = 0, i, hw_count;
200200
struct vb2_queue *src_vq;
201+
unsigned long flags;
201202

202203
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
203204
if (!ctx)
@@ -268,9 +269,9 @@ static int fops_vcodec_open(struct file *file)
268269

269270
ctx->dev->vdec_pdata->init_vdec_params(ctx);
270271

271-
mutex_lock(&dev->dev_ctx_lock);
272+
spin_lock_irqsave(&dev->dev_ctx_lock, flags);
272273
list_add(&ctx->list, &dev->ctx_list);
273-
mutex_unlock(&dev->dev_ctx_lock);
274+
spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
274275
mtk_vcodec_dbgfs_create(ctx);
275276

276277
mutex_unlock(&dev->dev_mutex);
@@ -295,6 +296,7 @@ static int fops_vcodec_release(struct file *file)
295296
{
296297
struct mtk_vcodec_dec_dev *dev = video_drvdata(file);
297298
struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(file->private_data);
299+
unsigned long flags;
298300

299301
mtk_v4l2_vdec_dbg(0, ctx, "[%d] decoder", ctx->id);
300302
mutex_lock(&dev->dev_mutex);
@@ -313,9 +315,9 @@ static int fops_vcodec_release(struct file *file)
313315
v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
314316

315317
mtk_vcodec_dbgfs_remove(dev, ctx->id);
316-
mutex_lock(&dev->dev_ctx_lock);
318+
spin_lock_irqsave(&dev->dev_ctx_lock, flags);
317319
list_del_init(&ctx->list);
318-
mutex_unlock(&dev->dev_ctx_lock);
320+
spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
319321
kfree(ctx);
320322
mutex_unlock(&dev->dev_mutex);
321323
return 0;
@@ -408,7 +410,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
408410
for (i = 0; i < MTK_VDEC_HW_MAX; i++)
409411
mutex_init(&dev->dec_mutex[i]);
410412
mutex_init(&dev->dev_mutex);
411-
mutex_init(&dev->dev_ctx_lock);
413+
spin_lock_init(&dev->dev_ctx_lock);
412414
spin_lock_init(&dev->irqlock);
413415

414416
snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",

drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ struct mtk_vcodec_dec_dev {
283283
/* decoder hardware mutex lock */
284284
struct mutex dec_mutex[MTK_VDEC_HW_MAX];
285285
struct mutex dev_mutex;
286-
struct mutex dev_ctx_lock;
286+
spinlock_t dev_ctx_lock;
287287
struct workqueue_struct *decode_workqueue;
288288

289289
spinlock_t irqlock;

drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,16 +75,17 @@ static void handle_get_param_msg_ack(const struct vdec_vpu_ipi_get_param_ack *ms
7575
static bool vpu_dec_check_ap_inst(struct mtk_vcodec_dec_dev *dec_dev, struct vdec_vpu_inst *vpu)
7676
{
7777
struct mtk_vcodec_dec_ctx *ctx;
78+
unsigned long flags;
7879
int ret = false;
7980

80-
mutex_lock(&dec_dev->dev_ctx_lock);
81+
spin_lock_irqsave(&dec_dev->dev_ctx_lock, flags);
8182
list_for_each_entry(ctx, &dec_dev->ctx_list, list) {
8283
if (!IS_ERR_OR_NULL(ctx) && ctx->vpu_inst == vpu) {
8384
ret = true;
8485
break;
8586
}
8687
}
87-
mutex_unlock(&dec_dev->dev_ctx_lock);
88+
spin_unlock_irqrestore(&dec_dev->dev_ctx_lock, flags);
8889

8990
return ret;
9091
}

drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ static int fops_vcodec_open(struct file *file)
117117
struct mtk_vcodec_enc_ctx *ctx = NULL;
118118
int ret = 0;
119119
struct vb2_queue *src_vq;
120+
unsigned long flags;
120121

121122
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
122123
if (!ctx)
@@ -177,9 +178,9 @@ static int fops_vcodec_open(struct file *file)
177178
mtk_v4l2_venc_dbg(2, ctx, "Create instance [%d]@%p m2m_ctx=%p ",
178179
ctx->id, ctx, ctx->m2m_ctx);
179180

180-
mutex_lock(&dev->dev_ctx_lock);
181+
spin_lock_irqsave(&dev->dev_ctx_lock, flags);
181182
list_add(&ctx->list, &dev->ctx_list);
182-
mutex_unlock(&dev->dev_ctx_lock);
183+
spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
183184

184185
mutex_unlock(&dev->dev_mutex);
185186
mtk_v4l2_venc_dbg(0, ctx, "%s encoder [%d]", dev_name(&dev->plat_dev->dev),
@@ -204,6 +205,7 @@ static int fops_vcodec_release(struct file *file)
204205
{
205206
struct mtk_vcodec_enc_dev *dev = video_drvdata(file);
206207
struct mtk_vcodec_enc_ctx *ctx = fh_to_enc_ctx(file->private_data);
208+
unsigned long flags;
207209

208210
mtk_v4l2_venc_dbg(1, ctx, "[%d] encoder", ctx->id);
209211
mutex_lock(&dev->dev_mutex);
@@ -214,9 +216,9 @@ static int fops_vcodec_release(struct file *file)
214216
v4l2_fh_exit(&ctx->fh);
215217
v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
216218

217-
mutex_lock(&dev->dev_ctx_lock);
219+
spin_lock_irqsave(&dev->dev_ctx_lock, flags);
218220
list_del_init(&ctx->list);
219-
mutex_unlock(&dev->dev_ctx_lock);
221+
spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
220222
kfree(ctx);
221223
mutex_unlock(&dev->dev_mutex);
222224
return 0;
@@ -298,7 +300,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
298300

299301
mutex_init(&dev->enc_mutex);
300302
mutex_init(&dev->dev_mutex);
301-
mutex_init(&dev->dev_ctx_lock);
303+
spin_lock_init(&dev->dev_ctx_lock);
302304
spin_lock_init(&dev->irqlock);
303305

304306
snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",

drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ struct mtk_vcodec_enc_dev {
206206
/* encoder hardware mutex lock */
207207
struct mutex enc_mutex;
208208
struct mutex dev_mutex;
209-
struct mutex dev_ctx_lock;
209+
spinlock_t dev_ctx_lock;
210210
struct workqueue_struct *encode_workqueue;
211211

212212
int enc_irq;

drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,17 @@ static void handle_enc_encode_msg(struct venc_vpu_inst *vpu, const void *data)
4545
static bool vpu_enc_check_ap_inst(struct mtk_vcodec_enc_dev *enc_dev, struct venc_vpu_inst *vpu)
4646
{
4747
struct mtk_vcodec_enc_ctx *ctx;
48+
unsigned long flags;
4849
int ret = false;
4950

50-
mutex_lock(&enc_dev->dev_ctx_lock);
51+
spin_lock_irqsave(&enc_dev->dev_ctx_lock, flags);
5152
list_for_each_entry(ctx, &enc_dev->ctx_list, list) {
5253
if (!IS_ERR_OR_NULL(ctx) && ctx->vpu_inst == vpu) {
5354
ret = true;
5455
break;
5556
}
5657
}
57-
mutex_unlock(&enc_dev->dev_ctx_lock);
58+
spin_unlock_irqrestore(&enc_dev->dev_ctx_lock, flags);
5859

5960
return ret;
6061
}

0 commit comments

Comments
 (0)