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

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

[Manivannan Sadhasivam]

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().

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

The dmaengine driver change can go separately as there is no build dependency.
But both MHI and PCI EP changes should go together. 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 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):
      dmaengine: dw-edma: Implement device_synchronize() callback
      bus: mhi: ep: Add mhi_cntrl->flush_async() callback to flush the async read/write
      PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write

 drivers/bus/mhi/ep/main.c                    |  7 +++++++
 drivers/dma/dw-edma/dw-edma-core.c           | 16 ++++++++++++++++
 drivers/pci/endpoint/functions/pci-epf-mhi.c | 10 ++++++++++
 include/linux/mhi_ep.h                       |  2 ++
 4 files changed, 35 insertions(+)
---
base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
change-id: 20260627-mhi-ep-flush-b50502718a9d

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

[PATCH v2 1/3] dmaengine: dw-edma: Implement device_synchronize() callback

[Manivannan Sadhasivam]

device_synchronize() callback is required by the client drivers to ensure
all the DMA operations are completed so that they can free the memory
associated with the complete callbacks.

So implement this callback by first making sure that all the in-flight DMA
operations are completed and then call vchan_synchronize() to drain the
DMA tasklet.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/dma/dw-edma/dw-edma-core.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index c2feb3adc79f..df0d1a946ed0 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -12,6 +12,7 @@
 #include <linux/dmaengine.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
+#include <linux/iopoll.h>
 #include <linux/irq.h>
 #include <linux/dma/edma.h>
 #include <linux/dma-mapping.h>
@@ -331,6 +332,20 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
 	return err;
 }
 
+static void dw_edma_device_synchronize(struct dma_chan *dchan)
+{
+	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+
+	/*
+	 * Make sure all the in-flight DMA operations are completed before
+	 * draining the tasklet using vchan_synchronize().
+	 */
+	read_poll_timeout(READ_ONCE, chan->status, chan->status != EDMA_ST_BUSY,
+			  10, 0, false, chan->status);
+
+	vchan_synchronize(&chan->vc);
+}
+
 static void dw_edma_device_issue_pending(struct dma_chan *dchan)
 {
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
@@ -968,6 +983,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
 	dma->device_pause = dw_edma_device_pause;
 	dma->device_resume = dw_edma_device_resume;
 	dma->device_terminate_all = dw_edma_device_terminate_all;
+	dma->device_synchronize = dw_edma_device_synchronize;
 	dma->device_issue_pending = dw_edma_device_issue_pending;
 	dma->device_tx_status = dw_edma_device_tx_status;
 	dma->device_prep_slave_sg = dw_edma_device_prep_slave_sg;

-- 
2.43.0

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

[Manivannan Sadhasivam]

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() before setting xfer_cb to NULL.

Note that flush_async() blocks until all the in-flight async transfers are
completed, so calling it with the chan->lock held would needlessly stall
the transfer paths on that channel for the whole duration of the drain. So
drop chan->lock around the flush and clear xfer_cb() only afterwards, once
all the pending completions are drained.

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 | 7 +++++++
 include/linux/mhi_ep.h    | 2 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index 0277e1ab1198..329a4855d397 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;
@@ -1636,6 +1637,12 @@ static void mhi_ep_remove(struct device *dev)
 		}
 
 		mhi_chan->state = MHI_CH_STATE_DISABLED;
+		mutex_unlock(&mhi_chan->lock);
+
+		if (mhi_cntrl->flush_async)
+			mhi_cntrl->flush_async(mhi_cntrl);
+
+		mutex_lock(&mhi_chan->lock);
 		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 v2 3/3] PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write

[Manivannan Sadhasivam]

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 v2 1/3] dmaengine: dw-edma: Implement device_synchronize() callback

[sashiko-bot]

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

New issues:
- [High] Misuse of the `read_poll_timeout` macro causes an unlocked read-modify-write data race on `chan->status`, which can overwrite the interrupt handler's state completion and lead to an infinite loop.

Pre-existing issues:
- [High] The `dw_edma_device_terminate_all()` function performs read and write access to the state machine variables `chan->status` and `chan->request` without acquiring the required `chan->vc.lock` spinlock.
--

--- Patch [1]: [PATCH v2 1/3] dmaengine: dw-edma: Implement device_synchronize() callback ---
commit dad772ddd69c60671b5bb40802d893826d2fa9c5
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

dmaengine: dw-edma: Implement device_synchronize() callback

