dmaengine: fsl_raid: switch __b32 to u32

PATCH New Rosen Penev 2 participants 2 messages Sep 10 dmaengine

Share & tools

Rosen Penev

author

2026-09-10 19:46 UTC #0 lore raw

PATCH v2 1/1 dmaengine: fsl_raid: switch __b32 to u32

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.

Reported-by kernel test robot <[email protected]>

Closes https://lore.kernel.org/oe-kbuild-all/[email protected]/ open here

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 <[email protected]>

Reviewed-by Frank Li <[email protected]>

---
v2: split patch up.

 drivers/dma/fsl_raid.h | 54 +++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

drivers/dma/fsl_raid.h +27 ~~-27~~

index adbfede330a7..6fde5192ab77 100644
@@ -110,59 +110,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 */
 };
 
 /*

Signature · 1 line

2.55.0

[email protected]

AI review Reviewed-by Frank Li on v2 1/1

2026-09-10 20:00 UTC #1 lore raw

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

Pre-existing issues:
- [High] The commit message claims that the CDB structures keep their __be32 annotations because they are correctly written via cpu_to_be32(). However, the driver directly assigns native u32 values to the cdb32 fields of the XOR, PQ, and MOVE structures without using cpu_to_be32(), violating the claim and leaving an endianness bug on little-endian architectures.
- [High] Lock held while invoking client DMA callback, causing a self-deadlock if the client submits a new transaction.
- [High] Missing free_irq() leads to IRQ leak and potential use-after-free.
- [High] Erroneous bitwise logic for splitting ring base addresses causes silent truncation.
--

commit 7b9c559b3810d3491f22a6353857e63e9743b1e1
Author: Rosen Penev <[email protected]>

dmaengine: fsl_raid: switch __b32 to u32

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.

Reported-by kernel test robot <[email protected]>

Closes https://lore.kernel.org/oe-kbuild-all/[email protected]/ open here

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 <[email protected]>

Reviewed-by Frank Li <[email protected]>

Quoted 3 lines

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.

[Severity: High]
This isn't a bug introduced by this patch, but the commit message claims that the CDB structures keep their __be32 annotations because they are correctly written via cpu_to_be32().

Is this description accurate? Looking at fsl_re_prep_dma_genq(), the driver directly assigns native u32 values to the cdb32 field of the XOR structure without using cpu_to_be32():

drivers/dma/fsl_raid.c:fsl_re_prep_dma_genq() {
    ...
    xor->cdb32 = cdb;
    ...
}

Similar direct assignments occur in fsl_re_prep_dma_pq() for pq->cdb32 and fsl_re_prep_dma_memcpy() for move->cdb32.

Could this lead to the DMA hardware receiving a byteswapped command descriptor block on little-endian architectures, causing it to misinterpret opcodes and potentially corrupt memory?

[Severity: High]
This is a pre-existing issue, but can a self-deadlock occur if the client callback submits a new transaction?

drivers/dma/fsl_raid.c:fsl_re_tx_submit() {
    ...
    spin_lock_irqsave(&re_chan->desc_lock, flags);
    ...
}

drivers/dma/fsl_raid.c:fsl_re_dequeue() {
    ...
    fsl_re_desc_done(desc);
    ...
}

If a client submits a new DMA transaction from the callback triggered by fsl_re_desc_done() while re_chan->desc_lock is still held, fsl_re_tx_submit() will unconditionally attempt to acquire the exact same lock again. Does this result in a complete system hang?

[Severity: High]
This is a pre-existing issue, but does this code leak the IRQ and potentially cause a use-after-free?

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

drivers/dma/fsl_raid.c:fsl_re_remove_chan() {
    ...
    tasklet_kill(&chan->irqtask);
    ...
}

The teardown function kills the tasklet but appears to miss the required free_irq() call. If the module is unloaded or the device is unbound, and an interrupt fires after the re_chan structure is freed, will the ISR fsl_re_isr() dereference the freed pointer?

[Severity: High]
This is a pre-existing issue, but is the bitwise logic for splitting ring base addresses correct here?

drivers/dma/fsl_raid.c:fsl_re_chan_probe() {
    ...
    out_be32(&chan->jrregs->inbring_base_h,
             chan->inb_phys_addr & FSL_RE_ADDR_BIT_MASK);
    out_be32(&chan->jrregs->oubring_base_h,
             chan->oub_phys_addr & FSL_RE_ADDR_BIT_MASK);
    out_be32(&chan->jrregs->inbring_base_l,
             chan->inb_phys_addr >> FSL_RE_ADDR_BIT_SHIFT);
    ...
}

This logic appears to use an incorrect mask and shift, truncating the high bits and completely shifting out the low bits. Could this result in the DMA rings being placed at the wrong physical memory locations, leading to immediate DMA failure or memory corruption?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1