[PATCH v3 0/3] bus: mhi: ep: Implement flush_async() callback to flush async read/write

[PATCH v3 0/3] bus: mhi: ep: Implement flush_async() callback to flush async read/write

[Manivannan Sadhasivam via B4 Relay]

Hi,

This series introduces a new mhi_cntrl->flush_async() callback to flush the
async read/write operations performed by the MHI controller using offload
mechanisms such as DMA.

The MHI EPF driver implements this callback by flushing the DMA wq. With this
series, the MHI EP stack can guarnatee that the channel specific xfer_cb() won't
be run after calling mhi_ep_remove() and mhi_ep_abort_transfer().

Merge Strategy
==============

Both MHI and PCI EP changes should go together. So I'm planning to take both
MHI and PCI EP patches through MHI tree.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
Changes in v3:
- Dropped the dmaengine patch as a variant of it already got merged into
  dmaengine tree
- Move flush_async() callback before triggering ENOTCONN
- Link to v2: https://patch.msgid.link/20260714-mhi-ep-flush-v2-0-b6a9db011e85@oss.qualcomm.com

Changes in v2:
- Switched to read_poll_timeout() in dw_edma_device_synchronize()
- Switched to dmaengine_synchronize() in pci_epf_mhi_edma_flush_async()
- Link to v1: https://patch.msgid.link/20260629-mhi-ep-flush-v1-0-714e0d56e87c@oss.qualcomm.com

---
Manivannan Sadhasivam (3):
      bus: mhi: ep: Add mhi_cntrl->flush_async() callback to flush the async read/write
      bus: mhi: ep: Flush async transfers before notifying disconnect in mhi_ep_abort_transfer()
      PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write

 drivers/bus/mhi/ep/main.c                    | 41 +++++++++++++++++++++++-----
 drivers/pci/endpoint/functions/pci-epf-mhi.c | 10 +++++++
 include/linux/mhi_ep.h                       |  2 ++
 3 files changed, 46 insertions(+), 7 deletions(-)
---
base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
change-id: 20260627-mhi-ep-flush-b50502718a9d

Best regards,
--  
மணிவண்ணன் சதாசிவம்

[PATCH v3 1/3] bus: mhi: ep: Add mhi_cntrl->flush_async() callback to flush the async read/write

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

MHI EP stack makes use of the MHI controller drivers like MHI EPF to do
read/write to the host memory. And that driver is free to use mechanisms
like DMA to offload the read/write operations.

So if DMA is used for offload, then there is no guarantee that those DMA
operations would be completed by the time mhi_ep_remove() gets called. This
can lead to UAF (Use-After-Free) issues as the DMA callback can trigger
xfer_cb() even after mhi_ep_remove() has returned.

So to fix this issue, introduce the mhi_cntrl->flush_async() callback and
call it in mhi_ep_remove() to drain all the in-flight async transfers
before disconnecting the channels.

The completion handlers keep triggering xfer_cb() as long as it is set. So
flushing the transfers after notifying the client about the disconnect
(-ENOTCONN) would still let a success callback slip through afterwards and
lead to the same UAF. So disable the channels first to prevent new
transfers, then flush the in-flight transfers so that their completions are
delivered while xfer_cb() is still valid and only then notify the
disconnect and clear xfer_cb().

