Re: [PATCH v3 2/3] bus: mhi: ep: Flush async transfers before notifying disconnect in mhi_ep_abort_transfer()

[Frank Li <Frank.li@oss.nxp.com>]

On Wed, Jul 22, 2026 at 07:54:45AM +0200, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <mani@kernel.org>
>
> mhi_ep_abort_transfer() notifies the client drivers about the channel
> disconnect using -ENOTCONN and only then flushes the ring workqueue to
> drain the in-flight transfers. But the async DMA transfers issued by the
> ring workers can still complete after the notification. And the completion
> handlers trigger the client xfer_cb() as long as it is set.
>
> So a transfer completing during the flush can deliver a success callback to
> the client even after it has been notified about the disconnect. This can
> lead to UAF (Use-After-Free) issues as the client can free its per-transfer
> resources in response to the -ENOTCONN notification and the trailing
> success callback would then reference the freed resources.
>
> So to fix this issue, disable all the channels first to prevent new
> transfers and then drain both the ring workqueue and the in-flight async
> transfers before notifying the disconnect. The completion and queue paths
> bail out once the channel state is not MHI_CH_STATE_RUNNING, so disabling
> the channels upfront makes sure that no new transfer sneaks in during the
> drain and all the pending completions are delivered while xfer_cb() is
> still valid.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/bus/mhi/ep/main.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index b94c571f01d8..51735f87017f 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -1025,26 +1025,37 @@ static void mhi_ep_abort_transfer(struct mhi_ep_cntrl *mhi_cntrl)
>         struct mhi_ep_chan *mhi_chan;
>         int i;
>
> -       /* Stop all the channels */
> +       /* Disable all the channels to prevent new transfers */
> +       for (i = 0; i < mhi_cntrl->max_chan; i++) {
> +               mhi_chan = &mhi_cntrl->mhi_chan[i];
> +               if (!mhi_chan->ring.started)
> +                       continue;
> +
> +               mutex_lock(&mhi_chan->lock);
> +               mhi_chan->state = MHI_CH_STATE_DISABLED;
> +               mutex_unlock(&mhi_chan->lock);
> +       }
> +
> +       /* Drain ring workers and in-flight transfers before notifying disconnect */
> +       flush_workqueue(mhi_cntrl->wq);
> +       if (mhi_cntrl->flush_async)
> +               mhi_cntrl->flush_async(mhi_cntrl);
> +
> +       /* Send channel disconnect status to client drivers */
>         for (i = 0; i < mhi_cntrl->max_chan; i++) {
>                 mhi_chan = &mhi_cntrl->mhi_chan[i];
>                 if (!mhi_chan->ring.started)
>                         continue;
>
>                 mutex_lock(&mhi_chan->lock);
> -               /* Send channel disconnect status to client drivers */
>                 if (mhi_chan->xfer_cb) {
>                         result.transaction_status = -ENOTCONN;
>                         result.bytes_xferd = 0;
>                         mhi_chan->xfer_cb(mhi_chan->mhi_dev, &result);
>                 }
> -
> -               mhi_chan->state = MHI_CH_STATE_DISABLED;
>                 mutex_unlock(&mhi_chan->lock);
>         }
>
> -       flush_workqueue(mhi_cntrl->wq);
> -
>         /* Destroy devices associated with all channels */
>         device_for_each_child(&mhi_cntrl->mhi_dev->dev, NULL, mhi_ep_destroy_device);
>
>
> --
> 2.43.0
>
>