tcp: limit payload size of sacked skbs
authorEric Dumazet <edumazet@google.com>
Sun, 16 Jun 2019 00:31:03 +0000 (17:31 -0700)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Mon, 17 Jun 2019 17:52:43 +0000 (19:52 +0200)
commit 3b4929f65b0d8249f19a50245cd88ed1a2f78cff upstream.

Jonathan Looney reported that TCP can trigger the following crash
in tcp_shifted_skb() :

BUG_ON(tcp_skb_pcount(skb) < pcount);

This can happen if the remote peer has advertized the smallest
MSS that linux TCP accepts : 48

An skb can hold 17 fragments, and each fragment can hold 32KB
on x86, or 64KB on PowerPC.

This means that the 16bit witdh of TCP_SKB_CB(skb)->tcp_gso_segs
can overflow.

Note that tcp_sendmsg() builds skbs with less than 64KB
of payload, so this problem needs SACK to be enabled.
SACK blocks allow TCP to coalesce multiple skbs in the retransmit
queue, thus filling the 17 fragments to maximal capacity.

CVE-2019-11477 -- u16 overflow of TCP_SKB_CB(skb)->tcp_gso_segs

Backport notes, provided by Joao Martins <joao.m.martins@oracle.com>

v4.15 or since commit 737ff314563 ("tcp: use sequence distance to
detect reordering") had switched from the packet-based FACK tracking and
switched to sequence-based.

v4.14 and older still have the old logic and hence on
tcp_skb_shift_data() needs to retain its original logic and have
@fack_count in sync. In other words, we keep the increment of pcount with
tcp_skb_pcount(skb) to later used that to update fack_count. To make it
more explicit we track the new skb that gets incremented to pcount in
@next_pcount, and we get to avoid the constant invocation of
tcp_skb_pcount(skb) all together.

Fixes: 832d11c5cd07 ("tcp: Try to restore large SKBs while SACK processing")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Jonathan Looney <jtl@netflix.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Bruce Curtis <brucec@netflix.com>
Cc: Jonathan Lemon <jonathan.lemon@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
include/linux/tcp.h
include/net/tcp.h
net/ipv4/tcp.c
net/ipv4/tcp_input.c
net/ipv4/tcp_output.c

index fe322fa611e663e50a67f5153f7a1d683e573d34..60aea230dc6a75b79a6554136f302ae3883dfe4f 100644 (file)
@@ -450,4 +450,8 @@ static inline u16 tcp_mss_clamp(const struct tcp_sock *tp, u16 mss)
 
        return (user_mss && user_mss < mss) ? user_mss : mss;
 }
+
+int tcp_skb_shift(struct sk_buff *to, struct sk_buff *from, int pcount,
+                 int shiftlen);
+
 #endif /* _LINUX_TCP_H */
index d0c2dbe94e21d6b1bdee35e0e200ec2af01fdc23..1179ef4f07683508d0275b552ade7bf091a995dd 100644 (file)
@@ -57,6 +57,8 @@ void tcp_time_wait(struct sock *sk, int state, int timeo);
 
 #define MAX_TCP_HEADER (128 + MAX_HEADER)
 #define MAX_TCP_OPTION_SPACE 40