This commit implements the device_synchronize() callback for the dw-edma
driver. It uses read_poll_timeout() to wait for all in-flight DMA operations
to complete by polling the channel status. Once complete, it drains the
tasklet using vchan_synchronize().

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index c2feb3adc79fa..df0d1a946ed06 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -331,6 +332,20 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
>  	return err;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does
dw_edma_device_terminate_all() perform read and write accesses to state
machine variables without acquiring the required chan->vc.lock spinlock?

Looking at dw_edma_device_terminate_all(), it modifies chan->status and
chan->request locklessly:

    } else if (chan->request > EDMA_REQ_PAUSE) {
        err = -EPERM;
    } else {
        chan->request = EDMA_REQ_STOP;
    }

Since the DMA engine API allows asynchronous termination, could this race with
dw_edma_done_interrupt() or dw_edma_device_issue_pending(), both of which hold
the spinlock while modifying the same fields? If the interrupt handler reads
chan->request right before terminate_all writes it, it might mistakenly start
the next pending transfer instead of stopping it.

>  
> +static void dw_edma_device_synchronize(struct dma_chan *dchan)
> +{
> +	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +
> +	/*
> +	 * Make sure all the in-flight DMA operations are completed before
> +	 * draining the tasklet using vchan_synchronize().
> +	 */
> +	read_poll_timeout(READ_ONCE, chan->status, chan->status != EDMA_ST_BUSY,
> +			  10, 0, false, chan->status);

[Severity: High]
Could this read_poll_timeout() macro usage introduce a regression by causing
an unlocked read-modify-write data race on chan->status?

The read_poll_timeout() macro expands to (val) = op(args). By passing
chan->status as the val parameter, it expands to:

    (chan->status) = READ_ONCE(chan->status)

This loop executes without the chan->vc.lock spinlock. If the hardware
interrupt handler dw_edma_done_interrupt() fires exactly after READ_ONCE()
reads EDMA_ST_BUSY but before the assignment back to chan->status, it will
cleanly set chan->status = EDMA_ST_IDLE. 

Would the polling thread then overwrite the newly set EDMA_ST_IDLE state with
its stale EDMA_ST_BUSY value? Because the timeout parameter is 0, if this
corrupted EDMA_ST_BUSY state is written back, no further interrupts will arrive
to clear it, which could result in a permanent hang.

> +
> +	vchan_synchronize(&chan->vc);
> +}

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

Re: [PATCH v2 2/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 5 potential issue(s) to consider:

New issues:
- [High] Draining async transfers *after* sending a disconnect notification causes out-of-order successful callbacks to be delivered to a disconnected client.
- [High] Controller-wide `flush_async()` called inside a per-channel loop disrupts active channels and causes livelocks.

Pre-existing issues:
- [Critical] The `MHI_PKT_TYPE_RESET_CHAN_CMD` handler frees the transfer ring without flushing in-flight async DMA operations.
- [High] Lockless TOCTOU race on `mhi_chan->xfer_cb` in completion handlers leads to a Use-After-Free.
- [High] Null pointer dereference on `mhi_chan->mhi_dev` in `mhi_ep_process_ch_ring` during controller teardown.
--

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

This patch introduces a flush_async() callback to the MHI EP controller and
invokes it during device removal. It aims to prevent use-after-free issues
by ensuring that any in-flight asynchronous DMA operations are completed
before the channel's transfer callback is unregistered.

> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c

[ ... ]

> @@ -1636,6 +1637,12 @@ static void mhi_ep_remove(struct device *dev)
>  		}
>  
>  		mhi_chan->state = MHI_CH_STATE_DISABLED;
> +		mutex_unlock(&mhi_chan->lock);
> +
> +		if (mhi_cntrl->flush_async)
> +			mhi_cntrl->flush_async(mhi_cntrl);
> +

[Severity: High]
Does this sequence result in successful callbacks being delivered after the
client has already been notified of disconnection?

When mhi_ep_remove() sends -ENOTCONN to mhi_chan->xfer_cb(), it signals the
client to free its context. If we then drop the lock and block on
flush_async() while xfer_cb is still registered, pending DMA completions
can invoke mhi_ep_read_completion():

mhi_ep_read_completion()
    if (mhi_chan->xfer_cb) {
        ...
        mhi_chan->xfer_cb(mhi_dev, &result);
    }

Can this lead to the client driver receiving a success callback after it has
already freed its resources in response to the -ENOTCONN notification?

