[PATCH v2] dmaengine: fsl-edma: tracing: no ptr dereference during log output

[PATCH v2] dmaengine: fsl-edma: tracing: no ptr dereference during log output

[Martin Kaiser]

The fsl edma events store a pointer to a struct fsl_edma_engine in the
ringbuffer and dereference it when a log entry is printed. At this time,
the pointer may no longer be valid.

Event injection can be used to trigger a crash:

$ cd /sys/kernel/tracing
$ echo 'value = 0' > events/fsl_edma/edma_writeb/inject
$ cat trace

The log output needs only edma->membase. Add a membase field at the end
of the event and use the new field for log output. Keep the existing
fields for backward compatibility.

Fixes: 11102d0c343b ("dmaengine: fsl-edma: add trace event support")
Cc: stable@vger.kernel.org
Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v2:
 - dmaengine prefix in the subject line
 - Cc stable
 - Steven's Reviewed tag

 drivers/dma/fsl-edma-trace.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/fsl-edma-trace.h b/drivers/dma/fsl-edma-trace.h
index d3541301a247..45d964a3726d 100644
--- a/drivers/dma/fsl-edma-trace.h
+++ b/drivers/dma/fsl-edma-trace.h
@@ -19,14 +19,16 @@ DECLARE_EVENT_CLASS(edma_log_io,
 		__field(struct fsl_edma_engine *, edma)
 		__field(void __iomem *, addr)
 		__field(u32, value)
+		__field(void __iomem *, membase)
 	),
 	TP_fast_assign(
 		__entry->edma = edma;
 		__entry->addr = addr;
 		__entry->value = value;
+		__entry->membase = edma->membase;
 	),
 	TP_printk("offset %08x: value %08x",
-		(u32)(__entry->addr - __entry->edma->membase), __entry->value)
+		(u32)(__entry->addr - __entry->membase), __entry->value)
 );
 
 DEFINE_EVENT(edma_log_io, edma_readl,
-- 
2.43.7

Re: [PATCH v2] dmaengine: fsl-edma: tracing: no ptr dereference during log output

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Double byte-swapping of TCD register values on Big Endian CPUs.
--

--- Patch [1]: [PATCH v2] dmaengine: fsl-edma: tracing: no ptr dereference during log output ---
commit 3052808ada723a8fc5d5327178f31b6879486260
Author: Martin Kaiser <martin@kaiser.cx>
Date:   Sat Jul 18 12:58:48 2026 +0000

    dmaengine: fsl-edma: tracing: no ptr dereference during log output

    The fsl edma events store a pointer to a struct fsl_edma_engine in the
    ringbuffer and dereference it when a log entry is printed. This adds a
    membase field to the event to avoid dereferencing the pointer, preventing
    a use-after-free if the edma pointer becomes invalid.

    Fixes: 11102d0c343b ("dmaengine: fsl-edma: add trace event support")
    Cc: stable@vger.kernel.org

> diff --git a/drivers/dma/fsl-edma-trace.h b/drivers/dma/fsl-edma-trace.h
> index d3541301a2470..45d964a3726d6 100644
> --- a/drivers/dma/fsl-edma-trace.h
> +++ b/drivers/dma/fsl-edma-trace.h

[ ... ]

>  	TP_fast_assign(
>  		__entry->edma = edma;
>  		__entry->addr = addr;
>  		__entry->value = value;
> +		__entry->membase = edma->membase;
>  	),
>  	TP_printk("offset %08x: value %08x",
> -		(u32)(__entry->addr - __entry->edma->membase), __entry->value)
> +		(u32)(__entry->addr - __entry->membase), __entry->value)
>  );

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential problem with how
the value being traced here is written to the hardware on Big Endian CPUs.

When a DMA transfer is initiated, fsl_edma_set_tcd_to_le() prepares the
in-memory descriptor fields in little-endian format. Then
edma_cp_tcd_to_reg() passes this little-endian value to edma_writel() via
edma_write_tcdreg_c():

