stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, syzbot <syzkaller@googlegroups.com>,
	Willem de Bruijn <willemb@google.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>
Subject: [PATCH 5.1 37/40] net: correct zerocopy refcnt with udp MSG_MORE
Date: Mon,  3 Jun 2019 11:09:30 +0200	[thread overview]
Message-ID: <20190603090524.739303756@linuxfoundation.org> (raw)
In-Reply-To: <20190603090522.617635820@linuxfoundation.org>

From: Willem de Bruijn <willemb@google.com>

[ Upstream commit 100f6d8e09905c59be45b6316f8f369c0be1b2d8 ]

TCP zerocopy takes a uarg reference for every skb, plus one for the
tcp_sendmsg_locked datapath temporarily, to avoid reaching refcnt zero
as it builds, sends and frees skbs inside its inner loop.

UDP and RAW zerocopy do not send inside the inner loop so do not need
the extra sock_zerocopy_get + sock_zerocopy_put pair. Commit
52900d22288ed ("udp: elide zerocopy operation in hot path") introduced
extra_uref to pass the initial reference taken in sock_zerocopy_alloc
to the first generated skb.

But, sock_zerocopy_realloc takes this extra reference at the start of
every call. With MSG_MORE, no new skb may be generated to attach the
extra_uref to, so refcnt is incorrectly 2 with only one skb.

Do not take the extra ref if uarg && !tcp, which implies MSG_MORE.
Update extra_uref accordingly.

This conditional assignment triggers a false positive may be used
uninitialized warning, so have to initialize extra_uref at define.

Changes v1->v2: fix typo in Fixes SHA1

Fixes: 52900d22288e7 ("udp: elide zerocopy operation in hot path")
Reported-by: syzbot <syzkaller@googlegroups.com>
Diagnosed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/core/skbuff.c     |    6 +++++-
 net/ipv4/ip_output.c  |    4 ++--
 net/ipv6/ip6_output.c |    4 ++--
 3 files changed, 9 insertions(+), 5 deletions(-)

