dmaengine: fsl_raid: fix 64-bit ring base address programming

drivers/dma/fsl_raid.h line 72

[Severity: High]
This is a pre-existing issue, but I noticed the return value of fsl_re_chan_probe() is ignored, which can lead to a NULL pointer dereference on driver removal.

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

If fsl_re_chan_probe() fails, re_priv->total_chans is still incremented, and re_priv->re_jrs[q] remains NULL. During teardown, fsl_re_remove() calls fsl_re_remove_chan() which unconditionally dereferences it:

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

Will this cause a kernel panic on driver unbind if any channel failed to probe?

[Severity: High]
This isn't a bug introduced by this patch, but I noticed request_irq() is called without a corresponding free_irq(), which can cause a use-after-free panic on module removal.

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

The interrupt handler doesn't appear to be freed in error paths or during teardown in fsl_re_remove_chan():

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

	dma_pool_free(chan->re_dev->hw_desc_pool, chan->inb_ring_virt_addr,
		      chan->inb_phys_addr);
    ...
}

If a hardware interrupt fires after the module is unloaded, will the kernel jump into freed module memory?

[Severity: High]
This is a pre-existing issue, but it looks like channels are never added to the DMA device's channel list, which might result in a non-functional driver.

drivers/dma/fsl_raid.c:fsl_re_chan_probe() {
    ...
	re_priv->re_jrs[q] = chan;
	chan->chan.device = dma_dev;
	chan->chan.private = chan;
	chan->dev = chandev;
    ...
}

The global channel list is initialized via INIT_LIST_HEAD(&dma_dev->channels), but list_add_tail(&chan->chan.device_node, &dma_dev->channels) is never called. Will dma_async_device_register() iterate over an empty list and register zero channels?

[Severity: High]
This isn't a bug introduced by this patch, but the interrupt handler is registered before critical driver data is initialized, which could lead to crashes from early interrupts.

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

If a spurious or early interrupt triggers immediately after request_irq(), the ISR will execute dev_get_drvdata() and receive NULL:

drivers/dma/fsl_raid.c:fsl_re_isr() {
    ...
	re_chan = dev_get_drvdata((struct device *)data);

	irqstate = in_be32(&re_chan->jrregs->jr_interrupt_status);
    ...
}

Could this cause a NULL pointer dereference panic while reading jr_interrupt_status?

[Severity: High]
This is a pre-existing issue, but the driver invokes the DMA client completion callback while holding desc_lock, which can cause an AB-BA self-deadlock.

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

If the client submits a chained DMA request within the callback, fsl_re_tx_submit() attempts to re-acquire the exact same desc_lock:

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

Will this instantly deadlock the CPU?

[Severity: High]
This isn't a bug introduced by this patch, but fsl_re_free_chan_resources() modifies the free_q list without holding desc_lock and improperly loops based on alloc_count.

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);
    ...
		kfree(desc);
	}
}

If any descriptors are still in flight, free_q will be prematurely empty, causing list_first_entry() to return an invalid pointer which is then freed. Should this be protected by desc_lock and use list_empty() checks?

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