Cc: <stable+noautosel@kernel.org> # Needs dmaengine driver fix as well
Fixes: 2547beb00ddb ("bus: mhi: ep: Add support for async DMA read operation")
Fixes: ee08acb58fe4 ("bus: mhi: ep: Add support for async DMA write operation")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/bus/mhi/ep/main.c | 18 +++++++++++++++++-
 include/linux/mhi_ep.h    |  2 ++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index 0277e1ab1198..b94c571f01d8 100644
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -1612,6 +1612,7 @@ static void mhi_ep_remove(struct device *dev)
 {
 	struct mhi_ep_device *mhi_dev = to_mhi_ep_device(dev);
 	struct mhi_ep_driver *mhi_drv = to_mhi_ep_driver(dev->driver);
+	struct mhi_ep_cntrl *mhi_cntrl = mhi_dev->mhi_cntrl;
 	struct mhi_result result = {};
 	struct mhi_ep_chan *mhi_chan;
 	int dir;
@@ -1620,6 +1621,22 @@ static void mhi_ep_remove(struct device *dev)
 	if (mhi_dev->dev_type == MHI_DEVICE_CONTROLLER)
 		return;
 
+	/* Disable the channels to prevent new transfers */
+	for (dir = 0; dir < 2; dir++) {
+		mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
+
+		if (!mhi_chan)
+			continue;
+
+		mutex_lock(&mhi_chan->lock);
+		mhi_chan->state = MHI_CH_STATE_DISABLED;
+		mutex_unlock(&mhi_chan->lock);
+	}
+
+	/* Flush in-flight transfers before notifying disconnect */
+	if (mhi_cntrl->flush_async)
+		mhi_cntrl->flush_async(mhi_cntrl);
+
 	/* Disconnect the channels associated with the driver */
 	for (dir = 0; dir < 2; dir++) {
 		mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
@@ -1635,7 +1652,6 @@ static void mhi_ep_remove(struct device *dev)
 			mhi_chan->xfer_cb(mhi_chan->mhi_dev, &result);
 		}
 
-		mhi_chan->state = MHI_CH_STATE_DISABLED;
 		mhi_chan->xfer_cb = NULL;
 		mutex_unlock(&mhi_chan->lock);
 	}
diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
index 7b40fc8cbe77..f6383a57a872 100644
--- a/include/linux/mhi_ep.h
+++ b/include/linux/mhi_ep.h
@@ -107,6 +107,7 @@ struct mhi_ep_buf_info {
  * @write_sync: CB function for writing to host memory synchronously
  * @read_async: CB function for reading from host memory asynchronously
  * @write_async: CB function for writing to host memory asynchronously
+ * @flush_async: CB function for flushing asynchronous read/writes
  * @mhi_state: MHI Endpoint state
  * @max_chan: Maximum channels supported by the endpoint controller
  * @mru: MRU (Maximum Receive Unit) value of the endpoint controller
@@ -164,6 +165,7 @@ struct mhi_ep_cntrl {
 	int (*write_sync)(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_buf_info *buf_info);
 	int (*read_async)(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_buf_info *buf_info);
 	int (*write_async)(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_buf_info *buf_info);
+	void (*flush_async)(struct mhi_ep_cntrl *mhi_cntrl);
 
 	enum mhi_state mhi_state;
 

-- 
2.43.0

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

[PATCH v3 3/3] PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <mani@kernel.org>

The MHI core needs to make sure that all the current DMA transactions are
completed before removing the channels. So implement the
mhi_cntrl->flush_async() callback by first making sure all the in-flight
DMA operations are completed and then flushing the DMA workqueue.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/endpoint/functions/pci-epf-mhi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
index 7f5326925ed5..8d2d9d01cfd2 100644
--- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
+++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
@@ -644,6 +644,15 @@ static int pci_epf_mhi_edma_write_async(struct mhi_ep_cntrl *mhi_cntrl,
 	return ret;
 }
 
+static void pci_epf_mhi_edma_flush_async(struct mhi_ep_cntrl *mhi_cntrl)
+{
+	struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl);
+
+	dmaengine_synchronize(epf_mhi->dma_chan_rx);
+	dmaengine_synchronize(epf_mhi->dma_chan_tx);
+	flush_workqueue(epf_mhi->dma_wq);
+}
+
 struct epf_dma_filter {
 	struct device *dev;
 	u32 dma_mask;
@@ -812,6 +821,7 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf)
 		mhi_cntrl->write_sync = pci_epf_mhi_edma_write;
 		mhi_cntrl->read_async = pci_epf_mhi_edma_read_async;
 		mhi_cntrl->write_async = pci_epf_mhi_edma_write_async;
+		mhi_cntrl->flush_async = pci_epf_mhi_edma_flush_async;
 	}
 
 	/* Register the MHI EP controller */

-- 
2.43.0

Re: [PATCH v3 3/3] PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The newly introduced `pci_epf_mhi_edma_flush_async` callback unconditionally accesses `dma_chan_rx` and `dma_chan_tx` without verification, leading to fatal crashes if DMA initialization fails or the PCIe link toggles.
- [High] `pci_epf_mhi_edma_flush_async()` misuses `dmaengine_synchronize()` by not terminating in-flight transfers, invoking undefined behavior and leading to Use-After-Free when hardware completes transfers after teardown.

