[Devel] [PATCH RH7 2/3] netfilter: nf_tables: fix oops during rule dump

Pavel Tikhomirov ptikhomirov at virtuozzo.com
Wed Nov 1 13:46:54 MSK 2023


From: Florian Westphal <fw at strlen.de>

We can oops in nf_tables_fill_rule_info().

Its not possible to fetch previous element in rcu-protected lists
when deletions are not prevented somehow: list_del_rcu poisons
the ->prev pointer value.

Before rcu-conversion this was safe as dump operations did hold
nfnetlink mutex.

Pass previous rule as argument, obtained by keeping a pointer to
the previous rule during traversal.

Fixes: d9adf22a291883 ("netfilter: nf_tables: use call_rcu in netlink dumps")
Signed-off-by: Florian Westphal <fw at strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>

(cherry picked from commit 2c82c7e724ff51cab78e1afd5c2aaa31994fe41e)
Changes:
- move hunks from __nf_tables_dump_rules to nf_tables_dump_rules

https://virtuozzo.atlassian.net/browse/PSBM-150147
Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
---
 net/netfilter/nf_tables_api.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 2a3680da9cd2..21b7b0f81f8f 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1890,13 +1890,13 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
 				    u32 flags, int family,
 				    const struct nft_table *table,
 				    const struct nft_chain *chain,
-				    const struct nft_rule *rule)
+				    const struct nft_rule *rule,
+				    const struct nft_rule *prule)
 {
 	struct nlmsghdr *nlh;
 	struct nfgenmsg *nfmsg;
 	const struct nft_expr *expr, *next;
 	struct nlattr *list;
-	const struct nft_rule *prule;
 	int type = event | NFNL_SUBSYS_NFTABLES << 8;
 
 	nlh = nlmsg_put(skb, portid, seq, type, sizeof(struct nfgenmsg),
@@ -1917,8 +1917,7 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
 			 NFTA_RULE_PAD))
 		goto nla_put_failure;
 
-	if ((event != NFT_MSG_DELRULE) && (rule->list.prev != &chain->rules)) {
-		prule = list_entry(rule->list.prev, struct nft_rule, list);
+	if (event != NFT_MSG_DELRULE && prule) {
 		if (nla_put_be64(skb, NFTA_RULE_POSITION,
 				 cpu_to_be64(prule->handle),
 				 NFTA_RULE_PAD))
@@ -1967,7 +1966,7 @@ static int nf_tables_rule_notify(const struct nft_ctx *ctx,
 
 	err = nf_tables_fill_rule_info(skb, ctx->net, ctx->portid, ctx->seq,
 				       event, 0, ctx->afi->family, ctx->table,
-				       ctx->chain, rule);
+				       ctx->chain, rule, NULL);
 	if (err < 0) {
 		kfree_skb(skb);
 		goto err;
@@ -1996,7 +1995,7 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
 	const struct nft_af_info *afi;
 	const struct nft_table *table;
 	const struct nft_chain *chain;
-	const struct nft_rule *rule;
+	const struct nft_rule *rule, *prule = NULL;
 	unsigned int idx = 0, s_idx = cb->args[0];
 	struct net *net = sock_net(skb->sk);
 	int family = nfmsg->nfgen_family;
@@ -2020,7 +2019,7 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
 
 				list_for_each_entry_rcu(rule, &chain->rules, list) {
 					if (!nft_is_active(net, rule))
-						goto cont;
+						goto cont_skip;
 					if (idx < s_idx)
 						goto cont;
 					if (idx > s_idx)
@@ -2030,11 +2029,13 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
 								      cb->nlh->nlmsg_seq,
 								      NFT_MSG_NEWRULE,
 								      NLM_F_MULTI | NLM_F_APPEND,
-								      afi->family, table, chain, rule) < 0)
+								      afi->family, table, chain, rule, prule) < 0)
 						goto done;
 
 					nl_dump_check_consistent(cb, nlmsg_hdr(skb));
 cont:
+					prule = rule;
+cont_skip:
 					idx++;
 				}
 			}
@@ -2134,7 +2135,7 @@ static int nf_tables_getrule(struct sock *nlsk, struct sk_buff *skb,
 
 	err = nf_tables_fill_rule_info(skb2, net, NETLINK_CB(skb).portid,
 				       nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0,
-				       family, table, chain, rule);
+				       family, table, chain, rule, NULL);
 	if (err < 0)
 		goto err;
 
-- 
2.41.0



More information about the Devel mailing list