From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 705BACA6016 for ; Thu, 8 Oct 2026 21:31:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:MIME-Version: Content-Transfer-Encoding:Content-Type:References:In-Reply-To:Message-ID:Date :Cc:To:From:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=elwTlGsGIEAUxLqe0dTIpTBpzp3+HyabLtiZ0xgNSAg=; b=MKeMDWsznILWTv/aoPp/E2W8WT xA3H4zDSIsIZpMd/uOmqSwa3DxC8siJ0u6XOC8q4QhCiW5qZ3hcUNcRpMgq+RVY1s5N5A0TpXFqfO 4k7sUQa+tHdFkT4dJ3u+8c212w7gpB4+AVDcZLjCAJRdXALQvfSGhv1mp2Two0oRW0QM4KMJiCEgd mhMMRg5w1USo/izmfg2Jsx/lfIRMkpCqJPxtz5QrN3hnrTow69SLbxeRVwK0BvokBAognrMBVju1n VxUiru0iMnf33ca7aOGwKI1od+82pTb8f+5hbBcwpR2NQ7jlMQJo3GubXpYv+g7xcDdOoYVjTE0T0 E4eeCybg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1xEviB-000000054x3-14xo; Thu, 08 Oct 2026 21:31:35 +0000 Received: from tor.source.kernel.org ([172.105.4.254]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1xEviA-000000054wo-0fF6 for linux-arm-kernel@lists.infradead.org; Thu, 08 Oct 2026 21:31:34 +0000 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by tor.source.kernel.org (Postfix) with ESMTP id BD854600CB; Thu, 8 Oct 2026 21:31:32 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 56FA01F000FF; Thu, 8 Oct 2026 21:31:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1791495092; bh=elwTlGsGIEAUxLqe0dTIpTBpzp3+HyabLtiZ0xgNSAg=; h=Subject:From:To:Cc:Date:In-Reply-To:References; b=BCp/0fQOtFuIOcrAJXCoouPA6rc3xayTuyzfq1YBFqHD317EGLlW/ARoHuzMcs42n EQY3WQ0u4xWZ3C6UD5aqFkCJ0HaGwV44/C/oklkOtDNcjU9yh4/BTDJBM9BEaCvZeg ngJsqFljQleUPOJy0FMftIZ+3SIh9OknPIQ/gaLWLzvlS2JCTV4tNcCAR8Q752Afk7 J2SNCXnPKVG/8FdEoFG1dOX/v0j1v4aixws9hhrIWpX/GHI751744jsCm/TR/X2P0p MLI3TVnuBTpHvqG6xBja+B+fbd0PaTYZ0zfQZeaIYTfY3vWc4LZ4KIvP7rmW8KW/iV BpGuFx8MlU1iQ== Subject: Re: [PATCH net-next 1/3] net: selftests: use skb_share_check() in the loopback receive hook From: netdev-bot+sashiko@kernel.org To: nb@tipi-net.de Cc: davem@davemloft.net, edumazet@kernel.org, kuba@kernel.org, pabeni@redhat.com, horms@kernel.org, o.rempel@pengutronix.de, maxime.chevallier@bootlin.com, andrew+netdev@lunn.ch, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, Jose.Abreu@synopsys.com, saeedm@nvidia.com, tariqt@nvidia.com, mbloch@nvidia.com, leon@kernel.org, dtatulea@nvidia.com, alazar@nvidia.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-rdma@vger.kernel.org Date: Thu, 08 Oct 2026 21:31:30 +0000 Message-ID: <179149509090.434549.9918598206439291164@kernel.org> In-Reply-To: <20261007-nb-net-selftest-shared-net-v1-1-458369bfa1ba@tipi-net.de> References: <20261007-nb-net-selftest-shared-net-v1-1-458369bfa1ba@tipi-net.de> X-sashiko-severity: High Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit MIME-Version: 1.0 X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider. Critical: 0 · High: 1 · Medium: 0 · Low: 0 (1) If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [High] The series fixes only part of this bug class. Pre-existing issues: - [Low] Pre-existing weak length validation in net_test_loopback_validate() (net/core/selftests.c). -- net: selftests: use skb_share_check() in the loopback receive hook This patch replaces skb_unshare() with skb_share_check() in net_test_loopback_validate(). With the change, a shared but non-cloned skb is cloned before skb_linearize() runs. Without it, a non-linear frame that reaches the hook while another ETH_P_IP handler is bound to the device can hit BUG_ON(skb_shared()). > diff --git a/net/core/selftests.c b/net/core/selftests.c > index 36b949ae520b3..80029a67b2d2b 100644 > --- a/net/core/selftests.c > +++ b/net/core/selftests.c > @@ -170,7 +170,7 @@ static int net_test_loopback_validate(struct sk_buff *skb, > struct tcphdr *thdr; > struct iphdr *ihdr; > > - skb = skb_unshare(skb, GFP_ATOMIC); > + skb = skb_share_check(skb, GFP_ATOMIC); > if (!skb) > goto out; > [Severity: High] This change looks correct for net_test_loopback_validate(). The same pattern still seems to be present in stmmac_test_vlan_validate() in drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c, though. A later commit in this series, "net: stmmac: selftests: use skb_share_check() in the loopback receive hook", converts stmmac_test_loopback_validate(). The second copy of this handler in the same file is left as: stmmac_test_vlan_validate() { ... skb = skb_unshare(skb, GFP_ATOMIC); if (!skb) goto out; if (skb_linearize(skb)) goto out; ... } __stmmac_test_vlanfilt() and the other VLAN tests register this handler as a device-bound packet_type. The type is ETH_P_IP, ETH_P_8021Q, or either one depending on svlan: tpriv->pt.type = htons(ETH_P_IP); tpriv->pt.func = stmmac_test_vlan_validate; tpriv->pt.dev = priv->dev; Suppose another handler for the same type sits later in the device's ptype list, for example an AF_PACKET socket bound to ETH_P_IP on that interface. deliver_skb() then increments skb->users, so the hook gets an skb that is shared but not cloned. skb_unshare() only checks skb_cloned(), so it returns that skb unchanged. For a non-linear frame with less tailroom than data_len, this path follows: skb_linearize() __pskb_pull_tail() pskb_expand_head() BUG_ON(skb_shared(skb)); Should stmmac_test_vlan_validate() get the same skb_share_check() conversion, so the series covers every copy of this hook? The xgbe sibling, xgbe_test_loopback_validate(), does not seem to be affected. It already calls skb_share_check() before it expands a shared skb. [Severity: Low] This is a pre-existing issue that this patch did not introduce. The length check further down in net_test_loopback_validate() looks too short for what the function parses afterwards: if (skb_headlen(skb) < (NET_TEST_PKT_SIZE - ETH_HLEN)) goto out; NET_TEST_PKT_SIZE in include/net/selftests.h covers only ethhdr, iphdr and netsfhdr. That guarantees 33 bytes past the network header and does not include the TCP/UDP header. The function then reads: thdr = (struct tcphdr *)((u8 *)ihdr + 4 * ihdr->ihl); ... if (shdr->magic != cpu_to_be64(NET_TEST_PKT_MAGIC)) It never checks ihl >= 5, the IP version, tot_len or the L4 header length, and double_vlan adds another 4 bytes. Could an IPv4 frame that arrives during ethtool -t make the hook read up to about 64 bytes past skb->tail? Examples would be a minimum-size TCP segment or a frame with ihl=15. The over-read seems to stay inside the skb head allocation, because skb_shared_info follows skb->end. If so, the only effect is a possibly wrong selftest result. -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007-nb-net-selftest-shared-net-v1-0-458369bfa1ba%40tipi-net.de