From patchwork Wed Sep 30 14:45:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vincent MAILHOL X-Patchwork-Id: 297411 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.0 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D1534C4727C for ; Wed, 30 Sep 2020 14:48:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A263920789 for ; Wed, 30 Sep 2020 14:48:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730653AbgI3Ost (ORCPT ); Wed, 30 Sep 2020 10:48:49 -0400 Received: from smtp09.smtpout.orange.fr ([80.12.242.131]:23166 "EHLO smtp.smtpout.orange.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730586AbgI3Ost (ORCPT ); Wed, 30 Sep 2020 10:48:49 -0400 Received: from tomoyo.flets-east.jp ([153.230.197.127]) by mwinf5d69 with ME id aEoZ2300H2lQRaH03EohP6; Wed, 30 Sep 2020 16:48:47 +0200 X-ME-Helo: tomoyo.flets-east.jp X-ME-Auth: bWFpbGhvbC52aW5jZW50QHdhbmFkb28uZnI= X-ME-Date: Wed, 30 Sep 2020 16:48:47 +0200 X-ME-IP: 153.230.197.127 From: Vincent Mailhol To: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, linux-can@vger.kernel.org, Wolfgang Grandegger , Marc Kleine-Budde , "David S . Miller" , Jakub Kicinski Cc: Vincent Mailhol , Oliver Neukum , Greg Kroah-Hartman , Arunachalam Santhanam , Masahiro Yamada , linux-usb@vger.kernel.org (open list:USB ACM DRIVER) Subject: [PATCH v2 1/6] can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ context Date: Wed, 30 Sep 2020 23:45:28 +0900 Message-Id: <20200930144602.10290-2-mailhol.vincent@wanadoo.fr> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200926175810.278529-1-mailhol.vincent@wanadoo.fr> References: <20200926175810.278529-1-mailhol.vincent@wanadoo.fr> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org 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 --- 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; }