[PATCH 0/3] Renesas CANFD Fixes

[PATCH 0/3] Renesas CANFD Fixes

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

This series fixes the pre-existing issues reported by sashiko[1].

This patch series depend upon [2]

[1]https://lore.kernel.org/linux-renesas-soc/aj2PBvZYaVs0G-be@shikoro/
[2]https://lore.kernel.org/all/20260625135216.130450-1-biju.das.jz@bp.renesas.com/

Biju Das (3):
  can: rcar_canfd: Fix dangling pointer on channel probe failure
  can: rcar_canfd: Fix double-cleanup and unclocked register access on
    init failure
  can: rcar_canfd: Fix interrupt registration order

 drivers/net/can/rcar/rcar_canfd.c | 33 ++++++++++++++++---------------
 1 file changed, 17 insertions(+), 16 deletions(-)

-- 
2.43.0

[PATCH 1/3] can: rcar_canfd: Fix dangling pointer on channel probe failure

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

In rcar_canfd_channel_probe(), the channel pointer is saved globally
via gpriv->ch[priv->channel] = priv. If register_candev() fails
shortly after, the error path uses free_candev(ndev) to release the
memory but leaves the pointer in the global array.

During teardown, rcar_canfd_probe() calls rcar_canfd_channel_remove()
for all channels in the mask. Since the dangling pointer still
evaluates to true, this will result in a use-after-free and double
free of the memory.

Clear gpriv->ch[priv->channel] in the failure path so the slot is
correctly seen as empty during teardown.

Fixes: dd3bd23eb438 ("can: rcar_canfd: Add Renesas R-Car CAN FD driver")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/net/can/rcar/rcar_canfd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index fcc37b73ed43..57f1710caa89 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1982,6 +1982,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 	netif_napi_del(&priv->napi);
 fail:
 	free_candev(ndev);
+	gpriv->ch[priv->channel] = NULL;
 	return err;
 }
 
-- 
2.43.0

Re: [PATCH 1/3] can: rcar_canfd: Fix dangling pointer on channel probe failure

[sashiko-bot]

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

New issues:
- [High] The patch introduces a Use-After-Free by dereferencing `priv` immediately after it has been freed, which can lead to an Out-Of-Bounds NULL write primitive.

Pre-existing issues:
- [High] The lifecycle of the dynamically allocated `priv` is shorter than the `devm`-managed IRQ handlers, leading to a potential use-after-free if an interrupt occurs during teardown.
--

commit e7cd96500f76ea63fe797db226dd1a89ff157846
Author: Biju Das <biju.das.jz@bp.renesas.com>

can: rcar_canfd: Fix dangling pointer on channel probe failure
    
In rcar_canfd_channel_probe(), the channel pointer is saved globally
via gpriv->ch[priv->channel] = priv. If register_candev() fails
shortly after, the error path uses free_candev(ndev) to release the
memory but leaves the pointer in the global array.

> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index eaf8cac780387..5aafbab85da67 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -1982,6 +1982,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
>  	netif_napi_del(&priv->napi);
>  fail:
>  	free_candev(ndev);

[Severity: High]
This is a pre-existing issue, but does the use of devm_request_irq() earlier
in rcar_canfd_channel_probe() create a lifetime mismatch with this
free_candev() call?

The driver requests channel interrupts using devm_request_irq() with priv
(which is allocated via alloc_candev()) as the dev_id. If a failure occurs
and we hit this fail path, or during teardown in rcar_canfd_channel_remove(),
the memory for priv is synchronously freed by free_candev().

However, because the IRQ was allocated with the devm_ API, free_irq() is not
called until the device's devres cleanup phase, which executes later.

If a spurious hardware interrupt fires during the vulnerable window between
free_candev() and devres cleanup, could the handler
(rcar_canfd_channel_err_interrupt) execute with and dereference the freed
priv pointer?

> +	gpriv->ch[priv->channel] = NULL;

[Severity: High]
Does this cause a use-after-free?

The free_candev(ndev) call immediately above synchronously frees the memory
block containing both ndev and the inline private data priv. Evaluating
gpriv->ch[priv->channel] directly dereferences the newly freed priv pointer.

If memory debugging is enabled (like SLUB poisoning), the freed memory might
be filled with poison bytes, which could lead to an out-of-bounds write here
when it reads a poison value for priv->channel.