Pre-existing issues:
- [High] Teardown paths in `pci_epf_mhi_unbind()`, `pci_epf_mhi_epc_deinit()`, and `pci_epf_mhi_link_down()` are vulnerable to a severe Use-After-Free (UAF) and Double-Free due to dangling pointers.
--

--- Patch [3]: [PATCH v3 3/3] PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write ---
commit e8cb88746eba8db4d5212c60aa7ddf4b58afafd5
Author: Manivannan Sadhasivam <mani@kernel.org>

PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write

The MHI core needs to make sure that all the current DMA transactions are
completed before removing the channels. So implement the
mhi_cntrl->flush_async() callback by first making sure all the in-flight
DMA operations are completed and then flushing the DMA workqueue.

> diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> @@ -644,6 +644,15 @@ static int pci_epf_mhi_edma_write_async(struct mhi_ep_cntrl *mhi_cntrl,
>  	return ret;
>  }
>  
> +static void pci_epf_mhi_edma_flush_async(struct mhi_ep_cntrl *mhi_cntrl)
> +{
> +	struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl);
> +
> +	dmaengine_synchronize(epf_mhi->dma_chan_rx);
> +	dmaengine_synchronize(epf_mhi->dma_chan_tx);

[Severity: High]
If pci_epf_mhi_dma_init() fails during epc_init(), the DMA channels are not
initialized, but the flush_async callback is still registered later in
pci_epf_mhi_link_up().  Will this unconditionally dereference dma_chan_rx
and dma_chan_tx and cause a crash if the link goes down?

Additionally, if the host toggles the PCIe link down then up,
pci_epf_mhi_link_down() destroys the channels and sets them to NULL, but the
subsequent link_up does not reinitialize them.  Does this also lead to a
NULL pointer dereference here?

[Severity: High]
Does dmaengine_synchronize() safely stop the hardware or wait for pending
descriptors to execute?

The DMA Engine API states that dmaengine_synchronize() synchronizes only the
termination of a channel and does not stop the hardware.

If dmaengine_terminate_async() is never called, will this return while
hardware DMA is still processing in the background?  When the active DMA
transactions eventually finish, the hardware might write to unmapped or freed
memory, potentially causing memory corruption and enqueuing a freed transfer
context to the dma_wq.

> +	flush_workqueue(epf_mhi->dma_wq);
> +}
> +
[ ... ]
> @@ -812,6 +821,7 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf)
>  		mhi_cntrl->write_sync = pci_epf_mhi_edma_write;
>  		mhi_cntrl->read_async = pci_epf_mhi_edma_read_async;
>  		mhi_cntrl->write_async = pci_epf_mhi_edma_write_async;
> +		mhi_cntrl->flush_async = pci_epf_mhi_edma_flush_async;
>  	}
>  
>  	/* Register the MHI EP controller */

[Severity: High]
This is a pre-existing issue, but teardown paths appear vulnerable to a
double-free or use-after-free.

If the endpoint is unbound via pci_epf_mhi_unbind() after a link down event,
the teardown logic executes again:

pci_epf_mhi_unbind() {
    ...
    if (mhi_cntrl->mhi_dev) {
        mhi_ep_power_down(mhi_cntrl);
        if (info->flags & MHI_EPF_USE_DMA)
            pci_epf_mhi_dma_deinit(epf_mhi);
        mhi_ep_unregister_controller(mhi_cntrl);
    }
    ...
}

Since mhi_ep_unregister_controller() and pci_epf_mhi_dma_deinit() do not set
mhi_cntrl->mhi_dev to NULL or clear out the destroyed workqueues, will this
conditional evaluate to true on a dangling pointer and run the teardown
logic a second time, resulting in a double-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-mhi-ep-flush-v3-0-d855e715264e@oss.qualcomm.com?part=3

Re: [PATCH v3 1/3] bus: mhi: ep: Add mhi_cntrl->flush_async() callback to flush the async read/write

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:

New issues:
- [High] Lock gap in `mhi_ep_remove()` during `flush_async()` allows the host to restart the channel and bypass teardown synchronization.
- [High] Memory leak of TRE buffers and SKBs if `flush_async` cancels DMA operations.

