dmaengine: fsl_raid: free resources in probe

drivers/dma/fsl_raid.c static int fsl_re_chan_probe(struct platform_device *ofdev, · line 673

[Severity: High]
This is a pre-existing issue, but does requesting the IRQ here make the interrupt handler live before subsequent initialization steps?

These important structures are initialized too late:

drivers/dma/fsl_raid.c:fsl_re_chan_probe() {
    ...
	spin_lock_init(&chan->desc_lock);
	INIT_LIST_HEAD(&chan->ack_q);
	INIT_LIST_HEAD(&chan->active_q);
    ...
}

Can this cause a kernel crash due to uninitialized spinlock and list operations if the hardware has a pending interrupt from a previous boot or kexec when the driver probes?

drivers/dma/fsl_raid.c static int fsl_re_chan_probe(struct platform_device *ofdev, · line 688

[Severity: High]
If fsl_re_chan_probe() fails after this assignment, the channel pointer is stored in the driver's private array before the final allocations occur. If those allocations fail, doesn't this leave a dangling pointer?

Later in fsl_re_probe(), the return value of fsl_re_chan_probe() is ignored:

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

So the driver commits the failed channel to the active channel count.

When the driver is subsequently removed, fsl_re_remove_chan() will be called again for the already freed channel. Since this patch adds free_irq() and of_platform_device_destroy() to the error unwind path, will this result in a double-free of the IRQ and a use-after-free of the platform device?

[Severity: High]
This is a pre-existing issue, but while the driver properly assigns the framework device pointers:

drivers/dma/fsl_raid.c:fsl_re_chan_probe() {
    ...
	chan->chan.device = dma_dev;
	chan->chan.private = chan;
    ...
}

It seems to fail to link the channel into the framework's channels list. Does this missing DMA channel registration render the driver completely useless by silently failing to register any channels into the dmaengine framework?

[Severity: Critical]
This is a pre-existing issue, but does this misconfigure the DMA engine?

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);
	out_be32(&chan->jrregs->oubring_base_l,
		 chan->oub_phys_addr >> FSL_RE_ADDR_BIT_SHIFT);
    ...
}

It appears the lower 32 bits are written into the high address register and the upper 32 bits into the low address register. Can this result in invalid physical memory accesses by the hardware, leading to silent memory corruption or IOMMU faults?

[ ... ]

drivers/dma/fsl_raid.c static int fsl_re_chan_probe(struct platform_device *ofdev, · line 746

[Severity: High]
The patch added free_irq() and tasklet_kill() after freeing the DMA memory in err_free_1.

If an interrupt fires during this window, could the handler schedule the tasklet which then accesses the freed or unallocated DMA rings? Does this incorrect error unwind order create a use-after-free race condition?

[ ... ]

drivers/dma/fsl_raid.c static int fsl_re_probe(struct platform_device *ofdev) · line 851

[Severity: High]
This is a pre-existing issue, but if fsl_re_chan_probe() fails early in its execution, re_priv->re_jrs[q] is left as NULL.

Because total_chans is unconditionally incremented on early error in fsl_re_probe():

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

Will this dereference the channel pointer without checking if it is NULL, causing an immediate kernel crash?

[Severity: Critical]
This is a pre-existing issue, but if the device tree has multiple job queues and a subsequent one fails validation:

drivers/dma/fsl_raid.c:fsl_re_probe() {
    ...
		rc = of_property_read_u32(np, "reg", &off);
		if (rc) {
			dev_err(dev, "Reg property not found in JQ node\n");
			return -ENODEV;
		}
    ...
}

Returning an error here abandons previously successful channels without tearing down their active resources. Because re_priv is managed by devres, it will be freed, leaving the active IRQ handler pointing to freed memory. Can this lead to a resource leak and use-after-free memory corruption if an interrupt fires?

[Severity: High]
This is a pre-existing issue, but is there a potential deadlock in the DMA callback execution?

The spinlock is acquired to protect the descriptor queues:

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

Then the client callback is invoked synchronously without dropping the spinlock first:

drivers/dma/fsl_raid.c:fsl_re_desc_done() {
    ...
	dmaengine_desc_get_callback_invoke(&desc->async_tx, NULL);
    ...
}

Could this lead to an ABBA deadlock if the client attempts to allocate or submit a new descriptor from its completion callback?

[Severity: High]
This is a pre-existing issue, but are the underlying channel resources (IRQs, tasklets, DMA pools) completely destroyed here:

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]);
    ...
}

Before the framework unregistration occurs?

drivers/dma/fsl_raid.c:fsl_re_remove() {
    ...
	dma_async_device_unregister(&re_priv->dma_dev);
}

Does this leave a window for active clients to use freed resources if the driver is unloaded or unbound while a concurrent DMA client is still active?

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