[PATCH] dmaengine: fsl_raid: fix sparse warnings

[PATCH] dmaengine: fsl_raid: fix sparse warnings

[Rosen Penev]

The register maps in fsl_raid.h annotate every field as __be32, but
they are accessed only through ioread32be()/iowrite32be(), which
already perform the byte swap and take plain u32 __iomem pointers.
The __be32 annotation therefore makes sparse reject each access:

  drivers/dma/fsl_raid.c:116:17: sparse: incorrect type in argument 1
      (different base types) ... got restricted __be32 [noderef] __iomem *

Drop __be32 from struct fsl_re_ctrl and struct fsl_re_chan_cfg; the
descriptor/CDB structures written via cpu_to_be32() keep their
annotations.

Also fix the remaining sparse warnings in fsl_raid.c:

  - compare the desc pointer with !desc instead of desc <= 0 in the
    three prep functions,
  - byte swap cdb32 with cpu_to_be32() when programming the XOR, PQ
    and MOVE command descriptor blocks; the plain u32 assignment is
    broken on little-endian hosts.

No warnings remain.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202608200637.ro1CqMxi-lkp@intel.com/
Fixes: 68b7fbc23529 ("dma: fsl_raid: keep MMIO bases as void __iomem and cast at access")
Assisted-by: opencode:deepseek-v4-flash-free
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsl_raid.c | 12 +++++-----
 drivers/dma/fsl_raid.h | 54 +++++++++++++++++++++---------------------
 2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
index a35ddc1297ce..524a7329c3c4 100644
--- a/drivers/dma/fsl_raid.c
+++ b/drivers/dma/fsl_raid.c
@@ -350,7 +350,7 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_genq(
 	}
 
 	desc = fsl_re_chan_alloc_desc(re_chan, flags);