[Severity: High]
Can calling a controller-wide flush operation inside a per-channel loop
disrupt other active channels or cause a livelock?

The code loops over the device's channels (UL then DL). When disabling the UL
channel, it calls mhi_cntrl->flush_async(). Since flush_async() operates on
the controller context and MHI EP architectures typically share RX/TX DMA
channels across the entire controller, the DL channel might still be active
and queuing new transfers.

Could waiting for all controller DMAs to flush while other channels are active
cause a livelock or drain traffic inappropriately?

> +		mutex_lock(&mhi_chan->lock);
>  		mhi_chan->xfer_cb = NULL;
>  		mutex_unlock(&mhi_chan->lock);
>  	}

[Severity: High]
This is a pre-existing issue, but does conditionally skipping the flush
leave the fallback path unprotected against concurrent teardown?

If mhi_cntrl->flush_async is NULL, the flush is skipped. However, DMA
completion handlers evaluate the callback pointer locklessly:

mhi_ep_read_completion()
    if (mhi_chan->xfer_cb) {
        ...
        mhi_chan->xfer_cb(mhi_dev, &result);
    }

If the thread is preempted here, mhi_ep_remove() can run, set
mhi_chan->xfer_cb = NULL, and free the device. When the handler resumes, it
could invoke the stale callback pointer. Does the lockless read of xfer_cb
lack the necessary synchronization?

While reviewing this file, I also noticed a couple of pre-existing issues
elsewhere in the code:

[Severity: Critical]
This is a pre-existing issue, but does resetting the channel ring without
flushing async DMAs lead to a NULL pointer dereference?

When the host sends a reset command, mhi_ep_process_ctrl_interrupt() calls
mhi_ep_ring_reset() which frees ring->ring_cache. Because this path does not
cancel in-flight async DMAs, a pending DMA completion handler might execute
shortly after:

mhi_ep_read_completion()
    struct mhi_ring_element *el = &ring->ring_cache[ring->rd_offset];

Can this result in a crash if the completion handler dereferences the cleared
ring_cache?

[Severity: High]
This is a pre-existing issue, but is there a risk of a NULL pointer
dereference during controller teardown?

In mhi_ep_abort_transfer(), client devices are destroyed using
device_for_each_child(), which sets mhi_chan->mhi_dev = NULL. Because the
channel rings are not stopped until later, a doorbell interrupt can fire and
queue mhi_ep_ch_ring_worker().

The worker calls mhi_ep_process_ch_ring() which checks for a missing client
driver:

mhi_ep_process_ch_ring()
    if (!mhi_chan->xfer_cb) {
        dev_err(&mhi_chan->mhi_dev->dev, "Client driver not available\n");

Since mhi_dev is now NULL, can this dev_err() call trigger a panic?

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

Re: [PATCH v2 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 2 potential issue(s) to consider:

New issues:
- [High] Deadlock / Infinite wait in pci_epf_mhi_edma_flush_async() when synchronizing globally shared DMA channels.

Pre-existing issues:
- [High] Pre-existing NULL pointer dereference on PCI link flap exacerbated by unconditional cleanup dereference.
--

--- Patch [3]: [PATCH v2 3/3] PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write ---
commit c83c3a93b470add22e521d4fb56e38b2a49dd98b
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

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);
> +	flush_workqueue(epf_mhi->dma_wq);

[Severity: High]
Can this cause an indefinite hang if other channels on the same controller
are active?

If a user unloads an MHI client module (invoking mhi_ep_remove), it calls
this function to drain the DMA channel. However, because
epf_mhi->dma_chan_rx and dma_chan_tx are globally shared across the entire
PCI endpoint, and dmaengine_synchronize() polls until the channel is no
longer busy, continuous data traffic from a different independent MHI
client might prevent this from ever returning.

[Severity: High]
This is a pre-existing issue, but could this also lead to a NULL pointer
dereference after a PCI link flap?

When the PCI link goes down, pci_epf_mhi_link_down() calls
pci_epf_mhi_dma_deinit(), which frees the DMA channels and workqueue and
sets them to NULL.

When the link comes back up, pci_epf_mhi_link_up() unconditionally assigns
these EDMA callbacks but fails to call pci_epf_mhi_dma_init() to reallocate
the resources.

If the link goes down again (or a client device is removed), invoking this
new flush_async() callback will pass NULL pointers to
dmaengine_synchronize() and flush_workqueue(), causing a crash.

> +}
> +

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