[PATCH v2] bus: mhi: host: Fix race between channel preparation and M0 event

[PATCH v2] bus: mhi: host: Fix race between channel preparation and M0 event

[Qiang Yu]

There is a race condition where mhi_prepare_channel() updates the
read and write pointers as the base address and in parallel, if
an M0 transition occurs, the tasklet goes ahead and rings
doorbells for all channels with a delta in TRE rings assuming
they are already enabled. This causes a null pointer access. Fix
it by adding a channel enabled check before ringing channel
doorbells.

Fixes: a6e2e3522f29 "bus: mhi: core: Add support for PM state transitions"
Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
v1->v2: add Fixes tags

 drivers/bus/mhi/host/pm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c
index 4a42186..0834590 100644
--- a/drivers/bus/mhi/host/pm.c
+++ b/drivers/bus/mhi/host/pm.c
@@ -301,7 +301,8 @@ int mhi_pm_m0_transition(struct mhi_controller *mhi_cntrl)
 		read_lock_irq(&mhi_chan->lock);
 
 		/* Only ring DB if ring is not empty */
-		if (tre_ring->base && tre_ring->wp  != tre_ring->rp)
+		if (tre_ring->base && tre_ring->wp  != tre_ring->rp &&
+		    mhi_chan->ch_state == MHI_CH_STATE_ENABLED)
 			mhi_ring_chan_db(mhi_cntrl, mhi_chan);
 		read_unlock_irq(&mhi_chan->lock);
 	}
-- 
2.7.4

Re: [PATCH v2] bus: mhi: host: Fix race between channel preparation and M0 event

[Manivannan Sadhasivam]

On Sun, Oct 16, 2022 at 11:05:32AM +0800, Qiang Yu wrote:
> There is a race condition where mhi_prepare_channel() updates the
> read and write pointers as the base address and in parallel, if
> an M0 transition occurs, the tasklet goes ahead and rings
> doorbells for all channels with a delta in TRE rings assuming
> they are already enabled. This causes a null pointer access. Fix
> it by adding a channel enabled check before ringing channel
> doorbells.
> 
> Fixes: a6e2e3522f29 "bus: mhi: core: Add support for PM state transitions"
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>

Can you also CC stable list for backporting?

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

Thanks,
Mani

> ---
> v1->v2: add Fixes tags
> 
>  drivers/bus/mhi/host/pm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c
> index 4a42186..0834590 100644
> --- a/drivers/bus/mhi/host/pm.c
> +++ b/drivers/bus/mhi/host/pm.c
> @@ -301,7 +301,8 @@ int mhi_pm_m0_transition(struct mhi_controller *mhi_cntrl)
>  		read_lock_irq(&mhi_chan->lock);
>  
>  		/* Only ring DB if ring is not empty */
> -		if (tre_ring->base && tre_ring->wp  != tre_ring->rp)
> +		if (tre_ring->base && tre_ring->wp  != tre_ring->rp &&
> +		    mhi_chan->ch_state == MHI_CH_STATE_ENABLED)
>  			mhi_ring_chan_db(mhi_cntrl, mhi_chan);
>  		read_unlock_irq(&mhi_chan->lock);
>  	}
> -- 
> 2.7.4
> 
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2] bus: mhi: host: Fix race between channel preparation and M0 event

[Manivannan Sadhasivam]

On Fri, Oct 28, 2022 at 10:24:01PM +0530, Manivannan Sadhasivam wrote:
> On Sun, Oct 16, 2022 at 11:05:32AM +0800, Qiang Yu wrote:
> > There is a race condition where mhi_prepare_channel() updates the
> > read and write pointers as the base address and in parallel, if
> > an M0 transition occurs, the tasklet goes ahead and rings
> > doorbells for all channels with a delta in TRE rings assuming
> > they are already enabled. This causes a null pointer access. Fix
> > it by adding a channel enabled check before ringing channel
> > doorbells.
> > 
> > Fixes: a6e2e3522f29 "bus: mhi: core: Add support for PM state transitions"
> > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> 
> Can you also CC stable list for backporting?
> 

Nvm, I added stable list and applied the patch to mhi-next! Because of the
addition of MHI EP support, this patch can be backported without modifications
till 5.19 only. If you want to backport to older kernels, please send them to
stable list once this got merged.

Thanks,
Mani

> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> 
> Thanks,
> Mani
> 
> > ---
> > v1->v2: add Fixes tags
> > 
> >  drivers/bus/mhi/host/pm.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/bus/mhi/host/pm.c b/drivers/bus/mhi/host/pm.c
> > index 4a42186..0834590 100644
> > --- a/drivers/bus/mhi/host/pm.c
> > +++ b/drivers/bus/mhi/host/pm.c
> > @@ -301,7 +301,8 @@ int mhi_pm_m0_transition(struct mhi_controller *mhi_cntrl)
> >  		read_lock_irq(&mhi_chan->lock);
> >  
> >  		/* Only ring DB if ring is not empty */
> > -		if (tre_ring->base && tre_ring->wp  != tre_ring->rp)
> > +		if (tre_ring->base && tre_ring->wp  != tre_ring->rp &&
> > +		    mhi_chan->ch_state == MHI_CH_STATE_ENABLED)
> >  			mhi_ring_chan_db(mhi_cntrl, mhi_chan);
> >  		read_unlock_irq(&mhi_chan->lock);
> >  	}
> > -- 
> > 2.7.4
> > 
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்
> 

-- 
மணிவண்ணன் சதாசிவம்