-	if (desc <= 0)
+	if (!desc)
 		return NULL;
 
 	if (scf && (flags & DMA_PREP_CONTINUE)) {
@@ -365,7 +365,7 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_genq(
 	cdb |= FSL_RE_INTR_ON_ERROR << FSL_RE_CDB_ERROR_SHIFT;
 	cdb |= FSL_RE_DATA_DEP << FSL_RE_CDB_DEPEND_SHIFT;
 	xor = desc->cdb_addr;
-	xor->cdb32 = cdb;
+	xor->cdb32 = cpu_to_be32(cdb);
 
 	if (scf) {
 		/* compute q = src0*coef0^src1*coef1^..., * is GF(8) mult */
@@ -474,7 +474,7 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_pq(
 		src_cnt += 3;
 
 	desc = fsl_re_chan_alloc_desc(re_chan, flags);
-	if (desc <= 0)
+	if (!desc)
 		return NULL;
 
 	/* Filling GenQQ CDB */
@@ -485,7 +485,7 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_pq(
 	cdb |= FSL_RE_DATA_DEP << FSL_RE_CDB_DEPEND_SHIFT;
 
 	pq = desc->cdb_addr;
-	pq->cdb32 = cdb;
+	pq->cdb32 = cpu_to_be32(cdb);
 
 	p = pq->gfm_q1;
 	/* Init gfm_q1[] */
@@ -558,7 +558,7 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_memcpy(
 	}
 
 	desc = fsl_re_chan_alloc_desc(re_chan, flags);
-	if (desc <= 0)
+	if (!desc)
 		return NULL;
 
 	/* Filling move CDB */
@@ -568,7 +568,7 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_memcpy(
 	cdb |= FSL_RE_DATA_DEP << FSL_RE_CDB_DEPEND_SHIFT;
 
 	move = desc->cdb_addr;
-	move->cdb32 = cdb;
+	move->cdb32 = cpu_to_be32(cdb);
 
 	/* Filling frame 0 of CFD with move CDB */
 	cf = desc->cf_addr;
diff --git a/drivers/dma/fsl_raid.h b/drivers/dma/fsl_raid.h
index 748047996bbc..1b6f6b82c7d3 100644
--- a/drivers/dma/fsl_raid.h
+++ b/drivers/dma/fsl_raid.h
@@ -109,59 +109,59 @@
 
 struct fsl_re_ctrl {
 	/* General Configuration Registers */
-	__be32 global_config;	/* Global Configuration Register */
+	u32 global_config;	/* Global Configuration Register */
 	u8     rsvd1[4];
-	__be32 galois_field_config; /* Galois Field Configuration Register */
+	u32 galois_field_config; /* Galois Field Configuration Register */
 	u8     rsvd2[4];
-	__be32 jq_wrr_config;   /* WRR Configuration register */
+	u32 jq_wrr_config;	/* WRR Configuration register */
 	u8     rsvd3[4];
-	__be32 crc_config;	/* CRC Configuration register */
+	u32 crc_config;		/* CRC Configuration register */
 	u8     rsvd4[228];
-	__be32 system_reset;	/* System Reset Register */
+	u32 system_reset;	/* System Reset Register */
 	u8     rsvd5[252];
-	__be32 global_status;	/* Global Status Register */
+	u32 global_status;	/* Global Status Register */
 	u8     rsvd6[832];
-	__be32 re_liodn_base;	/* LIODN Base Register */
+	u32 re_liodn_base;	/* LIODN Base Register */
 	u8     rsvd7[1712];
-	__be32 re_version_id;	/* Version ID register of RE */
-	__be32 re_version_id_2; /* Version ID 2 register of RE */
+	u32 re_version_id;	/* Version ID register of RE */
+	u32 re_version_id_2;	/* Version ID 2 register of RE */
 	u8     rsvd8[512];
-	__be32 host_config;	/* Host I/F Configuration Register */
+	u32 host_config;	/* Host I/F Configuration Register */
 };
 
 struct fsl_re_chan_cfg {
 	/* Registers for JR interface */
-	__be32 jr_config_0;	/* Job Queue Configuration 0 Register */
-	__be32 jr_config_1;	/* Job Queue Configuration 1 Register */
-	__be32 jr_interrupt_status; /* Job Queue Interrupt Status Register */
+	u32 jr_config_0;	/* Job Queue Configuration 0 Register */
+	u32 jr_config_1;	/* Job Queue Configuration 1 Register */
+	u32 jr_interrupt_status; /* Job Queue Interrupt Status Register */
 	u8     rsvd1[4];
-	__be32 jr_command;	/* Job Queue Command Register */
+	u32 jr_command;		/* Job Queue Command Register */
 	u8     rsvd2[4];
-	__be32 jr_status;	/* Job Queue Status Register */
+	u32 jr_status;		/* Job Queue Status Register */
 	u8     rsvd3[228];
 
 	/* Input Ring */
-	__be32 inbring_base_h;	/* Inbound Ring Base Address Register - High */
-	__be32 inbring_base_l;	/* Inbound Ring Base Address Register - Low */
-	__be32 inbring_size;	/* Inbound Ring Size Register */
+	u32 inbring_base_h;	/* Inbound Ring Base Address Register - High */
+	u32 inbring_base_l;	/* Inbound Ring Base Address Register - Low */
+	u32 inbring_size;	/* Inbound Ring Size Register */
 	u8     rsvd4[4];
-	__be32 inbring_slot_avail; /* Inbound Ring Slot Available Register */
+	u32 inbring_slot_avail; /* Inbound Ring Slot Available Register */
 	u8     rsvd5[4];
-	__be32 inbring_add_job;	/* Inbound Ring Add Job Register */
+	u32 inbring_add_job;	/* Inbound Ring Add Job Register */
 	u8     rsvd6[4];
-	__be32 inbring_cnsmr_indx; /* Inbound Ring Consumer Index Register */
+	u32 inbring_cnsmr_indx; /* Inbound Ring Consumer Index Register */
 	u8     rsvd7[220];
 
 	/* Output Ring */
-	__be32 oubring_base_h;	/* Outbound Ring Base Address Register - High */
-	__be32 oubring_base_l;	/* Outbound Ring Base Address Register - Low */
-	__be32 oubring_size;	/* Outbound Ring Size Register */
+	u32 oubring_base_h;	/* Outbound Ring Base Address Register - High */
+	u32 oubring_base_l;	/* Outbound Ring Base Address Register - Low */
+	u32 oubring_size;	/* Outbound Ring Size Register */
 	u8     rsvd8[4];
-	__be32 oubring_job_rmvd; /* Outbound Ring Job Removed Register */
+	u32 oubring_job_rmvd;	/* Outbound Ring Job Removed Register */
 	u8     rsvd9[4];
-	__be32 oubring_slot_full; /* Outbound Ring Slot Full Register */
+	u32 oubring_slot_full;	/* Outbound Ring Slot Full Register */
 	u8     rsvd10[4];
-	__be32 oubring_prdcr_indx; /* Outbound Ring Producer Index */
+	u32 oubring_prdcr_indx;	/* Outbound Ring Producer Index */
 };
 
 /*
-- 
2.55.0

Re: [PATCH] dmaengine: fsl_raid: fix sparse warnings

[sashiko-bot]

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

Pre-existing issues:
- [High] Incomplete Endianness Fix
- [High] Use-After-Free / Resource Leak
- [High] Incorrect teardown ordering in `fsl_re_remove()`
- [High] Memory Corruption due to unconditional pop in `fsl_re_free_chan_resources()`
- [High] Deadlock during client callback invocation
- [High] NULL Pointer Dereference on probe failure
- [Medium] Uninitialized DMA Memory Leak
--

--- Patch [1]: [PATCH] dmaengine: fsl_raid: fix sparse warnings ---
commit 736f7849012b086f7c1f995cc45ee838fbb53650
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: fsl_raid: fix sparse warnings

The register maps in fsl_raid.h annotate every field as __be32, but
they are accessed only through ioread32be()/iowrite32be(), which
already perform the byte swap and take plain u32 __iomem pointers.
The __be32 annotation therefore makes sparse reject each access.

Closes: https://lore.kernel.org/oe-kbuild-all/202608200637.ro1CqMxi-lkp@intel.com/

> diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
> index 99945845d8b5e..97103c84dd70b 100644
> --- a/drivers/dma/fsl_raid.c
> +++ b/drivers/dma/fsl_raid.c
> @@ -334,7 +334,7 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_genq(
>  	}
>  
>  	desc = fsl_re_chan_alloc_desc(re_chan, flags);
> -	if (desc <= 0)
> +	if (!desc)

[Severity: Medium]
This is a pre-existing issue, but does this allocation leak uninitialized
kernel memory to the device?

Looking at fsl_re_chan_alloc_desc():

drivers/dma/fsl_raid.c:fsl_re_chan_alloc_desc() {
    ...
    cf = dma_pool_alloc(re_chan->re_dev->cf_desc_pool, GFP_NOWAIT, &paddr);
    ...
}

Since the compound frame descriptors are dynamically allocated without zeroing
(__GFP_ZERO), could the uninitialized dpi and trailing gfm array fields expose
kernel heap state to the device?

>  		return NULL;
>  
>  	if (scf && (flags & DMA_PREP_CONTINUE)) {
> @@ -349,7 +349,7 @@ static struct dma_async_tx_descriptor *fsl_re_prep_dma_genq(
>  	cdb |= FSL_RE_INTR_ON_ERROR << FSL_RE_CDB_ERROR_SHIFT;
>  	cdb |= FSL_RE_DATA_DEP << FSL_RE_CDB_DEPEND_SHIFT;
>  	xor = desc->cdb_addr;
> -	xor->cdb32 = cdb;
> +	xor->cdb32 = cpu_to_be32(cdb);

[Severity: High]
This is a pre-existing issue, but does swapping only cdb32 here leave the
driver broken on little-endian platforms?

Looking at fill_cfd_frame() which is called shortly after:

drivers/dma/fsl_raid.c:fill_cfd_frame() {
    cf[index].efrl32 = efrl;
    cf[index].addr_high = upper_32_bits(addr);
    cf[index].addr_low = lower_32_bits(addr);
    ...
}

It appears native u32 values are still assigned to __be32 fields without
swapping. Will this incomplete endianness conversion cause issues on
little-endian hosts?

[ ... ]

[Severity: High]
This is a pre-existing issue, but does the driver leak the IRQ resource or
cause a use-after-free on unbind?

Looking at fsl_re_chan_probe():

drivers/dma/fsl_raid.c:fsl_re_chan_probe() {
    ...
    ret = request_irq(chan->irq, fsl_re_isr, 0, chan->name, chandev);
    ...
}

The IRQ is requested here but appears to lack cleanup in error paths and the
remove function. Upon device unbind, could the IRQ remain active while the
channel structure is freed, leading to a use-after-free on the next interrupt?

[Severity: High]
This is a pre-existing issue, but does the teardown order in fsl_re_remove()
violate DMA engine lifecycle constraints?

Looking at fsl_re_remove():

drivers/dma/fsl_raid.c:fsl_re_remove() {
    ...
    for (i = 0; i < re_priv->total_chans; i++)
        fsl_re_remove_chan(re_priv->re_jrs[i]);

    dma_async_device_unregister(&re_priv->dma_dev);
    ...
}

Since it destroys channel resources before unregistering the DMA device, does
this race condition permit concurrent client requests to operate on freed
memory?

[Severity: High]
This is a pre-existing issue, but can the loop in fsl_re_free_chan_resources()
cause memory corruption?

Looking at fsl_re_free_chan_resources():

drivers/dma/fsl_raid.c:fsl_re_free_chan_resources() {
    ...
    while (re_chan->alloc_count--) {
        desc = list_first_entry(&re_chan->free_q, struct fsl_re_desc, node);
        list_del(&desc->node);
        ...
    }
    ...
}

The cleanup routine loops blindly based on alloc_count. If descriptors are
currently active or submitted, could free_q run out early and cause
list_first_entry() to pop invalid pointers?

[Severity: High]
This is a pre-existing issue, but could fsl_re_dequeue() deadlock during client
callback invocation?

Looking at fsl_re_dequeue():

drivers/dma/fsl_raid.c:fsl_re_dequeue() {
    spin_lock_irqsave(&re_chan->desc_lock, flags);
    ...
    if (found) {
        fsl_re_desc_done(desc);
    ...
}

The client callback is triggered while a non-recursive spinlock is held. Will
this self-deadlock if the callback immediately submits a new transaction?

[Severity: High]
This is a pre-existing issue, but does fsl_re_probe() risk a NULL pointer
dereference on probe failure?

Looking at fsl_re_probe():

drivers/dma/fsl_raid.c:fsl_re_probe() {
    ...
    if (rc) {
        fsl_re_chan_probe(ofdev, child, ridx++, off);
        re_priv->total_chans++;
    ...
}

Since the return code of fsl_re_chan_probe() is ignored and total_chans is
incremented unconditionally, will failed channel allocations leave re_jrs[q]
as NULL, causing them to be blindly dereferenced on module removal?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820002816.17382-1-rosenp@gmail.com?part=1