>  	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630135119.104774-1-biju.das.jz@bp.renesas.com?part=1

[PATCH 2/3] can: rcar_canfd: Fix double-cleanup and unclocked register access on init failure

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

In rcar_canfd_probe(), if rcar_canfd_global_init() encounters an
error, it disables the clocks and asserts the resets before
returning. However, the probe function then jumps to the fail_mode
label and unconditionally calls rcar_canfd_global_deinit().

The deinit function calls rcar_canfd_disable_global_interrupts(),
which writes to the hardware registers. Since the peripheral clock
has already been disabled by the failed init, this leads to a
synchronous external abort.

Make the rcar_canfd_global_init() failure path jump directly to
fail_dev, skipping the redundant global_deinit() call, since
global_init() has already unwound its own clock/reset state on
error.

Fixes: fa5f4ec8fff8 ("can: rcar_canfd: Extract rcar_canfd_global_{,de}init()")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/net/can/rcar/rcar_canfd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 57f1710caa89..e88554388553 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -2257,7 +2257,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 
 	err = rcar_canfd_global_init(gpriv);
 	if (err)
-		goto fail_mode;
+		goto fail_dev;
 
 	for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels) {
 		err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq,
@@ -2275,7 +2275,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 fail_channel:
 	for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels)
 		rcar_canfd_channel_remove(gpriv, ch);
-fail_mode:
+
 	rcar_canfd_global_deinit(gpriv, false);
 fail_dev:
 	return err;
-- 
2.43.0

Re: [PATCH 2/3] can: rcar_canfd: Fix double-cleanup and unclocked register access on init failure

[sashiko-bot]

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

New issues:
- [High] Use-after-free read on the error path of rcar_canfd_channel_probe().
- [High] The CAN network device is registered and exposed to userspace before its global and shared interrupts are requested, introducing a race condition.

Pre-existing issues:
- [High] Use-After-Free vulnerability in error and removal paths due to mismatched devm-managed interrupt lifetimes and dangling pointers.
- [High] Incorrect clearing of W0C (Write-0-To-Clear) hardware status registers can silently drop concurrent interrupts.
--

commit a601711f6b0c8a2b4d0262288f6d110cc98b8332
Author: Biju Das <biju.das.jz@bp.renesas.com>

can: rcar_canfd: Fix double-cleanup and unclocked register access on init failure

In rcar_canfd_probe(), if rcar_canfd_global_init() encounters an error,
it disables the clocks and asserts the resets before returning. However,
the probe function then jumps to the fail_mode label and unconditionally
calls rcar_canfd_global_deinit(), which writes to hardware registers and
causes a synchronous external abort. This commit makes the failure path
jump directly to fail_dev, skipping the redundant global_deinit() call.

> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index 5aafbab85da67..1f3dc4b354f87 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -2257,7 +2257,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  
>  	err = rcar_canfd_global_init(gpriv);
>  	if (err)
> -		goto fail_mode;
> +		goto fail_dev;
>  
>  	for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels) {

[Severity: High]
Could this code expose the network interface to userspace before its interrupts
are fully set up?

The loop registers the channels via rcar_canfd_channel_probe(), which calls
register_candev(). However, the shared global interrupts (ch_irq, g_irq) are
not requested via devm_request_irq() until after the channel loop completes.

If a userspace process (e.g., systemd-networkd) brings the interface UP
immediately upon registration, could rcar_canfd_open() start the hardware and
enable interrupts before the OS has registered the IRQ handlers, leading to
unhandled interrupts?

>  		err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq,
>  					       transceivers[ch]);
[ ... ]
> @@ -2275,7 +2275,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  fail_channel:
>  	for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels)
>  		rcar_canfd_channel_remove(gpriv, ch);

[Severity: High]
This is a pre-existing issue, but is there a use-after-free vulnerability
here due to mismatched interrupt lifetimes?

When rcar_canfd_channel_remove() frees the network device and its inline
private data priv via free_candev(priv->ndev), the devm_request_irq()
shared interrupt handlers remain active until devres cleanup runs after
probe() or remove() returns.

If a shared interrupt fires during this window, wouldn't the handler
(e.g., rcar_canfd_global_interrupt()) execute and unconditionally
dereference gpriv->ch[ch]->base, accessing the freed priv memory?