Pre-existing issues:
- [High] Missing `flush_async()` in `mhi_ep_abort_transfer()`, `STOP_CHAN`, and `RESET_CHAN` leaves UAF and introduces NULL pointer dereference.
- [High] Inconsistent locking for `xfer_cb` leading to potential AB-BA deadlocks in client drivers.
- [High] Use-Before-Initialization race condition due to early `xfer_cb` publication in `probe()`.
- [High] Invalid Memory Free (`kfree()` used on a `kmem_cache` object).
- [Medium] Duplicate `-ENOTCONN` notifications sent to client driver.
--

--- Patch [1]: [PATCH v3 1/3] bus: mhi: ep: Add mhi_cntrl->flush_async() callback to flush the async read/write ---
commit 8e4fde6115727693f6dc8095e55dc40c2cd91987
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

bus: mhi: ep: Add mhi_cntrl->flush_async() callback to flush the async read/write

This commit introduces a flush_async callback to the MHI endpoint controller
and calls it during device removal. This ensures that any pending asynchronous
DMA read or write operations are completed before disconnecting the channels.
The change aims to prevent use-after-free issues that occur when DMA
callbacks trigger after channel teardown.

> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index 0277e1ab11988..b94c571f01d88 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -1620,6 +1621,22 @@ static void mhi_ep_remove(struct device *dev)
>  	if (mhi_dev->dev_type == MHI_DEVICE_CONTROLLER)
>  		return;
>  
> +	/* Disable the channels to prevent new transfers */
> +	for (dir = 0; dir < 2; dir++) {
> +		mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
> +
> +		if (!mhi_chan)
> +			continue;
> +
> +		mutex_lock(&mhi_chan->lock);
> +		mhi_chan->state = MHI_CH_STATE_DISABLED;
> +		mutex_unlock(&mhi_chan->lock);
> +	}
> +
> +	/* Flush in-flight transfers before notifying disconnect */
> +	if (mhi_cntrl->flush_async)
> +		mhi_cntrl->flush_async(mhi_cntrl);

[Severity: High]
Does dropping the channel locks while calling the blocking flush_async
callback leave a window where the host could restart the channel?

If a START_CHAN command is processed during this gap,
mhi_ep_process_cmd_ring() could reacquire the lock and set the state back to
MHI_CH_STATE_RUNNING. This would allow new DMA operations to be queued
before the xfer_cb is cleared later in this function, potentially leading to
a use-after-free.

[Severity: High]
If the controller implements flush_async() by cancelling pending transfers
(for example, via dmaengine_terminate_async()), could this cause a memory
leak?

Since the MHI EP core doesn't appear to track pending operations, abandoning
the buffers allocated from tre_buf_cache without invoking their completion
callbacks might leak both the TRE buffers and associated SKBs.

[Severity: High]
This is a pre-existing issue, but since flush_async() is being added here to
handle pending transfers, should it also be called when the host resets or
stops a channel?

During host commands like RESET_CHAN or STOP_CHAN, or during power down via
mhi_ep_abort_transfer(), channels are disabled without flushing pending DMA
transfers. For instance, in mhi_ep_process_cmd_ring() handling a RESET_CHAN,
mhi_ep_ring_reset() frees ring->ring_cache and sets it to NULL. If an
in-flight async DMA read subsequently completes, mhi_ep_read_completion()
will dereference this NULL ring_cache.

>  	/* Disconnect the channels associated with the driver */
>  	for (dir = 0; dir < 2; dir++) {
>  		mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
> @@ -1635,7 +1652,6 @@ static void mhi_ep_remove(struct device *dev)
>  			mhi_chan->xfer_cb(mhi_chan->mhi_dev, &result);
>  		}

[Severity: Medium]
This is a pre-existing issue, but does this lead to duplicate -ENOTCONN
notifications being sent to the client driver during power down?

When mhi_ep_abort_transfer() is called, it sends an initial -ENOTCONN via
xfer_cb but leaves the callback set. Device destruction then synchronously
invokes mhi_ep_remove(), which finds the callback still valid and sends a
second -ENOTCONN notification.

[Severity: High]
This isn't a bug introduced by this patch, but is there an inconsistent
locking expectation for xfer_cb that could lead to deadlocks in client
drivers?

