From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C6B213D0914; Mon, 13 Jul 2026 09:19:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783934392; cv=none; b=fqiTi3cizPmTlkIh3lcBWqx4CyVANpTXHklhOuW997m/OnUPvohrhbGH7agazeJGVNUmcjslP5ssyXv1Xi7riYMR50rjkMRtzRqvkT1+f9QxqqgR9SsI8GsLaMm1NQ+USjc32ti7BtZrRzopltoRDoDo0vVgOhuhWLjj4PxsY0Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783934392; c=relaxed/simple; bh=ouTRcIFjz+QYZNuSX6uuuPwKcabd7zspmQIFbqeA7Rs=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=X1yZxMscnvF0osZtzcLS+S3IkXij4AttVe/MAeXMTB0Sto5mQwggf51tpbxOTWlU+CKpfPr1bSEosIpE0NucdQ6ssuSYkrZIWW3WjsPQXZiFQifU/mWD4J7jkTQw9GD53OKJXOY7xb21W5CqgVgPnvQGJ1JRPKgjGjURR0rAj5M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=L1aSuhNZ; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="L1aSuhNZ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 047A01F00A3A; Mon, 13 Jul 2026 09:19:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1783934391; bh=b80d1FGTgriFwEBTyhkzxIjSntfSE9mLoTvcQaX991I=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=L1aSuhNZA9IZ78B8MXNOSqdtXdo50nqUqs8DtmOUNUlI7H4S+6tBA265drxJPFssa fvB1LZqx+LmHoIAtVC1byWB+r6D3nOSnjKseQ/fITyP/vMIZ3XptJHHdj1XX5mmD7+ y7vmuTBHVYjXBW6HLjwgDMH5fndi6u/evZq4DfhohwuvWwK4bXPGQ7BSFBrl3EPIUL GKdraD47xhl0IanmUr7NVcJlWNo+XaHIi5hDP1DsSXoi4Q1bu6KDWSNk7Kwo8HGH+S i2NSZq9TOPUXPs8/NGEz2KsM5dV07zlhPRjCp7uJw7Gg1WFqeoM1T43m80T3DocX2n UDHajYThuYSng== From: sashiko-bot@kernel.org Subject: Re: [PATCH v6 5/6] can: flexcan: add FLEXCAN_QUIRK_IRQ_BERR quirk Reply-To: sashiko-reviews@lists.linux.dev To: "Ciprian Costea" Cc: "Oleksij Rempel" , robh@kernel.org, imx@lists.linux.dev, "Marc Kleine-Budde" , "Vincent Mailhol" , "Oliver Hartkopp" , linux-can@vger.kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org, Frank.Li@kernel.org In-Reply-To: <20260713085306.2643794-6-ciprianmarian.costea@oss.nxp.com> References: <20260713085306.2643794-1-ciprianmarian.costea@oss.nxp.com> <20260713085306.2643794-6-ciprianmarian.costea@oss.nxp.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Mon, 13 Jul 2026 09:19:50 +0000 Message-Id: <20260713091951.047A01F00A3A@smtp.kernel.org> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: Pre-existing issues: - [High] Concurrent execution of separate hardware interrupt handlers (`dev= ->irq` and `priv->irq_err`) modifies the lockless `can_rx_offload` IRQ queu= e (`skb_irq_queue`), leading to list corruption. -- --- Patch [5]: [PATCH v6 5/6] can: flexcan: add FLEXCAN_QUIRK_IRQ_BERR quir= k --- commit 97d482f1edd202225930276cf268822cd49ea3d2 Author: Ciprian Marian Costea can: flexcan: add FLEXCAN_QUIRK_IRQ_BERR quirk Introduce FLEXCAN_QUIRK_IRQ_BERR quirk to handle hardware integration where the FlexCAN module has a dedicated interrupt line for signaling bus errors and device state changes. This adds the flexcan_irq_esr() handler and is required for NXP S32N79 SoC support. > diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/fle= xcan/flexcan-core.c > index 8b70952f7f05d..e7e3fb41f55e7 100644 > --- a/drivers/net/can/flexcan/flexcan-core.c > +++ b/drivers/net/can/flexcan/flexcan-core.c > @@ -1300,6 +1300,22 @@ static irqreturn_t flexcan_irq_boff(int irq, void = *dev_id) > return handled; > } > =20 > +/* Combined bus error and state change IRQ handler */ > +static irqreturn_t flexcan_irq_esr(int irq, void *dev_id) > +{ > + struct net_device *dev =3D dev_id; > + struct flexcan_priv *priv =3D netdev_priv(dev); > + irqreturn_t handled; > + > + handled =3D flexcan_do_state(dev); > + handled |=3D flexcan_do_berr(dev); > + > + if (handled) > + can_rx_offload_irq_finish(&priv->offload); > + > + return handled; > +} [Severity: High] This is a pre-existing issue, but could this lead to list corruption if these separate IRQs run concurrently on different CPUs? Since flexcan_irq_esr() and flexcan_irq_mb() are registered as separate interrupts, they might fire at the same time. If flexcan_irq_esr() processes a bus error and flexcan_irq_mb() processes a mailbox interrupt, they both end up modifying the lockless offload->skb_irq_queue. CPU1 handling dev->irq (flexcan_irq_mb): flexcan_irq_mb() can_rx_offload_irq_offload_timestamp() __skb_queue_add_sort(&offload->skb_irq_queue, ...) CPU2 handling priv->irq_err (flexcan_irq_esr): flexcan_irq_esr() flexcan_do_berr() flexcan_irq_bus_err() can_rx_offload_queue_timestamp() __skb_queue_add_sort(&offload->skb_irq_queue, ...) Furthermore, both handlers unconditionally call can_rx_offload_irq_finish(), which locklessly splices the skb_irq_queue list: can_rx_offload_irq_finish() skb_queue_splice_tail_init(&offload->skb_irq_queue, ...) Are there existing synchronization mechanisms preventing these IRQ handlers from running simultaneously and corrupting the lockless skb_irq_queue list pointers? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260713085306.2643= 794-1-ciprianmarian.costea@oss.nxp.com?part=3D5