From ffd219efd9ee1ceccc7ccfa9361fd40705680fc3 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Fri, 20 May 2022 00:02:03 +0200 Subject: netfilter: nfnetlink: fix warn in nfnetlink_unbind syzbot reports following warn: WARNING: CPU: 0 PID: 3600 at net/netfilter/nfnetlink.c:703 nfnetlink_unbind+0x357/0x3b0 net/netfilter/nfnetlink.c:694 The syzbot generated program does this: socket(AF_NETLINK, SOCK_RAW, NETLINK_NETFILTER) = 3 setsockopt(3, SOL_NETLINK, NETLINK_DROP_MEMBERSHIP, [1], 4) = 0 ... which triggers 'WARN_ON_ONCE(nfnlnet->ctnetlink_listeners == 0)' check. Instead of counting, just enable reporting for every bind request and check if we still have listeners on unbind. While at it, also add the needed bounds check on nfnl_group2type[] access. Reported-by: Reported-by: Fixes: 2794cdb0b97b ("netfilter: nfnetlink: allow to detect if ctnetlink listeners exist") Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nfnetlink.c | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c index ad3bbe34ca88..2f7c477fc9e7 100644 --- a/net/netfilter/nfnetlink.c +++ b/net/netfilter/nfnetlink.c @@ -45,7 +45,6 @@ MODULE_DESCRIPTION("Netfilter messages via netlink socket"); static unsigned int nfnetlink_pernet_id __read_mostly; struct nfnl_net { - unsigned int ctnetlink_listeners; struct sock *nfnl; }; @@ -673,18 +672,8 @@ static int nfnetlink_bind(struct net *net, int group) #ifdef CONFIG_NF_CONNTRACK_EVENTS if (type == NFNL_SUBSYS_CTNETLINK) { - struct nfnl_net *nfnlnet = nfnl_pernet(net); - nfnl_lock(NFNL_SUBSYS_CTNETLINK); - - if (WARN_ON_ONCE(nfnlnet->ctnetlink_listeners == UINT_MAX)) { - nfnl_unlock(NFNL_SUBSYS_CTNETLINK); - return -EOVERFLOW; - } - - nfnlnet->ctnetlink_listeners++; - if (nfnlnet->ctnetlink_listeners == 1) - WRITE_ONCE(net->ct.ctnetlink_has_listener, true); + WRITE_ONCE(net->ct.ctnetlink_has_listener, true); nfnl_unlock(NFNL_SUBSYS_CTNETLINK); } #endif @@ -694,15 +683,12 @@ static int nfnetlink_bind(struct net *net, int group) static void nfnetlink_unbind(struct net *net, int group) { #ifdef CONFIG_NF_CONNTRACK_EVENTS - int type = nfnl_group2type[group]; - - if (type == NFNL_SUBSYS_CTNETLINK) { - struct nfnl_net *nfnlnet = nfnl_pernet(net); + if (group <= NFNLGRP_NONE || group > NFNLGRP_MAX) + return; + if (nfnl_group2type[group] == NFNL_SUBSYS_CTNETLINK) { nfnl_lock(NFNL_SUBSYS_CTNETLINK); - WARN_ON_ONCE(nfnlnet->ctnetlink_listeners == 0); - nfnlnet->ctnetlink_listeners--; - if (nfnlnet->ctnetlink_listeners == 0) + if (!nfnetlink_has_listeners(net, group)) WRITE_ONCE(net->ct.ctnetlink_has_listener, false); nfnl_unlock(NFNL_SUBSYS_CTNETLINK); } -- cgit v1.2.3 From 56b14ecec97f39118bf85c9ac2438c5a949509ed Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Fri, 20 May 2022 00:02:04 +0200 Subject: netfilter: conntrack: re-fetch conntrack after insertion In case the conntrack is clashing, insertion can free skb->_nfct and set skb->_nfct to the already-confirmed entry. This wasn't found before because the conntrack entry and the extension space used to free'd after an rcu grace period, plus the race needs events enabled to trigger. Reported-by: Fixes: 71d8c47fc653 ("netfilter: conntrack: introduce clash resolution on insertion race") Fixes: 2ad9d7747c10 ("netfilter: conntrack: free extension area immediately") Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- include/net/netfilter/nf_conntrack_core.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h index 6406cfee34c2..37866c8386e2 100644 --- a/include/net/netfilter/nf_conntrack_core.h +++ b/include/net/netfilter/nf_conntrack_core.h @@ -58,8 +58,13 @@ static inline int nf_conntrack_confirm(struct sk_buff *skb) int ret = NF_ACCEPT; if (ct) { - if (!nf_ct_is_confirmed(ct)) + if (!nf_ct_is_confirmed(ct)) { ret = __nf_conntrack_confirm(skb); + + if (ret == NF_ACCEPT) + ct = (struct nf_conn *)skb_nfct(skb); + } + if (ret == NF_ACCEPT && nf_ct_ecache_exist(ct)) nf_ct_deliver_cached_events(ct); } -- cgit v1.2.3 From aeed55a08d0b2bf2e3986b110b04d2a5aca192a6 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Fri, 20 May 2022 00:02:05 +0200 Subject: netfilter: cttimeout: fix slab-out-of-bounds read in cttimeout_net_exit syzbot reports: BUG: KASAN: slab-out-of-bounds in __list_del_entry_valid+0xcc/0xf0 lib/list_debug.c:42 [..] list_del include/linux/list.h:148 [inline] cttimeout_net_exit+0x211/0x540 net/netfilter/nfnetlink_cttimeout.c:617 No reproducer so far. Looking at recent changes in this area its clear that the free_head must not be at the end of the structure because nf_ct_timeout structure has variable size. Reported-by: Fixes: 78222bacfca9 ("netfilter: cttimeout: decouple unlink and free on netns destruction") Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nfnetlink_cttimeout.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c index f069c24c6146..af15102bc696 100644 --- a/net/netfilter/nfnetlink_cttimeout.c +++ b/net/netfilter/nfnetlink_cttimeout.c @@ -35,12 +35,13 @@ static unsigned int nfct_timeout_id __read_mostly; struct ctnl_timeout { struct list_head head; + struct list_head free_head; struct rcu_head rcu_head; refcount_t refcnt; char name[CTNL_TIMEOUT_NAME_MAX]; - struct nf_ct_timeout timeout; - struct list_head free_head; + /* must be at the end */ + struct nf_ct_timeout timeout; }; struct nfct_timeout_pernet { -- cgit v1.2.3 From b53c116642502b0c85ecef78bff4f826a7dd4145 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Fri, 20 May 2022 00:02:06 +0200 Subject: netfilter: nf_tables: set element extended ACK reporting support Report the element that causes problems via netlink extended ACK for set element commands. Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_tables_api.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index f296dfe86b62..f4f1d0a2da43 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -5348,8 +5348,10 @@ static int nf_tables_getsetelem(struct sk_buff *skb, nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) { err = nft_get_set_elem(&ctx, set, attr); - if (err < 0) + if (err < 0) { + NL_SET_BAD_ATTR(extack, attr); break; + } } return err; @@ -6126,8 +6128,10 @@ static int nf_tables_newsetelem(struct sk_buff *skb, nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) { err = nft_add_set_elem(&ctx, set, attr, info->nlh->nlmsg_flags); - if (err < 0) + if (err < 0) { + NL_SET_BAD_ATTR(extack, attr); return err; + } } if (nft_net->validate_state == NFT_VALIDATE_DO) @@ -6397,8 +6401,10 @@ static int nf_tables_delsetelem(struct sk_buff *skb, nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) { err = nft_del_setelem(&ctx, set, attr); - if (err < 0) + if (err < 0) { + NL_SET_BAD_ATTR(extack, attr); break; + } } return err; } -- cgit v1.2.3