Skip to content

Commit 100f6d8

Browse files
wdebruijdavem330
authored andcommitted
net: correct zerocopy refcnt with udp MSG_MORE
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: 52900d2 ("udp: elide zerocopy operation in hot path") Reported-by: syzbot <[email protected]> Diagnosed-by: Eric Dumazet <[email protected]> Signed-off-by: Willem de Bruijn <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent b73484b commit 100f6d8

File tree

3 files changed

+9
-5
lines changed

3 files changed

+9
-5
lines changed

net/core/skbuff.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1036,7 +1036,11 @@ struct ubuf_info *sock_zerocopy_realloc(struct sock *sk, size_t size,
10361036
uarg->len++;
10371037
uarg->bytelen = bytelen;
10381038
atomic_set(&sk->sk_zckey, ++next);
1039-
sock_zerocopy_get(uarg);
1039+
1040+
/* no extra ref when appending to datagram (MSG_MORE) */
1041+
if (sk->sk_type == SOCK_STREAM)
1042+
sock_zerocopy_get(uarg);
1043+
10401044
return uarg;
10411045
}
10421046
}

net/ipv4/ip_output.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -878,7 +878,7 @@ static int __ip_append_data(struct sock *sk,
878878
int csummode = CHECKSUM_NONE;
879879
struct rtable *rt = (struct rtable *)cork->dst;
880880
unsigned int wmem_alloc_delta = 0;
881-
bool paged, extra_uref;
881+
bool paged, extra_uref = false;
882882
u32 tskey = 0;
883883

884884
skb = skb_peek_tail(queue);
@@ -918,7 +918,7 @@ static int __ip_append_data(struct sock *sk,
918918
uarg = sock_zerocopy_realloc(sk, length, skb_zcopy(skb));
919919
if (!uarg)
920920
return -ENOBUFS;
921-
extra_uref = true;
921+
extra_uref = !skb; /* only extra ref if !MSG_MORE */
922922
if (rt->dst.dev->features & NETIF_F_SG &&
923923
csummode == CHECKSUM_PARTIAL) {
924924
paged = true;

net/ipv6/ip6_output.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1275,7 +1275,7 @@ static int __ip6_append_data(struct sock *sk,
12751275
int csummode = CHECKSUM_NONE;
12761276
unsigned int maxnonfragsize, headersize;
12771277
unsigned int wmem_alloc_delta = 0;
1278-
bool paged, extra_uref;
1278+
bool paged, extra_uref = false;
12791279

12801280
skb = skb_peek_tail(queue);
12811281
if (!skb) {
@@ -1344,7 +1344,7 @@ static int __ip6_append_data(struct sock *sk,
13441344
uarg = sock_zerocopy_realloc(sk, length, skb_zcopy(skb));
13451345
if (!uarg)
13461346
return -ENOBUFS;
1347-
extra_uref = true;
1347+
extra_uref = !skb; /* only extra ref if !MSG_MORE */
13481348
if (rt->dst.dev->features & NETIF_F_SG &&
13491349
csummode == CHECKSUM_PARTIAL) {
13501350
paged = true;

0 commit comments

Comments
 (0)