[openwrt/openwrt] kernel: netfilter: fix dst entries in flowtable offload
LEDE Commits
lede-commits at lists.infradead.org
Fri Mar 23 12:57:02 PDT 2018
nbd pushed a commit to openwrt/openwrt.git, branch master:
https://git.lede-project.org/c89e338fe68fd5af61b80ef37c55a657721c6542
commit c89e338fe68fd5af61b80ef37c55a657721c6542
Author: Felix Fietkau <nbd at nbd.name>
AuthorDate: Thu Mar 15 18:03:22 2018 +0100
kernel: netfilter: fix dst entries in flowtable offload
Signed-off-by: Felix Fietkau <nbd at nbd.name>
---
...f_flow_table-clean-up-and-fix-dst-handlin.patch | 89 ++++++++++++++++++++++
.../650-netfilter-add-xt_OFFLOAD-target.patch | 33 +++++---
2 files changed, 111 insertions(+), 11 deletions(-)
diff --git a/target/linux/generic/backport-4.14/366-netfilter-nf_flow_table-clean-up-and-fix-dst-handlin.patch b/target/linux/generic/backport-4.14/366-netfilter-nf_flow_table-clean-up-and-fix-dst-handlin.patch
new file mode 100644
index 0000000..491f057
--- /dev/null
+++ b/target/linux/generic/backport-4.14/366-netfilter-nf_flow_table-clean-up-and-fix-dst-handlin.patch
@@ -0,0 +1,89 @@
+From: Felix Fietkau <nbd at nbd.name>
+Date: Thu, 15 Mar 2018 18:21:43 +0100
+Subject: [PATCH] netfilter: nf_flow_table: clean up and fix dst handling
+
+dst handling in the code is inconsistent and possibly wrong. In my test,
+skb_dst(skb) holds the dst entry after routing but before NAT, so the
+code could possibly return the same dst entry for both directions of a
+connection.
+Additionally, there was some confusion over the dst entry vs the address
+passed as parameter to rt_nexthop/rt6_nexthop.
+
+Do an explicit dst lookup for both ends of the connection and always use
+the source address for it. When running the IP hook, use the dst entry
+for the opposite direction for determining the route.
+
+Signed-off-by: Felix Fietkau <nbd at nbd.name>
+---
+
+--- a/net/netfilter/nf_flow_table_ip.c
++++ b/net/netfilter/nf_flow_table_ip.c
+@@ -238,7 +238,7 @@ nf_flow_offload_ip_hook(void *priv, stru
+
+ dir = tuplehash->tuple.dir;
+ flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
+- rt = (const struct rtable *)flow->tuplehash[dir].tuple.dst_cache;
++ rt = (const struct rtable *)flow->tuplehash[!dir].tuple.dst_cache;
+
+ if (unlikely(nf_flow_exceeds_mtu(skb, flow->tuplehash[dir].tuple.mtu)) &&
+ (ip_hdr(skb)->frag_off & htons(IP_DF)) != 0)
+@@ -455,7 +455,7 @@ nf_flow_offload_ipv6_hook(void *priv, st
+
+ dir = tuplehash->tuple.dir;
+ flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
+- rt = (struct rt6_info *)flow->tuplehash[dir].tuple.dst_cache;
++ rt = (struct rt6_info *)flow->tuplehash[!dir].tuple.dst_cache;
+
+ if (unlikely(nf_flow_exceeds_mtu(skb, flow->tuplehash[dir].tuple.mtu)))
+ return NF_ACCEPT;
+--- a/net/netfilter/nft_flow_offload.c
++++ b/net/netfilter/nft_flow_offload.c
+@@ -17,27 +17,38 @@ struct nft_flow_offload {
+ struct nft_flowtable *flowtable;
+ };
+
+-static int nft_flow_route(const struct nft_pktinfo *pkt,
+- const struct nf_conn *ct,
+- struct nf_flow_route *route,
+- enum ip_conntrack_dir dir)
++static struct dst_entry *
++nft_flow_dst(const struct nf_conn *ct, enum ip_conntrack_dir dir,
++ const struct nft_pktinfo *pkt)
+ {
+- struct dst_entry *this_dst = skb_dst(pkt->skb);
+- struct dst_entry *other_dst = NULL;
++ struct dst_entry *dst;
+ struct flowi fl;
+
+ memset(&fl, 0, sizeof(fl));
+ switch (nft_pf(pkt)) {
+ case NFPROTO_IPV4:
+- fl.u.ip4.daddr = ct->tuplehash[!dir].tuple.dst.u3.ip;
++ fl.u.ip4.daddr = ct->tuplehash[dir].tuple.src.u3.ip;
+ break;
+ case NFPROTO_IPV6:
+- fl.u.ip6.daddr = ct->tuplehash[!dir].tuple.dst.u3.in6;
++ fl.u.ip6.daddr = ct->tuplehash[dir].tuple.src.u3.in6;
+ break;
+ }
+
+- nf_route(nft_net(pkt), &other_dst, &fl, false, nft_pf(pkt));
+- if (!other_dst)
++ nf_route(nft_net(pkt), &dst, &fl, false, nft_pf(pkt));
++
++ return dst;
++}
++
++static int nft_flow_route(const struct nft_pktinfo *pkt,
++ const struct nf_conn *ct,
++ struct nf_flow_route *route,
++ enum ip_conntrack_dir dir)
++{
++ struct dst_entry *this_dst, *other_dst;
++
++ this_dst = nft_flow_dst(ct, dir, pkt);
++ other_dst = nft_flow_dst(ct, !dir, pkt);
++ if (!this_dst || !other_dst)
+ return -ENOENT;
+
+ route->tuple[dir].dst = this_dst;
diff --git a/target/linux/generic/hack-4.14/650-netfilter-add-xt_OFFLOAD-target.patch b/target/linux/generic/hack-4.14/650-netfilter-add-xt_OFFLOAD-target.patch
index 85826b8..7296cfa 100644
--- a/target/linux/generic/hack-4.14/650-netfilter-add-xt_OFFLOAD-target.patch
+++ b/target/linux/generic/hack-4.14/650-netfilter-add-xt_OFFLOAD-target.patch
@@ -98,7 +98,7 @@ Signed-off-by: Felix Fietkau <nbd at nbd.name>
obj-$(CONFIG_NETFILTER_XT_TARGET_LED) += xt_LED.o
--- /dev/null
+++ b/net/netfilter/xt_FLOWOFFLOAD.c
-@@ -0,0 +1,340 @@
+@@ -0,0 +1,351 @@
+/*
+ * Copyright (C) 2018 Felix Fietkau <nbd at nbd.name>
+ *
@@ -290,27 +290,38 @@ Signed-off-by: Felix Fietkau <nbd at nbd.name>
+ return false;
+}
+
-+static int
-+xt_flowoffload_route(struct sk_buff *skb, const struct nf_conn *ct,
-+ const struct xt_action_param *par,
-+ struct nf_flow_route *route, enum ip_conntrack_dir dir)
++static struct dst_entry *
++xt_flowoffload_dst(const struct nf_conn *ct, enum ip_conntrack_dir dir,
++ const struct xt_action_param *par)
+{
-+ struct dst_entry *this_dst = skb_dst(skb);
-+ struct dst_entry *other_dst = NULL;
++ struct dst_entry *dst;
+ struct flowi fl;
+
+ memset(&fl, 0, sizeof(fl));
+ switch (xt_family(par)) {
+ case NFPROTO_IPV4:
-+ fl.u.ip4.daddr = ct->tuplehash[!dir].tuple.dst.u3.ip;
++ fl.u.ip4.daddr = ct->tuplehash[dir].tuple.src.u3.ip;
+ break;
+ case NFPROTO_IPV6:
-+ fl.u.ip6.daddr = ct->tuplehash[!dir].tuple.dst.u3.in6;
++ fl.u.ip6.daddr = ct->tuplehash[dir].tuple.src.u3.in6;
+ break;
+ }
+
-+ nf_route(xt_net(par), &other_dst, &fl, false, xt_family(par));
-+ if (!other_dst)
++ nf_route(xt_net(par), &dst, &fl, false, xt_family(par));
++
++ return dst;
++}
++
++static int
++xt_flowoffload_route(struct sk_buff *skb, const struct nf_conn *ct,
++ const struct xt_action_param *par,
++ struct nf_flow_route *route, enum ip_conntrack_dir dir)
++{
++ struct dst_entry *this_dst, *other_dst;
++
++ this_dst = xt_flowoffload_dst(ct, dir, par);
++ other_dst = xt_flowoffload_dst(ct, !dir, par);
++ if (!this_dst || !other_dst)
+ return -ENOENT;
+
+ route->tuple[dir].dst = this_dst;
More information about the lede-commits
mailing list