Skip to content

Commit 9669822

Browse files
author
CKI KWF Bot
committed
Merge: netfilter: nf_tables: spurious false negative matching for set elements during transactions
MR: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-10/-/merge_requests/1638 JIRA: https://issues.redhat.com/browse/RHEL-113001 The hash, hash_fast, rhash and bitwise sets may indicate no result even though a matching element exists during a short time window while other cpu is finalizing the transaction. This happens when the hash lookup/bitwise lookup function has picked up the old genbit, right before it was toggled by nf_tables_commit(), but then the same cpu managed to unlink the matching old element from the hash table. Signed-off-by: Florian Westphal <fwestpha@redhat.com> Approved-by: Murphy Zhou <xzhou@redhat.com> Approved-by: Phil Sutter <psutter@redhat.com> Approved-by: CKI KWF Bot <cki-ci-bot+kwf-gitlab-com@redhat.com> Merged-by: CKI GitLab Kmaint Pipeline Bot <26919896-cki-kmaint-pipeline-bot@users.noreply.gitlab.com>
2 parents 326e624 + 39c4be3 commit 9669822

File tree

13 files changed

+281
-306
lines changed

13 files changed

+281
-306
lines changed

‎include/net/netfilter/nf_tables.h‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -459,19 +459,13 @@ struct nft_set_ext;
459459
* control plane functions.
460460
*/
461461
struct nft_set_ops {
462-
bool (*lookup)(const struct net *net,
462+
const struct nft_set_ext * (*lookup)(const struct net *net,
463463
const struct nft_set *set,
464+
const u32 *key);
465+
const struct nft_set_ext * (*update)(struct nft_set *set,
464466
const u32 *key,
465-
const struct nft_set_ext **ext);
466-
bool (*update)(struct nft_set *set,
467-
const u32 *key,
468-
struct nft_elem_priv *
469-
(*new)(struct nft_set *,
470-
const struct nft_expr *,
471-
struct nft_regs *),
472467
const struct nft_expr *expr,
473-
struct nft_regs *regs,
474-
const struct nft_set_ext **ext);
468+
struct nft_regs *regs);
475469
bool (*delete)(const struct nft_set *set,
476470
const u32 *key);
477471

@@ -1913,7 +1907,6 @@ struct nftables_pernet {
19131907
struct mutex commit_mutex;
19141908
u64 table_handle;
19151909
u64 tstamp;
1916-
unsigned int base_seq;
19171910
unsigned int gc_seq;
19181911
u8 validate_state;
19191912
struct work_struct destroy_work;

‎include/net/netfilter/nf_tables_core.h‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -94,34 +94,35 @@ extern const struct nft_set_type nft_set_pipapo_type;
9494
extern const struct nft_set_type nft_set_pipapo_avx2_type;
9595

9696
#ifdef CONFIG_MITIGATION_RETPOLINE
97-
bool nft_rhash_lookup(const struct net *net, const struct nft_set *set,
98-
const u32 *key, const struct nft_set_ext **ext);
99-
bool nft_rbtree_lookup(const struct net *net, const struct nft_set *set,
100-
const u32 *key, const struct nft_set_ext **ext);
101-
bool nft_bitmap_lookup(const struct net *net, const struct nft_set *set,
102-
const u32 *key, const struct nft_set_ext **ext);
103-
bool nft_hash_lookup_fast(const struct net *net,
104-
const struct nft_set *set,
105-
const u32 *key, const struct nft_set_ext **ext);
106-
bool nft_hash_lookup(const struct net *net, const struct nft_set *set,
107-
const u32 *key, const struct nft_set_ext **ext);
108-
bool nft_set_do_lookup(const struct net *net, const struct nft_set *set,
109-
const u32 *key, const struct nft_set_ext **ext);
110-
#else
111-
static inline bool
112-
nft_set_do_lookup(const struct net *net, const struct nft_set *set,
113-
const u32 *key, const struct nft_set_ext **ext)
114-
{
115-
return set->ops->lookup(net, set, key, ext);
116-
}
97+
const struct nft_set_ext *
98+
nft_rhash_lookup(const struct net *net, const struct nft_set *set,
99+
const u32 *key);
100+
const struct nft_set_ext *
101+
nft_rbtree_lookup(const struct net *net, const struct nft_set *set,
102+
const u32 *key);
103+
const struct nft_set_ext *
104+
nft_bitmap_lookup(const struct net *net, const struct nft_set *set,
105+
const u32 *key);
106+
const struct nft_set_ext *
107+
nft_hash_lookup_fast(const struct net *net, const struct nft_set *set,
108+
const u32 *key);
109+
const struct nft_set_ext *
110+
nft_hash_lookup(const struct net *net, const struct nft_set *set,
111+
const u32 *key);
117112
#endif
118113

114+
const struct nft_set_ext *
115+
nft_set_do_lookup(const struct net *net, const struct nft_set *set,
116+
const u32 *key);
117+
119118
/* called from nft_pipapo_avx2.c */
120-
bool nft_pipapo_lookup(const struct net *net, const struct nft_set *set,
121-
const u32 *key, const struct nft_set_ext **ext);
119+
const struct nft_set_ext *
120+
nft_pipapo_lookup(const struct net *net, const struct nft_set *set,
121+
const u32 *key);
122122
/* called from nft_set_pipapo.c */
123-
bool nft_pipapo_avx2_lookup(const struct net *net, const struct nft_set *set,
124-
const u32 *key, const struct nft_set_ext **ext);
123+
const struct nft_set_ext *
124+
nft_pipapo_avx2_lookup(const struct net *net, const struct nft_set *set,
125+
const u32 *key);
125126

126127
void nft_counter_init_seqcount(void);
127128

@@ -181,4 +182,7 @@ void nft_objref_eval(const struct nft_expr *expr, struct nft_regs *regs,
181182
const struct nft_pktinfo *pkt);
182183
void nft_objref_map_eval(const struct nft_expr *expr, struct nft_regs *regs,
183184
const struct nft_pktinfo *pkt);
185+
struct nft_elem_priv *nft_dynset_new(struct nft_set *set,
186+
const struct nft_expr *expr,
187+
struct nft_regs *regs);
184188
#endif /* _NET_NF_TABLES_CORE_H */

‎include/net/netns/nftables.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#define _NETNS_NFTABLES_H_
44

55
struct netns_nftables {
6+
unsigned int base_seq;
67
u8 gencursor;
78
};
89

‎net/netfilter/nf_tables_api.c‎

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,11 +1106,14 @@ nf_tables_chain_type_lookup(struct net *net, const struct nlattr *nla,
11061106
return ERR_PTR(-ENOENT);
11071107
}
11081108

1109-
static __be16 nft_base_seq(const struct net *net)
1109+
static unsigned int nft_base_seq(const struct net *net)
11101110
{
1111-
struct nftables_pernet *nft_net = nft_pernet(net);
1111+
return READ_ONCE(net->nft.base_seq);
1112+
}
11121113

1113-
return htons(nft_net->base_seq & 0xffff);
1114+
static __be16 nft_base_seq_be16(const struct net *net)
1115+
{
1116+
return htons(nft_base_seq(net) & 0xffff);
11141117
}
11151118

11161119
static const struct nla_policy nft_table_policy[NFTA_TABLE_MAX + 1] = {
@@ -1130,7 +1133,7 @@ static int nf_tables_fill_table_info(struct sk_buff *skb, struct net *net,
11301133

11311134
event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
11321135
nlh = nfnl_msg_put(skb, portid, seq, event, flags, family,
1133-
NFNETLINK_V0, nft_base_seq(net));
1136+
NFNETLINK_V0, nft_base_seq_be16(net));
11341137
if (!nlh)
11351138
goto nla_put_failure;
11361139

@@ -1222,7 +1225,7 @@ static int nf_tables_dump_tables(struct sk_buff *skb,
12221225

12231226
rcu_read_lock();
12241227
nft_net = nft_pernet(net);
1225-
cb->seq = READ_ONCE(nft_net->base_seq);
1228+
cb->seq = nft_base_seq(net);
12261229

12271230
list_for_each_entry_rcu(table, &nft_net->tables, list) {
12281231
if (family != NFPROTO_UNSPEC && family != table->family)
@@ -1992,7 +1995,7 @@ static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net,
19921995

19931996
event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
19941997
nlh = nfnl_msg_put(skb, portid, seq, event, flags, family,
1995-
NFNETLINK_V0, nft_base_seq(net));
1998+
NFNETLINK_V0, nft_base_seq_be16(net));
19961999
if (!nlh)
19972000
goto nla_put_failure;
19982001

@@ -2093,7 +2096,7 @@ static int nf_tables_dump_chains(struct sk_buff *skb,
20932096

20942097
rcu_read_lock();
20952098
nft_net = nft_pernet(net);
2096-
cb->seq = READ_ONCE(nft_net->base_seq);
2099+
cb->seq = nft_base_seq(net);
20972100

20982101
list_for_each_entry_rcu(table, &nft_net->tables, list) {
20992102
if (family != NFPROTO_UNSPEC && family != table->family)
@@ -3604,7 +3607,7 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
36043607
u16 type = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
36053608

36063609
nlh = nfnl_msg_put(skb, portid, seq, type, flags, family, NFNETLINK_V0,
3607-
nft_base_seq(net));
3610+
nft_base_seq_be16(net));
36083611
if (!nlh)
36093612
goto nla_put_failure;
36103613

@@ -3772,7 +3775,7 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
37723775

37733776
rcu_read_lock();
37743777
nft_net = nft_pernet(net);
3775-
cb->seq = READ_ONCE(nft_net->base_seq);
3778+
cb->seq = nft_base_seq(net);
37763779

37773780
list_for_each_entry_rcu(table, &nft_net->tables, list) {
37783781
if (family != NFPROTO_UNSPEC && family != table->family)
@@ -3983,7 +3986,7 @@ static int nf_tables_getrule_reset(struct sk_buff *skb,
39833986
buf = kasprintf(GFP_ATOMIC, "%.*s:%u",
39843987
nla_len(nla[NFTA_RULE_TABLE]),
39853988
(char *)nla_data(nla[NFTA_RULE_TABLE]),
3986-
nft_net->base_seq);
3989+
nft_base_seq(net));
39873990
audit_log_nfcfg(buf, info->nfmsg->nfgen_family, 1,
39883991
AUDIT_NFT_OP_RULE_RESET, GFP_ATOMIC);
39893992
kfree(buf);
@@ -4819,7 +4822,7 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
48194822

48204823
event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
48214824
nlh = nfnl_msg_put(skb, portid, seq, event, flags, ctx->family,
4822-
NFNETLINK_V0, nft_base_seq(ctx->net));
4825+
NFNETLINK_V0, nft_base_seq_be16(ctx->net));
48234826
if (!nlh)
48244827
goto nla_put_failure;
48254828

@@ -4963,7 +4966,7 @@ static int nf_tables_dump_sets(struct sk_buff *skb, struct netlink_callback *cb)
49634966

49644967
rcu_read_lock();
49654968
nft_net = nft_pernet(net);
4966-
cb->seq = READ_ONCE(nft_net->base_seq);
4969+
cb->seq = nft_base_seq(net);
49674970

49684971
list_for_each_entry_rcu(table, &nft_net->tables, list) {
49694972
if (ctx->family != NFPROTO_UNSPEC &&
@@ -6140,7 +6143,7 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
61406143

61416144
rcu_read_lock();
61426145
nft_net = nft_pernet(net);
6143-
cb->seq = READ_ONCE(nft_net->base_seq);
6146+
cb->seq = nft_base_seq(net);
61446147

61456148
list_for_each_entry_rcu(table, &nft_net->tables, list) {
61466149
if (dump_ctx->ctx.family != NFPROTO_UNSPEC &&
@@ -6169,7 +6172,7 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
61696172
seq = cb->nlh->nlmsg_seq;
61706173

61716174
nlh = nfnl_msg_put(skb, portid, seq, event, NLM_F_MULTI,
6172-
table->family, NFNETLINK_V0, nft_base_seq(net));
6175+
table->family, NFNETLINK_V0, nft_base_seq_be16(net));
61736176
if (!nlh)
61746177
goto nla_put_failure;
61756178

@@ -6262,7 +6265,7 @@ static int nf_tables_fill_setelem_info(struct sk_buff *skb,
62626265

62636266
event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
62646267
nlh = nfnl_msg_put(skb, portid, seq, event, flags, ctx->family,
6265-
NFNETLINK_V0, nft_base_seq(ctx->net));
6268+
NFNETLINK_V0, nft_base_seq_be16(ctx->net));
62666269
if (!nlh)
62676270
goto nla_put_failure;
62686271

@@ -6561,7 +6564,7 @@ static int nf_tables_getsetelem_reset(struct sk_buff *skb,
65616564
}
65626565
nelems++;
65636566
}
6564-
audit_log_nft_set_reset(dump_ctx.ctx.table, nft_net->base_seq, nelems);
6567+
audit_log_nft_set_reset(dump_ctx.ctx.table, nft_base_seq(info->net), nelems);
65656568

65666569
out_unlock:
65676570
rcu_read_unlock();
@@ -8312,7 +8315,7 @@ static int nf_tables_fill_obj_info(struct sk_buff *skb, struct net *net,
83128315

83138316
event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
83148317
nlh = nfnl_msg_put(skb, portid, seq, event, flags, family,
8315-
NFNETLINK_V0, nft_base_seq(net));
8318+
NFNETLINK_V0, nft_base_seq_be16(net));
83168319
if (!nlh)
83178320
goto nla_put_failure;
83188321

@@ -8376,7 +8379,7 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
83768379

83778380
rcu_read_lock();
83788381
nft_net = nft_pernet(net);
8379-
cb->seq = READ_ONCE(nft_net->base_seq);
8382+
cb->seq = nft_base_seq(net);
83808383

83818384
list_for_each_entry_rcu(table, &nft_net->tables, list) {
83828385
if (family != NFPROTO_UNSPEC && family != table->family)
@@ -8410,7 +8413,7 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
84108413
idx++;
84118414
}
84128415
if (ctx->reset && entries)
8413-
audit_log_obj_reset(table, nft_net->base_seq, entries);
8416+
audit_log_obj_reset(table, nft_base_seq(net), entries);
84148417
if (rc < 0)
84158418
break;
84168419
}
@@ -8579,7 +8582,7 @@ static int nf_tables_getobj_reset(struct sk_buff *skb,
85798582
buf = kasprintf(GFP_ATOMIC, "%.*s:%u",
85808583
nla_len(nla[NFTA_OBJ_TABLE]),
85818584
(char *)nla_data(nla[NFTA_OBJ_TABLE]),
8582-
nft_net->base_seq);
8585+
nft_base_seq(net));
85838586
audit_log_nfcfg(buf, info->nfmsg->nfgen_family, 1,
85848587
AUDIT_NFT_OP_OBJ_RESET, GFP_ATOMIC);
85858588
kfree(buf);
@@ -8684,9 +8687,8 @@ void nft_obj_notify(struct net *net, const struct nft_table *table,
86848687
struct nft_object *obj, u32 portid, u32 seq, int event,
86858688
u16 flags, int family, int report, gfp_t gfp)
86868689
{
8687-
struct nftables_pernet *nft_net = nft_pernet(net);
86888690
char *buf = kasprintf(gfp, "%s:%u",
8689-
table->name, nft_net->base_seq);
8691+
table->name, nft_base_seq(net));
86908692

86918693
audit_log_nfcfg(buf,
86928694
family,
@@ -9352,7 +9354,7 @@ static int nf_tables_fill_flowtable_info(struct sk_buff *skb, struct net *net,
93529354

93539355
event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
93549356
nlh = nfnl_msg_put(skb, portid, seq, event, flags, family,
9355-
NFNETLINK_V0, nft_base_seq(net));
9357+
NFNETLINK_V0, nft_base_seq_be16(net));
93569358
if (!nlh)
93579359
goto nla_put_failure;
93589360

@@ -9419,7 +9421,7 @@ static int nf_tables_dump_flowtable(struct sk_buff *skb,
94199421

94209422
rcu_read_lock();
94219423
nft_net = nft_pernet(net);
9422-
cb->seq = READ_ONCE(nft_net->base_seq);
9424+
cb->seq = nft_base_seq(net);
94239425

94249426
list_for_each_entry_rcu(table, &nft_net->tables, list) {
94259427
if (family != NFPROTO_UNSPEC && family != table->family)
@@ -9604,17 +9606,16 @@ static void nf_tables_flowtable_destroy(struct nft_flowtable *flowtable)
96049606
static int nf_tables_fill_gen_info(struct sk_buff *skb, struct net *net,
96059607
u32 portid, u32 seq)
96069608
{
9607-
struct nftables_pernet *nft_net = nft_pernet(net);
96089609
struct nlmsghdr *nlh;
96099610
char buf[TASK_COMM_LEN];
96109611
int event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, NFT_MSG_NEWGEN);
96119612

96129613
nlh = nfnl_msg_put(skb, portid, seq, event, 0, AF_UNSPEC,
9613-
NFNETLINK_V0, nft_base_seq(net));
9614+
NFNETLINK_V0, nft_base_seq_be16(net));
96149615
if (!nlh)
96159616
goto nla_put_failure;
96169617

9617-
if (nla_put_be32(skb, NFTA_GEN_ID, htonl(nft_net->base_seq)) ||
9618+
if (nla_put_be32(skb, NFTA_GEN_ID, htonl(nft_base_seq(net))) ||
96189619
nla_put_be32(skb, NFTA_GEN_PROC_PID, htonl(task_pid_nr(current))) ||
96199620
nla_put_string(skb, NFTA_GEN_PROC_NAME, get_task_comm(buf, current)))
96209621
goto nla_put_failure;
@@ -10790,11 +10791,12 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
1079010791
* Bump generation counter, invalidate any dump in progress.
1079110792
* Cannot fail after this point.
1079210793
*/
10793-
base_seq = READ_ONCE(nft_net->base_seq);
10794+
base_seq = nft_base_seq(net);
1079410795
while (++base_seq == 0)
1079510796
;
1079610797

10797-
WRITE_ONCE(nft_net->base_seq, base_seq);
10798+
/* pairs with smp_load_acquire in nft_lookup_eval */
10799+
smp_store_release(&net->nft.base_seq, base_seq);
1079810800

1079910801
gc_seq = nft_gc_seq_begin(nft_net);
1080010802

@@ -11003,7 +11005,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
1100311005

1100411006
nft_commit_notify(net, NETLINK_CB(skb).portid);
1100511007
nf_tables_gen_notify(net, skb, NFT_MSG_NEWGEN);
11006-
nf_tables_commit_audit_log(&adl, nft_net->base_seq);
11008+
nf_tables_commit_audit_log(&adl, nft_base_seq(net));
1100711009

1100811010
nft_gc_seq_end(nft_net, gc_seq);
1100911011
nft_net->validate_state = NFT_VALIDATE_SKIP;
@@ -11328,7 +11330,7 @@ static bool nf_tables_valid_genid(struct net *net, u32 genid)
1132811330
mutex_lock(&nft_net->commit_mutex);
1132911331
nft_net->tstamp = get_jiffies_64();
1133011332

11331-
genid_ok = genid == 0 || nft_net->base_seq == genid;
11333+
genid_ok = genid == 0 || nft_base_seq(net) == genid;
1133211334
if (!genid_ok)
1133311335
mutex_unlock(&nft_net->commit_mutex);
1133411336

@@ -12006,7 +12008,7 @@ static int __net_init nf_tables_init_net(struct net *net)
1200612008
INIT_LIST_HEAD(&nft_net->module_list);
1200712009
INIT_LIST_HEAD(&nft_net->notify_list);
1200812010
mutex_init(&nft_net->commit_mutex);
12009-
nft_net->base_seq = 1;
12011+
net->nft.base_seq = 1;
1201012012
nft_net->gc_seq = 0;
1201112013
nft_net->validate_state = NFT_VALIDATE_SKIP;
1201212014
INIT_WORK(&nft_net->destroy_work, nf_tables_trans_destroy_work);

‎net/netfilter/nft_dynset.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ static int nft_dynset_expr_setup(const struct nft_dynset *priv,
4444
return 0;
4545
}
4646

47-
static struct nft_elem_priv *nft_dynset_new(struct nft_set *set,
48-
const struct nft_expr *expr,
49-
struct nft_regs *regs)
47+
struct nft_elem_priv *nft_dynset_new(struct nft_set *set,
48+
const struct nft_expr *expr,
49+
struct nft_regs *regs)
5050
{
5151
const struct nft_dynset *priv = nft_expr_priv(expr);
5252
struct nft_set_ext *ext;
@@ -91,8 +91,8 @@ void nft_dynset_eval(const struct nft_expr *expr,
9191
return;
9292
}
9393

94-
if (set->ops->update(set, &regs->data[priv->sreg_key], nft_dynset_new,
95-
expr, regs, &ext)) {
94+
ext = set->ops->update(set, &regs->data[priv->sreg_key], expr, regs);
95+
if (ext) {
9696
if (priv->op == NFT_DYNSET_OP_UPDATE &&
9797
nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT) &&
9898
READ_ONCE(nft_set_ext_timeout(ext)->timeout) != 0) {

0 commit comments

Comments
 (0)