From: Ciprian Costea <ciprianmarian.costea@oss.nxp.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>,
Vincent Mailhol <mailhol@kernel.org>,
Nicolas Ferre <nicolas.ferre@microchip.com>,
Alexandre Belloni <alexandre.belloni@bootlin.com>,
Claudiu Beznea <claudiu.beznea@tuxon.dev>,
Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>
Cc: linux-can@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, imx@lists.linux.dev,
NXP S32 Linux Team <s32@nxp.com>,
Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
Subject: [PATCH v5 2/3] can: at91_can: fix rx-offload cleanup on unbind and probe errors
Date: Mon, 7 Sep 2026 12:49:41 +0200 [thread overview]
Message-ID: <20260907104942.17089-3-ciprianmarian.costea@oss.nxp.com> (raw)
In-Reply-To: <20260907104942.17089-1-ciprianmarian.costea@oss.nxp.com>
From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
Making can_rx_offload's skb_irq_queue per-CPU (previous patch) adds an
alloc_percpu() to can_rx_offload_add_*(). That allocation has to be freed
on teardown and can fail with -ENOMEM, which exposes three problems in
at91_can:
- at91_can_remove() does not call can_rx_offload_del(), so the NAPI
instance and the per-CPU queues are leaked on unbind and module removal.
- The probe error path after a failed register_candev() jumps straight to
free_candev() without can_rx_offload_del() and leaks the same objects.
- The return value of can_rx_offload_add_timestamp() is ignored. It can now
return -ENOMEM with offload->skb_irq_queue == NULL, probe still succeeds,
and the first RX interrupt dereferences that NULL pointer in
can_rx_offload_irq_offload_timestamp() via get_cpu_ptr().
Check the return value and call can_rx_offload_del() from at91_can_remove()
and from a new error label taken when register_candev() fails.
Fixes: 137f59d5dab4 ("can: at91_can: switch to rx-offload implementation")
Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
---
drivers/net/can/at91_can.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index 58da323f14d7..3f6c5bb373d3 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -1123,7 +1123,9 @@ static int at91_can_probe(struct platform_device *pdev)
priv->offload.mb_first = devtype_data->rx_first;
priv->offload.mb_last = devtype_data->rx_last;
- can_rx_offload_add_timestamp(dev, &priv->offload);
+ err = can_rx_offload_add_timestamp(dev, &priv->offload);
+ if (err)
+ goto exit_free;
if (transceiver)
priv->can.bitrate_max = transceiver->attrs.max_link_rate;
@@ -1137,7 +1139,7 @@ static int at91_can_probe(struct platform_device *pdev)
err = register_candev(dev);
if (err) {
dev_err(&pdev->dev, "registering netdev failed\n");
- goto exit_free;
+ goto exit_offload;
}
dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
@@ -1145,6 +1147,8 @@ static int at91_can_probe(struct platform_device *pdev)
return 0;
+ exit_offload:
+ can_rx_offload_del(&priv->offload);
exit_free:
free_candev(dev);
exit_iounmap:
@@ -1165,6 +1169,8 @@ static void at91_can_remove(struct platform_device *pdev)
unregister_netdev(dev);
+ can_rx_offload_del(&priv->offload);
+
iounmap(priv->reg_base);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
--
2.43.0
next prev parent reply other threads:[~2026-09-07 10:50 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-07 10:49 [PATCH v5 0/3] can_rx_offload keeps a lockless irq_queue that the IRQ handlers fill and that is later spliced under skb_queue.lock into the NAPI-facing skb_queue. This works as long as a single context fills the irq_queue. flexcan with FLEXCAN_QUIRK_SECONDARY_MB_IRQ and mcf5441x use two mailbox IRQ lines. When those are affined to different CPUs the two handlers can enqueue into the same list at the same time and corrupt it Ciprian Costea
2026-09-07 10:49 ` [PATCH v5 1/3] can: rx-offload: make skb_irq_queue per-CPU Ciprian Costea
2026-09-07 10:49 ` Ciprian Costea [this message]
2026-09-07 10:49 ` [PATCH v5 3/3] can: gs_usb: check can_rx_offload_add_manual() return value Ciprian Costea
2026-09-07 13:49 ` [PATCH v5 0/3] can_rx_offload keeps a lockless irq_queue that the IRQ handlers fill and that is later spliced under skb_queue.lock into the NAPI-facing skb_queue. This works as long as a single context fills the irq_queue. flexcan with FLEXCAN_QUIRK_SECONDARY_MB_IRQ and mcf5441x use two mailbox IRQ lines. When those are affined to different CPUs the two handlers can enqueue into the same list at the same time and corrupt it Marc Kleine-Budde
2026-09-07 15:07 ` Ciprian Marian Costea
2026-09-08 8:58 ` Marc Kleine-Budde
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=20260907104942.17089-3-ciprianmarian.costea@oss.nxp.com \
--to=ciprianmarian.costea@oss.nxp.com \
--cc=alexandre.belloni@bootlin.com \
--cc=claudiu.beznea@tuxon.dev \
--cc=dev.kurt@vandijck-laurijssen.be \
--cc=imx@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mailhol@kernel.org \
--cc=mkl@pengutronix.de \
--cc=nicolas.ferre@microchip.com \
--cc=s32@nxp.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox