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, Guillaume Nault <gnault@redhat.com>,
	Eric Dumazet <edumazet@google.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 5.4 17/37] tcp: fix rejected syncookies due to stale timestamps
Date: Tue, 17 Dec 2019 21:09:38 +0100	[thread overview]
Message-ID: <20191217200727.097873159@linuxfoundation.org> (raw)
In-Reply-To: <20191217200721.741054904@linuxfoundation.org>

From: Guillaume Nault <gnault@redhat.com>

[ Upstream commit 04d26e7b159a396372646a480f4caa166d1b6720 ]

If no synflood happens for a long enough period of time, then the
synflood timestamp isn't refreshed and jiffies can advance so much
that time_after32() can't accurately compare them any more.

Therefore, we can end up in a situation where time_after32(now,
last_overflow + HZ) returns false, just because these two values are
too far apart. In that case, the synflood timestamp isn't updated as
it should be, which can trick tcp_synq_no_recent_overflow() into
rejecting valid syncookies.

For example, let's consider the following scenario on a system
with HZ=1000:

  * The synflood timestamp is 0, either because that's the timestamp
    of the last synflood or, more commonly, because we're working with
    a freshly created socket.

  * We receive a new SYN, which triggers synflood protection. Let's say
    that this happens when jiffies == 2147484649 (that is,
    'synflood timestamp' + HZ + 2^31 + 1).

  * Then tcp_synq_overflow() doesn't update the synflood timestamp,
    because time_after32(2147484649, 1000) returns false.
    With:
      - 2147484649: the value of jiffies, aka. 'now'.
      - 1000: the value of 'last_overflow' + HZ.

  * A bit later, we receive the ACK completing the 3WHS. But
    cookie_v[46]_check() rejects it because tcp_synq_no_recent_overflow()
    says that we're not under synflood. That's because
    time_after32(2147484649, 120000) returns false.
    With:
      - 2147484649: the value of jiffies, aka. 'now'.
      - 120000: the value of 'last_overflow' + TCP_SYNCOOKIE_VALID.

    Of course, in reality jiffies would have increased a bit, but this
    condition will last for the next 119 seconds, which is far enough
    to accommodate for jiffie's growth.

Fix this by updating the overflow timestamp whenever jiffies isn't
within the [last_overflow, last_overflow + HZ] range. That shouldn't
have any performance impact since the update still happens at most once
per second.

Now we're guaranteed to have fresh timestamps while under synflood, so
tcp_synq_no_recent_overflow() can safely use it with time_after32() in
such situations.

Stale timestamps can still make tcp_synq_no_recent_overflow() return
the wrong verdict when not under synflood. This will be handled in the
next patch.

For 64 bits architectures, the problem was introduced with the
conversion of ->tw_ts_recent_stamp to 32 bits integer by commit
cca9bab1b72c ("tcp: use monotonic timestamps for PAWS").
The problem has always been there on 32 bits architectures.

Fixes: cca9bab1b72c ("tcp: use monotonic timestamps for PAWS")
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Guillaume Nault <gnault@redhat.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/time.h |   13 +++++++++++++
 include/net/tcp.h    |    5 +++--
 2 files changed, 16 insertions(+), 2 deletions(-)

