netdev - [PATCH net v4 0/3] tcp: fix handling of stale syncookies timestamps

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <cover.1575631229.git.gnault@redhat.com>
Date:   Fri, 6 Dec 2019 12:38:26 +0100
From:   Guillaume Nault <gnault@...hat.com>
To:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     netdev@...r.kernel.org, Eric Dumazet <edumazet@...gle.com>,
        Arnd Bergmann <arnd@...db.de>,
        John Stultz <john.stultz@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: [PATCH net v4 0/3] tcp: fix handling of stale syncookies timestamps

The synflood timestamps (->ts_recent_stamp and ->synq_overflow_ts) are
only refreshed when the syncookie protection triggers. Therefore, their
value can become very far apart from jiffies if no synflood happens for
a long time.

If jiffies grows too much and wraps while the synflood timestamp isn't
refreshed, then time_after32() might consider the later to be in the
future. This can trick tcp_synq_no_recent_overflow() into returning
erroneous values and rejecting valid ACKs.

Patch 1 handles the case of ACKs using legitimate syncookies.
Patch 2 handles the case of stray ACKs.
Patch 3 annotates lockless timestamp operations with READ_ONCE() and
WRITE_ONCE().

Changes from v3:
  - Fix description of time_between32() (found by Eric Dumazet).
  - Use more accurate Fixes tag in patch 3 (suggested by Eric Dumazet).

Changes from v2:
  - Define and use time_between32() instead of a pair of
    time_before32/time_after32 (suggested by Eric Dumazet).
  - Use 'last_overflow - HZ' as lower bound in
    tcp_synq_no_recent_overflow(), to accommodate for concurrent
    timestamp updates (found by Eric Dumazet).
  - Add a third patch to annotate lockless accesses to .ts_recent_stamp.

Changes from v1:
  - Initialising timestamps at socket creation time is not enough
    because jiffies wraps in 24 days with HZ=1000 (Eric Dumazet).
    Handle stale timestamps in tcp_synq_overflow() and
    tcp_synq_no_recent_overflow() instead.
  - Rework commit description.
  - Add a second patch to handle the case of stray ACKs.

Guillaume Nault (3):
  tcp: fix rejected syncookies due to stale timestamps
  tcp: tighten acceptance of ACKs not matching a child socket
  tcp: Protect accesses to .ts_recent_stamp with {READ,WRITE}_ONCE()

 include/linux/time.h | 13 +++++++++++++
 include/net/tcp.h    | 27 +++++++++++++++++++--------
 2 files changed, 32 insertions(+), 8 deletions(-)

-- 
2.21.0