Re: [PATCH v7 08/15] can: grcan: set DMA mask for GRCAN and GRCANFD to 32-bit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Pre-existing NULL pointer dereference if driver is bound manually.
- [Low] Misuse of `dev_err_probe()` causing redundant error logging.
--

commit d15f249cce5269da5a1fa1e8f9d22ab136f42013
Author: Daniel Hellstrom <daniel@gaisler.com>

can: grcan: set DMA mask for GRCAN and GRCANFD to 32-bit

Sets the DMA mask for GRCAN and GRCANFD devices to 32-bit.
Setting the DMA mask and coherent DMA mask to 32-bit ensures proper
memory addressing during DMA operations.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
> @@ -1746,6 +1746,13 @@ static int grcan_probe(struct platform_device *ofdev)
>  		goto exit_error;
>  	}
>  
> +	/* On 64-bit systems.. GRCAN and GRCANFD can only address 32-bit */
> +	err = dma_set_mask_and_coherent(&ofdev->dev, DMA_BIT_MASK(32));
> +	if (err) {
> +		dev_err_probe(&ofdev->dev, err, "No usable DMA configuration.\n");
> +		goto exit_error;

Does this dev_err_probe() call result in redundant error logging?

If dma_set_mask_and_coherent() fails, dev_err_probe() logs the error, but
its return value is ignored and execution falls through to the exit_error
label.

The exit_error path unconditionally calls dev_err() to log the failure:

drivers/net/can/grcan.c:grcan_probe() {
    ...
exit_error:
    dev_err(&ofdev->dev,
        "%s socket CAN driver initialization failed with error %d\n",
        DRV_NAME, err);
    return err;
}

Could this cause the error to be logged twice for a single failure
condition?

> +	}
> +
>  	irq = irq_of_parse_and_map(np, GRCAN_IRQIX_IRQ);

This wasn't introduced by this patch, but is it possible for
device_get_match_data() to return a null pointer earlier in this function?

If the driver is bound manually via sysfs or instantiated as a platform
device without a matching device tree node, device_get_match_data() will
return NULL:

drivers/net/can/grcan.c:grcan_probe() {
    ...
    hwcap = device_get_match_data(&ofdev->dev);

    if (!hwcap->txbug_possible) {
        txbug = false;
    ...
}

Could dereferencing hwcap without a check cause a kernel panic in that
scenario?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=8