Re: [PATCH net-next 17/37] can: rcar_canfd: Derive max_channels from the device tree

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] can: rcar_canfd: out-of-bounds MMIO and broken sparse DTs
--

commit a1d79c18cfbc6d4e1569e45058d228d182b10de9
Author: Biju Das <biju.das.jz@bp.renesas.com>
can: rcar_canfd: Derive max_channels from the device tree

This commit modifies the driver to obtain the maximum number of channels
dynamically from the device tree child count rather than hardcoding it
per SoC in struct rcar_canfd_hw_info.

Link: https://patch.msgid.link/20261001075742.21961-3-biju.das.jz@bp.renesas.com

> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index 74c3c3f20991f..41c3951189490 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -436,7 +436,6 @@ struct rcar_canfd_hw_info {
>  	u8 rnc_field_width;
>  	u8 max_aflpn;
>  	u8 max_cftml;
> -	u8 max_channels;

[Severity: High]
Does removing the structural enforcement of the hardware capability limit
allow the device tree to dictate bounds for MMIO register access? The
driver should enforce physical hardware limits independently of the device
tree to prevent out-of-bounds access.

>  	u8 postdiv;
>  	/* hardware features */

[ ... ]

> @@ -2145,14 +2139,18 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  	bool fdmode = true;			/* CAN FD only mode - default */
>  	char name[9] = "channelX";
>  	u32 ch, fcan_freq;
> +	u8 max_channels;
>  	int i;
>  
>  	info = of_device_get_match_data(dev);
> +	max_channels = of_get_child_count(dev->of_node);
> +	if (max_channels > RCANFD_NUM_CHANNELS)
> +		return -EINVAL;
>  
>  	if (of_property_read_bool(dev->of_node, "renesas,no-can-fd"))
>  		fdmode = false;			/* Classical CAN only mode */
>  
> -	for (i = 0; i < info->max_channels; ++i) {
> +	for (i = 0; i < max_channels; ++i) {

[Severity: High]
Is the child node count wrongly used as an absolute channel limit and
upper bound for iteration here?

Conflating the child population count with the maximum channel index
breaks sparse device trees. If a device tree explicitly omits an
intermediate channel (for example, via /delete-node/ channel1), the child
count shrinks. For a 3-channel SoC where channel1 is omitted, max_channels
becomes 2, causing this loop to terminate before discovering channel2.

Also, if a malformed device tree specifies more channels than the SoC
physically supports, the driver will attempt to configure non-existent
channels since the SoC-specific limit was removed. Accessing unmapped
or reserved MMIO registers for those extra channels could cause a
synchronous external abort.

>  		name[7] = '0' + i;
>  		of_child = of_get_available_child_by_name(dev->of_node, name);
>  		if (of_child) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009134323.64064-1-mkl@pengutronix.de?part=17