--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -96,4 +96,17 @@ static inline bool itimerspec64_valid(co
  */
 #define time_after32(a, b)	((s32)((u32)(b) - (u32)(a)) < 0)
 #define time_before32(b, a)	time_after32(a, b)
+
+/**
+ * time_between32 - check if a 32-bit timestamp is within a given time range
+ * @t:	the time which may be within [l,h]
+ * @l:	the lower bound of the range
+ * @h:	the higher bound of the range
+ *
+ * time_before32(t, l, h) returns true if @l <= @t <= @h. All operands are
+ * treated as 32-bit integers.
+ *
+ * Equivalent to !(time_before32(@t, @l) || time_after32(@t, @h)).
+ */
+#define time_between32(t, l, h) ((u32)(h) - (u32)(l) >= (u32)(t) - (u32)(l))
 #endif
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -494,14 +494,15 @@ static inline void tcp_synq_overflow(con
 		reuse = rcu_dereference(sk->sk_reuseport_cb);
 		if (likely(reuse)) {
 			last_overflow = READ_ONCE(reuse->synq_overflow_ts);
-			if (time_after32(now, last_overflow + HZ))
+			if (!time_between32(now, last_overflow,
+					    last_overflow + HZ))
 				WRITE_ONCE(reuse->synq_overflow_ts, now);
 			return;
 		}
 	}
 
 	last_overflow = tcp_sk(sk)->rx_opt.ts_recent_stamp;
-	if (time_after32(now, last_overflow + HZ))
+	if (!time_between32(now, last_overflow, last_overflow + HZ))
 		tcp_sk(sk)->rx_opt.ts_recent_stamp = now;
 }
 



  parent reply	other threads:[~2019-12-17 20:10 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-17 20:09 [PATCH 5.4 00/37] 5.4.5-stable review Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 01/37] inet: protect against too small mtu values Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 02/37] mqprio: Fix out-of-bounds access in mqprio_dump Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 03/37] net: bridge: deny dev_set_mac_address() when unregistering Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 04/37] net: dsa: fix flow dissection on Tx path Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 05/37] net: ethernet: ti: cpsw: fix extra rx interrupt Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 06/37] net: sched: fix dump qlen for sch_mq/sch_mqprio with NOLOCK subqueues Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 07/37] net_sched: validate TCA_KIND attribute in tc_chain_tmplt_add() Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 08/37] net-sysfs: Call dev_hold always in netdev_queue_add_kobject Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 09/37] net: thunderx: start phy before starting autonegotiation Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 10/37] net/tls: Fix return values to avoid ENOTSUPP Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 11/37] openvswitch: support asymmetric conntrack Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 12/37] tcp: md5: fix potential overestimation of TCP option space Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 13/37] tipc: fix ordering of tipc module init and exit routine Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 14/37] net/mlx5e: Query global pause state before setting prio2buffer Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 15/37] net: ipv6: add net argument to ip6_dst_lookup_flow Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 16/37] net: ipv6_stub: use ip6_dst_lookup_flow instead of ip6_dst_lookup Greg Kroah-Hartman
2019-12-17 20:09 ` Greg Kroah-Hartman [this message]
2019-12-17 20:09 ` [PATCH 5.4 18/37] tcp: tighten acceptance of ACKs not matching a child socket Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 19/37] tcp: Protect accesses to .ts_recent_stamp with {READ,WRITE}_ONCE() Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 20/37] net: core: rename indirect block ingress cb function Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 21/37] net: sched: allow indirect blocks to bind to clsact in TC Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 22/37] cls_flower: Fix the behavior using port ranges with hw-offload Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 23/37] gre: refetch erspan header from skb->data after pskb_may_pull() Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 24/37] Fixed updating of ethertype in function skb_mpls_pop Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 25/37] hsr: fix a NULL pointer dereference in hsr_dev_xmit() Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 26/37] net: Fixed updating of ethertype in skb_mpls_push() Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 27/37] net/mlx5e: Fix TXQ indices to be sequential Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 28/37] act_ct: support asymmetric conntrack Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 29/37] net/mlx5e: Fix SFF 8472 eeprom length Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 30/37] net/mlx5e: Fix freeing flow with kfree() and not kvfree() Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 31/37] net/mlx5e: Fix translation of link mode into speed Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 32/37] net/mlx5e: ethtool, Fix analysis of speed setting Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 33/37] page_pool: do not release pool until inflight == 0 Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 34/37] xdp: obtain the mem_id mutex before trying to remove an entry Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 35/37] ionic: keep users rss hash across lif reset Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 36/37] net: mscc: ocelot: unregister the PTP clock on deinit Greg Kroah-Hartman
2019-12-17 20:09 ` [PATCH 5.4 37/37] r8169: add missing RX enabling for WoL on RTL8125 Greg Kroah-Hartman
2019-12-18 10:24 ` [PATCH 5.4 00/37] 5.4.5-stable review Jon Hunter
2019-12-18 14:34   ` Greg Kroah-Hartman
2019-12-18 14:22 ` Naresh Kamboju
2019-12-18 14:35   ` Greg Kroah-Hartman
2019-12-18 14:48 ` Guenter Roeck
2019-12-18 14:53   ` Greg Kroah-Hartman
2019-12-18 20:59 ` shuah
2019-12-19  8:44   ` Greg Kroah-Hartman
2019-12-18 21:02 ` Geert Uytterhoeven
2019-12-19  8:45   ` 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=20191217200727.097873159@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gnault@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /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).