Linux-Can Archive on lore.kernel.org
 help / color / Atom feed
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
To: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	linux-can@vger.kernel.org,
	Wolfgang Grandegger <wg@grandegger.com>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>
Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>,
	Oliver Neukum <oneukum@suse.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Arunachalam Santhanam <arunachalam.santhanam@in.bosch.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	linux-usb@vger.kernel.org (open list:USB ACM DRIVER)
Subject: [PATCH v3 1/7] can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ context
Date: Sat,  3 Oct 2020 00:41:45 +0900
Message-ID: <20201002154219.4887-2-mailhol.vincent@wanadoo.fr> (raw)
In-Reply-To: <20201002154219.4887-1-mailhol.vincent@wanadoo.fr>

If a driver calls can_get_echo_skb() during a hardware IRQ (which is
often, but not always, the case), the 'WARN_ON(in_irq)' in
net/core/skbuff.c#skb_release_head_state() might be triggered, under
network congestion circumstances, together with the potential risk of
a NULL pointer dereference.

The root cause of this issue is the call to kfree_skb() instead of
dev_kfree_skb_irq() in net/core/dev.c#enqueue_to_backlog().

This patch prevents the skb to be freed within the call to netif_rx()
by incrementing its reference count with skb_get(). The skb is finally
freed by one of the in-irq-context safe functions:
dev_consume_skb_any() or dev_kfree_skb_any().  The "any" version is
used because some drivers might call can_get_echo_skb() in a normal
context.

The reason for this issue to occur is that initially, in the core
network stack, loopback skb were not supposed to be received in
hardware IRQ context. The CAN stack is an exeption.

This bug was previously reported back in 2017 in [1] but the proposed
patch never got accepted.

While [1] directly modifies net/core/dev.c, we try to propose here a
smoother modification local to CAN network stack (the assumption
behind is that only CAN devices are affected by this issue).

[1] https://patchwork.ozlabs.org/patch/835236/

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---

Changes in v3: None

Changes in v2:
 - Minor changes of link format in the changelog.
---
 drivers/net/can/dev.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 68834a2853c9..e291fda395a0 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -512,7 +512,11 @@ unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx)
 	if (!skb)
 		return 0;
 
-	netif_rx(skb);
+	skb_get(skb);
+	if (netif_rx(skb) == NET_RX_SUCCESS)
+		dev_consume_skb_any(skb);
+	else
+		dev_kfree_skb_any(skb);
 
 	return len;
 }
-- 
2.26.2


  reply index

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-26 17:57 [PATCH 0/6] can: add support for ETAS ES58X CAN USB Vincent Mailhol
2020-09-26 17:57 ` [PATCH 1/6] can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ context Vincent Mailhol
2020-09-26 17:57 ` [PATCH 2/6] can: dev: add a helper function to get the correct length of Classical frames Vincent Mailhol
2020-09-26 17:57 ` [PATCH 3/6] can: dev: __can_get_echo_skb(): fix the return length Vincent Mailhol
2020-09-26 17:57 ` [PATCH 4/6] can: dev: add a helper function to calculate the duration of one bit Vincent Mailhol
2020-09-26 17:57 ` [PATCH 6/6] USB: cdc-acm: blacklist ETAS ES58X device Vincent Mailhol
2020-09-27  5:45   ` Greg Kroah-Hartman
2020-09-27  5:52     ` Greg Kroah-Hartman
2020-09-29  2:15       ` Vincent Mailhol
2020-09-30 14:45 ` [PATCH v2 0/6] can: add support for ETAS ES58X CAN USB Vincent Mailhol
2020-09-30 14:45 ` [PATCH v2 1/6] can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ context Vincent Mailhol
2020-09-30 14:45 ` [PATCH v2 2/6] can: dev: add a helper function to get the correct length of Classical frames Vincent Mailhol
2020-09-30 15:21   ` Marc Kleine-Budde
2020-10-01 15:45     ` [PATCH v2 2/6] can: dev: add a helper function Vincent Mailhol
2020-10-01 15:51       ` Marc Kleine-Budde
2020-09-30 14:45 ` [PATCH v2 3/6] can: dev: __can_get_echo_skb(): fix the return length Vincent Mailhol
2020-09-30 14:45 ` [PATCH v2 4/6] can: dev: add a helper function to calculate the duration of one bit Vincent Mailhol
2020-09-30 14:45 ` [PATCH v2 6/6] usb: cdc-acm: add quirk to blacklist ETAS ES58X devices Vincent Mailhol
     [not found] ` <20200930144602.10290-6-mailhol.vincent@wanadoo.fr>
2020-09-30 16:18   ` [PATCH v2 5/6] can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces Greg Kroah-Hartman
2020-10-01 15:56     ` Vincent Mailhol
2020-10-02 15:41 ` [PATCH v3 0/7] can: add support for ETAS ES58X CAN USB Vincent Mailhol
2020-10-02 15:41   ` Vincent Mailhol [this message]
2020-10-02 15:41   ` [PATCH v3 2/7] can: dev: fix type of get_can_dlc() and get_canfd_dlc() macros Vincent Mailhol
2020-10-02 15:41   ` [PATCH v3 3/7] can: dev: add a helper function to get the correct length of Classical frames Vincent Mailhol
2020-10-02 15:41   ` [PATCH v3 4/7] can: dev: __can_get_echo_skb(): fix the return length Vincent Mailhol
2020-10-02 15:41   ` [PATCH v3 5/7] can: dev: add a helper function to calculate the duration of one bit Vincent Mailhol
2020-10-04 11:06     ` Marc Kleine-Budde
2020-10-04 11:10       ` Marc Kleine-Budde
2020-10-02 15:41   ` [PATCH v3 7/7] usb: cdc-acm: add quirk to blacklist ETAS ES58X devices Vincent Mailhol
2020-10-05 11:14     ` Greg Kroah-Hartman
     [not found]   ` <20201002154219.4887-7-mailhol.vincent@wanadoo.fr>
2020-10-04 12:06     ` [PATCH v3 6/7] can: usb: etas_es58X: add support for ETAS ES58X CAN USB interfaces Marc Kleine-Budde
2020-10-10  8:12       ` Vincent Mailhol

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=20201002154219.4887-2-mailhol.vincent@wanadoo.fr \
    --to=mailhol.vincent@wanadoo.fr \
    --cc=arunachalam.santhanam@in.bosch.com \
    --cc=davem@davemloft.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=kuba@kernel.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=oneukum@suse.com \
    --cc=wg@grandegger.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

Linux-Can Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-can/0 linux-can/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-can linux-can/ https://lore.kernel.org/linux-can \
		linux-can@vger.kernel.org
	public-inbox-index linux-can

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-can


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git