Re: [PATCH nf-next v2] netfilter: nft_synproxy: only handle pure SYN and ACK packets

[Fernando Fernandez Mancera <fmancera@suse.de>]

On 10/5/26 5:12 PM, Nguyen Ngoc Thang wrote:
> nft_synproxy treats any segment with SYN set as a client's initial SYN
> and any segment with ACK set as the client's final ACK. A SYN-ACK thus
> gets answered with a fresh SYN-ACK cookie.
> 
> If that reply is routed back to the same host, e.g. to a peer address
> covered by an address on lo, it re-enters the input hook, is answered
> again, and never stops. Every such SYN starts its own endless loop over
> the loopback backlog, so NET_RX softirq keeps the CPU busy and memory
> fills with skbs and rtable entries until workqueues stall and the
> machine OOMs.
> 
> Use the same flag checks as ip(6)t_SYNPROXY: SYN without ACK, FIN or
> RST is an initial SYN; ACK without SYN, FIN or RST is the client ACK.
> Anything else falls through to the next expression.
> 
> Reported-by: syzbot+d7795c8487ca4e20ed88@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d7795c8487ca4e20ed88
> Reported-by: syzbot+d0d2f1a65f45b319d25d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d0d2f1a65f45b319d25d
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
> v2:
> - Retarget to nf-next (Pablo).
> - Drop the Fixes: tag accordingly.

This should have been sent as a standalone thread not as a reply under 
the v1. In any case, I agree with the patch.

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>

> 
> v1: https://lore.kernel.org/all/20261004160615.142456-1-ngocthang2710.1999@gmail.com/
> 
> Notes (not for the changelog):
> 
> Reproduced in QEMU (x86_64, 2 vCPUs, KASAN+lockdep) with syzbot's C
> repro for d7795c8487ca4e20ed88. Minimised: only the nft batch and the
> injected SYN matter, which is exactly the d0d2f1a65f45b319d25d program.
> 
> The rule is an inet table, base chain on input, with an unconditional
> "synproxy" expression. The SYN goes from 172.20.20.187 to .170 via tun;
> syzkaller puts 172.20.20.10/24 on lo, so the whole /24 is local and the
> SYN-ACK cookie comes back through lo. Instrumented trace:
> 
>    synproxy tx .170->.187 in=syz_tun out=lo syn=1 ack=1
>    synproxy tx .187->.170 in=lo      out=lo syn=1 ack=1
>    synproxy tx .170->.187 in=lo      out=lo syn=1 ack=1
>    ...
> 
> Before (3fd2ff60f6d6, full C repro):
>    - synproxy syn_received ~400k per CPU per netns after 15s
>    - rtable 80k -> 295k, skbuff_head_cache >1M, softirq time explodes
>    - "BUG: workqueue lockup", then OOM / hung task, VM wedges
> 
> After (2 runs x 300s, full C repro):
>    - no lockup, hung task or OOM
>    - rtable flat (~2.2k), skbuff_head_cache flat (~80k)
>    - syn_received grows slowly (~9.5k/300s): real SYNs still answered
> 
> ACK|FIN and ACK|RST no longer hit the cookie check (and its NF_DROP);
> they continue, as with ip(6)t_SYNPROXY's XT_CONTINUE. The usual
> "ct state invalid drop" rule still catches them.
> 
>   net/netfilter/nft_synproxy.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/nft_synproxy.c b/net/netfilter/nft_synproxy.c
> index 9ed288c9d168..a580cb7324da 100644
> --- a/net/netfilter/nft_synproxy.c
> +++ b/net/netfilter/nft_synproxy.c
> @@ -53,13 +53,13 @@ static void nft_synproxy_eval_v4(const struct nft_synproxy *priv,
>   	struct synproxy_net *snet = synproxy_pernet(net);
>   	struct sk_buff *skb = pkt->skb;
>   
> -	if (tcp->syn) {
> +	if (tcp->syn && !(tcp->ack || tcp->fin || tcp->rst)) {
>   		/* Initial SYN from client */
>   		nft_synproxy_tcp_options(opts, tcp, snet, &info);
>   		synproxy_send_client_synack(net, skb, tcp, opts);
>   		consume_skb(skb);
>   		regs->verdict.code = NF_STOLEN;
> -	} else if (tcp->ack) {
> +	} else if (tcp->ack && !(tcp->fin || tcp->rst || tcp->syn)) {
>   		/* ACK from client */
>   		if (synproxy_recv_client_ack(net, skb, tcp, opts,
>   					     ntohl(tcp->seq))) {
> @@ -84,13 +84,13 @@ static void nft_synproxy_eval_v6(const struct nft_synproxy *priv,
>   	struct synproxy_net *snet = synproxy_pernet(net);
>   	struct sk_buff *skb = pkt->skb;
>   
> -	if (tcp->syn) {
> +	if (tcp->syn && !(tcp->ack || tcp->fin || tcp->rst)) {
>   		/* Initial SYN from client */
>   		nft_synproxy_tcp_options(opts, tcp, snet, &info);
>   		synproxy_send_client_synack_ipv6(net, skb, tcp, opts);
>   		consume_skb(skb);
>   		regs->verdict.code = NF_STOLEN;
> -	} else if (tcp->ack) {
> +	} else if (tcp->ack && !(tcp->fin || tcp->rst || tcp->syn)) {
>   		/* ACK from client */
>   		if (synproxy_recv_client_ack_ipv6(net, skb, tcp, opts,
>   						  ntohl(tcp->seq))) {