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

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

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

[Manivannan Sadhasivam via B4 Relay]

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

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..7b12dfe8cfd3 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -331,6 +331,21 @@ 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);
+	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
+
+	/*
+	 * Make sure all the in-flight DMA operations are completed before
+	 * draining the tasklet using vchan_synchronize().
+	 */
+	while (chan->status == EDMA_ST_BUSY && time_before(jiffies, timeout))
+		cpu_relax();
+
+	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 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..7b12dfe8cfd3 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -331,6 +331,21 @@ 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);
+	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
+
+	/*
+	 * Make sure all the in-flight DMA operations are completed before
+	 * draining the tasklet using vchan_synchronize().
+	 */
+	while (chan->status == EDMA_ST_BUSY && time_before(jiffies, timeout))
+		cpu_relax();
+
+	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

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

[sashiko-bot]

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

New issues:
- [Critical] dw_edma_device_synchronize() silently fails if the timeout is reached, returning to the caller while the hardware channel is still active.
- [High] dw_edma_device_synchronize() implements a 5000ms busy-wait loop using cpu_relax() in a sleepable context.
- [Medium] Lockless read of chan->status without READ_ONCE() or memory barriers in dw_edma_device_synchronize().

Pre-existing issues:
- [High] dw_edma_device_terminate_all() fails to flush pending virt-dma descriptor queues, causing memory leaks and the risk of stale transfers resuming.
- [High] Virt-DMA descriptors are permanently leaked during channel teardown because vchan_free_chan_resources() is not called.
- [High] dw_edma_device_terminate_all() modifies channel state without holding the required spinlock.
--

commit dde223ef8e9950c9d18889da8638ba0fd963f5d8
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

dmaengine: dw-edma: Implement device_synchronize() callback

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.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index c2feb3adc79fa..7b12dfe8cfd3e 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -331,6 +331,21 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)

[Severity: High]
This is a pre-existing issue, but looking at dw_edma_device_terminate_all(),
it seems to lack the necessary virt-dma calls (like vchan_get_all_descriptors)
to extract and free pending descriptors:

    } else {
        chan->request = EDMA_REQ_STOP;
    }

Since dmaengine_terminate_all() requires all active and pending descriptors
to be aborted and removed from the channel's queues, could setting this
software flag without flushing the virt-dma descriptor lists cause queued
descriptors to remain in vc->desc_issued and vc->desc_submitted?

If a client later submits new transfers, might the hardware unintentionally
resume processing the old, supposedly terminated descriptors?

[Severity: High]
This is a pre-existing issue, but does dw_edma_device_terminate_all()
modify channel state without holding the required spinlock?

    } else if (chan->status == EDMA_ST_PAUSE) {
        chan->status = EDMA_ST_IDLE;
        chan->configured = false;

Since chan->status, chan->request, and chan->configured are concurrently
accessed and updated by the interrupt handler and issue pending paths under
chan->vc.lock, does modifying them unlocked introduce a race condition?

Could this corrupt the channel's internal state machine when a client
calls dmaengine_terminate_all() during interrupt processing?

>  	return err;
>  }
>  
> +static void dw_edma_device_synchronize(struct dma_chan *dchan)
> +{
> +	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
> +
> +	/*
> +	 * Make sure all the in-flight DMA operations are completed before
> +	 * draining the tasklet using vchan_synchronize().
> +	 */
> +	while (chan->status == EDMA_ST_BUSY && time_before(jiffies, timeout))

[Severity: Medium]
Since chan->status is concurrently mutated by the interrupt handler under
vc->lock, does reading it locklessly in this while loop without READ_ONCE()
or memory barriers constitute a data race?

Could this cause synchronization bypass if the state update is observed out
of order relative to the tasklet scheduling?

> +		cpu_relax();

[Severity: High]
Is it safe to implement a 5000ms busy-wait loop using cpu_relax() in a
sleepable context like dw_edma_device_synchronize()?

Since busy-waiting for up to 5 seconds blocks a CPU core entirely, could
this starve other tasks and trigger soft lockups when a client driver calls
dmaengine_synchronize() to wait for DMA completion? Since this callback is
explicitly allowed to sleep, should a sleeping wait like wait_event_timeout()
or msleep() be used instead?

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

[Severity: Critical]
Does dw_edma_device_synchronize() silently fail if the timeout is reached?

If the 5-second timeout expires while the channel is still busy, the loop
exits and the function returns to the caller without stopping the ongoing
hardware transfer or warning the client.

Since the client relies on device_synchronize() to ensure all active DMA
operations have fully completed before unmapping and freeing memory, could
this lead to silent data corruption and a use-after-free if the hardware
is still actively transferring data?

Should the function either wait indefinitely or forcefully abort the hardware
and emit a warning on timeout?

> +}