The callback is invoked with mhi_chan->lock held in mhi_ep_process_ch_ring(),
but it is called locklessly from the workqueue in mhi_ep_skb_completion().
If a client driver responds to a receive callback by queueing a new buffer
via mhi_ep_queue_skb() (which acquires mhi_chan->lock), it could self-deadlock
when called from the locked context.

[Severity: High]
This isn't a bug introduced by this patch, but could the xfer_cb be called
before the client driver has finished initializing?

In mhi_ep_probe(), xfer_cb is assigned to the channel before the client
driver's probe function completes:

mhi_ep_probe() {
    ...
    ul_chan->xfer_cb = mhi_drv->ul_xfer_cb;
    dl_chan->xfer_cb = mhi_drv->dl_xfer_cb;

    return mhi_drv->probe(mhi_dev, mhi_dev->id);
}

If the host sends an early doorbell interrupt, mhi_ep_ch_ring_worker() could
start processing the ring and invoke the callback on a client structure that
is not fully initialized.

[Severity: High]
This is a pre-existing issue, but is kfree() being used incorrectly on an
object allocated from a slab cache?

In mhi_ep_ch_ring_worker(), if the ring is stopped concurrently, the iterator
is freed using kfree():

mhi_ep_ch_ring_worker() {
    ...
    if (!ring->started) {
        mutex_unlock(&chan->lock);
        kfree(itr);
        continue;
    }
}

However, the iterator was allocated from the mhi_cntrl->ring_item_cache via
kmem_cache_zalloc(), so this might cause slab memory corruption.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-mhi-ep-flush-v3-0-d855e715264e@oss.qualcomm.com?part=1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] Race condition in split loop design leaves channel in RUNNING state after disconnect notification.

Pre-existing issues:
- [Critical] STOP and RESET handlers lack flush_async() leading to UAF and deterministic NULL pointer dereference.
- [High] Late IRQ re-queues ch_ring_work causing UAF and freed memory access during DL channel processing.
- [High] Double -ENOTCONN disconnect notification triggers potential double-free in client drivers.
--

--- Patch [2]: [PATCH v3 2/3] bus: mhi: ep: Flush async transfers before notifying disconnect in mhi_ep_abort_transfer() ---
commit f2f701565f9c8f759ea3acdcd7ee4b4c1972f847
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

bus: mhi: ep: Flush async transfers before notifying disconnect in mhi_ep_abort_transfer()

mhi_ep_abort_transfer() modifies the abort process by first disabling
channels, draining workqueues and async transfers, and then notifying
clients. This aims to prevent UAFs caused by trailing completions.

> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index b94c571f01d88..51735f87017f0 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;

[Severity: High]
Since this loop skips unstarted channels, is it possible for an unstarted
channel to be started during the flush_workqueue() below?

If cmd_ring_work processes a START_CHAN_CMD during the flush, it could
start a previously unstarted channel and unconditionally set its state to
MHI_CH_STATE_RUNNING. The second loop would then observe the channel as
started, send the -ENOTCONN notification, but leave the channel in the
RUNNING state since it was bypassed by the first loop. This could allow
the channel to continue processing transfers and deliver trailing success
callbacks after the client has freed its resources.

> +
> +		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);

[Severity: High]
This is a pre-existing issue, but can late doorbell interrupts repopulate
the workqueue after it is flushed?

flush_workqueue() is called here, but hardware interrupts are not masked
until mhi_ep_mmio_mask_interrupts() is called later in
mhi_ep_abort_transfer(). A late interrupt could queue ch_ring_work again.

When mhi_ep_process_ch_ring() executes for a DL channel, it lacks a
MHI_CH_STATE_RUNNING check:

drivers/bus/mhi/ep/main.c:mhi_ep_process_ch_ring() {
    ...
		/* DL channel */
		result.dir = mhi_chan->dir;
		mhi_chan->xfer_cb(mhi_chan->mhi_dev, &result);
    ...
}

This could unconditionally invoke xfer_cb() after the disconnect loop has
notified the client with -ENOTCONN, potentially resulting in a
Use-After-Free. Additionally, if ch_ring_work runs after
mhi_ep_destroy_device() completes, mhi_chan->mhi_dev will be a dangling
pointer.

> +	if (mhi_cntrl->flush_async)
> +		mhi_cntrl->flush_async(mhi_cntrl);

[Severity: Critical]
This is a pre-existing issue, but does mhi_ep_process_cmd_ring() also need
to drain async transfers during STOP and RESET commands?

