From: alex@badcode.im
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: alexvancasper@gmail.com, stable@vger.kernel.org
Subject: Re: [PATCH net 2/2] wireguard: noise: recheck replay and flood under write lock on initiation
Date: Tue, 10 Nov 2026 18:08:38 +0000 [thread overview]
Message-ID: <0770649757ee4a30024424f66a16b964@badcode.im> (raw)
In-Reply-To: <avNLlkTOYf_yYTHE@zx2c4.com>
I don't think it changes which packets take the exclusive lock. Before
this patch, every initiation that passed the read-locked check already
went on to take down_write() unconditionally to copy the new state into
the handshake. With the patch, exactly the same set of packets reaches
down_write(). The only difference is that a packet that lost the race
now sees the updated latest_timestamp/last_initiation_consumption and
bails out right away, instead of overwriting the handshake state and
generating a second response. So the time spent under the write lock
goes down for those packets, not up.
Everything else is still filtered under the read lock as before. Once
the first copy has stored its timestamp and consumption time, later
replays, and any initiation within the next 1/INITIATIONS_PER_SECOND,
are rejected at the down_read() fast path and never touch the write
lock. Only copies of the same initiation that are processed truly in
parallel, so at most one per CPU, can get to down_write(), and that was
already the case before.
This is the same pattern the response side uses after 2a73c9b2c0ca:
check under the read lock as a fast path, then check again after
down_write() before committing.
Thanks,
Alexander
-------- Исходное сообщение --------
Тема: Re: [PATCH net 2/2] wireguard: noise: recheck replay and flood
under write lock on initiation
Дата: 10.11.2026 16:51
От: "Jason A. Donenfeld" <Jason@zx2c4.com>
Кому: Alexander Ildiyarov <alex@badcode.im>
On Thu, Nov 05, 2026 at 05:08:15PM +0300, Alexander Ildiyarov wrote:
> wg_noise_handshake_consume_initiation() checks the initiation's
> timestamp against latest_timestamp, and last_initiation_consumption
> against the current time, while holding handshake->lock for reading.
> It then drops the lock and takes it again for writing to store the new
> state, without repeating the checks.
>
> Incoming handshake packets are spread round-robin over all online CPUs,
> so two copies of the same initiation, received back to back, are
> processed in parallel. Both copies can pass the checks before either of
> them updates latest_timestamp and last_initiation_consumption, so the
> replay and flood protection is bypassed. The responder then creates two
> responses, each with a new ephemeral key, and installs the keypair of
> the last one, while the initiator accepts whichever response arrives
> first. When those differ, the session is unusable until the initiator
> gives up on it and starts a new handshake, and an attacker who can see
> and duplicate initiations can repeat this for every handshake.
>
> The window is narrow, since the expensive DH computations happen before
> the check, so it is between up_read() and down_write() only. With netem
> duplicating every initiation between two namespaces, no double response
> was observed in 200 handshakes on a 4-CPU VM. With a usleep_range(1000,
> 2000) inserted before down_write() to widen the window, 30 of 50
> duplicated initiations produced two responses, and in 12 of 50 cases
> the session was unusable after the handshake. With this patch applied,
> and the same delay, every duplicate was rejected and all 50 sessions
> worked.
>
> Fix this by doing the checks again once the write lock is held, keeping
> the read-locked check as a fast path for rejecting replays.
The downside, though, is that now it means there's potential to do flood
attacks that take the exclusive lock, which is what was trying to be
avoided, right?
next prev parent reply other threads:[~2026-11-10 18:08 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20261105140815.92841-1-alex@badcode.im>
2026-11-05 14:08 ` [PATCH net 1/2] wireguard: account peer and allowedips allocations to memcg Alexander Ildiyarov
2026-11-10 16:52 ` Jason A. Donenfeld
2026-11-05 14:08 ` [PATCH net 2/2] wireguard: noise: recheck replay and flood under write lock on initiation Alexander Ildiyarov
2026-11-10 16:51 ` Jason A. Donenfeld
2026-11-10 18:08 ` alex [this message]
[not found] <20261105141006.95083-1-alex@badcode.im>
2026-11-05 14:10 ` Alexander Ildiyarov
2026-11-09 15:20 ` netdev-bot+sashiko
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=0770649757ee4a30024424f66a16b964@badcode.im \
--to=alex@badcode.im \
--cc=Jason@zx2c4.com \
--cc=alexvancasper@gmail.com \
--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