netdev - Re: [PATCH net v2] Fix race for duplicate reqsk on identical SYN

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <22b3012d-3d70-492a-8786-d6c6beef2eb4@orange.com>
Date: Mon, 17 Jun 2024 01:45:52 +0200
From: alexandre.ferrieux@...nge.com
To: luoxuanqiang <luoxuanqiang@...inos.cn>, <edumazet@...gle.com>
CC: <davem@...emloft.net>, <dsahern@...nel.org>, <fw@...len.de>,
	<kuba@...nel.org>, <netdev@...r.kernel.org>, <pabeni@...hat.com>,
	<kuniyu@...zon.com>
Subject: Re: [PATCH net v2] Fix race for duplicate reqsk on identical SYN

On 14/06/2024 12:26, luoxuanqiang wrote:
> When bonding is configured in BOND_MODE_BROADCAST mode, if two identical
> SYN packets are received at the same time and processed on different CPUs,
> it can potentially create the same sk (sock) but two different reqsk
> (request_sock) in tcp_conn_request().
>
> These two different reqsk will respond with two SYNACK packets, and since
> the generation of the seq (ISN) incorporates a timestamp, the final two
> SYNACK packets will have different seq values.
>
> The consequence is that when the Client receives and replies with an ACK
> to the earlier SYNACK packet, we will reset(RST) it.
>
> ========================================================================
This is close, but not identical, to a race we observed on a *single* CPU with
the TPROXY iptables target, in the following situation:

  - two identical SYNs, sent one second apart from the same client socket,
    arrive back-to-back on the interface (due to network jitter)

  - they happen to be handled in the same batch of packet from one softirq
    name_your_nic_poll()

  - there, two loops run sequentially: one for netfilter (doing TPROXY), one
    for the network stack (doing TCP processing)

  - the first generates two distinct contexts for the two SYNs

  - the second respects these contexts and never gets a chance to merge them

The result is exactly as you describe, but in this case there is no need for 
bonding,
and everything happens in one single CPU, which is pretty ironic for a race.
My uneducated feeling is that the two loops are the cause of a simulated
parallelism, yielding the race. If each packet of the batch was handled
"to completion" (full netfilter handling followed immediately by full network
stack ingestion), the problem would not exist.

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.