* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.