linux: layer7: Don't linearize the skb in the match
Commit Message
skb_linearize() can free the old skb head while ipt_do_table() keeps
using its cached IP header pointer for the next rule, which KFENCE
reports as a use-after-free when QoS is enabled. Read the application
data without modifying the skb instead.
Fixes: #13953
Signed-off-by: Antonio Quartulli <antonio@mandelbit.com>
---
How to reproduce (no QoS configuration needed):
1. The match must see received, non-cloned, non-linear skbs with a small
kmalloc'ed head, e.g. on Intel I226-V (igc), or on a VM whose red virtio
NIC has mergeable rx buffers disabled and receives GSO packets:
<driver name='vhost'><host mrg_rxbuf='off'/></driver>
2. Add the same rules QoS creates (a layer7 match that is false for
unclassified connections, followed by another rule):
iptables -t mangle -N L7T
iptables -t mangle -A PREROUTING -i red0 -j L7T
iptables -t mangle -A L7T -m layer7 --l7dir /etc/l7-protocols/protocols \
! --l7proto unset -j MARK --set-xmark 0x1/0xf
iptables -t mangle -A L7T -j CONNMARK --save-mark --mask 0xf
3. Make KFENCE sample often:
echo 1 > /sys/module/kfence/parameters/sample_interval
4. Open many short TCP connections from the firewall to a host on red that
sends data back (e.g. 5000 connections downloading 256 KiB each).
Without this patch, "BUG: KFENCE: use-after-free read in ipt_do_table"
shows up within seconds (31 reports for 5000 connections on a Core 205
VM). With it, there were none after 50000 connections, and layer7 still
classified all connections correctly.
src/patches/linux/linux-6.18-layer7.patch | 80 ++++++++++++++++-------
1 file changed, 55 insertions(+), 25 deletions(-)
@@ -1501,7 +1501,7 @@ new file mode 100644
index 000000000000..a7eb760123bf
--- /dev/null
+++ b/net/netfilter/xt_layer7.c
-@@ -0,0 +1,666 @@
+@@ -0,0 +1,696 @@
+/*
+ Kernel module to match application layer (OSI layer 7) data in connections.
+
@@ -1572,6 +1572,10 @@ index 000000000000..a7eb760123bf
+
+DEFINE_SPINLOCK(l7_lock);
+
++/* Copy of the application data of the packet being matched (maxdatalen
++bytes), protected by l7_lock */
++static char *l7_scratch;
++
+#ifdef CONFIG_IP_NF_MATCH_LAYER7_DEBUG
+/* Converts an unfriendly string into a friendly one by
+replacing unprintables with periods and all whitespace with " ". */
@@ -1763,7 +1767,8 @@ index 000000000000..a7eb760123bf
+ return 1;
+}
+
-+/* Returns offset the into the skb->data that the application data starts */
++/* Returns offset the into the skb->data that the application data starts,
++or -1 if it can't be determined */
+static int app_data_offset(const struct sk_buff *skb)
+{
+ /* In case we are ported somewhere (ebtables?) where ip_hdr(skb)
@@ -1774,10 +1779,15 @@ index 000000000000..a7eb760123bf
+ /* 12 == offset into TCP header for the header length field.
+ Can't get this with skb->h.th->doff because the tcphdr
+ struct doesn't get set when routing (this is confirmed to be
-+ true in Netfilter as well as QoS.) */
-+ int tcp_hl = 4*(skb->data[ip_hl + 12] >> 4);
++ true in Netfilter as well as QoS.) The TCP header might not be
++ in the linear part, so don't read it from skb->data directly. */
++ u8 _doff, *doff;
++
++ doff = skb_header_pointer(skb, ip_hl + 12, sizeof(_doff), &_doff);
++ if (!doff)
++ return -1;
+
-+ return ip_hl + tcp_hl;
++ return ip_hl + 4 * (*doff >> 4);
+ } else if( ip_hdr(skb)->protocol == IPPROTO_UDP ) {
+ return ip_hl + 8; /* UDP header is always 8 bytes */
+ } else if( ip_hdr(skb)->protocol == IPPROTO_ICMP ) {
@@ -1945,6 +1955,7 @@ index 000000000000..a7eb760123bf
+ struct nf_conn *master_conntrack, *conntrack;
+ unsigned char * app_data;
+ unsigned int pattern_result, appdatalen;
++ int appdataoff;
+ regexp * comppattern;
+
+ /* Be paranoid/incompetent - lock the entire match function. */
@@ -1992,20 +2003,6 @@ index 000000000000..a7eb760123bf
+ return (pattern_result ^ info->invert);
+ }
+
-+ if(skb_is_nonlinear(skb)){
-+ if(skb_linearize(skb) != 0){
-+ if (net_ratelimit())
-+ printk(KERN_ERR "layer7: failed to linearize "
-+ "packet, bailing.\n");
-+ spin_unlock_bh(&l7_lock);
-+ return info->invert;
-+ }
-+ }
-+
-+ /* now that the skb is linearized, it's safe to set these. */
-+ app_data = skb->data + app_data_offset(skb);
-+ appdatalen = skb_tail_pointer(skb) - app_data;
-+
+ /* the return value gets checked later, when we're ready to use it */
+ comppattern = compile_and_cache(info->pattern, info->protocol);
+
@@ -2028,6 +2025,25 @@ index 000000000000..a7eb760123bf
+ if(!skb->layer7_flags[0]){
+ int newbytes;
+ master_conntrack->layer7.packets++;
++
++ /* Don't linearize the skb to get at the application data: a
++ match must not modify the skb, because ipt_do_table() keeps
++ using pointers into its old head, which skb_linearize() might
++ free. Copy the data into l7_scratch instead if it is not in
++ the linear part. */
++ appdataoff = app_data_offset(skb);
++ if(appdataoff < 0 || appdataoff > skb->len){
++ app_data = NULL;
++ appdatalen = 0;
++ } else {
++ appdatalen = min_t(unsigned int, skb->len - appdataoff,
++ maxdatalen);
++ app_data = skb_header_pointer(skb, appdataoff,
++ appdatalen, l7_scratch);
++ if(!app_data)
++ appdatalen = 0;
++ }
++
+ newbytes = add_data(master_conntrack, app_data, appdatalen);
+ if(newbytes == 0) { /* didn't add any data */
+ skb->layer7_flags[0] = 1;
@@ -2137,11 +2153,9 @@ index 000000000000..a7eb760123bf
+
+static int __init xt_layer7_init(void)
+{
-+// need_conntrack();
++ int ret;
+
-+ // Register proc interface
-+ proc_create_data("layer7_numpackets", 0644,
-+ init_net.proc_net, &layer7_numpackets_proc_fops, NULL);
++// need_conntrack();
+
+ if(maxdatalen < 1) {
+ printk(KERN_WARNING "layer7: maxdatalen can't be < 1, "
@@ -2155,14 +2169,30 @@ index 000000000000..a7eb760123bf
+ "using 65536\n");
+ maxdatalen = 65536;
+ }
-+ return xt_register_matches(xt_layer7_match,
-+ ARRAY_SIZE(xt_layer7_match));
++
++ l7_scratch = kmalloc(maxdatalen, GFP_KERNEL);
++ if(!l7_scratch)
++ return -ENOMEM;
++
++ // Register proc interface
++ proc_create_data("layer7_numpackets", 0644,
++ init_net.proc_net, &layer7_numpackets_proc_fops, NULL);
++
++ ret = xt_register_matches(xt_layer7_match,
++ ARRAY_SIZE(xt_layer7_match));
++ if(ret) {
++ remove_proc_entry("layer7_numpackets", init_net.proc_net);
++ kfree(l7_scratch);
++ }
++
++ return ret;
+}
+
+static void __exit xt_layer7_fini(void)
+{
+ remove_proc_entry("layer7_numpackets", init_net.proc_net);
+ xt_unregister_matches(xt_layer7_match, ARRAY_SIZE(xt_layer7_match));
++ kfree(l7_scratch);
+}
+
+module_init(xt_layer7_init);