From c89e338fe68fd5af61b80ef37c55a657721c6542 Mon Sep 17 00:00:00 2001 From: Felix Fietkau Date: Thu, 15 Mar 2018 18:03:22 +0100 Subject: [PATCH] kernel: netfilter: fix dst entries in flowtable offload Signed-off-by: Felix Fietkau --- ...w_table-clean-up-and-fix-dst-handlin.patch | 89 +++++++++++++++++++ .../650-netfilter-add-xt_OFFLOAD-target.patch | 33 ++++--- 2 files changed, 111 insertions(+), 11 deletions(-) create mode 100644 target/linux/generic/backport-4.14/366-netfilter-nf_flow_table-clean-up-and-fix-dst-handlin.patch 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 0000000000..491f057858 --- /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 +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 +--- + +--- 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 85826b8706..7296cfa6c4 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 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 + * @@ -290,27 +290,38 @@ Signed-off-by: Felix Fietkau + 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;