dccp/tcp: Avoid negative sk_forward_alloc by ipv6_pinfo.pktoptions.

commit ca43ccf41224b023fc290073d5603a755fd12eed upstream.

Eric Dumazet pointed out [0] that when we call skb_set_owner_r()
for ipv6_pinfo.pktoptions, sk_rmem_schedule() has not been called,
resulting in a negative sk_forward_alloc.

We add a new helper which clones a skb and sets its owner only
when sk_rmem_schedule() succeeds.

Note that we move skb_set_owner_r() forward in (dccp|tcp)_v6_do_rcv()
because tcp_send_synack() can make sk_forward_alloc negative before
ipv6_opt_accepted() in the crossed SYN-ACK or self-connect() cases.

[0]: https://lore.kernel.org/netdev/CANn89iK9oc20Jdi_41jb9URdF210r7d1Y-+uypbMSbOfY6jqrg@mail.gmail.com/

Fixes: e4df55faa9e3 ("net: dccp: Add handling of IPV6_PKTOPTIONS to dccp_v6_do_rcv()")
Fixes: 1bbb399fda07 ("[DCCP]: Introduce DCCPv6")
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/net/sock.h b/include/net/sock.h
index 5fa255b1e0a6550b645256278e4bfdbdd97b6d4d..976433438c6f22df212195e758e3db0698db9d34 100644 (file)
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2167,6 +2167,19 @@ static inline __must_check bool skb_set_owner_sk_safe(struct sk_buff *skb, struc
        return false;
 }
 
+static inline struct sk_buff *skb_clone_and_charge_r(struct sk_buff *skb, struct sock *sk)
+{
+       skb = skb_clone(skb, sk_gfp_mask(sk, GFP_ATOMIC));
+       if (skb) {
+               if (sk_rmem_schedule(sk, skb, skb->truesize)) {
+                       skb_set_owner_r(skb, sk);
+                       return skb;
+               }
+               __kfree_skb(skb);
+       }
+       return NULL;
+}
+
 void sk_reset_timer(struct sock *sk, struct timer_list *timer,
                    unsigned long expires);
 

diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 1bf267d36a9c8f90ebe7a604ff008d96bf2cf1e3..c2844fb442c4fdeb124ce64a15262c6d19dd0549 100644 (file)
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -541,11 +541,9 @@ static struct sock *dccp_v6_request_recv_sock(const struct sock *sk,
        *own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash), NULL);
        /* Clone pktoptions received with SYN, if we own the req */
        if (*own_req && ireq->pktopts) {
-               newnp->pktoptions = skb_clone(ireq->pktopts, GFP_ATOMIC);
+               newnp->pktoptions = skb_clone_and_charge_r(ireq->pktopts, newsk);
                consume_skb(ireq->pktopts);
                ireq->pktopts = NULL;
-               if (newnp->pktoptions)
-                       skb_set_owner_r(newnp->pktoptions, newsk);
        }
 
        return newsk;
@@ -605,7 +603,7 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
                                               --ANK (980728)
         */
        if (np->rxopt.all)
-               opt_skb = skb_clone(skb, GFP_ATOMIC);
+               opt_skb = skb_clone_and_charge_r(skb, sk);
 
        if (sk->sk_state == DCCP_OPEN) { /* Fast path */
                if (dccp_rcv_established(sk, skb, dccp_hdr(skb), skb->len))
@@ -669,7 +667,6 @@ ipv6_pktoptions:
                        np->flow_label = ip6_flowlabel(ipv6_hdr(opt_skb));
                if (ipv6_opt_accepted(sk, opt_skb,
                                      &DCCP_SKB_CB(opt_skb)->header.h6)) {
-                       skb_set_owner_r(opt_skb, sk);
                        memmove(IP6CB(opt_skb),
                                &DCCP_SKB_CB(opt_skb)->header.h6,
                                sizeof(struct inet6_skb_parm));

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index e84b79357b2ffc221cfa03de4c179814f3db3b06..eefd76ca19145045adf461d01027878f171aa592 100644 (file)
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1318,14 +1318,11 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
 
                /* Clone pktoptions received with SYN, if we own the req */
                if (ireq->pktopts) {
-                       newnp->pktoptions = skb_clone(ireq->pktopts,
-                                                     sk_gfp_mask(sk, GFP_ATOMIC));
+                       newnp->pktoptions = skb_clone_and_charge_r(ireq->pktopts, newsk);
                        consume_skb(ireq->pktopts);
                        ireq->pktopts = NULL;
-                       if (newnp->pktoptions) {
+                       if (newnp->pktoptions)
                                tcp_v6_restore_cb(newnp->pktoptions);
-                               skb_set_owner_r(newnp->pktoptions, newsk);
-                       }
                }
        } else {
                if (!req_unhash && found_dup_sk) {
@@ -1393,7 +1390,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
                                               --ANK (980728)
         */
        if (np->rxopt.all)
-               opt_skb = skb_clone(skb, sk_gfp_mask(sk, GFP_ATOMIC));
+               opt_skb = skb_clone_and_charge_r(skb, sk);
 
        if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
                struct dst_entry *dst;
@@ -1475,7 +1472,6 @@ ipv6_pktoptions:
                if (np->repflow)
                        np->flow_label = ip6_flowlabel(ipv6_hdr(opt_skb));
                if (ipv6_opt_accepted(sk, opt_skb, &TCP_SKB_CB(opt_skb)->header.h6)) {
-                       skb_set_owner_r(opt_skb, sk);
                        tcp_v6_restore_cb(opt_skb);
                        opt_skb = xchg(&np->pktoptions, opt_skb);
                } else {