• Guillaume Nault's avatar
    tcp: tighten acceptance of ACKs not matching a child socket · b4347c76
    Guillaume Nault authored
    BugLink: https://bugs.launchpad.net/bugs/1858489
    
    [ Upstream commit cb44a08f ]
    
    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: default avatarGuillaume Nault <gnault@redhat.com>
    Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: default avatarConnor Kuehl <connor.kuehl@canonical.com>
    Signed-off-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
    b4347c76
tcp.h 55.2 KB