| Message ID | 20231027084329.4067-1-peter.wang@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] ufs: core: fix racing issue between ufshcd_mcq_abort and ISR | expand |
Hi all, Gentle ping for this bug fix review. Thanks. On Fri, 2023-10-27 at 16:43 +0800, peter.wang@mediatek.com wrote: > From: Peter Wang <peter.wang@mediatek.com> > > If command timeout happen and cq complete irq raise at the same time, > ufshcd_mcq_abort null the lprb->cmd and NULL poiner KE in ISR. > Below is error log. > > ufshcd_abort: Device abort task at tag 18 > Unable to handle kernel NULL pointer dereference at virtual address > 0000000000000108 > pc : [0xffffffe27ef867ac] scsi_dma_unmap+0xc/0x44 > lr : [0xffffffe27f1b898c] ufshcd_release_scsi_cmd+0x24/0x114 > > Signed-off-by: Peter Wang <peter.wang@mediatek.com> > --- > drivers/ufs/core/ufs-mcq.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c > index 2ba8ec254dce..6ea96406f2bf 100644 > --- a/drivers/ufs/core/ufs-mcq.c > +++ b/drivers/ufs/core/ufs-mcq.c > @@ -630,6 +630,7 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd) > int tag = scsi_cmd_to_rq(cmd)->tag; > struct ufshcd_lrb *lrbp = &hba->lrb[tag]; > struct ufs_hw_queue *hwq; > + unsigned long flags; > int err = FAILED; > > if (!ufshcd_cmd_inflight(lrbp->cmd)) { > @@ -670,8 +671,10 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd) > } > > err = SUCCESS; > + spin_lock_irqsave(&hwq->cq_lock, flags); > if (ufshcd_cmd_inflight(lrbp->cmd)) > ufshcd_release_scsi_cmd(hba, lrbp); > + spin_unlock_irqrestore(&hwq->cq_lock, flags); > > out: > return err;
On 10/27/23 01:43, peter.wang@mediatek.com wrote: > If command timeout happen and cq complete irq raise at the same time, > ufshcd_mcq_abort null the lprb->cmd and NULL poiner KE in ISR. Please add a Fixes: tag. Otherwise this patch looks good to me. Thanks, Bart.
On Fri, 2023-11-03 at 07:57 -0700, Bart Van Assche wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 10/27/23 01:43, peter.wang@mediatek.com wrote: > > If command timeout happen and cq complete irq raise at the same > time, > > ufshcd_mcq_abort null the lprb->cmd and NULL poiner KE in ISR. > > Please add a Fixes: tag. Otherwise this patch looks good to me. > > Thanks, > > Bart. Hi Bart, Thank you for review, will add fixes tag next version. Thanks. Peter
diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c index 2ba8ec254dce..6ea96406f2bf 100644 --- a/drivers/ufs/core/ufs-mcq.c +++ b/drivers/ufs/core/ufs-mcq.c @@ -630,6 +630,7 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd) int tag = scsi_cmd_to_rq(cmd)->tag; struct ufshcd_lrb *lrbp = &hba->lrb[tag]; struct ufs_hw_queue *hwq; + unsigned long flags; int err = FAILED; if (!ufshcd_cmd_inflight(lrbp->cmd)) { @@ -670,8 +671,10 @@ int ufshcd_mcq_abort(struct scsi_cmnd *cmd) } err = SUCCESS; + spin_lock_irqsave(&hwq->cq_lock, flags); if (ufshcd_cmd_inflight(lrbp->cmd)) ufshcd_release_scsi_cmd(hba, lrbp); + spin_unlock_irqrestore(&hwq->cq_lock, flags); out: return err;