> -fail_mode:
> +
>  	rcar_canfd_global_deinit(gpriv, false);
>  fail_dev:
>  	return err;


I also noticed a few other issues in this file:

[Severity: High]
This is a pre-existing issue, but won't the driver incorrectly clear
W0C (Write-0-To-Clear) hardware status registers?

In functions like rcar_canfd_tx_done(), rcar_canfd_rx_poll(), and
rcar_canfd_global_error(), the driver reads the status register into sts
and writes back sts & ~FLAG:

drivers/net/can/rcar/rcar_canfd.c:rcar_canfd_tx_done() {
    ...
    rcar_canfd_write(priv->base, RCANFD_CFSTS(gpriv, ch, RCANFD_CFFIFO_IDX),
                     sts & ~RCANFD_CFSTS_CFTXIF);
    ...
}

Because this bitwise operation leaves 0s in bit positions that were
originally 0, won't writing 0 inadvertently clear any flag that was
asserted by the hardware after the register was initially read, leading to
silent loss of hardware interrupts and dropped packets?


[Severity: High]
Is there a use-after-free read in the error path of rcar_canfd_channel_probe()?

When device registration fails, the error path calls free_candev(ndev)
which frees the net_device and its inline private data priv:

drivers/net/can/rcar/rcar_canfd.c:rcar_canfd_channel_probe() {
    ...
fail:
	free_candev(ndev);
	gpriv->ch[priv->channel] = NULL;
	return err;
}

Immediately following the free, the code attempts to nullify the global
pointer array using priv->channel. Since priv has already been freed,
isn't this a use-after-free read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630135119.104774-1-biju.das.jz@bp.renesas.com?part=2

[PATCH 3/3] can: rcar_canfd: Fix interrupt registration order

[Biju]

From: Biju Das <biju.das.jz@bp.renesas.com>

In rcar_canfd_probe(), global interrupts are requested via
devm_request_irq() before rcar_canfd_global_init() enables the clocks
and before the per-channel structures in gpriv->ch[] are allocated.

If the interrupt is shared and fires immediately, the handler
rcar_canfd_global_err_interrupt() will invoke
rcar_canfd_handle_global_err(), leading to a NULL pointer
dereference.

Move the rcar_canfd_global_init() and per-channel probe calls ahead
of the devm_request_irq() calls so that clocks are enabled and
gpriv->ch[] is fully populated before any global interrupt can be
delivered. The IRQ failure paths now jump to fail_channel instead of
fail_dev, since channels are allocated earlier and must be torn down
on this path too.

Fixes: dd3bd23eb438 ("can: rcar_canfd: Add Renesas R-Car CAN FD driver")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/net/can/rcar/rcar_canfd.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index e88554388553..4ee108abffb8 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -2216,6 +2216,17 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	gpriv->base = addr;
 	gpriv->fcbase = addr + gpriv->info->regs->coffset;
 
