All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: Biju <biju.das.au@gmail.com>
Cc: "Vincent Mailhol" <mailhol@kernel.org>,
	"Marc Kleine-Budde" <mkl@pengutronix.de>,
	wsa+renesas@sang-engineering.com,
	"Oleksij Rempel" <o.rempel@pengutronix.de>,
	"Oliver Hartkopp" <socketcan@hartkopp.net>,
	linux-can@vger.kernel.org
Subject: Re: [PATCH 1/3] can: rcar_canfd: Fix dangling pointer on channel probe failure
Date: Tue, 30 Jun 2026 13:59:35 +0000	[thread overview]
Message-ID: <20260630135936.558A51F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260630135119.104774-2-biju.das.jz@bp.renesas.com>

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

  reply	other threads:[~2026-06-30 13:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-30 13:51 [PATCH 0/3] Renesas CANFD Fixes Biju
2026-06-30 13:51 ` [PATCH 1/3] can: rcar_canfd: Fix dangling pointer on channel probe failure Biju
2026-06-30 13:59   ` sashiko-bot [this message]
2026-06-30 13:51 ` [PATCH 2/3] can: rcar_canfd: Fix double-cleanup and unclocked register access on init failure Biju
2026-06-30 14:06   ` sashiko-bot
2026-06-30 13:51 ` [PATCH 3/3] can: rcar_canfd: Fix interrupt registration order Biju
2026-06-30 14:05   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260630135936.558A51F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=biju.das.au@gmail.com \
    --cc=linux-can@vger.kernel.org \
    --cc=mailhol@kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=o.rempel@pengutronix.de \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=socketcan@hartkopp.net \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.