dmaengine: bestcomm: set bcom_eng to NULL on probe failure

drivers/dma/bestcomm/bestcomm.c static int mpc52xx_bcom_probe(struct platform_device *op) · line 445

[Severity: High]
This is a pre-existing issue, but does this mismatch in size during release cause a permanent resource leak?

The memory region is requested earlier in mpc52xx_bcom_probe() using the device tree node size:

if (!request_mem_region(res_bcom.start, resource_size(&res_bcom),
DRIVER_NAME)) {

However, the error path above attempts to release the region using a fixed compile-time size: sizeof(struct mpc52xx_sdma). Since __release_region() strictly verifies the boundaries of the requested resource, if these sizes differ, the check fails and the I/O memory region is never freed. Driver unbinding in mpc52xx_bcom_remove() also uses this same fixed size.

drivers/dma/bestcomm/bestcomm.c static int mpc52xx_bcom_probe(struct platform_device *op) · line 448

[Severity: High]
This is a pre-existing issue, but by assigning bcom_eng early, do we expose uninitialized state to concurrent accessors?

In mpc52xx_bcom_probe(), the global pointer is published immediately after allocation:

    bcom_eng = kzalloc_obj(struct bcom_engine);
    if (!bcom_eng) {

Concurrent API calls like bcom_task_alloc() rely on the pointer not being NULL as a safety check:

    if (!bcom_eng)
        return NULL;
    spin_lock(&bcom_eng->lock);

Because the pointer is published before bcom_engine_init() sets up the lock and structures, a concurrent client could dereference uninitialized fields. While adding bcom_eng = NULL closes the use-after-free window during cleanup, the early publication race still exists.

drivers/dma/bestcomm/bestcomm.c static int mpc52xx_bcom_probe(struct platform_device *op) · line 451

[Severity: High]
This is a pre-existing issue, but is there a missing check for an error pointer from rh_create() that could cause a kernel panic?

In drivers/dma/bestcomm/sram.c:bcom_sram_init(), the result of rh_create() is directly used without validation:

    /* Create an rheap (defaults to 32 bits word alignment) */
    bcom_sram->rh = rh_create(4);

    /* Attach the free zones */
    regaddr_p = NULL;
    psize = 0;

    if (!regaddr_p || !psize) {
        /* Attach the whole zone */
        rh_attach_region(bcom_sram->rh, 0, bcom_sram->size);

If memory allocation fails under pressure, rh_create() returns ERR_PTR(-ENOMEM). Passing this directly to rh_attach_region() will cause a crash when it accesses internal fields.

[Severity: Medium]
This is a pre-existing issue, but does locklessly clearing the stop marker create a data race and risk premature reallocation?

In drivers/dma/bestcomm/bestcomm.c:bcom_task_free(), the task marker state is cleared without holding the required lock:

    /* Clear TDT */
    bcom_eng->tdt[tsk->tasknum].start = 0;
    bcom_eng->tdt[tsk->tasknum].stop  = 0;

The same happens on the allocation error path in bcom_task_alloc():

    bcom_eng->tdt[tasknum].stop = 0;

Since bcom_task_alloc() enforces a lock to safely locate free tasks, if stop is cleared locklessly before task teardown completes, a concurrent allocator might identify the task as available and reallocate it while resources are still being freed.

[Severity: High]
This is a pre-existing issue, but does freeing a task leak the dynamically allocated SRAM used for microcode image descriptors?

When loading an image, drivers/dma/bestcomm/bestcomm.c:bcom_load_image() dynamically allocates SRAM and tracks it via tdt->start:

    desc = bcom_sram_alloc(hdr->desc_size * sizeof(u32), 4, &start_pa);
    if (!desc)
        return -ENOMEM;

    tdt->start = start_pa;

However, when the task is torn down, bcom_task_free() simply discards this physical address without freeing the block:

    /* Clear TDT */
    bcom_eng->tdt[tsk->tasknum].start = 0;
    bcom_eng->tdt[tsk->tasknum].stop  = 0;

Because SRAM is extremely limited on the MPC5200, repeatedly bringing a network interface up and down could exhaust SRAM and break bestcomm functionality. Should bcom_sram_free() be called here before zeroing the marker?

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