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

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <mani@xxxxxxxxxx>

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@xxxxxxxxxxxxxxxx>
---
 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