[ ... ]

[Severity: High]
This is a pre-existing issue, but does dw_edma_free_chan_resources()
permanently leak virt-dma descriptors during channel teardown?

Looking at dw_edma_free_chan_resources():

    static void dw_edma_free_chan_resources(struct dma_chan *dchan)
    {
        ...
        // No call to vchan_free_chan_resources(&chan->vc)
    }

When a DMA channel is released via dma_release_channel(), this teardown
function is called. Because dw_edma_device_terminate_all() leaves
uncompleted descriptors on the virt-dma lists, and this function fails to
invoke vchan_free_chan_resources(), are any pending or submitted
descriptors permanently leaked during a client driver unbind operation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-mhi-ep-flush-v1-0-714e0d56e87c@oss.qualcomm.com?part=1

[PATCH 2/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() 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 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

Re: [PATCH 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] Incorrect ordering of channel teardown, disconnect notification, and DMA flushing in mhi_ep_remove() causes data delivery after disconnect.
- [High] The newly introduced flush_async() callback lacks channel context, forcing a controller-wide flush that aborts DMA operations for all active channels.
- [High] Dropping the channel lock during flush_async exposes a window for worker threads to process disconnected channels, defeating the fix and re-introducing the UAF.

Pre-existing issues:
- [Critical] In-flight asynchronous DMA operations are not flushed before mhi_ep_ring_reset() frees ring->ring_cache.
- [High] Lockless access to mhi_chan->xfer_cb in completion handlers causes a TOCTOU data race with mhi_ep_remove().
--

commit 191c1d6e4eab8c4a6ee949c2b5b9499a56060800
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 in the MHI endpoint
controller. It is called during device removal to ensure any in-flight
asynchronous DMA operations are completed before continuing teardown.
This aims to prevent use-after-free issues caused by delayed DMA callbacks.

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

[Severity: High]
Does dropping the channel lock here leave a window for worker threads to
process disconnected channels?

If mhi_ep_ch_ring_worker wakes up while the lock is dropped, it sees
ring->started is still true. For Downlink channels, mhi_ep_process_ch_ring()
skips the state check and fires xfer_cb() directly:

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

Could this execute xfer_cb() mid-teardown and cause a use-after-free?

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

[Severity: High]
Does this order of operations allow successful data delivery callbacks after
disconnecting the client?

Since the channel lock is dropped, completing in-flight DMA operations during
flush_async() will invoke their completion handlers. Those handlers find
xfer_cb still populated and call it with a success status.

Should flush_async() be called before sending the -ENOTCONN disconnect
status to avoid notifying the client after it may have freed its contexts?

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

[Severity: High]
This is a pre-existing issue, but is there a data race when clearing xfer_cb
concurrently with completion handlers?

In mhi_ep_read_completion(), the callback is checked locklessly:

drivers/bus/mhi/ep/main.c:mhi_ep_read_completion() {
    if (mhi_chan->xfer_cb) {
        ...
        mhi_chan->xfer_cb(mhi_dev, &result);
    }
}

If mhi_ep_remove() clears mhi_chan->xfer_cb to NULL after the lockless check
but before execution, could this result in a NULL pointer dereference or
execution of freed memory?

>  		mutex_unlock(&mhi_chan->lock);
>  	}
> diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
> --- a/include/linux/mhi_ep.h
> +++ b/include/linux/mhi_ep.h
[ ... ]
> @@ -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);