--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1001,7 +1001,11 @@ struct ubuf_info *sock_zerocopy_realloc(
 			uarg->len++;
 			uarg->bytelen = bytelen;
 			atomic_set(&sk->sk_zckey, ++next);
-			sock_zerocopy_get(uarg);
+
+			/* no extra ref when appending to datagram (MSG_MORE) */
+			if (sk->sk_type == SOCK_STREAM)
+				sock_zerocopy_get(uarg);
+
 			return uarg;
 		}
 	}
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -883,7 +883,7 @@ static int __ip_append_data(struct sock
 	int csummode = CHECKSUM_NONE;
 	struct rtable *rt = (struct rtable *)cork->dst;
 	unsigned int wmem_alloc_delta = 0;
-	bool paged, extra_uref;
+	bool paged, extra_uref = false;
 	u32 tskey = 0;
 
 	skb = skb_peek_tail(queue);
@@ -923,7 +923,7 @@ static int __ip_append_data(struct sock
 		uarg = sock_zerocopy_realloc(sk, length, skb_zcopy(skb));
 		if (!uarg)
 			return -ENOBUFS;
-		extra_uref = true;
+		extra_uref = !skb;	/* only extra ref if !MSG_MORE */
 		if (rt->dst.dev->features & NETIF_F_SG &&
 		    csummode == CHECKSUM_PARTIAL) {
 			paged = true;
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1275,7 +1275,7 @@ static int __ip6_append_data(struct sock
 	int csummode = CHECKSUM_NONE;
 	unsigned int maxnonfragsize, headersize;
 	unsigned int wmem_alloc_delta = 0;
-	bool paged, extra_uref;
+	bool paged, extra_uref = false;
 
 	skb = skb_peek_tail(queue);
 	if (!skb) {
@@ -1344,7 +1344,7 @@ emsgsize:
 		uarg = sock_zerocopy_realloc(sk, length, skb_zcopy(skb));
 		if (!uarg)
 			return -ENOBUFS;
-		extra_uref = true;
+		extra_uref = !skb;	/* only extra ref if !MSG_MORE */
 		if (rt->dst.dev->features & NETIF_F_SG &&
 		    csummode == CHECKSUM_PARTIAL) {
 			paged = true;



  parent reply	other threads:[~2019-06-03  9:14 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-03  9:08 [PATCH 5.1 00/40] 5.1.7-stable review Greg Kroah-Hartman
2019-06-03  9:08 ` [PATCH 5.1 01/40] bonding/802.3ad: fix slave link initialization transition states Greg Kroah-Hartman
2019-06-03  9:08 ` [PATCH 5.1 02/40] cxgb4: offload VLAN flows regardless of VLAN ethtype Greg Kroah-Hartman
2019-06-03  9:08 ` [PATCH 5.1 03/40] ethtool: Check for vlan etype or vlan tci when parsing flow_rule Greg Kroah-Hartman
2019-06-03  9:08 ` [PATCH 5.1 04/40] inet: switch IP ID generator to siphash Greg Kroah-Hartman
2019-06-03  9:08 ` [PATCH 5.1 05/40] ipv4/igmp: fix another memory leak in igmpv3_del_delrec() Greg Kroah-Hartman
2019-06-03  9:08 ` [PATCH 5.1 06/40] ipv4/igmp: fix build error if !CONFIG_IP_MULTICAST Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 07/40] ipv6: Consider sk_bound_dev_if when binding a raw socket to an address Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 08/40] ipv6: Fix redirect with VRF Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 09/40] llc: fix skb leak in llc_build_and_send_ui_pkt() Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 10/40] mlxsw: spectrum_acl: Avoid warning after identical rules insertion Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 11/40] net: dsa: mv88e6xxx: fix handling of upper half of STATS_TYPE_PORT Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 12/40] net: fec: fix the clk mismatch in failed_reset path Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 13/40] net-gro: fix use-after-free read in napi_gro_frags() Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 14/40] net: mvneta: Fix err code path of probe Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 15/40] net: mvpp2: fix bad MVPP2_TXQ_SCHED_TOKEN_CNTR_REG queue value Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 16/40] net: phy: marvell10g: report if the PHY fails to boot firmware Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 17/40] net: sched: dont use tc_action->order during action dump Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 18/40] net: stmmac: fix reset gpio free missing Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 19/40] r8169: fix MAC address being lost in PCI D3 Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 20/40] usbnet: fix kernel crash after disconnect Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 21/40] net/mlx5: Avoid double free in fs init error unwinding path Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 22/40] tipc: Avoid copying bytes beyond the supplied data Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 23/40] net/mlx5: Allocate root ns memory using kzalloc to match kfree Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 24/40] net/mlx5e: Disable rxhash when CQE compress is enabled Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 26/40] net: stmmac: dma channel control register need to be init first Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 27/40] bnxt_en: Fix aggregation buffer leak under OOM condition Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 28/40] bnxt_en: Fix possible BUG() condition when calling pci_disable_msix() Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 29/40] bnxt_en: Reduce memory usage when running in kdump kernel Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 30/40] net/tls: fix lowat calculation if some data came from previous record Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 31/40] selftests/tls: test for lowat overshoot with multiple records Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 32/40] net/tls: fix no wakeup on partial reads Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 33/40] selftests/tls: add test for sleeping even though there is data Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 34/40] net/tls: fix state removal with feature flags off Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 35/40] net/tls: dont ignore netdev notifications if no TLS features Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 36/40] cxgb4: Revert "cxgb4: Remove SGE_HOST_PAGE_SIZE dependency on page size" Greg Kroah-Hartman
2019-06-03  9:09 ` Greg Kroah-Hartman [this message]
2019-06-03  9:09 ` [PATCH 5.1 38/40] crypto: vmx - ghash: do nosimd fallback manually Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 39/40] Revert "tipc: fix modprobe tipc failed after switch order of device registration" Greg Kroah-Hartman
2019-06-03  9:09 ` [PATCH 5.1 40/40] tipc: fix modprobe tipc failed after switch order of device registration Greg Kroah-Hartman
2019-06-03 14:49 ` [PATCH 5.1 00/40] 5.1.7-stable review kernelci.org bot
2019-06-03 18:28   ` Kevin Hilman
2019-06-04  5:49     ` Greg Kroah-Hartman
2019-06-03 17:17 ` Guenter Roeck
2019-06-04  5:51   ` Greg Kroah-Hartman
2019-06-03 18:34 ` Jon Hunter
2019-06-04  5:50   ` Greg Kroah-Hartman
2019-06-03 19:33 ` Naresh Kamboju
2019-06-04  5:51   ` Greg Kroah-Hartman
2019-06-03 23:31 ` shuah
2019-06-04  5:50   ` Greg Kroah-Hartman

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=20190603090524.739303756@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=syzkaller@googlegroups.com \
    --cc=willemb@google.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).