From patchwork Sat Sep 26 17:41:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vincent MAILHOL X-Patchwork-Id: 260124 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=-12.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham 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 E7FDDC2D0A8 for ; Sat, 26 Sep 2020 17:42:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A100322B2D for ; Sat, 26 Sep 2020 17:42:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729260AbgIZRmA (ORCPT ); Sat, 26 Sep 2020 13:42:00 -0400 Received: from smtp11.smtpout.orange.fr ([80.12.242.133]:34514 "EHLO smtp.smtpout.orange.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725208AbgIZRmA (ORCPT ); Sat, 26 Sep 2020 13:42:00 -0400 Received: from tomoyo.flets-east.jp ([153.230.197.127]) by mwinf5d89 with ME id Yhhl230032lQRaH03hhvkg; Sat, 26 Sep 2020 19:41:58 +0200 X-ME-Helo: tomoyo.flets-east.jp X-ME-Auth: bWFpbGhvbC52aW5jZW50QHdhbmFkb28uZnI= X-ME-Date: Sat, 26 Sep 2020 19:41:58 +0200 X-ME-IP: 153.230.197.127 From: Vincent Mailhol To: linux-can@vger.kernel.org, Wolfgang Grandegger , Marc Kleine-Budde , "David S . Miller" Cc: Vincent Mailhol , Jakub Kicinski , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/6] can: dev: can_get_echo_skb(): prevent call to kfree_skb() in hard IRQ context Date: Sun, 27 Sep 2020 02:41:22 +0900 Message-Id: <20200926174135.278009-1-mailhol.vincent@wanadoo.fr> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200926172953.277426-1-mailhol.vincent@wanadoo.fr> References: <20200926172953.277426-1-mailhol.vincent@wanadoo.fr> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@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 is exactly what is described in https://patchwork.ozlabs.org/patch/835236/ While above link proposes a patch that 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). 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; } From patchwork Sat Sep 26 17:45:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vincent MAILHOL X-Patchwork-Id: 260122 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=-12.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham 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 785AEC2D0A8 for ; Sat, 26 Sep 2020 17:46:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4910820878 for ; Sat, 26 Sep 2020 17:46:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730033AbgIZRqW (ORCPT ); Sat, 26 Sep 2020 13:46:22 -0400 Received: from smtp11.smtpout.orange.fr ([80.12.242.133]:45913 "EHLO smtp.smtpout.orange.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729997AbgIZRqU (ORCPT ); Sat, 26 Sep 2020 13:46:20 -0400 Received: from tomoyo.flets-east.jp ([153.230.197.127]) by mwinf5d89 with ME id Yhlm230082lQRaH03hmF5X; Sat, 26 Sep 2020 19:46:18 +0200 X-ME-Helo: tomoyo.flets-east.jp X-ME-Auth: bWFpbGhvbC52aW5jZW50QHdhbmFkb28uZnI= X-ME-Date: Sat, 26 Sep 2020 19:46:18 +0200 X-ME-IP: 153.230.197.127 From: Vincent Mailhol To: linux-can@vger.kernel.org, Wolfgang Grandegger , Marc Kleine-Budde , "David S . Miller" Cc: Vincent Mailhol , Jakub Kicinski , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 3/6] can: dev: __can_get_echo_skb(): fix the return length Date: Sun, 27 Sep 2020 02:45:33 +0900 Message-Id: <20200926174542.278166-4-mailhol.vincent@wanadoo.fr> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200926174542.278166-1-mailhol.vincent@wanadoo.fr> References: <20200926174542.278166-1-mailhol.vincent@wanadoo.fr> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The length of Remote Transmission Request (RTR) frames is always 0 bytes. The DLC represents the requested length, not the actual length of the RTR. But __can_get_echo_skb() returns the DLC value regardless. Apply get_can_len() function to retrieve the correct length. Signed-off-by: Vincent Mailhol --- drivers/net/can/dev.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c index e291fda395a0..8c3e11820e03 100644 --- a/drivers/net/can/dev.c +++ b/drivers/net/can/dev.c @@ -481,14 +481,9 @@ __can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr) } if (priv->echo_skb[idx]) { - /* Using "struct canfd_frame::len" for the frame - * length is supported on both CAN and CANFD frames. - */ struct sk_buff *skb = priv->echo_skb[idx]; - struct canfd_frame *cf = (struct canfd_frame *)skb->data; - u8 len = cf->len; - *len_ptr = len; + *len_ptr = get_can_len(skb); priv->echo_skb[idx] = NULL; return skb;