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

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

From: Fernando Fernandez Mancera <fmancera@suse.de>

nft_exthdr_tcp_set_eval() and nft_exthdr_tcp_strip_eval() fetched the
TCP header and options through nft_tcp_header_pointer(), which calls
skb_header_pointer() a second time for the full tcphdr_len into a 60
bytes stack buffer. Since commit 28427f368f0e ("netfilter: nft_exthdr:
Fix non-linear header modification") both functions then call
skb_ensure_writable() and re-read the header from skb->data, so the
buffer was never used.

Only the 20 bytes fixed header is needed to learn tcphdr_len, so fetch
just that, validate the length as before, and let skb_ensure_writable()
pull the rest. This also drops the 60 bytes stack buffer from both
functions.

Comparison of performance in mpps:

  op       linear/non-linear     before    after    delta
==================================================================
   set,         non-linear       4.193     4.348    +3.7% mpps
   strip,       non-linear       4.184     4.434    +6.0% mpps
   set,         linear           5.302     5.427    +2.4% mpps
   strip,       linear           5.452     5.527    +1.4% mpss

Comparison of performance in ns/expression:

  op       linear/non-linear     before    after    delta
==================================================================
   set,         non-linear       34.6      25.3     -9.4  ns
   strip,       non-linear       35.2      20.8     -14.3 ns
   set,         linear           23.6      19.7     -3.9  ns
   strip,       linear           22.9      19.8     -3.1  ns

./scripts/bloat-o-meter exthdr_before.o exthdr_after.o
add/remove: 0/1 grow/shrink: 3/0 up/down: 233/-207 (26)
Function                                     old     new   delta
nft_exthdr_tcp_eval                          792     946    +154
nft_exthdr_tcp_strip_eval                    825     891     +66
nft_exthdr_tcp_set_eval                      602     615     +13
nft_tcp_header_pointer.part.constprop        207       -    -207
Total: Before=6527, After=6553, chg +0.40%

This also allow the compiler to inline nft_tcp_header_pointer inside
nft_exthdr_tcp_eval() which could yield some minimal performance gain.

This patch also updates the logic to drop malformed packets, before this
patch NFT_BREAK was used as a verdict instead.

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_exthdr.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index 3492426dccf7..861bab93f435 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,
 				    struct nft_regs *regs,
 				    const struct nft_pktinfo *pkt)
 {
-	u8 buff[sizeof(struct tcphdr) + MAX_TCP_OPTION_SPACE];
 	struct nft_exthdr *priv = nft_expr_priv(expr);
 	unsigned int i, optl, tcphdr_len, offset;
-	struct tcphdr *tcph;
+	struct tcphdr *tcph, _tcph;
 	u8 *opt;
 
-	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,
 				      struct nft_regs *regs,
 				      const struct nft_pktinfo *pkt)
 {
-	u8 buff[sizeof(struct tcphdr) + MAX_TCP_OPTION_SPACE];
 	struct nft_exthdr *priv = nft_expr_priv(expr);
 	unsigned int i, tcphdr_len, optl;
-	struct tcphdr *tcph;
+	struct tcphdr *tcph, _tcph;
 	u8 *opt;
 
-	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;
 
-- 
2.47.3