Re: [PATCH v4 5/7] can: rcar_canfd: Add support for Renesas RZ/G3S

[sashiko-bot@kernel.org]

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

New issues:
- [High] Lack of validation for Classical CAN mode on CAN FD-only hardware.

Pre-existing issues:
- [High] Dangling pointer in global array leads to double free when register_candev fails.
--

commit 1c2240055c546ef67856ad4716f6c425dbce01c6
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

can: rcar_canfd: Add support for Renesas RZ/G3S

This commit adds support for the Renesas RZ/G3S CAN-FD controller.
The RZ/G3S controller supports two channels, 16 AFL pages, and operates
exclusively in CAN-FD mode.

> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index bed7cde1aa01b..48e0573a73a44 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
[ ... ]
> @@ -661,6 +661,26 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
>  	.fixed_canfd = 0,
>  };
>  
> +static const struct rcar_canfd_hw_info r9a08g045_hw_info = {
> +	.nom_bittiming = &rcar_canfd_gen4_nom_bittiming_const,
> +	.data_bittiming = &rcar_canfd_gen4_data_bittiming_const,
> +	.tdc_const = &rcar_canfd_gen4_tdc_const,
> +	.regs = &rcar_gen4_regs,
> +	.sh = &rcar_gen4_shift_data,
> +	.rnc_field_width = 16,
> +	.max_aflpn = 15,
> +	.max_cftml = 31,
> +	.max_channels = 2,
> +	.postdiv = 2,
> +	.shared_global_irqs = 0,
> +	.multi_channel_irqs = 1,
> +	.ch_interface_mode = 1,
> +	.shared_can_regs = 1,
> +	.external_clk = 1,
> +	.fcan_pclk = 1,
> +	.fixed_canfd = 1,

[Severity: High]
If the device tree specifies the "renesas,no-can-fd" property, could this
lead to a silent mismatch between the software network stack configuration
and the hardware state?

In drivers/net/can/rcar/rcar_canfd.c:rcar_canfd_probe(), if the property is
present, the driver will set fdmode = false:

	if (of_property_read_bool(dev->of_node, "renesas,no-can-fd"))
		fdmode = false;

However, when rcar_canfd_configure_controller() runs, it skips the
controller mode update because fixed_canfd is true:

	if (gpriv->info->fixed_canfd)
		continue;

This effectively leaves the hardware locked in CAN FD mode, while the rest of
the driver initializes the software stack for Classical CAN. For example,
rcar_canfd_channel_probe() initializes a max MTU of 16 bytes, and
rcar_canfd_configure_rx() restricts the physical hardware Rx FIFO payload
size to 8 bytes.

If the hardware receives a 64-byte CAN FD frame, will it attempt to write
64 bytes into an Rx FIFO configured for 8 bytes, causing a buffer overflow?

> +};
> +
[ ... ]
> @@ -2383,6 +2403,7 @@ static DEFINE_SIMPLE_DEV_PM_OPS(rcar_canfd_pm_ops, rcar_canfd_suspend,
>  
>  static const __maybe_unused struct of_device_id rcar_canfd_of_table[] = {
>  	{ .compatible = "renesas,r8a779a0-canfd", .data = &rcar_gen4_hw_info },
> +	{ .compatible = "renesas,r9a08g045-canfd", .data = &r9a08g045_hw_info },

[Severity: High]
This is a pre-existing issue, but does probe failure on this newly added
hardware expose a potential double-free?

In drivers/net/can/rcar/rcar_canfd.c:rcar_canfd_channel_probe(), the global
array is populated before device registration:

	gpriv->ch[priv->channel] = priv;
	...
	err = register_candev(ndev);
	if (err) {
		...
		goto fail_candev;
	}

If register_candev() fails, the error path calls free_candev(ndev), which
also frees the embedded priv structure, but leaves the dangling pointer in
gpriv->ch[priv->channel].

When the failure propagates back to rcar_canfd_probe(), the cleanup block
iterates over all configured channels and calls rcar_canfd_channel_remove()
for each, including the one that just failed:

drivers/net/can/rcar/rcar_canfd.c:rcar_canfd_channel_remove() {
	...
	unregister_candev(priv->ndev);
	...
	free_candev(priv->ndev);
}

Could this lead to memory corruption when it attempts to unregister and free
the already freed dangling pointer?

>  	{ .compatible = "renesas,r9a09g047-canfd", .data = &r9a09g047_hw_info },
>  	{ .compatible = "renesas,r9a09g077-canfd", .data = &r9a09g077_hw_info },
>  	{ .compatible = "renesas,rcar-gen3-canfd", .data = &rcar_gen3_hw_info },

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902141544.565763-1-claudiu.beznea+renesas@tuxon.dev?part=5