[Severity: High]
Does the flush_async callback need a channel argument to avoid flushing
all channels on the controller?

When mhi_ep_remove() calls this without specifying a channel, an
implementation like pci_epf_mhi_edma_flush_async() terminates the shared
DMA channels indiscriminately:

drivers/pci/endpoint/functions/pci-epf-mhi.c:pci_epf_mhi_edma_flush_async() {
    dmaengine_terminate_sync(epf_mhi->dma_chan_rx);
    dmaengine_terminate_sync(epf_mhi->dma_chan_tx);
}

Will this abruptly discard in-flight operations and cause data loss for all
other active MHI clients sharing the controller?

[Severity: Critical]
This is a pre-existing issue, but is a similar flush required in
mhi_ep_ring_reset() before freeing the ring cache?

When the host issues a reset command, mhi_ep_ring_reset() executes:

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

If in-flight asynchronous DMA operations complete after this,
mhi_ep_read_completion() unconditionally accesses the freed cache:

drivers/bus/mhi/ep/main.c:mhi_ep_read_completion() {
    struct mhi_ring_element *el = &ring->ring_cache[ring->rd_offset];
    ...
}

Could this cause a NULL pointer dereference when a remote host triggers a
channel reset?

>  
>  	enum mhi_state mhi_state;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-mhi-ep-flush-v1-0-714e0d56e87c@oss.qualcomm.com?part=2

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

[Manivannan Sadhasivam via B4 Relay]

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

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..4af3689921a3 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_terminate_sync(epf_mhi->dma_chan_rx);
+	dmaengine_terminate_sync(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

[PATCH 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..4af3689921a3 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_terminate_sync(epf_mhi->dma_chan_rx);
+	dmaengine_terminate_sync(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 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:
- [High] pci_epf_mhi_edma_flush_async() violently terminates globally shared DMA channels, which aborts active transfers for all unrelated MHI channels.
- [High] Calling dmaengine_terminate_sync() in pci_epf_mhi_edma_flush_async() cancels in-flight DMA operations, leaking dynamically allocated transfer objects, DMA mappings, and orphaning upstream MHI buffers.
--

commit 6865f2474af30b8ecf0f56f29a02aca13c8836a3
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Subject: PCI: epf-mhi: Implement mhi_cntrl->flush_async() to flush DMA read/write

This commit implements the flush_async callback for the MHI endpoint
controller. It terminates the DMA channels and flushes the workqueue to
ensure transactions complete before removing channels.

> diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> index 7f5326925ed54..4af3689921a3f 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_terminate_sync(epf_mhi->dma_chan_rx);
> +	dmaengine_terminate_sync(epf_mhi->dma_chan_tx);

[Severity: High]
Does this code globally abort DMA operations for all independent MHI channels?

When a single MHI client device is unbound, mhi_ep_remove() loops over the
client's channels and calls mhi_cntrl->flush_async().

Since dma_chan_tx and dma_chan_rx are globally shared by the endpoint,
unconditionally calling dmaengine_terminate_sync() here appears to forcibly
abort all active and pending DMA operations for all other unrelated MHI
client devices currently sharing the controller.

[Severity: High]
Can this also leak dynamically allocated pci_epf_mhi_dma_transfer objects
and their DMA mappings?

The dmaengine_terminate_sync() function immediately drops pending and active
DMA descriptors without executing their completion callbacks.

Because the pci_epf_mhi_dma_transfer structures and their associated DMA
mappings are only tracked and cleaned up inside the completion callback
pci_epf_mhi_dma_async_callback(), aborting the transfers here seems to
permanently leak these resources and orphan the upstream MHI buffers.

> +	flush_workqueue(epf_mhi->dma_wq);
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-mhi-ep-flush-v1-0-714e0d56e87c@oss.qualcomm.com?part=3