+	err = rcar_canfd_global_init(gpriv);
+	if (err)
+		goto fail_dev;
+
+	for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels) {
+		err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq,
+					       transceivers[ch]);
+		if (err)
+			goto fail_channel;
+	}
+
 	/* Request IRQ that's common for both channels */
 	if (info->shared_global_irqs) {
 		err = devm_request_irq(dev, ch_irq,
@@ -2224,7 +2235,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		if (err) {
 			dev_err(dev, "devm_request_irq %d failed: %pe\n",
 				ch_irq, ERR_PTR(err));
-			goto fail_dev;
+			goto fail_channel;
 		}
 
 		err = devm_request_irq(dev, g_irq, rcar_canfd_global_interrupt,
@@ -2232,7 +2243,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		if (err) {
 			dev_err(dev, "devm_request_irq %d failed: %pe\n",
 				g_irq, ERR_PTR(err));
-			goto fail_dev;
+			goto fail_channel;
 		}
 	} else {
 		err = devm_request_irq(dev, g_recc_irq,
@@ -2242,7 +2253,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		if (err) {
 			dev_err(dev, "devm_request_irq %d failed: %pe\n",
 				g_recc_irq, ERR_PTR(err));
-			goto fail_dev;
+			goto fail_channel;
 		}
 
 		err = devm_request_irq(dev, g_err_irq,
@@ -2251,19 +2262,8 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		if (err) {
 			dev_err(dev, "devm_request_irq %d failed: %pe\n",
 				g_err_irq, ERR_PTR(err));
-			goto fail_dev;
-		}
-	}
-
-	err = rcar_canfd_global_init(gpriv);
-	if (err)
-		goto fail_dev;
-
-	for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels) {
-		err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq,
-					       transceivers[ch]);
-		if (err)
 			goto fail_channel;
+		}
 	}
 
 	platform_set_drvdata(pdev, gpriv);
-- 
2.43.0

Re: [PATCH 3/3] can: rcar_canfd: Fix interrupt registration order

[sashiko-bot]

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

New issues:
- [High] Premature UAPI exposure causes unhandled hardware interrupts (IRQ storm).

Pre-existing issues:
- [High] Use-After-Free in global interrupt handlers during driver removal and error paths.
- [High] NULL pointer dereference due to early channel IRQ registration.
--

commit a0e1233eee22a1c4cd2d28a4473c39b527927759
Author: Biju Das <biju.das.jz@bp.renesas.com>

can: rcar_canfd: Fix interrupt registration order

In rcar_canfd_probe(), global interrupts are requested via
devm_request_irq() before rcar_canfd_global_init() enables the clocks
and before the per-channel structures in gpriv->ch[] are allocated.

> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index 1f3dc4b354f87..48f624c1eda9a 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -2216,6 +2216,17 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  	gpriv->base = addr;
>  	gpriv->fcbase = addr + gpriv->info->regs->coffset;
>  
> +	err = rcar_canfd_global_init(gpriv);
> +	if (err)
> +		goto fail_dev;
> +
> +	for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels) {
> +		err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq,
> +					       transceivers[ch]);

[Severity: High]
Does moving rcar_canfd_channel_probe() before the global interrupt
registration expose the device to userspace prematurely?

rcar_canfd_channel_probe() calls register_candev(), which makes the network
device visible. If a userspace daemon immediately brings the interface up, it
can transition to operational mode and receive CAN frames, asserting the
global receive interrupt before devm_request_irq() has registered the global
handler.

Could this lead to an unhandled hardware interrupt storm that permanently
disables the CAN interrupt line?

[Severity: High]
This is a pre-existing issue, but does rcar_canfd_channel_probe() suffer
from the exact same early interrupt registration bug being fixed here?

Inside rcar_canfd_channel_probe(), when multi_channel_irqs is enabled,
devm_request_irq() is called for the channel error interrupt before the
channel pointer is stored:

drivers/net/can/rcar/rcar_canfd.c:rcar_canfd_channel_probe() {
    ...
    err = devm_request_irq(dev, err_irq, rcar_canfd_channel_err_interrupt,
                           ...);
    ...
    gpriv->ch[priv->channel] = priv;
}

If a pending interrupt fires immediately, the handler calls
rcar_canfd_handle_channel_err(), which dereferences gpriv->ch[ch].

Since gpriv->ch[ch] is still NULL, will this result in a kernel panic?

> +		if (err)
> +			goto fail_channel;
> +	}
> +
>  	/* Request IRQ that's common for both channels */
>  	if (info->shared_global_irqs) {
>  		err = devm_request_irq(dev, ch_irq,
> @@ -2224,7 +2235,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  		if (err) {
>  			dev_err(dev, "devm_request_irq %d failed: %pe\n",
>  				ch_irq, ERR_PTR(err));
> -			goto fail_dev;
> +			goto fail_channel;

[Severity: High]
This is a pre-existing issue, but does the fail_channel error path have a
use-after-free race condition?

When tearing down the channels, rcar_canfd_channel_remove() calls
free_candev() but does not clear the gpriv->ch[ch] pointer:

drivers/net/can/rcar/rcar_canfd.c:rcar_canfd_channel_remove() {
    ...
    free_candev(priv->ndev);
    // gpriv->ch[ch] is left dangling
}

Since global interrupts remain enabled until rcar_canfd_global_deinit() runs
after the channel loop, if a global receive interrupt fires for an active
channel, the handler rcar_canfd_global_receive_fifo_interrupt() will iterate
over all channels and dereference the dangling gpriv->ch[ch] pointer.

Can this cause a kernel crash if CAN traffic is present during the error
path or driver removal?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630135119.104774-1-biju.das.jz@bp.renesas.com?part=3