Commit ca43ccf4 authored by Kuniyuki Iwashima's avatar Kuniyuki Iwashima Committed by Jakub Kicinski

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

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: 323fbd0e ("net: dccp: Add handling of IPV6_PKTOPTIONS to dccp_v6_do_rcv()")
Fixes: 3df80d93 ("[DCCP]: Introduce DCCPv6")
Fixes: 1da177e4 ("Linux-2.6.12-rc2")
Signed-off-by: default avatarKuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 7fa0b526
...@@ -2434,6 +2434,19 @@ static inline __must_check bool skb_set_owner_sk_safe(struct sk_buff *skb, struc ...@@ -2434,6 +2434,19 @@ static inline __must_check bool skb_set_owner_sk_safe(struct sk_buff *skb, struc
return false; 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;
}
static inline void skb_prepare_for_gro(struct sk_buff *skb) static inline void skb_prepare_for_gro(struct sk_buff *skb)
{ {
if (skb->destructor != sock_wfree) { if (skb->destructor != sock_wfree) {
......
...@@ -551,11 +551,9 @@ static struct sock *dccp_v6_request_recv_sock(const struct sock *sk, ...@@ -551,11 +551,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); *own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash), NULL);
/* Clone pktoptions received with SYN, if we own the req */ /* Clone pktoptions received with SYN, if we own the req */
if (*own_req && ireq->pktopts) { 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); consume_skb(ireq->pktopts);
ireq->pktopts = NULL; ireq->pktopts = NULL;
if (newnp->pktoptions)
skb_set_owner_r(newnp->pktoptions, newsk);
} }
return newsk; return newsk;
...@@ -615,7 +613,7 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb) ...@@ -615,7 +613,7 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
--ANK (980728) --ANK (980728)
*/ */
if (np->rxopt.all) 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 (sk->sk_state == DCCP_OPEN) { /* Fast path */
if (dccp_rcv_established(sk, skb, dccp_hdr(skb), skb->len)) if (dccp_rcv_established(sk, skb, dccp_hdr(skb), skb->len))
...@@ -679,7 +677,6 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb) ...@@ -679,7 +677,6 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
np->flow_label = ip6_flowlabel(ipv6_hdr(opt_skb)); np->flow_label = ip6_flowlabel(ipv6_hdr(opt_skb));
if (ipv6_opt_accepted(sk, opt_skb, if (ipv6_opt_accepted(sk, opt_skb,
&DCCP_SKB_CB(opt_skb)->header.h6)) { &DCCP_SKB_CB(opt_skb)->header.h6)) {
skb_set_owner_r(opt_skb, sk);
memmove(IP6CB(opt_skb), memmove(IP6CB(opt_skb),
&DCCP_SKB_CB(opt_skb)->header.h6, &DCCP_SKB_CB(opt_skb)->header.h6,
sizeof(struct inet6_skb_parm)); sizeof(struct inet6_skb_parm));
......
...@@ -1388,14 +1388,11 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff * ...@@ -1388,14 +1388,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 */ /* Clone pktoptions received with SYN, if we own the req */
if (ireq->pktopts) { if (ireq->pktopts) {
newnp->pktoptions = skb_clone(ireq->pktopts, newnp->pktoptions = skb_clone_and_charge_r(ireq->pktopts, newsk);
sk_gfp_mask(sk, GFP_ATOMIC));
consume_skb(ireq->pktopts); consume_skb(ireq->pktopts);
ireq->pktopts = NULL; ireq->pktopts = NULL;
if (newnp->pktoptions) { if (newnp->pktoptions)
tcp_v6_restore_cb(newnp->pktoptions); tcp_v6_restore_cb(newnp->pktoptions);
skb_set_owner_r(newnp->pktoptions, newsk);
}
} }
} else { } else {
if (!req_unhash && found_dup_sk) { if (!req_unhash && found_dup_sk) {
...@@ -1467,7 +1464,7 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb) ...@@ -1467,7 +1464,7 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
--ANK (980728) --ANK (980728)
*/ */
if (np->rxopt.all) if (np->rxopt.all)
opt_skb = skb_clone(skb, sk_gfp_mask(sk, GFP_ATOMIC)); opt_skb = skb_clone_and_charge_r(skb, sk);
reason = SKB_DROP_REASON_NOT_SPECIFIED; reason = SKB_DROP_REASON_NOT_SPECIFIED;
if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */ if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
...@@ -1553,7 +1550,6 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb) ...@@ -1553,7 +1550,6 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
if (np->repflow) if (np->repflow)
np->flow_label = ip6_flowlabel(ipv6_hdr(opt_skb)); np->flow_label = ip6_flowlabel(ipv6_hdr(opt_skb));
if (ipv6_opt_accepted(sk, opt_skb, &TCP_SKB_CB(opt_skb)->header.h6)) { 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); tcp_v6_restore_cb(opt_skb);
opt_skb = xchg(&np->pktoptions, opt_skb); opt_skb = xchg(&np->pktoptions, opt_skb);
} else { } else {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment