lkml.org 
[lkml]   [2019]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.4 18/37] tcp: tighten acceptance of ACKs not matching a child socket
    Date
    From: Guillaume Nault <gnault@redhat.com>

    [ Upstream commit cb44a08f8647fd2e8db5cc9ac27cd8355fa392d8 ]

    When no synflood occurs, the synflood timestamp isn't updated.
    Therefore it can be so old that time_after32() can consider it to be
    in the future.

    That's a problem for tcp_synq_no_recent_overflow() as it may report
    that a recent overflow occurred while, in fact, it's just that jiffies
    has grown past 'last_overflow' + TCP_SYNCOOKIE_VALID + 2^31.

    Spurious detection of recent overflows lead to extra syncookie
    verification in cookie_v[46]_check(). At that point, the verification
    should fail and the packet dropped. But we should have dropped the
    packet earlier as we didn't even send a syncookie.

    Let's refine tcp_synq_no_recent_overflow() to report a recent overflow
    only if jiffies is within the
    [last_overflow, last_overflow + TCP_SYNCOOKIE_VALID] interval. This
    way, no spurious recent overflow is reported when jiffies wraps and
    'last_overflow' becomes in the future from the point of view of
    time_after32().

    However, if jiffies wraps and enters the
    [last_overflow, last_overflow + TCP_SYNCOOKIE_VALID] interval (with
    'last_overflow' being a stale synflood timestamp), then
    tcp_synq_no_recent_overflow() still erroneously reports an
    overflow. In such cases, we have to rely on syncookie verification
    to drop the packet. We unfortunately have no way to differentiate
    between a fresh and a stale syncookie timestamp.

    In practice, using last_overflow as lower bound is problematic.
    If the synflood timestamp is concurrently updated between the time
    we read jiffies and the moment we store the timestamp in
    'last_overflow', then 'now' becomes smaller than 'last_overflow' and
    tcp_synq_no_recent_overflow() returns true, potentially dropping a
    valid syncookie.

    Reading jiffies after loading the timestamp could fix the problem,
    but that'd require a memory barrier. Let's just accommodate for
    potential timestamp growth instead and extend the interval using
    'last_overflow - HZ' as lower bound.

    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/net/tcp.h | 16 +++++++++++++---
    1 file changed, 13 insertions(+), 3 deletions(-)

    --- a/include/net/tcp.h
    +++ b/include/net/tcp.h
    @@ -518,13 +518,23 @@ static inline bool tcp_synq_no_recent_ov
    reuse = rcu_dereference(sk->sk_reuseport_cb);
    if (likely(reuse)) {
    last_overflow = READ_ONCE(reuse->synq_overflow_ts);
    - return time_after32(now, last_overflow +
    - TCP_SYNCOOKIE_VALID);
    + return !time_between32(now, last_overflow - HZ,
    + last_overflow +
    + TCP_SYNCOOKIE_VALID);
    }
    }

    last_overflow = tcp_sk(sk)->rx_opt.ts_recent_stamp;
    - return time_after32(now, last_overflow + TCP_SYNCOOKIE_VALID);
    +
    + /* If last_overflow <= jiffies <= last_overflow + TCP_SYNCOOKIE_VALID,
    + * then we're under synflood. However, we have to use
    + * 'last_overflow - HZ' as lower bound. That's because a concurrent
    + * tcp_synq_overflow() could update .ts_recent_stamp after we read
    + * jiffies but before we store .ts_recent_stamp into last_overflow,
    + * which could lead to rejecting a valid syncookie.
    + */
    + return !time_between32(now, last_overflow - HZ,
    + last_overflow + TCP_SYNCOOKIE_VALID);
    }

    static inline u32 tcp_cookie_time(void)

    \
     
     \ /
      Last update: 2019-12-17 21:12    [W:4.443 / U:0.140 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site