* [PATCH net 0/5] Netfilter fixes for net
@ 2022-05-31 21:58 Pablo Neira Ayuso
2022-05-31 21:58 ` [PATCH net 1/5] netfilter: nf_tables: sanitize nft_set_desc_concat_parse() Pablo Neira Ayuso
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2022-05-31 21:58 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet
Hi,
1) Missing proper sanitization for nft_set_desc_concat_parse().
2) Missing mutex in nf_tables pre_exit path.
3) Possible double hook unregistration from clean_net path.
4) Missing FLOWI_FLAG_ANYSRC flag in flowtable route lookup.
Fix incorrect source and destination address in case of NAT.
Patch from wenxu.
Please, pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git
Thanks.
----------------------------------------------------------------
The following changes since commit 09e545f7381459c015b6fa0cd0ac6f010ef8cc25:
xen/netback: fix incorrect usage of RING_HAS_UNCONSUMED_REQUESTS() (2022-05-31 12:22:22 +0200)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git HEAD
for you to fetch changes up to 97629b237a8cb7ac655c3969b8d5e57300ff6598:
netfilter: flowtable: fix nft_flow_route source address for nat case (2022-05-31 23:32:53 +0200)
----------------------------------------------------------------
Pablo Neira Ayuso (3):
netfilter: nf_tables: sanitize nft_set_desc_concat_parse()
netfilter: nf_tables: hold mutex on netns pre_exit path
netfilter: nf_tables: double hook unregistration in netns path
wenxu (2):
netfilter: flowtable: fix missing FLOWI_FLAG_ANYSRC flag
netfilter: flowtable: fix nft_flow_route source address for nat case
net/netfilter/nf_tables_api.c | 75 +++++++++++++++++++++++++++++++---------
net/netfilter/nft_flow_offload.c | 6 ++--
2 files changed, 62 insertions(+), 19 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net 1/5] netfilter: nf_tables: sanitize nft_set_desc_concat_parse() 2022-05-31 21:58 [PATCH net 0/5] Netfilter fixes for net Pablo Neira Ayuso @ 2022-05-31 21:58 ` Pablo Neira Ayuso 2022-06-01 4:10 ` patchwork-bot+netdevbpf 2022-05-31 21:58 ` [PATCH net 2/5] netfilter: nf_tables: hold mutex on netns pre_exit path Pablo Neira Ayuso ` (3 subsequent siblings) 4 siblings, 1 reply; 8+ messages in thread From: Pablo Neira Ayuso @ 2022-05-31 21:58 UTC (permalink / raw) To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet Add several sanity checks for nft_set_desc_concat_parse(): - validate desc->field_count not larger than desc->field_len array. - field length cannot be larger than desc->field_len (ie. U8_MAX) - total length of the concatenation cannot be larger than register array. Joint work with Florian Westphal. Fixes: f3a2181e16f1 ("netfilter: nf_tables: Support for sets with multiple ranged fields") Reported-by: <zhangziming.zzm@antgroup.com> Reviewed-by: Stefano Brivio <sbrivio@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nf_tables_api.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index f4f1d0a2da43..dcefb5f36b3a 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -4246,6 +4246,9 @@ static int nft_set_desc_concat_parse(const struct nlattr *attr, u32 len; int err; + if (desc->field_count >= ARRAY_SIZE(desc->field_len)) + return -E2BIG; + err = nla_parse_nested_deprecated(tb, NFTA_SET_FIELD_MAX, attr, nft_concat_policy, NULL); if (err < 0) @@ -4255,9 +4258,8 @@ static int nft_set_desc_concat_parse(const struct nlattr *attr, return -EINVAL; len = ntohl(nla_get_be32(tb[NFTA_SET_FIELD_LEN])); - - if (len * BITS_PER_BYTE / 32 > NFT_REG32_COUNT) - return -E2BIG; + if (!len || len > U8_MAX) + return -EINVAL; desc->field_len[desc->field_count++] = len; @@ -4268,7 +4270,8 @@ static int nft_set_desc_concat(struct nft_set_desc *desc, const struct nlattr *nla) { struct nlattr *attr; - int rem, err; + u32 num_regs = 0; + int rem, err, i; nla_for_each_nested(attr, nla, rem) { if (nla_type(attr) != NFTA_LIST_ELEM) @@ -4279,6 +4282,12 @@ static int nft_set_desc_concat(struct nft_set_desc *desc, return err; } + for (i = 0; i < desc->field_count; i++) + num_regs += DIV_ROUND_UP(desc->field_len[i], sizeof(u32)); + + if (num_regs > NFT_REG32_COUNT) + return -E2BIG; + return 0; } -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/5] netfilter: nf_tables: sanitize nft_set_desc_concat_parse() 2022-05-31 21:58 ` [PATCH net 1/5] netfilter: nf_tables: sanitize nft_set_desc_concat_parse() Pablo Neira Ayuso @ 2022-06-01 4:10 ` patchwork-bot+netdevbpf 0 siblings, 0 replies; 8+ messages in thread From: patchwork-bot+netdevbpf @ 2022-06-01 4:10 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel, davem, netdev, kuba, pabeni, edumazet Hello: This series was applied to netdev/net.git (master) by Pablo Neira Ayuso <pablo@netfilter.org>: On Tue, 31 May 2022 23:58:35 +0200 you wrote: > Add several sanity checks for nft_set_desc_concat_parse(): > > - validate desc->field_count not larger than desc->field_len array. > - field length cannot be larger than desc->field_len (ie. U8_MAX) > - total length of the concatenation cannot be larger than register array. > > Joint work with Florian Westphal. > > [...] Here is the summary with links: - [net,1/5] netfilter: nf_tables: sanitize nft_set_desc_concat_parse() https://git.kernel.org/netdev/net/c/fecf31ee395b - [net,2/5] netfilter: nf_tables: hold mutex on netns pre_exit path https://git.kernel.org/netdev/net/c/3923b1e44066 - [net,3/5] netfilter: nf_tables: double hook unregistration in netns path https://git.kernel.org/netdev/net/c/f9a43007d3f7 - [net,4/5] netfilter: flowtable: fix missing FLOWI_FLAG_ANYSRC flag https://git.kernel.org/netdev/net/c/f1896d45fee9 - [net,5/5] netfilter: flowtable: fix nft_flow_route source address for nat case https://git.kernel.org/netdev/net/c/97629b237a8c You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net 2/5] netfilter: nf_tables: hold mutex on netns pre_exit path 2022-05-31 21:58 [PATCH net 0/5] Netfilter fixes for net Pablo Neira Ayuso 2022-05-31 21:58 ` [PATCH net 1/5] netfilter: nf_tables: sanitize nft_set_desc_concat_parse() Pablo Neira Ayuso @ 2022-05-31 21:58 ` Pablo Neira Ayuso 2022-05-31 21:58 ` [PATCH net 3/5] netfilter: nf_tables: double hook unregistration in netns path Pablo Neira Ayuso ` (2 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Pablo Neira Ayuso @ 2022-05-31 21:58 UTC (permalink / raw) To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet clean_net() runs in workqueue while walking over the lists, grab mutex. Fixes: 767d1216bff8 ("netfilter: nftables: fix possible UAF over chains from packet path in netns") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nf_tables_api.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index dcefb5f36b3a..f77414e13de1 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -9896,7 +9896,11 @@ static int __net_init nf_tables_init_net(struct net *net) static void __net_exit nf_tables_pre_exit_net(struct net *net) { + struct nftables_pernet *nft_net = nft_pernet(net); + + mutex_lock(&nft_net->commit_mutex); __nft_release_hooks(net); + mutex_unlock(&nft_net->commit_mutex); } static void __net_exit nf_tables_exit_net(struct net *net) -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net 3/5] netfilter: nf_tables: double hook unregistration in netns path 2022-05-31 21:58 [PATCH net 0/5] Netfilter fixes for net Pablo Neira Ayuso 2022-05-31 21:58 ` [PATCH net 1/5] netfilter: nf_tables: sanitize nft_set_desc_concat_parse() Pablo Neira Ayuso 2022-05-31 21:58 ` [PATCH net 2/5] netfilter: nf_tables: hold mutex on netns pre_exit path Pablo Neira Ayuso @ 2022-05-31 21:58 ` Pablo Neira Ayuso 2022-06-01 3:59 ` Jakub Kicinski 2022-05-31 21:58 ` [PATCH net 4/5] netfilter: flowtable: fix missing FLOWI_FLAG_ANYSRC flag Pablo Neira Ayuso 2022-05-31 21:58 ` [PATCH net 5/5] netfilter: flowtable: fix nft_flow_route source address for nat case Pablo Neira Ayuso 4 siblings, 1 reply; 8+ messages in thread From: Pablo Neira Ayuso @ 2022-05-31 21:58 UTC (permalink / raw) To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet __nft_release_hooks() is called from pre_netns exit path which unregisters the hooks, then the NETDEV_UNREGISTER event is triggered which unregisters the hooks again. [ 565.221461] WARNING: CPU: 18 PID: 193 at net/netfilter/core.c:495 __nf_unregister_net_hook+0x247/0x270 [...] [ 565.246890] CPU: 18 PID: 193 Comm: kworker/u64:1 Tainted: G E 5.18.0-rc7+ #27 [ 565.253682] Workqueue: netns cleanup_net [ 565.257059] RIP: 0010:__nf_unregister_net_hook+0x247/0x270 [...] [ 565.297120] Call Trace: [ 565.300900] <TASK> [ 565.304683] nf_tables_flowtable_event+0x16a/0x220 [nf_tables] [ 565.308518] raw_notifier_call_chain+0x63/0x80 [ 565.312386] unregister_netdevice_many+0x54f/0xb50 Unregister and destroy netdev hook from netns pre_exit via kfree_rcu so the NETDEV_UNREGISTER path see unregistered hooks. Fixes: 767d1216bff8 ("netfilter: nftables: fix possible UAF over chains from packet path in netns") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nf_tables_api.c | 54 ++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index f77414e13de1..746be13438ef 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -222,12 +222,18 @@ static int nft_netdev_register_hooks(struct net *net, } static void nft_netdev_unregister_hooks(struct net *net, - struct list_head *hook_list) + struct list_head *hook_list, + bool release_netdev) { - struct nft_hook *hook; + struct nft_hook *hook, *next; - list_for_each_entry(hook, hook_list, list) + list_for_each_entry_safe(hook, next, hook_list, list) { nf_unregister_net_hook(net, &hook->ops); + if (release_netdev) { + list_del(&hook->list); + kfree_rcu(hook, rcu); + } + } } static int nf_tables_register_hook(struct net *net, @@ -253,9 +259,10 @@ static int nf_tables_register_hook(struct net *net, return nf_register_net_hook(net, &basechain->ops); } -static void nf_tables_unregister_hook(struct net *net, - const struct nft_table *table, - struct nft_chain *chain) +static void __nf_tables_unregister_hook(struct net *net, + const struct nft_table *table, + struct nft_chain *chain, + bool release_netdev) { struct nft_base_chain *basechain; const struct nf_hook_ops *ops; @@ -270,11 +277,19 @@ static void nf_tables_unregister_hook(struct net *net, return basechain->type->ops_unregister(net, ops); if (nft_base_chain_netdev(table->family, basechain->ops.hooknum)) - nft_netdev_unregister_hooks(net, &basechain->hook_list); + nft_netdev_unregister_hooks(net, &basechain->hook_list, + release_netdev); else nf_unregister_net_hook(net, &basechain->ops); } +static void nf_tables_unregister_hook(struct net *net, + const struct nft_table *table, + struct nft_chain *chain) +{ + return __nf_tables_unregister_hook(net, table, chain, false); +} + static void nft_trans_commit_list_add_tail(struct net *net, struct nft_trans *trans) { struct nftables_pernet *nft_net = nft_pernet(net); @@ -7307,13 +7322,25 @@ static void nft_unregister_flowtable_hook(struct net *net, FLOW_BLOCK_UNBIND); } -static void nft_unregister_flowtable_net_hooks(struct net *net, - struct list_head *hook_list) +static void __nft_unregister_flowtable_net_hooks(struct net *net, + struct list_head *hook_list, + bool release_netdev) { - struct nft_hook *hook; + struct nft_hook *hook, *next; - list_for_each_entry(hook, hook_list, list) + list_for_each_entry_safe(hook, next, hook_list, list) { nf_unregister_net_hook(net, &hook->ops); + if (release_netdev) { + list_del(&hook->list); + kfree_rcu(hook); + } + } +} + +static void nft_unregister_flowtable_net_hooks(struct net *net, + struct list_head *hook_list) +{ + __nft_unregister_flowtable_net_hooks(net, hook_list, false); } static int nft_register_flowtable_net_hooks(struct net *net, @@ -9755,9 +9782,10 @@ static void __nft_release_hook(struct net *net, struct nft_table *table) struct nft_chain *chain; list_for_each_entry(chain, &table->chains, list) - nf_tables_unregister_hook(net, table, chain); + __nf_tables_unregister_hook(net, table, chain, true); list_for_each_entry(flowtable, &table->flowtables, list) - nft_unregister_flowtable_net_hooks(net, &flowtable->hook_list); + __nft_unregister_flowtable_net_hooks(net, &flowtable->hook_list, + true); } static void __nft_release_hooks(struct net *net) -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net 3/5] netfilter: nf_tables: double hook unregistration in netns path 2022-05-31 21:58 ` [PATCH net 3/5] netfilter: nf_tables: double hook unregistration in netns path Pablo Neira Ayuso @ 2022-06-01 3:59 ` Jakub Kicinski 0 siblings, 0 replies; 8+ messages in thread From: Jakub Kicinski @ 2022-06-01 3:59 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel, davem, netdev, pabeni, edumazet On Tue, 31 May 2022 23:58:37 +0200 Pablo Neira Ayuso wrote: > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index f77414e13de1..746be13438ef 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -222,12 +222,18 @@ static int nft_netdev_register_hooks(struct net *net, > } > > static void nft_netdev_unregister_hooks(struct net *net, > - struct list_head *hook_list) > + struct list_head *hook_list, > + bool release_netdev) FWIW ERROR: code indent should use tabs where possible #115: FILE: net/netfilter/nf_tables_api.c:7327: +^I^I^I^I^I bool release_netdev)$ ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net 4/5] netfilter: flowtable: fix missing FLOWI_FLAG_ANYSRC flag 2022-05-31 21:58 [PATCH net 0/5] Netfilter fixes for net Pablo Neira Ayuso ` (2 preceding siblings ...) 2022-05-31 21:58 ` [PATCH net 3/5] netfilter: nf_tables: double hook unregistration in netns path Pablo Neira Ayuso @ 2022-05-31 21:58 ` Pablo Neira Ayuso 2022-05-31 21:58 ` [PATCH net 5/5] netfilter: flowtable: fix nft_flow_route source address for nat case Pablo Neira Ayuso 4 siblings, 0 replies; 8+ messages in thread From: Pablo Neira Ayuso @ 2022-05-31 21:58 UTC (permalink / raw) To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet From: wenxu <wenxu@chinatelecom.cn> The nf_flow_table gets route through ip_route_output_key. If the saddr is not local one, then FLOWI_FLAG_ANYSRC flag should be set. Without this flag, the route lookup for other_dst will fail. Fixes: 3412e1641828 (netfilter: flowtable: nft_flow_route use more data for reverse route) Signed-off-by: wenxu <wenxu@chinatelecom.cn> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nft_flow_offload.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c index a16cf47199b7..20e5f372dd76 100644 --- a/net/netfilter/nft_flow_offload.c +++ b/net/netfilter/nft_flow_offload.c @@ -237,6 +237,7 @@ static int nft_flow_route(const struct nft_pktinfo *pkt, fl.u.ip4.flowi4_iif = this_dst->dev->ifindex; fl.u.ip4.flowi4_tos = RT_TOS(ip_hdr(pkt->skb)->tos); fl.u.ip4.flowi4_mark = pkt->skb->mark; + fl.u.ip4.flowi4_flags = FLOWI_FLAG_ANYSRC; break; case NFPROTO_IPV6: fl.u.ip6.daddr = ct->tuplehash[dir].tuple.src.u3.in6; @@ -245,6 +246,7 @@ static int nft_flow_route(const struct nft_pktinfo *pkt, fl.u.ip6.flowi6_iif = this_dst->dev->ifindex; fl.u.ip6.flowlabel = ip6_flowinfo(ipv6_hdr(pkt->skb)); fl.u.ip6.flowi6_mark = pkt->skb->mark; + fl.u.ip6.flowi6_flags = FLOWI_FLAG_ANYSRC; break; } -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net 5/5] netfilter: flowtable: fix nft_flow_route source address for nat case 2022-05-31 21:58 [PATCH net 0/5] Netfilter fixes for net Pablo Neira Ayuso ` (3 preceding siblings ...) 2022-05-31 21:58 ` [PATCH net 4/5] netfilter: flowtable: fix missing FLOWI_FLAG_ANYSRC flag Pablo Neira Ayuso @ 2022-05-31 21:58 ` Pablo Neira Ayuso 4 siblings, 0 replies; 8+ messages in thread From: Pablo Neira Ayuso @ 2022-05-31 21:58 UTC (permalink / raw) To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet From: wenxu <wenxu@chinatelecom.cn> For snat and dnat cases, the saddr should be taken from reverse tuple. Fixes: 3412e1641828 (netfilter: flowtable: nft_flow_route use more data for reverse route) Signed-off-by: wenxu <wenxu@chinatelecom.cn> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nft_flow_offload.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c index 20e5f372dd76..a25c88bc8b75 100644 --- a/net/netfilter/nft_flow_offload.c +++ b/net/netfilter/nft_flow_offload.c @@ -232,7 +232,7 @@ static int nft_flow_route(const struct nft_pktinfo *pkt, switch (nft_pf(pkt)) { case NFPROTO_IPV4: fl.u.ip4.daddr = ct->tuplehash[dir].tuple.src.u3.ip; - fl.u.ip4.saddr = ct->tuplehash[dir].tuple.dst.u3.ip; + fl.u.ip4.saddr = ct->tuplehash[!dir].tuple.src.u3.ip; fl.u.ip4.flowi4_oif = nft_in(pkt)->ifindex; fl.u.ip4.flowi4_iif = this_dst->dev->ifindex; fl.u.ip4.flowi4_tos = RT_TOS(ip_hdr(pkt->skb)->tos); @@ -241,7 +241,7 @@ static int nft_flow_route(const struct nft_pktinfo *pkt, break; case NFPROTO_IPV6: fl.u.ip6.daddr = ct->tuplehash[dir].tuple.src.u3.in6; - fl.u.ip6.saddr = ct->tuplehash[dir].tuple.dst.u3.in6; + fl.u.ip6.saddr = ct->tuplehash[!dir].tuple.src.u3.in6; fl.u.ip6.flowi6_oif = nft_in(pkt)->ifindex; fl.u.ip6.flowi6_iif = this_dst->dev->ifindex; fl.u.ip6.flowlabel = ip6_flowinfo(ipv6_hdr(pkt->skb)); -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-06-01 4:10 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-05-31 21:58 [PATCH net 0/5] Netfilter fixes for net Pablo Neira Ayuso 2022-05-31 21:58 ` [PATCH net 1/5] netfilter: nf_tables: sanitize nft_set_desc_concat_parse() Pablo Neira Ayuso 2022-06-01 4:10 ` patchwork-bot+netdevbpf 2022-05-31 21:58 ` [PATCH net 2/5] netfilter: nf_tables: hold mutex on netns pre_exit path Pablo Neira Ayuso 2022-05-31 21:58 ` [PATCH net 3/5] netfilter: nf_tables: double hook unregistration in netns path Pablo Neira Ayuso 2022-06-01 3:59 ` Jakub Kicinski 2022-05-31 21:58 ` [PATCH net 4/5] netfilter: flowtable: fix missing FLOWI_FLAG_ANYSRC flag Pablo Neira Ayuso 2022-05-31 21:58 ` [PATCH net 5/5] netfilter: flowtable: fix nft_flow_route source address for nat case Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).