Netfilter/IPVS updates for net-next
Subsystems (6)
Pablo Neira Ayuso
maintainercover Netfilter/IPVS updates for net-next
Hi David,
The following patchset contains Netfilter/IPVS updates for your net-next tree. This batch comes with more input sanitization for xtables to address bug reports from fuzzers, preparation works to the flowtable infrastructure and assorted updates. In no particular order, they are:
1) Make sure userspace provides a valid standard target verdict, from Florian Westphal.
2) Sanitize error target size, also from Florian.
3) Validate that last rule in basechain matches underflow/policy since userspace assumes this when decoding the ruleset blob that comes from the kernel, from Florian.
4) Consolidate hook entry checks through xt_check_table_hooks(), patch from Florian.
5) Cap ruleset allocations at 512 mbytes, 134217728 rules and reject very large compat offset arrays, so we have a reasonable upper limit and fuzzers don't exercise the oom-killer. Patches from Florian.
6) Several WARN_ON checks on xtables mutex helper, from Florian.
7) xt_rateest now has a hashtable per net, from Cong Wang.
8) Consolidate counter allocation in xt_counters_alloc(), from Florian.
9) Earlier xt_table_unlock() call in {ip,ip6,arp,eb}tables, patch from Xin Long.
10) Set FLOW_OFFLOAD_DIR_* to IP_CT_DIR_* definitions, patch from Felix Fietkau.
11) Consolidate code through flow_offload_fill_dir(), also from Felix.
12) Inline ip6_dst_mtu_forward() just like ip_dst_mtu_maybe_forward() to remove a dependency with flowtable and ipv6.ko, from Felix.
13) Cache mtu size in flow_offload_tuple object, this is safe for forwarding as f87c10a8aa1e describes, from Felix.
14) Rename nf_flow_table.c to nf_flow_table_core.o, to simplify too modular infrastructure, from Felix.
15) Add rt0, rt2 and rt4 IPv6 routing extension support, patch from Ahmed Abdelsalam.
16) Remove unused parameter in nf_conncount_count(), from Yi-Hung Wei.
17) Support for counting only to nf_conncount infrastructure, patch from Yi-Hung Wei.
18) Add strict NFT_CT_{SRC_IP,DST_IP,SRC_IP6,DST_IP6} key datatypes to nft_ct.
19) Use boolean as return value from ipt_ah and from IPVS too, patch from Gustavo A. R. Silva.
20) Remove useless parameters in nfnl_acct_overquota() and nf_conntrack_broadcast_help(), from Taehee Yoo.
21) Use ipv6_addr_is_multicast() from xt_cluster, also from Taehee Yoo.
22) Statify nf_tables_obj_lookup_byhandle, patch from Fengguang Wu.
23) Fix typo in xt_limit, from Geert Uytterhoeven.
You can pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git
Thanks!
----------------------------------------------------------------
The following changes since commit ef3f6c256f0b4711a3ef1489797b95820be5ab01:
Merge branch 'mvpp2-jumbo-frames-support' (2018-03-05 12:55:55 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git HEAD
for you to fetch changes up to 90eee0957b655339d659c0b9bba64f5c90b2233b:
netfilter: nft_ct: add NFT_CT_{SRC,DST}_{IP,IP6} (2018-03-11 22:17:57 +0100)
----------------------------------------------------------------
Ahmed Abdelsalam (1):
netfilter: nf_tables: handle rt0 and rt2 properly
Cong Wang (1):
netfilter: make xt_rateest hash table per net
Felix Fietkau (5):
netfilter: nf_flow_table: use IP_CT_DIR_* values for FLOW_OFFLOAD_DIR_*
netfilter: nf_flow_table: clean up flow_offload_alloc
ipv6: make ip6_dst_mtu_forward inline
netfilter: nf_flow_table: cache mtu in struct flow_offload_tuple
netfilter: nf_flow_table: rename nf_flow_table.c to nf_flow_table_core.c
Florian Westphal (12):
netfilter: x_tables: check standard verdicts in core
netfilter: x_tables: check error target size too
netfilter: x_tables: move hook entry checks into core
netfilter: x_tables: enforce unique and ascending entry points
netfilter: x_tables: cap allocations at 512 mbyte
netfilter: x_tables: limit allocation requests for blob rule heads
netfilter: x_tables: add counters allocation wrapper
netfilter: compat: prepare xt_compat_init_offsets to return errors
netfilter: compat: reject huge allocation requests
netfilter: x_tables: make sure compat af mutex is held
netfilter: x_tables: ensure last rule in base chain matches underflow/policy
netfilter: x_tables: fix build with CONFIG_COMPAT=n
Geert Uytterhoeven (1):
netfilter: xt_limit: Spelling s/maxmum/maximum/
Gustavo A. R. Silva (2):
netfilter: ipt_ah: return boolean instead of integer
ipvs: use true and false for boolean values
Pablo Neira Ayuso (1):
netfilter: nft_ct: add NFT_CT_{SRC,DST}_{IP,IP6}Taehee Yoo (3):
netfilter: nfnetlink_acct: remove useless parameter
netfilter: xt_cluster: get rid of xt_cluster_ipv6_is_multicast
netfilter: nf_conntrack_broadcast: remove useless parameter
Xin Long (1):
netfilter: unlock xt_table earlier in __do_replace
Yi-Hung Wei (2):
netfilter: Refactor nf_conncount
netfilter: conncount: Support count only use case
kbuild test robot (1):
netfilter: nf_tables: nf_tables_obj_lookup_byhandle() can be static
include/linux/netfilter/nfnetlink_acct.h | 3 +-
include/linux/netfilter/x_tables.h | 5 +-
include/net/ip6_route.h | 21 +++
include/net/ipv6.h | 2 -
include/net/netfilter/nf_conntrack_count.h | 1 -
include/net/netfilter/nf_conntrack_helper.h | 3 +-
include/net/netfilter/nf_flow_table.h | 10 +-
include/net/netfilter/xt_rateest.h | 4 +-
include/uapi/linux/netfilter/nf_tables.h | 15 +-
net/bridge/netfilter/ebtables.c | 10 +-
net/ipv4/netfilter/arp_tables.c | 50 +++---
net/ipv4/netfilter/ip_tables.c | 48 +++---
net/ipv4/netfilter/ipt_ah.c | 2 +-
net/ipv4/netfilter/nf_flow_table_ipv4.c | 17 +-
net/ipv6/ip6_output.c | 22 ---
net/ipv6/netfilter/ip6_tables.c | 50 +++---
net/ipv6/netfilter/nf_flow_table_ipv6.c | 17 +-
net/netfilter/Makefile | 2 +
net/netfilter/ipvs/ip_vs_lblc.c | 4 +-
net/netfilter/ipvs/ip_vs_lblcr.c | 4 +-
net/netfilter/nf_conncount.c | 14 +-
net/netfilter/nf_conntrack_broadcast.c | 1 -
net/netfilter/nf_conntrack_netbios_ns.c | 5 +-
net/netfilter/nf_conntrack_snmp.c | 5 +-
.../{nf_flow_table.c => nf_flow_table_core.c} | 97 +++++------
net/netfilter/nf_tables_api.c | 8 +-
net/netfilter/nfnetlink_acct.c | 3 +-
net/netfilter/nft_ct.c | 38 +++++
net/netfilter/nft_exthdr.c | 3 +
net/netfilter/x_tables.c | 187 +++++++++++++++++++--
net/netfilter/xt_RATEEST.c | 91 +++++++---
net/netfilter/xt_cluster.c | 10 +-
net/netfilter/xt_connlimit.c | 4 +-
net/netfilter/xt_limit.c | 2 +-
net/netfilter/xt_nfacct.c | 2 +-
net/netfilter/xt_rateest.c | 10 +-
36 files changed, 500 insertions(+), 270 deletions(-)
rename net/netfilter/{nf_flow_table.c => nf_flow_table_core.c} (79%)Pablo Neira Ayuso
maintainerPATCH 1/30 netfilter: nf_tables: nf_tables_obj_lookup_byhandle() can be static
From: kbuild test robot <[email protected]>
--- net/netfilter/nf_tables_api.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
net/netfilter/nf_tables_api.c +4 -4
@@ -4328,9 +4328,9 @@ struct nft_object *nf_tables_obj_lookup(const struct nft_table *table, }EXPORT_SYMBOL_GPL (nf_tables_obj_lookup); -struct nft_object *nf_tables_obj_lookup_byhandle (const struct nft_table *table, -const struct nlattr *nla, -u32 objtype,u8 genmask) +static struct nft_object *nf_tables_obj_lookup_byhandle (const struct nft_table *table, +const struct nlattr *nla, +u32 objtype,u8 genmask) {struct nft_object *obj;
@@ -4850,7 +4850,7 @@ struct nft_flowtable *nf_tables_flowtable_lookup(const struct nft_table *table, }EXPORT_SYMBOL_GPL (nf_tables_flowtable_lookup); -struct nft_flowtable * +static struct nft_flowtable *nf_tables_flowtable_lookup_byhandle (const struct nft_table *table,const struct nlattr *nla,u8 genmask) {
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 2/30 netfilter: nfnetlink_acct: remove useless parameter
In reply to Pablo Neira AyusoFrom: Taehee Yoo <[email protected]>
parameter skb in nfnl_acct_overquota is not used anywhere.
--- include/linux/netfilter/nfnetlink_acct.h | 3 +-- net/netfilter/nfnetlink_acct.c | 3 +-- net/netfilter/xt_nfacct.c | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-)
include/linux/netfilter/nfnetlink_acct.h +1 -2
@@ -16,6 +16,5 @@ struct nf_acct;struct nf_acct *nfnl_acct_find_get (struct net *net,const char *filter_name);void nfnl_acct_put (struct nf_acct *acct);void nfnl_acct_update (const struct sk_buff *skb,struct nf_acct *nfacct); -int nfnl_acct_overquota (struct net *net,const struct sk_buff *skb, -struct nf_acct *nfacct); +int nfnl_acct_overquota (struct net *net,struct nf_acct *nfacct);#endif /* _NFNL_ACCT_H */
net/netfilter/nfnetlink_acct.c +1 -2
@@ -467,8 +467,7 @@ static void nfnl_overquota_report(struct net *net, struct nf_acct *nfacct) GFP_ATOMIC); } -int nfnl_acct_overquota (struct net *net,const struct sk_buff *skb, -struct nf_acct *nfacct) +int nfnl_acct_overquota (struct net *net,struct nf_acct *nfacct) {u64 now;u64 *quota;
net/netfilter/xt_nfacct.c +1 -1
@@ -28,7 +28,7 @@ static bool nfacct_mt(const struct sk_buff *skb, struct xt_action_param *par)nfnl_acct_update (skb, info->nfacct); - overquota =nfnl_acct_overquota (xt_net (par), skb, info->nfacct); + overquota =nfnl_acct_overquota (xt_net (par), info->nfacct);return overquota ==NFACCT_UNDERQUOTA ? false : true; }
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 3/30 netfilter: xt_cluster: get rid of xt_cluster_ipv6_is_multicast
In reply to Pablo Neira AyusoFrom: Taehee Yoo <[email protected]>
If use the ipv6_addr_is_multicast instead of xt_cluster_ipv6_is_multicast, then we can reduce code size.
--- net/netfilter/xt_cluster.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
net/netfilter/xt_cluster.c +1 -9
@@ -60,13 +60,6 @@ xt_cluster_hash(const struct nf_conn *ct, }static inline bool -xt_cluster_ipv6_is_multicast (const struct in6_addr *addr) -{ -__be32 st = addr->s6_addr32[0 ]; -return ((st &htonl (0xFF000000 )) ==htonl (0xFF000000 )); -} - -static inline bool xt_cluster_is_multicast_addr (const struct sk_buff *skb,u_int8_t family) {bool is_multicast = false;
@@ -76,8 +69,7 @@ xt_cluster_is_multicast_addr(const struct sk_buff *skb, u_int8_t family) is_multicast =ipv4_is_multicast (ip_hdr (skb)->daddr);break ;case NFPROTO_IPV6 : - is_multicast = -xt_cluster_ipv6_is_multicast (&ipv6_hdr (skb)->daddr); + is_multicast =ipv6_addr_is_multicast (&ipv6_hdr (skb)->daddr);break ;default :WARN_ON (1 );
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 4/30 netfilter: nf_conntrack_broadcast: remove useless parameter
In reply to Pablo Neira AyusoFrom: Taehee Yoo <[email protected]>
parameter protoff in nf_conntrack_broadcast_help is not used anywhere.
--- include/net/netfilter/nf_conntrack_helper.h | 3 +-- net/netfilter/nf_conntrack_broadcast.c | 1 - net/netfilter/nf_conntrack_netbios_ns.c | 5 +++-- net/netfilter/nf_conntrack_snmp.c | 5 +++-- 4 files changed, 7 insertions(+), 7 deletions(-)
include/net/netfilter/nf_conntrack_helper.h +1 -2
@@ -132,8 +132,7 @@ void nf_conntrack_helper_pernet_fini(struct net *net);int nf_conntrack_helper_init (void );void nf_conntrack_helper_fini (void ); -int nf_conntrack_broadcast_help (struct sk_buff *skb,unsigned int protoff, -struct nf_conn *ct, +int nf_conntrack_broadcast_help (struct sk_buff *skb,struct nf_conn *ct,enum ip_conntrack_info ctinfo,unsigned int timeout);
net/netfilter/nf_conntrack_broadcast.c +0 -1
@@ -20,7 +20,6 @@#include <net/netfilter/nf_conntrack_expect.h> int nf_conntrack_broadcast_help (struct sk_buff *skb, -unsigned int protoff,struct nf_conn *ct,enum ip_conntrack_info ctinfo,unsigned int timeout)
net/netfilter/nf_conntrack_netbios_ns.c +3 -2
@@ -41,9 +41,10 @@ static struct nf_conntrack_expect_policy exp_policy = { };static int netbios_ns_help (struct sk_buff *skb,unsigned int protoff, -struct nf_conn *ct,enum ip_conntrack_info ctinfo) +struct nf_conn *ct, +enum ip_conntrack_info ctinfo) { -return nf_conntrack_broadcast_help (skb, protoff, ct, ctinfo, timeout); +return nf_conntrack_broadcast_help (skb, ct, ctinfo, timeout); }static struct nf_conntrack_helper helper__read_mostly = {
net/netfilter/nf_conntrack_snmp.c +3 -2
@@ -36,11 +36,12 @@ int (*nf_nat_snmp_hook)(struct sk_buff *skb,EXPORT_SYMBOL_GPL (nf_nat_snmp_hook);static int snmp_conntrack_help (struct sk_buff *skb,unsigned int protoff, -struct nf_conn *ct,enum ip_conntrack_info ctinfo) +struct nf_conn *ct, +enum ip_conntrack_info ctinfo) {typeof (nf_nat_snmp_hook) nf_nat_snmp; -nf_conntrack_broadcast_help (skb, protoff, ct, ctinfo, timeout); +nf_conntrack_broadcast_help (skb, ct, ctinfo, timeout); nf_nat_snmp =rcu_dereference (nf_nat_snmp_hook);if (nf_nat_snmp && ct->status &IPS_NAT_MASK )
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 5/30 netfilter: ipt_ah: return boolean instead of integer
In reply to Pablo Neira AyusoFrom: "Gustavo A. R. Silva" <[email protected]>
Return statements in functions returning bool should use true/false instead of 1/0.
This issue was detected with the help of Coccinelle.
--- net/ipv4/netfilter/ipt_ah.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv4/netfilter/ipt_ah.c +1 -1
@@ -47,7 +47,7 @@ static bool ah_mt(const struct sk_buff *skb, struct xt_action_param *par)*/ pr_debug ("Dropping evil AH tinygram.\n" ); par->hotdrop = true; -return 0 ; +return false; } return spi_match(ahinfo->spis[0], ahinfo->spis[1],
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 6/30 netfilter: unlock xt_table earlier in __do_replace
In reply to Pablo Neira AyusoFrom: Xin Long <[email protected]>
Now it's doing cleanup_entry for oldinfo under the xt_table lock, but it's not really necessary. After the replacement job is done in xt_replace_table, oldinfo is not used elsewhere any more, and it can be freed without xt_table lock safely.
The important thing is that rtnl_lock is called in some xt_target destroy, which means rtnl_lock, a big lock is used in xt_table lock, a smaller one. It usually could be the reason why a dead lock may happen.
Besides, all xt_target/match checkentry is called out of xt_table lock. It's better also to move all cleanup_entry calling out of xt_table lock, just as do_replace_finish does for ebtables.
--- net/ipv4/netfilter/arp_tables.c | 3 ++- net/ipv4/netfilter/ip_tables.c | 3 ++- net/ipv6/netfilter/ip6_tables.c | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-)
net/ipv4/netfilter/arp_tables.c +2 -1
@@ -925,6 +925,8 @@ static int __do_replace(struct net *net, const char *name, (newinfo->number <= oldinfo->initial_entries))module_put (t->me); +xt_table_unlock (t); +get_old_counters (oldinfo, counters);/* Decrease module usage counts and free resource */
@@ -939,7 +941,6 @@ static int __do_replace(struct net *net, const char *name,net_warn_ratelimited ("arptables: counters copy to user failed while replacing table\n" ); }vfree (counters); -xt_table_unlock (t);return ret; put_module:
net/ipv4/netfilter/ip_tables.c +2 -1
@@ -1087,6 +1087,8 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks, (newinfo->number <= oldinfo->initial_entries))module_put (t->me); +xt_table_unlock (t); +get_old_counters (oldinfo, counters);/* Decrease module usage counts and free resource */
@@ -1100,7 +1102,6 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,net_warn_ratelimited ("iptables: counters copy to user failed while replacing table\n" ); }vfree (counters); -xt_table_unlock (t);return ret; put_module:
net/ipv6/netfilter/ip6_tables.c +2 -1
@@ -1105,6 +1105,8 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks, (newinfo->number <= oldinfo->initial_entries))module_put (t->me); +xt_table_unlock (t); +get_old_counters (oldinfo, counters);/* Decrease module usage counts and free resource */
@@ -1118,7 +1120,6 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,net_warn_ratelimited ("ip6tables: counters copy to user failed while replacing table\n" ); }vfree (counters); -xt_table_unlock (t);return ret; put_module:
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 7/30 netfilter: x_tables: check standard verdicts in core
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
Userspace must provide a valid verdict to the standard target.
The verdict can be either a jump (signed int > 0), or a return code.
Allowed return codes are either RETURN (pop from stack), NF_ACCEPT, DROP and QUEUE (latter is allowed for legacy reasons).
Jump offsets (verdict > 0) are checked in more detail later on when loop-detection is performed.
--- net/ipv4/netfilter/arp_tables.c | 5 ----- net/ipv4/netfilter/ip_tables.c | 5 ----- net/ipv6/netfilter/ip6_tables.c | 5 ----- net/netfilter/x_tables.c | 49 ++++++++++++++++++++++++++++++++++++----- 4 files changed, 43 insertions(+), 21 deletions(-)
net/ipv4/netfilter/arp_tables.c +0 -5
@@ -334,11 +334,6 @@ static int mark_source_chains(const struct xt_table_info *newinfo, t->verdict < 0) || visited) {unsigned int oldpos, size; -if ((strcmp (t->target.u.user.name, -XT_STANDARD_TARGET ) ==0 ) && - t->verdict < -NF_MAX_VERDICT -1 ) -return 0 ; -/* Return: backtrack through the last * big jump. */
net/ipv4/netfilter/ip_tables.c +0 -5
@@ -402,11 +402,6 @@ mark_source_chains(const struct xt_table_info *newinfo, t->verdict < 0) || visited) {unsigned int oldpos, size; -if ((strcmp (t->target.u.user.name, -XT_STANDARD_TARGET ) ==0 ) && - t->verdict < -NF_MAX_VERDICT -1 ) -return 0 ; -/* Return: backtrack through the last big jump. */ do {
net/ipv6/netfilter/ip6_tables.c +0 -5
@@ -420,11 +420,6 @@ mark_source_chains(const struct xt_table_info *newinfo, t->verdict < 0) || visited) {unsigned int oldpos, size; -if ((strcmp (t->target.u.user.name, -XT_STANDARD_TARGET ) ==0 ) && - t->verdict < -NF_MAX_VERDICT -1 ) -return 0 ; -/* Return: backtrack through the last big jump. */ do {
net/netfilter/x_tables.c +43 -6
67 lines not on this page · raw message
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 8/30 netfilter: x_tables: check error target size too
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
Check that userspace ERROR target (custom user-defined chains) match expected format, and the chain name is null terminated.
This is irrelevant for kernel, but iptables itself relies on sane input when it dumps rules from kernel.
--- net/netfilter/x_tables.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
net/netfilter/x_tables.c +23 -0
@@ -654,6 +654,11 @@ struct compat_xt_standard_target {compat_uint_t verdict; }; +struct compat_xt_error_target { +struct compat_xt_entry_target t; +char errorname[XT_FUNCTION_MAXNAMELEN ]; +}; +static bool verdict_ok (int verdict) {if (verdict >0 )
@@ -679,6 +684,12 @@ static bool verdict_ok(int verdict)return false; } +static bool error_tg_ok (unsigned int usersize,unsigned int kernsize, +const char *msg,unsigned int msglen) +{ +return usersize == kernsize &&strnlen (msg, msglen) < msglen; +} +int xt_compat_check_entry_offsets (const void *base,const char *elems,unsigned int target_offset,unsigned int next_offset)
@@ -708,6 +719,12 @@ int xt_compat_check_entry_offsets(const void *base, const char *elems,if (!verdict_ok (st->verdict))return -EINVAL ; + }else if (strcmp (t->u.user.name,XT_ERROR_TARGET ) ==0 ) { +const struct compat_xt_error_target *et = (const void *)t; + +if (!error_tg_ok (t->u.target_size,sizeof (*et), + et->errorname,sizeof (et->errorname))) +return -EINVAL ; }/* compat_xt_entry match has less strict alignment requirements,
@@ -796,6 +813,12 @@ int xt_check_entry_offsets(const void *base,if (!verdict_ok (st->verdict))return -EINVAL ; + }else if (strcmp (t->u.user.name,XT_ERROR_TARGET ) ==0 ) { +const struct xt_error_target *et = (const void *)t; + +if (!error_tg_ok (t->u.target_size,sizeof (*et), + et->errorname,sizeof (et->errorname))) +return -EINVAL ; } return xt_check_entry_match(elems, base + target_offset,
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 9/30 netfilter: x_tables: move hook entry checks into core
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
Allow followup patch to change on location instead of three.
--- include/linux/netfilter/x_tables.h | 2 ++ net/ipv4/netfilter/arp_tables.c | 13 +++---------- net/ipv4/netfilter/ip_tables.c | 13 +++---------- net/ipv6/netfilter/ip6_tables.c | 13 +++---------- net/netfilter/x_tables.c | 29 +++++++++++++++++++++++++++++ 5 files changed, 40 insertions(+), 30 deletions(-)
include/linux/netfilter/x_tables.h +2 -0
@@ -281,6 +281,8 @@ int xt_check_entry_offsets(const void *base, const char *elems,unsigned int target_offset, unsigned int next_offset); +int xt_check_table_hooks (const struct xt_table_info *info,unsigned int valid_hooks); +unsigned int *xt_alloc_entry_offsets (unsigned int size);bool xt_find_jump_offset (const unsigned int *offsets,unsigned int target,unsigned int size);
net/ipv4/netfilter/arp_tables.c +3 -10
@@ -555,16 +555,9 @@ static int translate_table(struct xt_table_info *newinfo, void *entry0,if (i != repl->num_entries) goto out_free; -/* Check hooks all assigned */ -for (i =0 ; i <NF_ARP_NUMHOOKS ; i++) { -/* Only hooks which are valid */ -if (!(repl->valid_hooks & (1 << i))) -continue ; -if (newinfo->hook_entry[i] ==0xFFFFFFFF ) - goto out_free; -if (newinfo->underflow[i] ==0xFFFFFFFF ) - goto out_free; - } + ret =xt_check_table_hooks (newinfo, repl->valid_hooks); +if (ret) + goto out_free;if (!mark_source_chains (newinfo, repl->valid_hooks, entry0, offsets)) { ret = -ELOOP ;
net/ipv4/netfilter/ip_tables.c +3 -10
@@ -702,16 +702,9 @@ translate_table(struct net *net, struct xt_table_info *newinfo, void *entry0,if (i != repl->num_entries) goto out_free; -/* Check hooks all assigned */ -for (i =0 ; i <NF_INET_NUMHOOKS ; i++) { -/* Only hooks which are valid */ -if (!(repl->valid_hooks & (1 << i))) -continue ; -if (newinfo->hook_entry[i] ==0xFFFFFFFF ) - goto out_free; -if (newinfo->underflow[i] ==0xFFFFFFFF ) - goto out_free; - } + ret =xt_check_table_hooks (newinfo, repl->valid_hooks); +if (ret) + goto out_free;if (!mark_source_chains (newinfo, repl->valid_hooks, entry0, offsets)) { ret = -ELOOP ;
net/ipv6/netfilter/ip6_tables.c +3 -10
@@ -720,16 +720,9 @@ translate_table(struct net *net, struct xt_table_info *newinfo, void *entry0,if (i != repl->num_entries) goto out_free; -/* Check hooks all assigned */ -for (i =0 ; i <NF_INET_NUMHOOKS ; i++) { -/* Only hooks which are valid */ -if (!(repl->valid_hooks & (1 << i))) -continue ; -if (newinfo->hook_entry[i] ==0xFFFFFFFF ) - goto out_free; -if (newinfo->underflow[i] ==0xFFFFFFFF ) - goto out_free; - } + ret =xt_check_table_hooks (newinfo, repl->valid_hooks); +if (ret) + goto out_free;if (!mark_source_chains (newinfo, repl->valid_hooks, entry0, offsets)) { ret = -ELOOP ;
net/netfilter/x_tables.c +29 -0
@@ -518,6 +518,35 @@ static int xt_check_entry_match(const char *match, const char *target,return 0 ; } +/** xt_check_table_hooks - check hook entry points are sane +* +* @info xt_table_info to check +* @valid_hooks - hook entry points that we can enter from +* +* Validates that the hook entry and underflows points are set up. +* +* Return: 0 on success, negative errno on failure. +*/ +int xt_check_table_hooks (const struct xt_table_info *info,unsigned int valid_hooks) +{ +unsigned int i; + +BUILD_BUG_ON (ARRAY_SIZE (info->hook_entry) !=ARRAY_SIZE (info->underflow)); + +for (i =0 ; i <ARRAY_SIZE (info->hook_entry); i++) { +if (!(valid_hooks & (1 << i))) +continue ; + +if (info->hook_entry[i] ==0xFFFFFFFF ) +return -EINVAL ; +if (info->underflow[i] ==0xFFFFFFFF ) +return -EINVAL ; + } + +return 0 ; +} +EXPORT_SYMBOL (xt_check_table_hooks); +#ifdef CONFIG_COMPAT int xt_compat_add_offset (u_int8_t af,unsigned int offset,int delta) {
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 10/30 netfilter: x_tables: enforce unique and ascending entry points
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
Harmless from kernel point of view, but iptables assumes that this is true when decoding a ruleset.
iptables walks the dumped blob from kernel, and, for each entry that creates a new chain it prints out rule/chain information. Base chains (hook entry points) are thus only shown when they appear in the rule blob. One base chain that is referenced multiple times in hook blob is then only printed once.
--- net/netfilter/x_tables.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
net/netfilter/x_tables.c +30 -1
@@ -529,10 +529,15 @@ static int xt_check_entry_match(const char *match, const char *target,*/ int xt_check_table_hooks (const struct xt_table_info *info,unsigned int valid_hooks) { -unsigned int i; +const char *err ="unsorted underflow" ; +unsigned int i, max_uflow, max_entry; +bool check_hooks = false;BUILD_BUG_ON (ARRAY_SIZE (info->hook_entry) !=ARRAY_SIZE (info->underflow)); + max_entry =0 ; + max_uflow =0 ; +for (i =0 ; i <ARRAY_SIZE (info->hook_entry); i++) {if (!(valid_hooks & (1 << i)))continue ;
@@ -541,9 +546,33 @@ int xt_check_table_hooks(const struct xt_table_info *info, unsigned int valid_horeturn -EINVAL ;if (info->underflow[i] ==0xFFFFFFFF )return -EINVAL ; + +if (check_hooks) { +if (max_uflow > info->underflow[i]) + goto error; + +if (max_uflow == info->underflow[i]) { + err ="duplicate underflow" ; + goto error; + } +if (max_entry > info->hook_entry[i]) { + err ="unsorted entry" ; + goto error; + } +if (max_entry == info->hook_entry[i]) { + err ="duplicate entry" ; + goto error; + } + } + max_entry = info->hook_entry[i]; + max_uflow = info->underflow[i]; + check_hooks = true; }return 0 ; +error: +pr_err_ratelimited ("%s at hook %d\n" , err, i); +return -EINVAL ; }EXPORT_SYMBOL (xt_check_table_hooks);
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 11/30 netfilter: x_tables: cap allocations at 512 mbyte
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
Arbitrary limit, however, this still allows huge rulesets (> 1 million rules). This helps with automated fuzzer as it prevents oom-killer invocation.
--- net/netfilter/x_tables.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
net/netfilter/x_tables.c +2 -1
@@ -40,6 +40,7 @@ MODULE_AUTHOR("Harald Welte <[email protected]>");MODULE_DESCRIPTION ("{ip,ip6,arp,eb}_tables backend module" );#define XT_PCPU_BLOCK_SIZE 4096 +#define XT_MAX_TABLE_SIZE (512 * 1024 * 1024)struct compat_delta {unsigned int offset;/* offset in kernel */
@@ -1117,7 +1118,7 @@ struct xt_table_info *xt_alloc_table_info(unsigned int size)struct xt_table_info *info =NULL ;size_t sz =sizeof (*info) + size; -if (sz <sizeof (*info)) +if (sz <sizeof (*info) || sz >=XT_MAX_TABLE_SIZE )return NULL ;/* __GFP_NORETRY is not fully supported by kvmalloc but it should
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 12/30 netfilter: x_tables: limit allocation requests for blob rule heads
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
This is a very conservative limit (134217728 rules), but good enough to not trigger frequent oom from syzkaller.
--- net/netfilter/x_tables.c | 3 +++ 1 file changed, 3 insertions(+)
net/netfilter/x_tables.c +3 -0
@@ -894,6 +894,9 @@ EXPORT_SYMBOL(xt_check_entry_offsets);*/ unsigned int *xt_alloc_entry_offsets (unsigned int size) { +if (size >XT_MAX_TABLE_SIZE /sizeof (unsigned int )) +return NULL ; +return kvmalloc_array (size,sizeof (unsigned int ),GFP_KERNEL | __GFP_ZERO); }
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 13/30 netfilter: x_tables: add counters allocation wrapper
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
allows to have size checks in a single spot.
This is supposed to reduce oom situations when fuzz-testing xtables.
--- include/linux/netfilter/x_tables.h | 1 + net/ipv4/netfilter/arp_tables.c | 2 +- net/ipv4/netfilter/ip_tables.c | 2 +- net/ipv6/netfilter/ip6_tables.c | 2 +- net/netfilter/x_tables.c | 15 +++++++++++++++ 5 files changed, 19 insertions(+), 3 deletions(-)
include/linux/netfilter/x_tables.h +1 -0
@@ -301,6 +301,7 @@ int xt_data_to_user(void __user *dst, const void *src,void *xt_copy_counters_from_user (const void __user *user,unsigned int len,struct xt_counters_info *info,bool compat); +struct xt_counters *xt_counters_alloc (unsigned int counters); struct xt_table *xt_register_table(struct net *net, const struct xt_table *table,
net/ipv4/netfilter/arp_tables.c +1 -1
@@ -883,7 +883,7 @@ static int __do_replace(struct net *net, const char *name,struct arpt_entry *iter; ret =0 ; - counters =vzalloc (num_counters *sizeof (struct xt_counters )); + counters =xt_counters_alloc (num_counters);if (!counters) { ret = -ENOMEM ; goto out;
net/ipv4/netfilter/ip_tables.c +1 -1
@@ -1045,7 +1045,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,struct ipt_entry *iter; ret =0 ; - counters =vzalloc (num_counters *sizeof (struct xt_counters )); + counters =xt_counters_alloc (num_counters);if (!counters) { ret = -ENOMEM ; goto out;
net/ipv6/netfilter/ip6_tables.c +1 -1
@@ -1063,7 +1063,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,struct ip6t_entry *iter; ret =0 ; - counters =vzalloc (num_counters *sizeof (struct xt_counters )); + counters =xt_counters_alloc (num_counters);if (!counters) { ret = -ENOMEM ; goto out;
net/netfilter/x_tables.c +15 -0
@@ -1290,6 +1290,21 @@ static int xt_jumpstack_alloc(struct xt_table_info *i)return 0 ; } +struct xt_counters *xt_counters_alloc (unsigned int counters) +{ +struct xt_counters *mem; + +if (counters ==0 || counters >INT_MAX /sizeof (*mem)) +return NULL ; + + counters *=sizeof (*mem); +if (counters >XT_MAX_TABLE_SIZE ) +return NULL ; + +return vzalloc (counters); +} +EXPORT_SYMBOL (xt_counters_alloc); +struct xt_table_info * xt_replace_table(struct xt_table *table, unsigned int num_counters,
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 14/30 netfilter: compat: prepare xt_compat_init_offsets to return errors
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
should have no impact, function still always returns 0. This patch is only to ease review.
--- include/linux/netfilter/x_tables.h | 2 +- net/bridge/netfilter/ebtables.c | 10 ++++++++-- net/ipv4/netfilter/arp_tables.c | 10 +++++++--- net/ipv4/netfilter/ip_tables.c | 8 ++++++-- net/ipv6/netfilter/ip6_tables.c | 10 +++++++--- net/netfilter/x_tables.c | 4 +++- 6 files changed, 32 insertions(+), 12 deletions(-)
include/linux/netfilter/x_tables.h +1 -1
@@ -510,7 +510,7 @@ void xt_compat_unlock(u_int8_t af);int xt_compat_add_offset (u_int8_t af,unsigned int offset,int delta);void xt_compat_flush_offsets (u_int8_t af); -void xt_compat_init_offsets (u_int8_t af,unsigned int number); +int xt_compat_init_offsets (u8 af,unsigned int number);int xt_compat_calc_jump (u_int8_t af,unsigned int offset);int xt_compat_match_offset (const struct xt_match *match);
net/bridge/netfilter/ebtables.c +8 -2
@@ -1819,10 +1819,14 @@ static int compat_table_info(const struct ebt_table_info *info, {unsigned int size = info->entries_size;const void *entries = info->entries; +int ret; newinfo->entries_size = size; -xt_compat_init_offsets (NFPROTO_BRIDGE , info->nentries); + ret =xt_compat_init_offsets (NFPROTO_BRIDGE , info->nentries); +if (ret) +return ret; +return EBT_ENTRY_ITERATE (entries, size, compat_calc_entry, info, entries, newinfo); }
@@ -2245,7 +2249,9 @@ static int compat_do_replace(struct net *net, void __user *user,xt_compat_lock (NFPROTO_BRIDGE ); -xt_compat_init_offsets (NFPROTO_BRIDGE , tmp.nentries); + ret =xt_compat_init_offsets (NFPROTO_BRIDGE , tmp.nentries); +if (ret <0 ) + goto out_unlock; ret =compat_copy_entries (entries_tmp, tmp.entries_size, &state);if (ret <0 ) goto out_unlock;
net/ipv4/netfilter/arp_tables.c +7 -3
@@ -769,7 +769,9 @@ static int compat_table_info(const struct xt_table_info *info,memcpy (newinfo, info, offsetof(struct xt_table_info , entries)); newinfo->initial_entries =0 ; loc_cpu_entry = info->entries; -xt_compat_init_offsets (NFPROTO_ARP , info->number); + ret =xt_compat_init_offsets (NFPROTO_ARP , info->number); +if (ret) +return ret; xt_entry_foreach(iter, loc_cpu_entry, info->size) { ret =compat_calc_entry (iter, info, loc_cpu_entry, newinfo);if (ret !=0 )
@@ -1156,7 +1158,7 @@ static int translate_compat_table(struct xt_table_info **pinfo,struct compat_arpt_entry *iter0;struct arpt_replace repl;unsigned int size; -int ret =0 ; +int ret; info = *pinfo; entry0 = *pentry0;
@@ -1165,7 +1167,9 @@ static int translate_compat_table(struct xt_table_info **pinfo, j =0 ;xt_compat_lock (NFPROTO_ARP ); -xt_compat_init_offsets (NFPROTO_ARP , compatr->num_entries); + ret =xt_compat_init_offsets (NFPROTO_ARP , compatr->num_entries); +if (ret) + goto out_unlock;/* Walk through entries, checking offsets. */ xt_entry_foreach(iter0, entry0, compatr->size) { ret = check_compat_entry_size_and_hooks(iter0, info, &size,
net/ipv4/netfilter/ip_tables.c +6 -2
@@ -933,7 +933,9 @@ static int compat_table_info(const struct xt_table_info *info,memcpy (newinfo, info, offsetof(struct xt_table_info , entries)); newinfo->initial_entries =0 ; loc_cpu_entry = info->entries; -xt_compat_init_offsets (AF_INET , info->number); + ret =xt_compat_init_offsets (AF_INET , info->number); +if (ret) +return ret; xt_entry_foreach(iter, loc_cpu_entry, info->size) { ret =compat_calc_entry (iter, info, loc_cpu_entry, newinfo);if (ret !=0 )
@@ -1407,7 +1409,9 @@ translate_compat_table(struct net *net, j =0 ;xt_compat_lock (AF_INET ); -xt_compat_init_offsets (AF_INET , compatr->num_entries); + ret =xt_compat_init_offsets (AF_INET , compatr->num_entries); +if (ret) + goto out_unlock;/* Walk through entries, checking offsets. */ xt_entry_foreach(iter0, entry0, compatr->size) { ret = check_compat_entry_size_and_hooks(iter0, info, &size,
net/ipv6/netfilter/ip6_tables.c +7 -3
@@ -950,7 +950,9 @@ static int compat_table_info(const struct xt_table_info *info,memcpy (newinfo, info, offsetof(struct xt_table_info , entries)); newinfo->initial_entries =0 ; loc_cpu_entry = info->entries; -xt_compat_init_offsets (AF_INET6 , info->number); + ret =xt_compat_init_offsets (AF_INET6 , info->number); +if (ret) +return ret; xt_entry_foreach(iter, loc_cpu_entry, info->size) { ret =compat_calc_entry (iter, info, loc_cpu_entry, newinfo);if (ret !=0 )
@@ -1414,7 +1416,7 @@ translate_compat_table(struct net *net,struct compat_ip6t_entry *iter0;struct ip6t_replace repl;unsigned int size; -int ret =0 ; +int ret; info = *pinfo; entry0 = *pentry0;
@@ -1423,7 +1425,9 @@ translate_compat_table(struct net *net, j =0 ;xt_compat_lock (AF_INET6 ); -xt_compat_init_offsets (AF_INET6 , compatr->num_entries); + ret =xt_compat_init_offsets (AF_INET6 , compatr->num_entries); +if (ret) + goto out_unlock;/* Walk through entries, checking offsets. */ xt_entry_foreach(iter0, entry0, compatr->size) { ret = check_compat_entry_size_and_hooks(iter0, info, &size,
net/netfilter/x_tables.c +3 -1
@@ -632,10 +632,12 @@ int xt_compat_calc_jump(u_int8_t af, unsigned int offset) }EXPORT_SYMBOL_GPL (xt_compat_calc_jump); -void xt_compat_init_offsets (u_int8_t af,unsigned int number) +int xt_compat_init_offsets (u8 af,unsigned int number) { xt[af].number = number; xt[af].cur =0 ; + +return 0 ; }EXPORT_SYMBOL (xt_compat_init_offsets);
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 15/30 netfilter: compat: reject huge allocation requests
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
no need to bother even trying to allocating huge compat offset arrays, such ruleset is rejected later on anyway becaus we refuse to allocate overly large rule blobs.
However, compat translation happens before blob allocation, so we should add a check there too.
This is supposed to help with fuzzing by avoiding oom-killer.
--- net/netfilter/x_tables.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
net/netfilter/x_tables.c +18 -8
@@ -582,14 +582,8 @@ int xt_compat_add_offset(u_int8_t af, unsigned int offset, int delta) {struct xt_af *xp = &xt[af]; -if (!xp->compat_tab) { -if (!xp->number) -return -EINVAL ; - xp->compat_tab =vmalloc (sizeof (struct compat_delta ) * xp->number); -if (!xp->compat_tab) -return -ENOMEM ; - xp->cur =0 ; - } +if (WARN_ON (!xp->compat_tab)) +return -ENOMEM ;if (xp->cur >= xp->number)return -EINVAL ;
@@ -634,6 +628,22 @@ EXPORT_SYMBOL_GPL(xt_compat_calc_jump);int xt_compat_init_offsets (u8 af,unsigned int number) { +size_t mem; + +if (!number || number > (INT_MAX /sizeof (struct compat_delta ))) +return -EINVAL ; + +if (WARN_ON (xt[af].compat_tab)) +return -EINVAL ; + + mem =sizeof (struct compat_delta ) * number; +if (mem >XT_MAX_TABLE_SIZE ) +return -ENOMEM ; + + xt[af].compat_tab =vmalloc (mem); +if (!xt[af].compat_tab) +return -ENOMEM ; + xt[af].number = number; xt[af].cur =0 ;
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 16/30 netfilter: x_tables: make sure compat af mutex is held
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
--- net/netfilter/x_tables.c | 6 ++++++ 1 file changed, 6 insertions(+)
net/netfilter/x_tables.c +6 -0
@@ -582,6 +582,8 @@ int xt_compat_add_offset(u_int8_t af, unsigned int offset, int delta) {struct xt_af *xp = &xt[af]; +WARN_ON (!mutex_is_locked (&xt[af].compat_mutex)); +if (WARN_ON (!xp->compat_tab))return -ENOMEM ;
@@ -599,6 +601,8 @@ EXPORT_SYMBOL_GPL(xt_compat_add_offset);void xt_compat_flush_offsets (u_int8_t af) { +WARN_ON (!mutex_is_locked (&xt[af].compat_mutex)); +if (xt[af].compat_tab) {vfree (xt[af].compat_tab); xt[af].compat_tab =NULL ;
@@ -630,6 +634,8 @@ int xt_compat_init_offsets(u8 af, unsigned int number) {size_t mem; +WARN_ON (!mutex_is_locked (&xt[af].compat_mutex)); +if (!number || number > (INT_MAX /sizeof (struct compat_delta )))return -EINVAL ;
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 17/30 netfilter: x_tables: ensure last rule in base chain matches underflow/policy
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
Harmless from kernel point of view, but again iptables assumes that this is true when decoding ruleset coming from kernel.
If a (syzkaller generated) ruleset doesn't have the underflow/policy stored as the last rule in the base chain, then iptables will abort() because it doesn't find the chain policy.
libiptc assumes that the policy is the last rule in the basechain, which is only true for iptables-generated rulesets.
Unfortunately this needs code duplication -- the functions need the struct layout of the rule head, but that is different for ip/ip6/arptables.
NB: pr_warn could be pr_debug but in case this break rulesets somehow its useful to know why blob was rejected.
--- net/ipv4/netfilter/arp_tables.c | 17 ++++++++++++++++- net/ipv4/netfilter/ip_tables.c | 17 ++++++++++++++++- net/ipv6/netfilter/ip6_tables.c | 17 ++++++++++++++++- 3 files changed, 48 insertions(+), 3 deletions(-)
net/ipv4/netfilter/arp_tables.c +16 -1
@@ -309,10 +309,13 @@ static int mark_source_chains(const struct xt_table_info *newinfo,for (hook =0 ; hook <NF_ARP_NUMHOOKS ; hook++) {unsigned int pos = newinfo->hook_entry[hook];struct arpt_entry *e = entry0 + pos; +unsigned int last_pos, depth;if (!(valid_hooks & (1 << hook)))continue ; + depth =0 ; + last_pos = pos;/* Set initial back pointer. */ e->counters.pcnt = pos;
@@ -343,6 +346,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo, pos = e->counters.pcnt; e->counters.pcnt =0 ; +if (depth) + --depth;/* We're at the start. */ if (pos == oldpos) goto next;
@@ -367,6 +372,9 @@ static int mark_source_chains(const struct xt_table_info *newinfo,if (!xt_find_jump_offset (offsets, newpos, newinfo->number))return 0 ; + +if (entry0 + newpos !=arpt_next_entry (e)) + ++depth; }else {/* ... this is a fallthru */ newpos = pos + e->next_offset;
@@ -377,8 +385,15 @@ static int mark_source_chains(const struct xt_table_info *newinfo, e->counters.pcnt = pos; pos = newpos; } +if (depth ==0 ) + last_pos = pos; + } +next: +if (last_pos != newinfo->underflow[hook]) { +pr_err_ratelimited ("last base chain position %u doesn't match underflow %u (hook %u)\n" , + last_pos, newinfo->underflow[hook], hook); +return 0 ; } -next: ; }return 1 ; }
net/ipv4/netfilter/ip_tables.c +16 -1
@@ -378,10 +378,13 @@ mark_source_chains(const struct xt_table_info *newinfo,for (hook =0 ; hook <NF_INET_NUMHOOKS ; hook++) {unsigned int pos = newinfo->hook_entry[hook];struct ipt_entry *e = entry0 + pos; +unsigned int last_pos, depth;if (!(valid_hooks & (1 << hook)))continue ; + depth =0 ; + last_pos = pos;/* Set initial back pointer. */ e->counters.pcnt = pos;
@@ -410,6 +413,8 @@ mark_source_chains(const struct xt_table_info *newinfo, pos = e->counters.pcnt; e->counters.pcnt =0 ; +if (depth) + --depth;/* We're at the start. */ if (pos == oldpos) goto next;
@@ -434,6 +439,9 @@ mark_source_chains(const struct xt_table_info *newinfo,if (!xt_find_jump_offset (offsets, newpos, newinfo->number))return 0 ; + +if (entry0 + newpos !=ipt_next_entry (e)) + ++depth; }else {/* ... this is a fallthru */ newpos = pos + e->next_offset;
@@ -444,8 +452,15 @@ mark_source_chains(const struct xt_table_info *newinfo, e->counters.pcnt = pos; pos = newpos; } +if (depth ==0 ) + last_pos = pos; + } +next: +if (last_pos != newinfo->underflow[hook]) { +pr_err_ratelimited ("last base chain position %u doesn't match underflow %u (hook %u)\n" , + last_pos, newinfo->underflow[hook], hook); +return 0 ; } -next: ; }return 1 ; }
net/ipv6/netfilter/ip6_tables.c +16 -1
@@ -396,10 +396,13 @@ mark_source_chains(const struct xt_table_info *newinfo,for (hook =0 ; hook <NF_INET_NUMHOOKS ; hook++) {unsigned int pos = newinfo->hook_entry[hook];struct ip6t_entry *e = entry0 + pos; +unsigned int last_pos, depth;if (!(valid_hooks & (1 << hook)))continue ; + depth =0 ; + last_pos = pos;/* Set initial back pointer. */ e->counters.pcnt = pos;
@@ -428,6 +431,8 @@ mark_source_chains(const struct xt_table_info *newinfo, pos = e->counters.pcnt; e->counters.pcnt =0 ; +if (depth) + --depth;/* We're at the start. */ if (pos == oldpos) goto next;
@@ -452,6 +457,9 @@ mark_source_chains(const struct xt_table_info *newinfo,if (!xt_find_jump_offset (offsets, newpos, newinfo->number))return 0 ; + +if (entry0 + newpos !=ip6t_next_entry (e)) + ++depth; }else {/* ... this is a fallthru */ newpos = pos + e->next_offset;
@@ -462,8 +470,15 @@ mark_source_chains(const struct xt_table_info *newinfo, e->counters.pcnt = pos; pos = newpos; } +if (depth ==0 ) + last_pos = pos; + } +next: +if (last_pos != newinfo->underflow[hook]) { +pr_err_ratelimited ("last base chain position %u doesn't match underflow %u (hook %u)\n" , + last_pos, newinfo->underflow[hook], hook); +return 0 ; } -next: ; }return 1 ; }
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 18/30 netfilter: make xt_rateest hash table per net
In reply to Pablo Neira AyusoFrom: Cong Wang <[email protected]>
As suggested by Eric, we need to make the xt_rateest hash table and its lock per netns to reduce lock contentions.
--- include/net/netfilter/xt_rateest.h | 4 +- net/netfilter/xt_RATEEST.c | 91 +++++++++++++++++++++++++++----------- net/netfilter/xt_rateest.c | 10 ++--- 3 files changed, 72 insertions(+), 33 deletions(-)
include/net/netfilter/xt_rateest.h +2 -2
@@ -21,7 +21,7 @@ struct xt_rateest { struct net_rate_estimator __rcu *rate_est; }; -struct xt_rateest *xt_rateest_lookup (const char *name); -void xt_rateest_put (struct xt_rateest *est); +struct xt_rateest *xt_rateest_lookup (struct net *net,const char *name); +void xt_rateest_put (struct net *net,struct xt_rateest *est);#endif /* _XT_RATEEST_H */
net/netfilter/xt_RATEEST.c +65 -26
191 lines not on this page · raw message
net/netfilter/xt_rateest.c +5 -5
@@ -95,13 +95,13 @@ static int xt_rateest_mt_checkentry(const struct xt_mtchk_param *par) } ret = -ENOENT ; - est1 =xt_rateest_lookup (info->name1); + est1 =xt_rateest_lookup (par->net, info->name1);if (!est1) goto err1; est2 =NULL ;if (info->flags &XT_RATEEST_MATCH_REL ) { - est2 =xt_rateest_lookup (info->name2); + est2 =xt_rateest_lookup (par->net, info->name2);if (!est2) goto err2; }
@@ -111,7 +111,7 @@ static int xt_rateest_mt_checkentry(const struct xt_mtchk_param *par)return 0 ; err2: -xt_rateest_put (est1); +xt_rateest_put (par->net, est1); err1:return ret; }
@@ -120,9 +120,9 @@ static void xt_rateest_mt_destroy(const struct xt_mtdtor_param *par) {struct xt_rateest_match_info *info = par->matchinfo; -xt_rateest_put (info->est1); +xt_rateest_put (par->net, info->est1);if (info->est2) -xt_rateest_put (info->est2); +xt_rateest_put (par->net, info->est2); }static struct xt_match xt_rateest_mt_reg__read_mostly = {
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 19/30 netfilter: xt_limit: Spelling s/maxmum/maximum/
In reply to Pablo Neira AyusoFrom: Geert Uytterhoeven <[email protected]>
--- net/netfilter/xt_limit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/netfilter/xt_limit.c +1 -1
@@ -46,7 +46,7 @@ MODULE_ALIAS("ip6t_limit");See Alexey's formal explanation in net/sched/sch_tbf.c. -To get the maxmum range, we multiply by this factor (ie. you get N +To get the maximum range, we multiply by this factor (ie. you get N credits per jiffy). We want to allow a rate as low as 1 per day (slowest userspace tool allows), which means CREDITS_PER_JIFFY*HZ*60*60*24 < 2^32. ie. */
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 20/30 netfilter: nf_flow_table: use IP_CT_DIR_* values for FLOW_OFFLOAD_DIR_*
In reply to Pablo Neira AyusoFrom: Felix Fietkau <[email protected]>
Simplifies further code cleanups
--- include/net/netfilter/nf_flow_table.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
include/net/netfilter/nf_flow_table.h +4 -4
@@ -6,6 +6,7 @@#include <linux/netdevice.h> #include <linux/rhashtable.h> #include <linux/rcupdate.h> +#include <linux/netfilter/nf_conntrack_tuple_common.h> #include <net/dst.h> struct nf_flowtable ;
@@ -27,11 +28,10 @@ struct nf_flowtable { };enum flow_offload_tuple_dir { -FLOW_OFFLOAD_DIR_ORIGINAL , -FLOW_OFFLOAD_DIR_REPLY , - __FLOW_OFFLOAD_DIR_MAX =FLOW_OFFLOAD_DIR_REPLY , +FLOW_OFFLOAD_DIR_ORIGINAL =IP_CT_DIR_ORIGINAL , +FLOW_OFFLOAD_DIR_REPLY =IP_CT_DIR_REPLY , +FLOW_OFFLOAD_DIR_MAX =IP_CT_DIR_MAX }; -#define FLOW_OFFLOAD_DIR_MAX (__FLOW_OFFLOAD_DIR_MAX + 1) struct flow_offload_tuple { union {
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 21/30 netfilter: nf_flow_table: clean up flow_offload_alloc
In reply to Pablo Neira AyusoFrom: Felix Fietkau <[email protected]>
Reduce code duplication and make it much easier to read
--- net/netfilter/nf_flow_table.c | 93 ++++++++++++++++--------------------------- 1 file changed, 34 insertions(+), 59 deletions(-)
net/netfilter/nf_flow_table.c +34 -59
@@ -16,6 +16,38 @@ struct flow_offload_entry {struct rcu_head rcu_head; }; +static void +flow_offload_fill_dir (struct flow_offload *flow,struct nf_conn *ct, +struct nf_flow_route *route, +enum flow_offload_tuple_dir dir) +{ +struct flow_offload_tuple *ft = &flow->tuplehash[dir].tuple; +struct nf_conntrack_tuple *ctt = &ct->tuplehash[dir].tuple; + + ft->dir = dir; + +switch (ctt->src.l3num) { +case NFPROTO_IPV4 : + ft->src_v4 = ctt->src.u3.in; + ft->dst_v4 = ctt->dst.u3.in; +break ; +case NFPROTO_IPV6 : + ft->src_v6 = ctt->src.u3.in6; + ft->dst_v6 = ctt->dst.u3.in6; +break ; + } + + ft->l3proto = ctt->src.l3num; + ft->l4proto = ctt->dst.protonum; + ft->src_port = ctt->src.u.tcp.port; + ft->dst_port = ctt->dst.u.tcp.port; + + ft->iifidx = route->tuple[dir].ifindex; + ft->oifidx = route->tuple[!dir].ifindex; + + ft->dst_cache = route->tuple[dir].dst; +} +struct flow_offload *flow_offload_alloc (struct nf_conn *ct,struct nf_flow_route *route) {
@@ -40,65 +72,8 @@ flow_offload_alloc(struct nf_conn *ct, struct nf_flow_route *route) entry->ct = ct; -switch (ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple.src.l3num) { -case NFPROTO_IPV4 : - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.src_v4 = - ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple.src.u3.in; - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.dst_v4 = - ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple.dst.u3.in; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.src_v4 = - ct->tuplehash[IP_CT_DIR_REPLY ].tuple.src.u3.in; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.dst_v4 = - ct->tuplehash[IP_CT_DIR_REPLY ].tuple.dst.u3.in; -break ; -case NFPROTO_IPV6 : - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.src_v6 = - ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple.src.u3.in6; - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.dst_v6 = - ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple.dst.u3.in6; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.src_v6 = - ct->tuplehash[IP_CT_DIR_REPLY ].tuple.src.u3.in6; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.dst_v6 = - ct->tuplehash[IP_CT_DIR_REPLY ].tuple.dst.u3.in6; -break ; - } - - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.l3proto = - ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple.src.l3num; - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.l4proto = - ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple.dst.protonum; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.l3proto = - ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple.src.l3num; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.l4proto = - ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple.dst.protonum; - - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.dst_cache = - route->tuple[FLOW_OFFLOAD_DIR_ORIGINAL ].dst; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.dst_cache = - route->tuple[FLOW_OFFLOAD_DIR_REPLY ].dst; - - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.src_port = - ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple.src.u.tcp.port; - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.dst_port = - ct->tuplehash[IP_CT_DIR_ORIGINAL ].tuple.dst.u.tcp.port; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.src_port = - ct->tuplehash[IP_CT_DIR_REPLY ].tuple.src.u.tcp.port; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.dst_port = - ct->tuplehash[IP_CT_DIR_REPLY ].tuple.dst.u.tcp.port; - - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.dir = -FLOW_OFFLOAD_DIR_ORIGINAL ; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.dir = -FLOW_OFFLOAD_DIR_REPLY ; - - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.iifidx = - route->tuple[FLOW_OFFLOAD_DIR_ORIGINAL ].ifindex; - flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL ].tuple.oifidx = - route->tuple[FLOW_OFFLOAD_DIR_REPLY ].ifindex; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.iifidx = - route->tuple[FLOW_OFFLOAD_DIR_REPLY ].ifindex; - flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY ].tuple.oifidx = - route->tuple[FLOW_OFFLOAD_DIR_ORIGINAL ].ifindex; +flow_offload_fill_dir (flow, ct, route,FLOW_OFFLOAD_DIR_ORIGINAL ); +flow_offload_fill_dir (flow, ct, route,FLOW_OFFLOAD_DIR_REPLY );if (ct->status &IPS_SRC_NAT ) flow->flags |=FLOW_OFFLOAD_SNAT ;
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 22/30 ipv6: make ip6_dst_mtu_forward inline
In reply to Pablo Neira AyusoFrom: Felix Fietkau <[email protected]>
Needed to remove a direct dependency on ipv6.ko from flowtable infrastructure. Make it inline like ip_dst_mtu_maybe_forward().
--- include/net/ip6_route.h | 21 +++++++++++++++++++++ include/net/ipv6.h | 2 -- net/ipv6/ip6_output.c | 22 ---------------------- 3 files changed, 21 insertions(+), 24 deletions(-)
include/net/ip6_route.h +21 -0
@@ -271,4 +271,25 @@ static inline bool rt6_duplicate_nexthop(struct rt6_info *a, struct rt6_info *b) !lwtunnel_cmp_encap(a->dst.lwtstate, b->dst.lwtstate); } +static inline unsigned int ip6_dst_mtu_forward (const struct dst_entry *dst) +{ +unsigned int mtu; +struct inet6_dev *idev; + +if (dst_metric_locked (dst,RTAX_MTU )) { + mtu =dst_metric_raw (dst,RTAX_MTU ); +if (mtu) +return mtu; + } + + mtu =IPV6_MIN_MTU ; +rcu_read_lock (); + idev =__in6_dev_get (dst->dev); +if (idev) + mtu = idev->cnf.mtu6; +rcu_read_unlock (); + +return mtu; +} +#endif
include/net/ipv6.h +0 -2
@@ -970,8 +970,6 @@ static inline struct sk_buff *ip6_finish_skb(struct sock *sk) &inet6_sk(sk)->cork); } -unsigned int ip6_dst_mtu_forward (const struct dst_entry *dst); -int ip6_dst_lookup (struct net *net,struct sock *sk,struct dst_entry **dst,struct flowi6 *fl6); struct dst_entry *ip6_dst_lookup_flow(const struct sock *sk, struct flowi6 *fl6,
net/ipv6/ip6_output.c +0 -22
@@ -378,28 +378,6 @@ static inline int ip6_forward_finish(struct net *net, struct sock *sk,return dst_output (net, sk, skb); } -unsigned int ip6_dst_mtu_forward (const struct dst_entry *dst) -{ -unsigned int mtu; -struct inet6_dev *idev; - -if (dst_metric_locked (dst,RTAX_MTU )) { - mtu =dst_metric_raw (dst,RTAX_MTU ); -if (mtu) -return mtu; - } - - mtu =IPV6_MIN_MTU ; -rcu_read_lock (); - idev =__in6_dev_get (dst->dev); -if (idev) - mtu = idev->cnf.mtu6; -rcu_read_unlock (); - -return mtu; -} -EXPORT_SYMBOL_GPL (ip6_dst_mtu_forward); -static bool ip6_pkt_too_big (const struct sk_buff *skb,unsigned int mtu) {if (skb->len <= mtu)
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 23/30 netfilter: nf_flow_table: cache mtu in struct flow_offload_tuple
In reply to Pablo Neira AyusoFrom: Felix Fietkau <[email protected]>
Reduces the number of cache lines touched in the offload forwarding path. This is safe because PMTU limits are bypassed for the forwarding path (see commit f87c10a8aa1e for more details).
--- include/net/netfilter/nf_flow_table.h | 2 ++ net/ipv4/netfilter/nf_flow_table_ipv4.c | 17 +++-------------- net/ipv6/netfilter/nf_flow_table_ipv6.c | 17 +++-------------- net/netfilter/nf_flow_table.c | 8 ++++++-- 4 files changed, 14 insertions(+), 30 deletions(-)
include/net/netfilter/nf_flow_table.h +2 -0
@@ -55,6 +55,8 @@ struct flow_offload_tuple {int oifidx; +u16 mtu; +struct dst_entry *dst_cache; };
net/ipv4/netfilter/nf_flow_table_ipv4.c +3 -14
@@ -177,7 +177,7 @@ static int nf_flow_tuple_ip(struct sk_buff *skb, const struct net_device *dev, }/* Based on ip_exceeds_mtu(). */ -static bool __nf_flow_exceeds_mtu (const struct sk_buff *skb,unsigned int mtu) +static bool nf_flow_exceeds_mtu (const struct sk_buff *skb,unsigned int mtu) {if (skb->len <= mtu)return false;
@@ -191,17 +191,6 @@ static bool __nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)return true; } -static bool nf_flow_exceeds_mtu (struct sk_buff *skb,const struct rtable *rt) -{ -u32 mtu; - - mtu =ip_dst_mtu_maybe_forward (&rt->dst, true); -if (__nf_flow_exceeds_mtu (skb, mtu)) -return true; - -return false; -} -unsigned int nf_flow_offload_ip_hook (void *priv,struct sk_buff *skb,const struct nf_hook_state *state)
@@ -232,9 +221,9 @@ nf_flow_offload_ip_hook(void *priv, struct sk_buff *skb, dir = tuplehash->tuple.dir; flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]); - rt = (const struct rtable *)flow->tuplehash[dir].tuple.dst_cache; -if (unlikely (nf_flow_exceeds_mtu (skb, rt))) + +if (unlikely (nf_flow_exceeds_mtu (skb, flow->tuplehash[dir].tuple.mtu)))return NF_ACCEPT ;if (skb_try_make_writable (skb,sizeof (*iph)))
net/ipv6/netfilter/nf_flow_table_ipv6.c +3 -14
@@ -173,7 +173,7 @@ static int nf_flow_tuple_ipv6(struct sk_buff *skb, const struct net_device *dev, }/* Based on ip_exceeds_mtu(). */ -static bool __nf_flow_exceeds_mtu (const struct sk_buff *skb,unsigned int mtu) +static bool nf_flow_exceeds_mtu (const struct sk_buff *skb,unsigned int mtu) {if (skb->len <= mtu)return false;
@@ -184,17 +184,6 @@ static bool __nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)return true; } -static bool nf_flow_exceeds_mtu (struct sk_buff *skb,const struct rt6_info *rt) -{ -u32 mtu; - - mtu =ip6_dst_mtu_forward (&rt->dst); -if (__nf_flow_exceeds_mtu (skb, mtu)) -return true; - -return false; -} -unsigned int nf_flow_offload_ipv6_hook (void *priv,struct sk_buff *skb,const struct nf_hook_state *state)
@@ -225,9 +214,9 @@ nf_flow_offload_ipv6_hook(void *priv, struct sk_buff *skb, dir = tuplehash->tuple.dir; flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]); - rt = (struct rt6_info *)flow->tuplehash[dir].tuple.dst_cache; -if (unlikely (nf_flow_exceeds_mtu (skb, rt))) + +if (unlikely (nf_flow_exceeds_mtu (skb, flow->tuplehash[dir].tuple.mtu)))return NF_ACCEPT ;if (skb_try_make_writable (skb,sizeof (*ip6h)))
net/netfilter/nf_flow_table.c +6 -2
@@ -4,6 +4,8 @@#include <linux/netfilter.h> #include <linux/rhashtable.h> #include <linux/netdevice.h> +#include <net/ip.h> +#include <net/ip6_route.h> #include <net/netfilter/nf_tables.h> #include <net/netfilter/nf_flow_table.h> #include <net/netfilter/nf_conntrack.h>
@@ -23,6 +25,7 @@ flow_offload_fill_dir(struct flow_offload *flow, struct nf_conn *ct, {struct flow_offload_tuple *ft = &flow->tuplehash[dir].tuple;struct nf_conntrack_tuple *ctt = &ct->tuplehash[dir].tuple; +struct dst_entry *dst = route->tuple[dir].dst; ft->dir = dir;
@@ -30,10 +33,12 @@ flow_offload_fill_dir(struct flow_offload *flow, struct nf_conn *ct,case NFPROTO_IPV4 : ft->src_v4 = ctt->src.u3.in; ft->dst_v4 = ctt->dst.u3.in; + ft->mtu =ip_dst_mtu_maybe_forward (dst, true);break ;case NFPROTO_IPV6 : ft->src_v6 = ctt->src.u3.in6; ft->dst_v6 = ctt->dst.u3.in6; + ft->mtu =ip6_dst_mtu_forward (dst);break ; }
@@ -44,8 +49,7 @@ flow_offload_fill_dir(struct flow_offload *flow, struct nf_conn *ct, ft->iifidx = route->tuple[dir].ifindex; ft->oifidx = route->tuple[!dir].ifindex; - - ft->dst_cache = route->tuple[dir].dst; + ft->dst_cache = dst; } struct flow_offload *
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 24/30 netfilter: nf_flow_table: rename nf_flow_table.c to nf_flow_table_core.c
In reply to Pablo Neira AyusoFrom: Felix Fietkau <[email protected]>
Preparation for adding more code to the same module
---
net/netfilter/Makefile | 2 ++
net/netfilter/{nf_flow_table.c => nf_flow_table_core.c} | 0
2 files changed, 2 insertions(+)
rename net/netfilter/{nf_flow_table.c => nf_flow_table_core.c} (100%)net/netfilter/Makefile +2 -0
@@ -112,6 +112,8 @@ obj-$(CONFIG_NFT_FWD_NETDEV) += nft_fwd_netdev.o # flow table infrastructure obj-$(CONFIG_NF_FLOW_TABLE) += nf_flow_table.o +nf_flow_table-objs := nf_flow_table_core.o + obj-$(CONFIG_NF_FLOW_TABLE_INET) += nf_flow_table_inet.o # generic X tables
net/netfilter/nf_flow_table_core.c renamed from net/netfilter/nf_flow_table.c +0 -0
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 25/30 netfilter: x_tables: fix build with CONFIG_COMPAT=n
In reply to Pablo Neira AyusoFrom: Florian Westphal <[email protected]>
I placed the helpers within CONFIG_COMPAT section, move them outside.
--- net/netfilter/x_tables.c | 62 ++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 31 deletions(-)
net/netfilter/x_tables.c +31 -31
@@ -577,6 +577,37 @@ int xt_check_table_hooks(const struct xt_table_info *info, unsigned int valid_ho }EXPORT_SYMBOL (xt_check_table_hooks); +static bool verdict_ok (int verdict) +{ +if (verdict >0 ) +return true; + +if (verdict <0 ) { +int v = -verdict -1 ; + +if (verdict ==XT_RETURN ) +return true; + +switch (v) { +case NF_ACCEPT :return true; +case NF_DROP :return true; +case NF_QUEUE :return true; +default : +break ; + } + +return false; + } + +return false; +} + +static bool error_tg_ok (unsigned int usersize,unsigned int kernsize, +const char *msg,unsigned int msglen) +{ +return usersize == kernsize &&strnlen (msg, msglen) < msglen; +} +#ifdef CONFIG_COMPAT int xt_compat_add_offset (u_int8_t af,unsigned int offset,int delta) {
@@ -736,37 +767,6 @@ struct compat_xt_error_target {char errorname[XT_FUNCTION_MAXNAMELEN ]; }; -static bool verdict_ok (int verdict) -{ -if (verdict >0 ) -return true; - -if (verdict <0 ) { -int v = -verdict -1 ; - -if (verdict ==XT_RETURN ) -return true; - -switch (v) { -case NF_ACCEPT :return true; -case NF_DROP :return true; -case NF_QUEUE :return true; -default : -break ; - } - -return false; - } - -return false; -} - -static bool error_tg_ok (unsigned int usersize,unsigned int kernsize, -const char *msg,unsigned int msglen) -{ -return usersize == kernsize &&strnlen (msg, msglen) < msglen; -} -int xt_compat_check_entry_offsets (const void *base,const char *elems,unsigned int target_offset,unsigned int next_offset)
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 26/30 ipvs: use true and false for boolean values
In reply to Pablo Neira AyusoFrom: "Gustavo A. R. Silva" <[email protected]>
Assign true or false to boolean variables instead of an integer value.
This issue was detected with the help of Coccinelle.
--- net/netfilter/ipvs/ip_vs_lblc.c | 4 ++-- net/netfilter/ipvs/ip_vs_lblcr.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
net/netfilter/ipvs/ip_vs_lblc.c +2 -2
@@ -238,7 +238,7 @@ static void ip_vs_lblc_flush(struct ip_vs_service *svc)int i;spin_lock_bh (&svc->sched_lock); - tbl->dead =1 ; + tbl->dead = true;for (i =0 ; i <IP_VS_LBLC_TAB_SIZE ; i++) {hlist_for_each_entry_safe (en, next, &tbl->bucket[i], list) {ip_vs_lblc_del (en);
@@ -369,7 +369,7 @@ static int ip_vs_lblc_init_svc(struct ip_vs_service *svc) tbl->max_size =IP_VS_LBLC_TAB_SIZE *16 ; tbl->rover =0 ; tbl->counter =1 ; - tbl->dead =0 ; + tbl->dead = false; tbl->svc = svc;/*
net/netfilter/ipvs/ip_vs_lblcr.c +2 -2
@@ -404,7 +404,7 @@ static void ip_vs_lblcr_flush(struct ip_vs_service *svc)struct hlist_node *next;spin_lock_bh (&svc->sched_lock); - tbl->dead =1 ; + tbl->dead = true;for (i =0 ; i <IP_VS_LBLCR_TAB_SIZE ; i++) {hlist_for_each_entry_safe (en, next, &tbl->bucket[i], list) {ip_vs_lblcr_free (en);
@@ -532,7 +532,7 @@ static int ip_vs_lblcr_init_svc(struct ip_vs_service *svc) tbl->max_size =IP_VS_LBLCR_TAB_SIZE *16 ; tbl->rover =0 ; tbl->counter =1 ; - tbl->dead =0 ; + tbl->dead = false; tbl->svc = svc;/*
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 27/30 netfilter: nf_tables: handle rt0 and rt2 properly
In reply to Pablo Neira AyusoFrom: Ahmed Abdelsalam <[email protected]>
This fixes Netfilter's bugzilla #1219.
Type 0 and 2 of the IPv6 Routing extension header are not handled properlyby exthdr_init_raw() in src/exthdr.c
In order to fix the bug, we extended the "enum nft_exthdr_op" to differentiate between rt, rt0, and rt2.
In this patch we extended the kernel implementation of nf_tables to recognize the new options
--- include/uapi/linux/netfilter/nf_tables.h | 3 +++ net/netfilter/nft_exthdr.c | 3 +++ 2 files changed, 6 insertions(+)
include/uapi/linux/netfilter/nf_tables.h +3 -0
@@ -731,6 +731,9 @@ enum nft_exthdr_flags {enum nft_exthdr_op {NFT_EXTHDR_OP_IPV6 ,NFT_EXTHDR_OP_TCPOPT , +NFT_EXTHDR_OP_RT0 , +NFT_EXTHDR_OP_RT2 , +NFT_EXTHDR_OP_RT4 , __NFT_EXTHDR_OP_MAX };#define NFT_EXTHDR_OP_MAX (__NFT_EXTHDR_OP_MAX - 1)
net/netfilter/nft_exthdr.c +3 -0
@@ -399,6 +399,9 @@ nft_exthdr_select_ops(const struct nft_ctx *ctx,return &nft_exthdr_tcp_ops;break ;case NFT_EXTHDR_OP_IPV6 : +case NFT_EXTHDR_OP_RT0 : +case NFT_EXTHDR_OP_RT2 : +case NFT_EXTHDR_OP_RT4 :if (tb[NFTA_EXTHDR_DREG ])return &nft_exthdr_ipv6_ops;break ;
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 28/30 netfilter: Refactor nf_conncount
In reply to Pablo Neira AyusoFrom: Yi-Hung Wei <[email protected]>
Remove parameter 'family' in nf_conncount_count() and count_tree(). It is because the parameter is not useful after commit 625c556118f3 ("netfilter: connlimit: split xt_connlimit into front and backend").
--- include/net/netfilter/nf_conntrack_count.h | 1 - net/netfilter/nf_conncount.c | 4 +--- net/netfilter/xt_connlimit.c | 4 ++-- 3 files changed, 3 insertions(+), 6 deletions(-)
include/net/netfilter/nf_conntrack_count.h +0 -1
@@ -11,7 +11,6 @@ void nf_conncount_destroy(struct net *net, unsigned int family,unsigned int nf_conncount_count (struct net *net,struct nf_conncount_data *data,const u32 *key, -unsigned int family,const struct nf_conntrack_tuple *tuple,const struct nf_conntrack_zone *zone);#endif
net/netfilter/nf_conncount.c +1 -3
@@ -158,7 +158,6 @@ static void tree_nodes_free(struct rb_root *root,static unsigned int count_tree (struct net *net,struct rb_root *root,const u32 *key,u8 keylen, -u8 family,const struct nf_conntrack_tuple *tuple,const struct nf_conntrack_zone *zone) {
@@ -246,7 +245,6 @@ count_tree(struct net *net, struct rb_root *root,unsigned int nf_conncount_count (struct net *net,struct nf_conncount_data *data,const u32 *key, -unsigned int family,const struct nf_conntrack_tuple *tuple,const struct nf_conntrack_zone *zone) {
@@ -259,7 +257,7 @@ unsigned int nf_conncount_count(struct net *net,spin_lock_bh (&nf_conncount_locks[hash %CONNCOUNT_LOCK_SLOTS ]); - count =count_tree (net, root, key, data->keylen, family, tuple, zone); + count =count_tree (net, root, key, data->keylen, tuple, zone);spin_unlock_bh (&nf_conncount_locks[hash %CONNCOUNT_LOCK_SLOTS ]);
net/netfilter/xt_connlimit.c +2 -2
@@ -67,8 +67,8 @@ connlimit_mt(const struct sk_buff *skb, struct xt_action_param *par) key[1 ] = zone->id; } - connections =nf_conncount_count (net, info->data, key, -xt_family (par), tuple_ptr, zone); + connections =nf_conncount_count (net, info->data, key, tuple_ptr, + zone);if (connections ==0 )/* kmalloc failed, drop it entirely */ goto hotdrop;
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 29/30 netfilter: conncount: Support count only use case
In reply to Pablo Neira AyusoFrom: Yi-Hung Wei <[email protected]>
Currently, nf_conncount_count() counts the number of connections that matches key and inserts a conntrack 'tuple' with the same key into the accounting data structure. This patch supports another use case that only counts the number of connections where 'tuple' is not provided. Therefore, proper changes are made on nf_conncount_count() to support the case where 'tuple' is NULL. This could be useful for querying statistics or debugging purpose.
--- net/netfilter/nf_conncount.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
net/netfilter/nf_conncount.c +8 -2
@@ -104,7 +104,7 @@ static unsigned int check_hlist(struct net *net,struct nf_conn *found_ct;unsigned int length =0 ; - *addit = true; + *addit = tuple ? true : false;/* check the saved connections */ hlist_for_each_entry_safe (conn, n, head, node) {
@@ -117,7 +117,7 @@ static unsigned int check_hlist(struct net *net, found_ct =nf_ct_tuplehash_to_ctrack (found); -if (nf_ct_tuple_equal (&conn->tuple, tuple)) { +if (tuple &&nf_ct_tuple_equal (&conn->tuple, tuple)) {/* * Just to be sure we have it only once in the list. * We should not see tuples twice unless someone hooks
@@ -220,6 +220,9 @@ count_tree(struct net *net, struct rb_root *root, goto restart; } +if (!tuple) +return 0 ; +/* no match, need to insert new node */ rbconn =kmem_cache_alloc (conncount_rb_cachep,GFP_ATOMIC );if (rbconn ==NULL )
@@ -242,6 +245,9 @@ count_tree(struct net *net, struct rb_root *root,return 1 ; } +/* Count and return number of conntrack entries in 'net' with particular 'key'. +* If 'tuple' is not null, insert it into the accounting data structure. +*/ unsigned int nf_conncount_count(struct net *net, struct nf_conncount_data *data, const u32 *key,
Signature · 1 line
2.11.0
Pablo Neira Ayuso
maintainerPATCH 30/30 netfilter: nft_ct: add NFT_CT_{SRC,DST}_{IP,IP6}
In reply to Pablo Neira AyusoAll existing keys, except the NFT_CT_SRC and NFT_CT_DST are assumed to have strict datatypes. This is causing problems with sets and concatenations given the specific length of these keys is not known.
--- include/uapi/linux/netfilter/nf_tables.h | 12 ++++++++-- net/netfilter/nft_ct.c | 38 ++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-)
include/uapi/linux/netfilter/nf_tables.h +10 -2
@@ -912,8 +912,8 @@ enum nft_rt_attributes {* @NFT_CT_EXPIRATION: relative conntrack expiration time in ms * @NFT_CT_HELPER: connection tracking helper assigned to conntrack * @NFT_CT_L3PROTOCOL: conntrack layer 3 protocol -* @NFT_CT_SRC: conntrack layer 3 protocol source (IPv4/IPv6 address) -* @NFT_CT_DST: conntrack layer 3 protocol destination (IPv4/IPv6 address) +* @NFT_CT_SRC: conntrack layer 3 protocol source (IPv4/IPv6 address, deprecated) +* @NFT_CT_DST: conntrack layer 3 protocol destination (IPv4/IPv6 address, deprecated) * @NFT_CT_PROTOCOL: conntrack layer 4 protocol * @NFT_CT_PROTO_SRC: conntrack layer 4 protocol source * @NFT_CT_PROTO_DST: conntrack layer 4 protocol destination
@@ -923,6 +923,10 @@ enum nft_rt_attributes {* @NFT_CT_AVGPKT: conntrack average bytes per packet * @NFT_CT_ZONE: conntrack zone * @NFT_CT_EVENTMASK: ctnetlink events to be generated for this conntrack +* @NFT_CT_SRC_IP: conntrack layer 3 protocol source (IPv4 address) +* @NFT_CT_DST_IP: conntrack layer 3 protocol destination (IPv4 address) +* @NFT_CT_SRC_IP6: conntrack layer 3 protocol source (IPv6 address) +* @NFT_CT_DST_IP6: conntrack layer 3 protocol destination (IPv6 address) */ enum nft_ct_keys {NFT_CT_STATE ,
@@ -944,6 +948,10 @@ enum nft_ct_keys {NFT_CT_AVGPKT ,NFT_CT_ZONE ,NFT_CT_EVENTMASK , +NFT_CT_SRC_IP , +NFT_CT_DST_IP , +NFT_CT_SRC_IP6 , + NFT_CT_DST_IP6, };/**
net/netfilter/nft_ct.c +38 -0
@@ -196,6 +196,26 @@ static void nft_ct_get_eval(const struct nft_expr *expr,case NFT_CT_PROTO_DST :nft_reg_store16 (dest, (__force u16 )tuple->dst.u.all);return ; +case NFT_CT_SRC_IP : +if (nf_ct_l3num (ct) !=NFPROTO_IPV4 ) + goto err; + *dest = tuple->src.u3.ip; +return ; +case NFT_CT_DST_IP : +if (nf_ct_l3num (ct) !=NFPROTO_IPV4 ) + goto err; + *dest = tuple->dst.u3.ip; +return ; +case NFT_CT_SRC_IP6 : +if (nf_ct_l3num (ct) !=NFPROTO_IPV6 ) + goto err; +memcpy (dest, tuple->src.u3.ip6,sizeof (struct in6_addr )); +return ; +case NFT_CT_DST_IP6 : +if (nf_ct_l3num (ct) !=NFPROTO_IPV6 ) + goto err; +memcpy (dest, tuple->dst.u3.ip6,sizeof (struct in6_addr )); +return ;default :break ; }
@@ -419,6 +439,20 @@ static int nft_ct_get_init(const struct nft_ctx *ctx,return -EAFNOSUPPORT ; }break ; +case NFT_CT_SRC_IP : +case NFT_CT_DST_IP : +if (tb[NFTA_CT_DIRECTION ] ==NULL ) +return -EINVAL ; + + len = FIELD_SIZEOF(struct nf_conntrack_tuple, src.u3.ip); +break ; +case NFT_CT_SRC_IP6 : +case NFT_CT_DST_IP6 : +if (tb[NFTA_CT_DIRECTION ] ==NULL ) +return -EINVAL ; + + len = FIELD_SIZEOF(struct nf_conntrack_tuple, src.u3.ip6); +break ;case NFT_CT_PROTO_SRC :case NFT_CT_PROTO_DST :if (tb[NFTA_CT_DIRECTION ] ==NULL )
@@ -588,6 +622,10 @@ static int nft_ct_get_dump(struct sk_buff *skb, const struct nft_expr *expr)switch (priv->key) {case NFT_CT_SRC :case NFT_CT_DST : +case NFT_CT_SRC_IP : +case NFT_CT_DST_IP : +case NFT_CT_SRC_IP6 : +case NFT_CT_DST_IP6 :case NFT_CT_PROTO_SRC :case NFT_CT_PROTO_DST :if (nla_put_u8 (skb,NFTA_CT_DIRECTION , priv->dir))
Signature · 1 line
2.11.0
David Miller
maintainerFrom: Pablo Neira Ayuso <[email protected]>
Date: Mon, 12 Mar 2018 18:58:50 +0100
Quoted 4 lines
The following patchset contains Netfilter/IPVS updates for your net-next tree. This batch comes with more input sanitization for xtables to address bug reports from fuzzers, preparation works to the flowtable infrastructure and assorted updates. In no particular order, they are:
Sorry, I've seen enough. I'm not pulling this.
What is the story with this flow table stuff? I tried to ask you about this before, but the response I was given was extremely vague and did not answer my question at all.
This is a lot of code, and a lot of infrastructure, yet I see no device using the infrastructure to offload conntack.
Nor can I see how this can possibly be even useful for such an application. What conntrack offload needs are things completely outside of what the flow table stuff provides. Mainly, they require that the SKB is completely abstracted away from all of the contrack code paths, and that the conntrack infrastructure operates on an abstract packet metadata concept.
If you are targetting one specific piece of hardware with TCAMs that you are familiar with. I'd like you to stop right there. Because if that is all that this infrastructure can actually be used for, it is definitely designed wrong.
This, as has been the case in the past, is what is wrong with netfilter approach to supporting offloading. We see all of this infrastructure before an actual working use case is provided for a specific piece of hardware for a specific driver in the tree.
Nobody can evaluate whether the approach is good or not without a clear driver change implementing support for it.
No other area of networking puts the cart before the horse like this.
I do not agree at all with the flow table infrastructure and I therefore do not want to pull any more flow table changes into my tree until there is an actual user of this stuff in that pull request which actually works in a way which is useful for people. It is completely dead and useless code currently.
If you disagree you have to not just say it, but show it with a driver that successfully and cleanly uses this code to offload conntrack.
Meanwhile, remove the flow table commits from this pull request out of your tree and ask me to pull in the rest.
Thanks.
Quoted 37 lines · David Miller
From: Pablo Neira Ayuso <[email protected]>
Date: Mon, 12 Mar 2018 18:58:50 +0100Quoted 4 lines
The following patchset contains Netfilter/IPVS updates for your net-next tree. This batch comes with more input sanitization for xtables to address bug reports from fuzzers, preparation works to the flowtable infrastructure and assorted updates. In no particular order, they are:
Sorry, I've seen enough. I'm not pulling this.
What is the story with this flow table stuff? I tried to ask you about this before, but the response I was given was extremely vague and did not answer my question at all.
This is a lot of code, and a lot of infrastructure, yet I see no device using the infrastructure to offload conntack.
Nor can I see how this can possibly be even useful for such an application. What conntrack offload needs are things completely outside of what the flow table stuff provides. Mainly, they require that the SKB is completely abstracted away from all of the contrack code paths, and that the conntrack infrastructure operates on an abstract packet metadata concept.
If you are targetting one specific piece of hardware with TCAMs that you are familiar with. I'd like you to stop right there. Because if that is all that this infrastructure can actually be used for, it is definitely designed wrong.
This, as has been the case in the past, is what is wrong with netfilter approach to supporting offloading. We see all of this infrastructure before an actual working use case is provided for a specific piece of hardware for a specific driver in the tree.
Nobody can evaluate whether the approach is good or not without a clear driver change implementing support for it.
Quoted 7 lines
No other area of networking puts the cart before the horse like this.
I do not agree at all with the flow table infrastructure and I therefore do not want to pull any more flow table changes into my tree until there is an actual user of this stuff in that pull request which actually works in a way which is useful for people. It is completely dead and useless code currently.
It's not dead and useless. In its current state, it has a software fast path that significantly improves nftables routing/NAT throughput, especially on embedded devices.
On some devices, I've seen "only" 20% throughput improvement (along with CPU usage reduction), on others it's quite a bit lot more. This is without any extra drivers or patches aside from what's posted.
Within OpenWrt, I'm working on a patch that makes the same available to legacy netfilter as well. This is the reason for a lot of the core refactoring that I did.
Hardware offload is still being worked on, not sure when we will have the first driver ready. But as it stands now, the code is already very useful and backported to OpenWrt for testing.
I think that in a couple of weeks this code will be ready to be enabled by default in OpenWrt, which means that a lot of users' setups will get a lot faster with no configuration change at all.
- Felix
David Miller
maintainerFrom: Felix Fietkau <[email protected]>
Date: Mon, 12 Mar 2018 20:30:01 +0100
Quoted 6 lines
It's not dead and useless. In its current state, it has a software fast path that significantly improves nftables routing/NAT throughput, especially on embedded devices.
On some devices, I've seen "only" 20% throughput improvement (along with CPU usage reduction), on others it's quite a bit lot more. This is without any extra drivers or patches aside from what's posted.
I wonder if this software fast path has the exploitability problems that things like the ipv4 routing cache and the per-cpu flow cache both had. And the reason for which both were removed.
I don't see how you can avoid this problem.
I'm willing to be shown otherwise :-)
Quoted 10 lines · David Miller
From: Felix Fietkau <[email protected]>
Date: Mon, 12 Mar 2018 20:30:01 +0100Quoted 6 lines
It's not dead and useless. In its current state, it has a software fast path that significantly improves nftables routing/NAT throughput, especially on embedded devices.
On some devices, I've seen "only" 20% throughput improvement (along with CPU usage reduction), on others it's quite a bit lot more. This is without any extra drivers or patches aside from what's posted.
Quoted 7 lines
I wonder if this software fast path has the exploitability problems that things like the ipv4 routing cache and the per-cpu flow cache both had. And the reason for which both were removed.
I don't see how you can avoid this problem.
I'm willing to be shown otherwise :-)
I don't think it suffers from the same issues, and if it does, it's a lot easier to mitigate. The ruleset can easily be configured to only offload connections that transferred a certain amount of data, handling only bulk flows.
It's easy to put an upper limit on the number of offloaded connections, and there's nothing in the code that just creates an offload entry per packet or per lookup or something like that.
If you have other concerns, I'm sure we can address them with follow-up patches, but as it stands, I think the code is already quite useful.
- Felix
Florian Westphal
maintainerQuoted 10 lines · David Miller
From: Felix Fietkau <[email protected]>
Date: Mon, 12 Mar 2018 20:30:01 +0100Quoted 6 lines
It's not dead and useless. In its current state, it has a software fast path that significantly improves nftables routing/NAT throughput, especially on embedded devices.
On some devices, I've seen "only" 20% throughput improvement (along with CPU usage reduction), on others it's quite a bit lot more. This is without any extra drivers or patches aside from what's posted.
Quoted 2 lines
I wonder if this software fast path has the exploitability problems that things like the ipv4 routing cache and the per-cpu flow cache both had.
No, entries in the flow table are backed by an entry in the conntrack table, and that has an upper ceiling.
As decision of when an entry gets placed into the flow table is configureable via ruleset (nftables, iptables will be coming too), one can tie the 'fastpathing' to almost-arbitrary criterion, e.g.
'only flows from trusted internal network'
'only flows that saw two-way communication'
'only flows that sent more than 100kbyte'
or any combination thereof.
Do you see another problem that needs to be addressed?
David Miller
maintainerFrom: Florian Westphal <[email protected]>
Date: Tue, 13 Mar 2018 14:41:39 +0100
Quoted 21 lines
Quoted 10 lines · David Miller
From: Felix Fietkau <[email protected]>
Date: Mon, 12 Mar 2018 20:30:01 +0100Quoted 6 lines
It's not dead and useless. In its current state, it has a software fast path that significantly improves nftables routing/NAT throughput, especially on embedded devices.
On some devices, I've seen "only" 20% throughput improvement (along with CPU usage reduction), on others it's quite a bit lot more. This is without any extra drivers or patches aside from what's posted.Quoted 2 lines
I wonder if this software fast path has the exploitability problems that things like the ipv4 routing cache and the per-cpu flow cache both had.
No, entries in the flow table are backed by an entry in the conntrack table, and that has an upper ceiling.
As decision of when an entry gets placed into the flow table is configureable via ruleset (nftables, iptables will be coming too), one can tie the 'fastpathing' to almost-arbitrary criterion, e.g.
Quoted 7 lines
'only flows from trusted internal network'
'only flows that saw two-way communication'
'only flows that sent more than 100kbyte'or any combination thereof.
Do you see another problem that needs to be addressed?
Ok, that seems to constrain the exposure.
We should talk at some point about how exposed conntrack itself is.
Florian Westphal
maintainerDavid Miller <[email protected]> wrote:
[ flow tables ]
Quoted 3 lines
Ok, that seems to constrain the exposure.
We should talk at some point about how exposed conntrack itself is.
Sure, we can do that.
If you have specific scenarios (synflood, peer that opens 100k (legitimate) connections, perpetual-fin, etc) in mind let me know, i do think that we could still do better in some cases.
Pablo Neira Ayuso
maintainerHi David,
Just for the record, this is a summary of what we have discussed so far:
1) The existing flowtable infrastructure provides a software fast path that is being useful for a valid number of usecases, in particular, OpenWRT/LEDE developers/users are very enthusiastic about this. Reason for this is that they have had no other choice rather than loading out of tree kernel modules to enable fast forwarding paths before this infrastructure has been mainlined. Fortunately, now they have an upstream alternative that can help them get rid of those modules. This fast path can be enabled very easily, actually one single rule to select what flows follow the alternative path is sufficient.
2) The software flowtable implementation is not affected by the problems that flow/routing cache used to have. An attacker that cycles through all key values by sending forged packets to fill up the hashtable will get no entries. Ruleset policy specifies when to offload entries into the flowtable, users can arbitrarily decide when to push the flow into the flowtable, eg.
add rule filter forward ct status assured flow offload @x
Worst case scenario is that users need to see two packets, one on each direction, to be able to place a flow in the flowtable.
3) There is no hardware offload integration yet. There's a public patch - waiting to have a driver - that proposes ndo hooks, this patch is not merged upstream. The flowtable design and the hardware offload patch has been the result of conversations with many vendors that represent a wide range of networking device classes, so it is an individual effort by looking at one single device. Stateful flowtable offload has been another main topic, pipeline is going to stall a bit if we cannot make incremental progress towards that direction.
Note that this batch was coming with a patch to reduce cache footprint of the flowtable entries, so there is already working-in-progress targeted at improving performance of this new software fast path. Also, preparation works to introduce iptables support has been in the radar while working on this.
We understand, we may have have spent more time in explaining all this in the mailing list, we are trying to amend this now. Therefore, we can probably convince someone here to write design documentation to be placed on the Documentation/flowtable/ directory in the next pull request if that makes it easier for the broader audience to understand our effort and rise concerns, if any.
Thanks.
David Miller
maintainerFrom: Pablo Neira Ayuso <[email protected]>
Date: Wed, 14 Mar 2018 19:38:48 +0100
Quoted 2 lines
Just for the record, this is a summary of what we have discussed so far:
...
Quoted 6 lines
Note that this batch was coming with a patch to reduce cache footprint of the flowtable entries, so there is already working-in-progress targeted at improving performance of this new software fast path. Also, preparation works to introduce iptables support has been in the radar while working on this.
Quoted 6 lines
We understand, we may have have spent more time in explaining all this in the mailing list, we are trying to amend this now. Therefore, we can probably convince someone here to write design documentation to be placed on the
Documentation/flowtable/directory in the next pull request if that makes it easier for the broader audience to understand our effort and rise concerns, if any.
Noted.
Please resend your pull request.
Thanks.
Quoted 7 lines · David Miller
From: Pablo Neira Ayuso <[email protected]>
Date: Mon, 12 Mar 2018 18:58:50 +0100Quoted 4 lines
The following patchset contains Netfilter/IPVS updates for your net-next tree. This batch comes with more input sanitization for xtables to address bug reports from fuzzers, preparation works to the flowtable infrastructure and assorted updates. In no particular order, they are:
Quoted 8 lines
Sorry, I've seen enough. I'm not pulling this.
What is the story with this flow table stuff? I tried to ask you about this before, but the response I was given was extremely vague and did not answer my question at all.
This is a lot of code, and a lot of infrastructure, yet I see no device using the infrastructure to offload conntack.
Hi David,
Pablo's code is a very welcome addition to the flow tables infrastructure. We at Mellanox already have customers asking for Offload of Connection Tracking. While a complete hardware implementation is yet to arrive. Pablo's contribution is blessed.
Using this infrastructure we are capable of completely offloading connection tracking (Without TCP window validation) and possibly do a complete offload once hardware support for TCP window Validation shows up.
I'm currently working closely with Pablo to create the first driver implementation to utilize hardware offloading. Needless to say - prior to having hardware implementation a software infrastructure is required.
Quoted 6 lines
Nor can I see how this can possibly be even useful for such an application. What conntrack offload needs are things completely outside of what the flow table stuff provides. Mainly, they require that the SKB is completely abstracted away from all of the contrack code paths, and that the conntrack infrastructure operates on an abstract packet metadata concept.
Assuming that the software maintains the flow in the system,
it is reasonable to allow software do the connection establishment and termination
and let the hardware do all the rest between (again - without TCP window validation, unless a specialized hardware exists)
Quoted 8 lines
This, as has been the case in the past, is what is wrong with netfilter approach to supporting offloading. We see all of this infrastructure before an actual working use case is provided for a specific piece of hardware for a specific driver in the tree.
Nobody can evaluate whether the approach is good or not without a clear driver change implementing support for it.
I'm speaking on behalf of Mellanox.
Would one driver support as demonstration suffice?
Thanks,
Guy
David Miller
maintainerFrom: Guy Shattah <[email protected]>
Date: Fri, 16 Mar 2018 18:39:03 +0200
Quoted 1 line
Would one driver support as demonstration suffice?
It would certinaly improve the reviewability of the changes.