While the global abort path drains in-flight transfers, the per-channel
STOP and RESET command handlers in mhi_ep_process_cmd_ring() send
-ENOTCONN disconnect notifications without calling flush_async().

For example, during a RESET command, the handler calls mhi_ep_ring_reset()
which immediately frees ring->ring_cache:

drivers/bus/mhi/ep/ring.c:mhi_ep_ring_reset() {
    ...
	kfree(ring->ring_cache);
	ring->ring_cache = NULL;
    ...
}

Trailing async DMA completions could then invoke mhi_ep_skb_completion() or
mhi_ep_read_completion(), which dereference
ring->ring_cache[ring->rd_offset], causing a NULL pointer dereference.

> +
> +	/* 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);
>  		}

[Severity: High]
This is a pre-existing issue, but should mhi_chan->xfer_cb be set to NULL
here to prevent double -ENOTCONN notifications?

The callback pointer is left populated, and immediately afterwards
device_for_each_child() triggers device destruction. This unbinds the
client driver and invokes its bus remove callback, mhi_ep_remove():

drivers/bus/mhi/ep/main.c:mhi_ep_remove() {
    ...
		if (mhi_chan->xfer_cb) {
			result.transaction_status = -ENOTCONN;
			result.bytes_xferd = 0;
			mhi_chan->xfer_cb(mhi_chan->mhi_dev, &result);
		}

		mhi_chan->xfer_cb = NULL;
    ...
}

Since the client driver may clean up its per-transfer resources on the
first -ENOTCONN, could delivering the teardown event a second time risk
double-free bugs in the client driver?

> -
> -		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);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-mhi-ep-flush-v3-0-d855e715264e@oss.qualcomm.com?part=2

Re: [PATCH v3 1/3] bus: mhi: ep: Add mhi_cntrl->flush_async() callback to flush the async read/write

[Frank Li]

On Wed, Jul 22, 2026 at 07:54:44AM +0200, Manivannan Sadhasivam via B4 Relay wrote:
> [You don't often get email from devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> MHI EP stack makes use of the MHI controller drivers like MHI EPF to do
> read/write to the host memory. And that driver is free to use mechanisms
> like DMA to offload the read/write operations.
>
> So if DMA is used for offload, then there is no guarantee that those DMA
> operations would be completed by the time mhi_ep_remove() gets called. This
> can lead to UAF (Use-After-Free) issues as the DMA callback can trigger
> xfer_cb() even after mhi_ep_remove() has returned.
>
> So to fix this issue, introduce the mhi_cntrl->flush_async() callback and
> call it in mhi_ep_remove() to drain all the in-flight async transfers
> before disconnecting the channels.
>
> The completion handlers keep triggering xfer_cb() as long as it is set. So
> flushing the transfers after notifying the client about the disconnect
> (-ENOTCONN) would still let a success callback slip through afterwards and
> lead to the same UAF. So disable the channels first to prevent new
> transfers, then flush the in-flight transfers so that their completions are
> delivered while xfer_cb() is still valid and only then notify the
> disconnect and clear xfer_cb().
>
> Cc: <stable+noautosel@kernel.org> # Needs dmaengine driver fix as well
> Fixes: 2547beb00ddb ("bus: mhi: ep: Add support for async DMA read operation")
> Fixes: ee08acb58fe4 ("bus: mhi: ep: Add support for async DMA write operation")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---

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

>  drivers/bus/mhi/ep/main.c | 18 +++++++++++++++++-
>  include/linux/mhi_ep.h    |  2 ++
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index 0277e1ab1198..b94c571f01d8 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -1612,6 +1612,7 @@ static void mhi_ep_remove(struct device *dev)
>  {
>         struct mhi_ep_device *mhi_dev = to_mhi_ep_device(dev);
>         struct mhi_ep_driver *mhi_drv = to_mhi_ep_driver(dev->driver);
> +       struct mhi_ep_cntrl *mhi_cntrl = mhi_dev->mhi_cntrl;
>         struct mhi_result result = {};
>         struct mhi_ep_chan *mhi_chan;
>         int dir;
> @@ -1620,6 +1621,22 @@ static void mhi_ep_remove(struct device *dev)
>         if (mhi_dev->dev_type == MHI_DEVICE_CONTROLLER)
>                 return;
>
> +       /* Disable the channels to prevent new transfers */
> +       for (dir = 0; dir < 2; dir++) {
> +               mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
> +
> +               if (!mhi_chan)
> +                       continue;
> +
> +               mutex_lock(&mhi_chan->lock);
> +               mhi_chan->state = MHI_CH_STATE_DISABLED;
> +               mutex_unlock(&mhi_chan->lock);
> +       }
> +
> +       /* Flush in-flight transfers before notifying disconnect */
> +       if (mhi_cntrl->flush_async)
> +               mhi_cntrl->flush_async(mhi_cntrl);
> +
>         /* Disconnect the channels associated with the driver */
>         for (dir = 0; dir < 2; dir++) {
>                 mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
> @@ -1635,7 +1652,6 @@ static void mhi_ep_remove(struct device *dev)
>                         mhi_chan->xfer_cb(mhi_chan->mhi_dev, &result);
>                 }
>
> -               mhi_chan->state = MHI_CH_STATE_DISABLED;
>                 mhi_chan->xfer_cb = NULL;
>                 mutex_unlock(&mhi_chan->lock);
>         }
> diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
> index 7b40fc8cbe77..f6383a57a872 100644
> --- a/include/linux/mhi_ep.h
> +++ b/include/linux/mhi_ep.h
> @@ -107,6 +107,7 @@ struct mhi_ep_buf_info {
>   * @write_sync: CB function for writing to host memory synchronously
>   * @read_async: CB function for reading from host memory asynchronously
>   * @write_async: CB function for writing to host memory asynchronously
> + * @flush_async: CB function for flushing asynchronous read/writes
>   * @mhi_state: MHI Endpoint state
>   * @max_chan: Maximum channels supported by the endpoint controller
>   * @mru: MRU (Maximum Receive Unit) value of the endpoint controller
> @@ -164,6 +165,7 @@ struct mhi_ep_cntrl {
>         int (*write_sync)(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_buf_info *buf_info);
>         int (*read_async)(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_buf_info *buf_info);
>         int (*write_async)(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_buf_info *buf_info);
> +       void (*flush_async)(struct mhi_ep_cntrl *mhi_cntrl);
>
>         enum mhi_state mhi_state;
>
>
> --
> 2.43.0
>
>

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

[Frank Li]

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

Re: [PATCH v3 3/3] PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write

[Frank Li]

On Wed, Jul 22, 2026 at 07:54:46AM +0200, Manivannan Sadhasivam via B4 Relay wrote:
>
> From: Manivannan Sadhasivam <mani@kernel.org>
>
> The MHI core needs to make sure that all the current DMA transactions are
> completed before removing the channels. So implement the
> mhi_cntrl->flush_async() callback by first making sure all the in-flight
> DMA operations are completed and then flushing the DMA workqueue.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---

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

>  drivers/pci/endpoint/functions/pci-epf-mhi.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> index 7f5326925ed5..8d2d9d01cfd2 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> @@ -644,6 +644,15 @@ static int pci_epf_mhi_edma_write_async(struct mhi_ep_cntrl *mhi_cntrl,
>         return ret;
>  }
>
> +static void pci_epf_mhi_edma_flush_async(struct mhi_ep_cntrl *mhi_cntrl)
> +{
> +       struct pci_epf_mhi *epf_mhi = to_epf_mhi(mhi_cntrl);
> +
> +       dmaengine_synchronize(epf_mhi->dma_chan_rx);
> +       dmaengine_synchronize(epf_mhi->dma_chan_tx);
> +       flush_workqueue(epf_mhi->dma_wq);
> +}
> +
>  struct epf_dma_filter {
>         struct device *dev;
>         u32 dma_mask;
> @@ -812,6 +821,7 @@ static int pci_epf_mhi_link_up(struct pci_epf *epf)
>                 mhi_cntrl->write_sync = pci_epf_mhi_edma_write;
>                 mhi_cntrl->read_async = pci_epf_mhi_edma_read_async;
>                 mhi_cntrl->write_async = pci_epf_mhi_edma_write_async;
> +               mhi_cntrl->flush_async = pci_epf_mhi_edma_flush_async;
>         }
>
>         /* Register the MHI EP controller */
>
> --
> 2.43.0
>
>