drivers/dma/fsl-edma-common.h:edma_write_tcdreg_c() {
    ...
    __iomem __le32 : edma_writel(chan->edma, (u32 __force)(_val), ...
    ...
}

However, edma_writel() uses iowrite32() which assumes native CPU-endian
input:

drivers/dma/fsl-edma-common.h:edma_writel() {
    if (edma->big_endian)
        iowrite32be(val, addr);
    else
        iowrite32(val, addr);

    trace_edma_writel(edma, addr, val);
}

On Big Endian CPUs, does this result in iowrite32() double-swapping the
already little-endian descriptor data and writing a corrupted value to the
target and source addresses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718130024.341243-1-martin@kaiser.cx?part=1

Re: [PATCH v2] dmaengine: fsl-edma: tracing: no ptr dereference during log output

[Frank Li]

On Sat, Jul 18, 2026 at 02:58:48PM +0200, Martin Kaiser wrote:
> The fsl edma events store a pointer to a struct fsl_edma_engine in the
> ringbuffer and dereference it when a log entry is printed. At this time,
> the pointer may no longer be valid.
>
> Event injection can be used to trigger a crash:
>
> $ cd /sys/kernel/tracing
> $ echo 'value = 0' > events/fsl_edma/edma_writeb/inject
> $ cat trace
>
> The log output needs only edma->membase. Add a membase field at the end
> of the event and use the new field for log output. Keep the existing
> fields for backward compatibility.
>
> Fixes: 11102d0c343b ("dmaengine: fsl-edma: add trace event support")
> Cc: stable@vger.kernel.org
> Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---

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


> v2:
>  - dmaengine prefix in the subject line
>  - Cc stable
>  - Steven's Reviewed tag
>
>  drivers/dma/fsl-edma-trace.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/fsl-edma-trace.h b/drivers/dma/fsl-edma-trace.h
> index d3541301a247..45d964a3726d 100644
> --- a/drivers/dma/fsl-edma-trace.h
> +++ b/drivers/dma/fsl-edma-trace.h
> @@ -19,14 +19,16 @@ DECLARE_EVENT_CLASS(edma_log_io,
>  		__field(struct fsl_edma_engine *, edma)
>  		__field(void __iomem *, addr)
>  		__field(u32, value)
> +		__field(void __iomem *, membase)
>  	),
>  	TP_fast_assign(
>  		__entry->edma = edma;
>  		__entry->addr = addr;
>  		__entry->value = value;
> +		__entry->membase = edma->membase;
>  	),
>  	TP_printk("offset %08x: value %08x",
> -		(u32)(__entry->addr - __entry->edma->membase), __entry->value)
> +		(u32)(__entry->addr - __entry->membase), __entry->value)
>  );
>
>  DEFINE_EVENT(edma_log_io, edma_readl,
> --
> 2.43.7
>

Re: [PATCH v2] dmaengine: fsl-edma: tracing: no ptr dereference during log output

[Martin Kaiser]

Thus wrote sashiko-bot@kernel.org (sashiko-bot@kernel.org):

> However, edma_writel() uses iowrite32() which assumes native CPU-endian
> input:

> drivers/dma/fsl-edma-common.h:edma_writel() {

static inline void edma_writel(struct fsl_edma_engine *edma,
			       u32 val, void __iomem *addr)

where val is always little-endian

>     if (edma->big_endian)

edma->big_endian is configured from a devicetree setting. It's possible that it
doesn't match cpu endianness.

Could we check __BYTE_ORDER__ instead (or additonally)?


>         iowrite32be(val, addr);
>     else
>         iowrite32(val, addr);

The potential endianness issue affects only the value that's written to the
hardware. It's unrelated to the tracing patch that is reviewed here.

>     trace_edma_writel(edma, addr, val);

The value that's traced is always little-endian. We might have to use
le32_to_cpu(val) in the trace event definition. But again, that would be a
separate patch.

May I ask that we put the sashiko comments asided and merge the trace event
fix? (I'm happy to submit another patch for le32_to_cpu in the event.)

Thanks,
Martin

Re: [PATCH v2] dmaengine: fsl-edma: tracing: no ptr dereference during log output

[Frank Li]

On Tue, Jul 28, 2026 at 10:19:24AM +0200, Martin Kaiser wrote:
> Thus wrote sashiko-bot@kernel.org (sashiko-bot@kernel.org):
>
> > However, edma_writel() uses iowrite32() which assumes native CPU-endian
> > input:
>
> > drivers/dma/fsl-edma-common.h:edma_writel() {
>
> static inline void edma_writel(struct fsl_edma_engine *edma,
> 			       u32 val, void __iomem *addr)
>
> where val is always little-endian
>
> >     if (edma->big_endian)
>
> edma->big_endian is configured from a devicetree setting. It's possible that it
> doesn't match cpu endianness.
>
> Could we check __BYTE_ORDER__ instead (or additonally)?
>
>
> >         iowrite32be(val, addr);
> >     else
> >         iowrite32(val, addr);
>
> The potential endianness issue affects only the value that's written to the
> hardware. It's unrelated to the tracing patch that is reviewed here.
>
> >     trace_edma_writel(edma, addr, val);
>
> The value that's traced is always little-endian. We might have to use
> le32_to_cpu(val) in the trace event definition. But again, that would be a
> separate patch.
>
> May I ask that we put the sashiko comments asided and merge the trace event
> fix? (I'm happy to submit another patch for le32_to_cpu in the event.)

This is preexisting issue, which should not impact pick this patch. Just
wait for vinod to pick it.

It will be good if you send new patch to fix these preexisting problem.

Frank

>
> Thanks,
> Martin