| Message ID | 1658311454-4707-1-git-send-email-quic_qianyu@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1,1/1] bus: mhi: host: Fix up null pointer access in mhi_irq_handler | expand |
On 7/20/2022 4:04 AM, Qiang Yu wrote: > The irq handler for a shared IRQ ought to be prepared for running > even now it's being freed. So let's check the pointer used by > mhi_irq_handler to avoid null pointer access since it is probably > released before freeing IRQ. > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> > --- > drivers/bus/mhi/host/main.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c > index f3aef77a..7959457 100644 > --- a/drivers/bus/mhi/host/main.c > +++ b/drivers/bus/mhi/host/main.c > @@ -430,12 +430,20 @@ irqreturn_t mhi_irq_handler(int irq_number, void *dev) > { > struct mhi_event *mhi_event = dev; > struct mhi_controller *mhi_cntrl = mhi_event->mhi_cntrl; > - struct mhi_event_ctxt *er_ctxt = > - &mhi_cntrl->mhi_ctxt->er_ctxt[mhi_event->er_index]; > + struct mhi_event_ctxt *er_ctxt; > struct mhi_ring *ev_ring = &mhi_event->ring; > - dma_addr_t ptr = le64_to_cpu(er_ctxt->rp); > + dma_addr_t ptr; > void *dev_rp; > > + if (!mhi_cntrl->mhi_ctxt) { > + dev_err(&mhi_cntrl->mhi_dev->dev, > + "mhi_ctxt has been freed\n"); dev_dbg since you identified a scenario where this is expected? > + return IRQ_HANDLED; > + } > + > + er_ctxt = &mhi_cntrl->mhi_ctxt->er_ctxt[mhi_event->er_index]; > + ptr = le64_to_cpu(er_ctxt->rp); > + > if (!is_valid_ring_ptr(ev_ring, ptr)) { > dev_err(&mhi_cntrl->mhi_dev->dev, > "Event ring rp points outside of the event ring\n");
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c index f3aef77a..7959457 100644 --- a/drivers/bus/mhi/host/main.c +++ b/drivers/bus/mhi/host/main.c @@ -430,12 +430,20 @@ irqreturn_t mhi_irq_handler(int irq_number, void *dev) { struct mhi_event *mhi_event = dev; struct mhi_controller *mhi_cntrl = mhi_event->mhi_cntrl; - struct mhi_event_ctxt *er_ctxt = - &mhi_cntrl->mhi_ctxt->er_ctxt[mhi_event->er_index]; + struct mhi_event_ctxt *er_ctxt; struct mhi_ring *ev_ring = &mhi_event->ring; - dma_addr_t ptr = le64_to_cpu(er_ctxt->rp); + dma_addr_t ptr; void *dev_rp; + if (!mhi_cntrl->mhi_ctxt) { + dev_err(&mhi_cntrl->mhi_dev->dev, + "mhi_ctxt has been freed\n"); + return IRQ_HANDLED; + } + + er_ctxt = &mhi_cntrl->mhi_ctxt->er_ctxt[mhi_event->er_index]; + ptr = le64_to_cpu(er_ctxt->rp); + if (!is_valid_ring_ptr(ev_ring, ptr)) { dev_err(&mhi_cntrl->mhi_dev->dev, "Event ring rp points outside of the event ring\n");
The irq handler for a shared IRQ ought to be prepared for running even now it's being freed. So let's check the pointer used by mhi_irq_handler to avoid null pointer access since it is probably released before freeing IRQ. Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> --- drivers/bus/mhi/host/main.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)