+#define TCP_MIN_SND_MSS                48
+#define TCP_MIN_GSO_SIZE       (TCP_MIN_SND_MSS - MAX_TCP_OPTION_SPACE)
 
 /*
  * Never offer a window over 32767 without using window scaling. Some
index 00ae9a1d44edae7e19dc52f0df8a06d8ed502937..d51571be9eac68c8885537387b09275a6a44da1c 100644 (file)
@@ -3480,6 +3480,7 @@ void __init tcp_init(void)
        unsigned long limit;
        unsigned int i;
 
+       BUILD_BUG_ON(TCP_MIN_SND_MSS <= MAX_TCP_OPTION_SPACE);
        BUILD_BUG_ON(sizeof(struct tcp_skb_cb) >
                     FIELD_SIZEOF(struct sk_buff, cb));
 
index 03bfec5a388af629f6868c1e42ee8a5c5d9bf58e..8e080f3b75bd8dea236c4baf32921ca7f3c558e2 100644 (file)
@@ -1329,7 +1329,7 @@ static bool tcp_shifted_skb(struct sock *sk, struct sk_buff *skb,
        TCP_SKB_CB(skb)->seq += shifted;
 
        tcp_skb_pcount_add(prev, pcount);
-       BUG_ON(tcp_skb_pcount(skb) < pcount);
+       WARN_ON_ONCE(tcp_skb_pcount(skb) < pcount);
        tcp_skb_pcount_add(skb, -pcount);
 
        /* When we're adding to gso_segs == 1, gso_size will be zero,
@@ -1396,6 +1396,21 @@ static int skb_can_shift(const struct sk_buff *skb)
        return !skb_headlen(skb) && skb_is_nonlinear(skb);
 }
 
+int tcp_skb_shift(struct sk_buff *to, struct sk_buff *from,
+                 int pcount, int shiftlen)
+{
+       /* TCP min gso_size is 8 bytes (TCP_MIN_GSO_SIZE)
+        * Since TCP_SKB_CB(skb)->tcp_gso_segs is 16 bits, we need
+        * to make sure not storing more than 65535 * 8 bytes per skb,
+        * even if current MSS is bigger.
+        */
+       if (unlikely(to->len + shiftlen >= 65535 * TCP_MIN_GSO_SIZE))
+               return 0;
+       if (unlikely(tcp_skb_pcount(to) + pcount > 65535))
+               return 0;
+       return skb_shift(to, from, shiftlen);
+}
+
 /* Try collapsing SACK blocks spanning across multiple skbs to a single
  * skb.
  */
@@ -1407,6 +1422,7 @@ static struct sk_buff *tcp_shift_skb_data(struct sock *sk, struct sk_buff *skb,
        struct tcp_sock *tp = tcp_sk(sk);
        struct sk_buff *prev;
        int mss;
+       int next_pcount;
        int pcount = 0;
        int len;
        int in_sack;
@@ -1504,7 +1520,7 @@ static struct sk_buff *tcp_shift_skb_data(struct sock *sk, struct sk_buff *skb,
        if (!after(TCP_SKB_CB(skb)->seq + len, tp->snd_una))
                goto fallback;
 
-       if (!skb_shift(prev, skb, len))
+       if (!tcp_skb_shift(prev, skb, pcount, len))
                goto fallback;
        if (!tcp_shifted_skb(sk, skb, state, pcount, len, mss, dup_sack))
                goto out;
@@ -1523,11 +1539,11 @@ static struct sk_buff *tcp_shift_skb_data(struct sock *sk, struct sk_buff *skb,
                goto out;
 
        len = skb->len;
-       if (skb_shift(prev, skb, len)) {
-               pcount += tcp_skb_pcount(skb);
-               tcp_shifted_skb(sk, skb, state, tcp_skb_pcount(skb), len, mss, 0);
+       next_pcount = tcp_skb_pcount(skb);
+       if (tcp_skb_shift(prev, skb, next_pcount, len)) {
+               pcount += next_pcount;
+               tcp_shifted_skb(sk, skb, state, next_pcount, len, mss, 0);
        }
-
 out:
        state->fack_count += pcount;
        return prev;
index 24bad638c2ecfe97c1eb6a7c2ae5930aa1330203..def09d1fd0e18d5e5ba948bd4758b5f79d9ea8a9 100644 (file)
@@ -1442,8 +1442,8 @@ static inline int __tcp_mtu_to_mss(struct sock *sk, int pmtu)
        mss_now -= icsk->icsk_ext_hdr_len;
 
        /* Then reserve room for full set of TCP options and 8 bytes of data */
-       if (mss_now < 48)
-               mss_now = 48;
+       if (mss_now < TCP_MIN_SND_MSS)
+               mss_now = TCP_MIN_SND_MSS;
        return mss_now;
 }
 
@@ -2724,7 +2724,7 @@ static bool tcp_collapse_retrans(struct sock *sk, struct sk_buff *skb)
                if (next_skb_size <= skb_availroom(skb))
                        skb_copy_bits(next_skb, 0, skb_put(skb, next_skb_size),
                                      next_skb_size);
-               else if (!skb_shift(skb, next_skb, next_skb_size))
+               else if (!tcp_skb_shift(skb, next_skb, 1, next_skb_size))
                        return false;
        }
        tcp_highest_sack_replace(sk, next_skb, skb);