| Message ID | 20230926105330.10281-1-dg573847474@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: s5p-mfc: Fix potential deadlock on condlock | expand |
On 26.09.2023 12:53, Chengfeng Ye wrote: > As &dev->condlock is acquired under irq context along the following > call chain from s5p_mfc_irq(), other acquisition of the same lock > inside process context or softirq context should disable irq avoid double > lock. enc_post_frame_start() seems to be one such function that execute > under process context or softirq context. > > <deadlock #1> > > enc_post_frame_start() > --> clear_work_bit() > --> spin_loc(&dev->condlock) > <interrupt> > --> s5p_mfc_irq() > --> s5p_mfc_handle_frame() > --> clear_work_bit() > --> spin_lock(&dev->condlock) > > This flaw was found by an experimental static analysis tool I am > developing for irq-related deadlock. > > To prevent the potential deadlock, the patch change clear_work_bit() > inside enc_post_frame_start() to clear_work_bit_irqsave(). > > Signed-off-by: Chengfeng Ye <dg573847474@gmail.com> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/media/platform/samsung/s5p-mfc/s5p_mfc_enc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_enc.c > index f62703cebb77..4b4c129c09e7 100644 > --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_enc.c > +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_enc.c > @@ -1297,7 +1297,7 @@ static int enc_post_frame_start(struct s5p_mfc_ctx *ctx) > if (ctx->state == MFCINST_FINISHING && ctx->ref_queue_cnt == 0) > src_ready = false; > if (!src_ready || ctx->dst_queue_cnt == 0) > - clear_work_bit(ctx); > + clear_work_bit_irqsave(ctx); > > return 0; > } Best regards
diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_enc.c index f62703cebb77..4b4c129c09e7 100644 --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_enc.c +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_enc.c @@ -1297,7 +1297,7 @@ static int enc_post_frame_start(struct s5p_mfc_ctx *ctx) if (ctx->state == MFCINST_FINISHING && ctx->ref_queue_cnt == 0) src_ready = false; if (!src_ready || ctx->dst_queue_cnt == 0) - clear_work_bit(ctx); + clear_work_bit_irqsave(ctx); return 0; }
As &dev->condlock is acquired under irq context along the following call chain from s5p_mfc_irq(), other acquisition of the same lock inside process context or softirq context should disable irq avoid double lock. enc_post_frame_start() seems to be one such function that execute under process context or softirq context. <deadlock #1> enc_post_frame_start() --> clear_work_bit() --> spin_loc(&dev->condlock) <interrupt> --> s5p_mfc_irq() --> s5p_mfc_handle_frame() --> clear_work_bit() --> spin_lock(&dev->condlock) This flaw was found by an experimental static analysis tool I am developing for irq-related deadlock. To prevent the potential deadlock, the patch change clear_work_bit() inside enc_post_frame_start() to clear_work_bit_irqsave(). Signed-off-by: Chengfeng Ye <dg573847474@gmail.com> --- drivers/media/platform/samsung/s5p-mfc/s5p_mfc_enc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)