Re: [PATCH net-next 09/15] netfilter: nft_exthdr: avoid 60 bytes stack buffer in TCP option mangling

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Mon, Oct 19, 2026 at 01:21:26AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] The commit message says "This patch also updates the logic to
>   drop malformed packets, before this patch NFT_BREAK was used as a…
> 
> --
> 
> netfilter: nft_exthdr: avoid 60 bytes stack buffer in TCP option mangling
> 
> This commit changes nft_exthdr_tcp_set_eval() and
> nft_exthdr_tcp_strip_eval() so they no longer call
> nft_tcp_header_pointer(). Each function now reads only the 20 byte fixed
> TCP header with skb_header_pointer(), validates doff, and leaves it to
> skb_ensure_writable() to pull the options. This removes a 60 byte stack
> buffer and a redundant second header copy from both functions.
> 
> > diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
> > index 3492426dccf7f..861bab93f4352 100644
> > --- a/net/netfilter/nft_exthdr.c
> > +++ b/net/netfilter/nft_exthdr.c
> > @@ -233,16 +233,23 @@ static void nft_exthdr_tcp_set_eval(const struct nft_expr *expr,
> 
> [ ... ]
> 
> > -	tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
> > +	if (pkt->tprot != IPPROTO_TCP || pkt->fragoff)
> > +		goto err;
> > +
> > +	tcph = skb_header_pointer(pkt->skb, nft_thoff(pkt), sizeof(_tcph), &_tcph);
> >  	if (!tcph)
> >  		goto err;
> >  
> > +	tcphdr_len = __tcp_hdrlen(tcph);
> > +	if (tcphdr_len < sizeof(*tcph) ||
> > +	    tcphdr_len > sizeof(*tcph) + MAX_TCP_OPTION_SPACE)
> > +		goto err;
> > +
> >  	if (skb_ensure_writable(pkt->skb, nft_thoff(pkt) + tcphdr_len))
> >  		goto err;
> >  
> > @@ -313,16 +320,23 @@ static void nft_exthdr_tcp_strip_eval(const struct nft_expr *expr,
> 
> [ ... ]
> 
> > -	tcph = nft_tcp_header_pointer(pkt, sizeof(buff), buff, &tcphdr_len);
> > +	if (pkt->tprot != IPPROTO_TCP || pkt->fragoff)
> > +		goto err;
> > +
> > +	tcph = skb_header_pointer(pkt->skb, nft_thoff(pkt), sizeof(_tcph), &_tcph);
> >  	if (!tcph)
> >  		goto err;
> >  
> > +	tcphdr_len = __tcp_hdrlen(tcph);
> > +	if (tcphdr_len < sizeof(*tcph) ||
> > +	    tcphdr_len > sizeof(*tcph) + MAX_TCP_OPTION_SPACE)
> > +		goto err;
> > +
> >  	if (skb_ensure_writable(pkt->skb, nft_thoff(pkt) + tcphdr_len))
> >  		goto drop;
> 
> [Severity: Medium]

Nothing wrong in this patch.

> The commit message says:
> 
>   This patch also updates the logic to drop malformed packets, before this
>   patch NFT_BREAK was used as a verdict instead.

I added this comment to make this AI robot and it keeps complaining...

> Does that match what the diff does? The diff does not touch any verdict
> code. The change in behaviour only comes from removing the second,
> full-length skb_header_pointer() call that nft_tcp_header_pointer() made:
> 
>   return skb_header_pointer(pkt->skb, nft_thoff(pkt), *tcphdr_len, buffer);
> 
> Before this patch, a truncated TCP packet (thoff + doff * 4 > skb->len)
> made that call return NULL. Both functions then took goto err and
> returned NFT_BREAK.

Yes, this is what the comment aims to say.