Skip to content

Commit 9b9ac46

Browse files
committed
Florian Westphal says: ==================== netfilter: updates for net First patch, from Phil Sutter, reduces number of audit notifications when userspace requests to re-set stateful objects. This change also comes with a selftest update. Second patch, also from Phil, moves the nftables audit selftest to its own netns to avoid interference with the init netns. Third patch, from Pablo Neira, fixes an inconsistency with the "rbtree" set backend: When set element X has expired, a request to delete element X should fail (like with all other backends). Finally, patch four, also from Pablo, reverts a recent attempt to speed up abort of a large pending update with the "pipapo" set backend. It could cause stray references to remain in the set, which then results in a double-free. * tag 'nf-23-10-18' of https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf: netfilter: nf_tables: revert do not remove elements if set backend implements .abort netfilter: nft_set_rbtree: .deactivate fails if element has expired selftests: netfilter: Run nft_audit.sh in its own netns netfilter: nf_tables: audit log object reset once per table ==================== Link: https://lore.kernel.org/r/20231018125605.27299-1-fw@strlen.de Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents 88343fb + f86fb94 commit 9b9ac46

File tree

3 files changed

+83
-26
lines changed

3 files changed

+83
-26
lines changed

net/netfilter/nf_tables_api.c

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7607,6 +7607,16 @@ static int nf_tables_fill_obj_info(struct sk_buff *skb, struct net *net,
76077607
return -1;
76087608
}
76097609

7610+
static void audit_log_obj_reset(const struct nft_table *table,
7611+
unsigned int base_seq, unsigned int nentries)
7612+
{
7613+
char *buf = kasprintf(GFP_ATOMIC, "%s:%u", table->name, base_seq);
7614+
7615+
audit_log_nfcfg(buf, table->family, nentries,
7616+
AUDIT_NFT_OP_OBJ_RESET, GFP_ATOMIC);
7617+
kfree(buf);
7618+
}
7619+
76107620
struct nft_obj_filter {
76117621
char *table;
76127622
u32 type;
@@ -7621,8 +7631,10 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
76217631
struct net *net = sock_net(skb->sk);
76227632
int family = nfmsg->nfgen_family;
76237633
struct nftables_pernet *nft_net;
7634+
unsigned int entries = 0;
76247635
struct nft_object *obj;
76257636
bool reset = false;
7637+
int rc = 0;
76267638

76277639
if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET)
76287640
reset = true;
@@ -7635,6 +7647,7 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
76357647
if (family != NFPROTO_UNSPEC && family != table->family)
76367648
continue;
76377649

7650+
entries = 0;
76387651
list_for_each_entry_rcu(obj, &table->objects, list) {
76397652
if (!nft_is_active(net, obj))
76407653
goto cont;
@@ -7650,34 +7663,27 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
76507663
filter->type != NFT_OBJECT_UNSPEC &&
76517664
obj->ops->type->type != filter->type)
76527665
goto cont;
7653-
if (reset) {
7654-
char *buf = kasprintf(GFP_ATOMIC,
7655-
"%s:%u",
7656-
table->name,
7657-
nft_net->base_seq);
7658-
7659-
audit_log_nfcfg(buf,
7660-
family,
7661-
obj->handle,
7662-
AUDIT_NFT_OP_OBJ_RESET,
7663-
GFP_ATOMIC);
7664-
kfree(buf);
7665-
}
76667666

7667-
if (nf_tables_fill_obj_info(skb, net, NETLINK_CB(cb->skb).portid,
7668-
cb->nlh->nlmsg_seq,
7669-
NFT_MSG_NEWOBJ,
7670-
NLM_F_MULTI | NLM_F_APPEND,
7671-
table->family, table,
7672-
obj, reset) < 0)
7673-
goto done;
7667+
rc = nf_tables_fill_obj_info(skb, net,
7668+
NETLINK_CB(cb->skb).portid,
7669+
cb->nlh->nlmsg_seq,
7670+
NFT_MSG_NEWOBJ,
7671+
NLM_F_MULTI | NLM_F_APPEND,
7672+
table->family, table,
7673+
obj, reset);
7674+
if (rc < 0)
7675+
break;
76747676

7677+
entries++;
76757678
nl_dump_check_consistent(cb, nlmsg_hdr(skb));
76767679
cont:
76777680
idx++;
76787681
}
7682+
if (reset && entries)
7683+
audit_log_obj_reset(table, nft_net->base_seq, entries);
7684+
if (rc < 0)
7685+
break;
76797686
}
7680-
done:
76817687
rcu_read_unlock();
76827688

76837689
cb->args[0] = idx;
@@ -7782,7 +7788,7 @@ static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info,
77827788

77837789
audit_log_nfcfg(buf,
77847790
family,
7785-
obj->handle,
7791+
1,
77867792
AUDIT_NFT_OP_OBJ_RESET,
77877793
GFP_ATOMIC);
77887794
kfree(buf);
@@ -10339,10 +10345,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
1033910345
break;
1034010346
}
1034110347
te = (struct nft_trans_elem *)trans->data;
10342-
if (!te->set->ops->abort ||
10343-
nft_setelem_is_catchall(te->set, &te->elem))
10344-
nft_setelem_remove(net, te->set, &te->elem);
10345-
10348+
nft_setelem_remove(net, te->set, &te->elem);
1034610349
if (!nft_setelem_is_catchall(te->set, &te->elem))
1034710350
atomic_dec(&te->set->nelems);
1034810351

net/netfilter/nft_set_rbtree.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,8 @@ static void *nft_rbtree_deactivate(const struct net *net,
568568
nft_rbtree_interval_end(this)) {
569569
parent = parent->rb_right;
570570
continue;
571+
} else if (nft_set_elem_expired(&rbe->ext)) {
572+
break;
571573
} else if (!nft_set_elem_active(&rbe->ext, genmask)) {
572574
parent = parent->rb_left;
573575
continue;

tools/testing/selftests/netfilter/nft_audit.sh

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ nft --version >/dev/null 2>&1 || {
1111
exit $SKIP_RC
1212
}
1313

14+
# Run everything in a separate network namespace
15+
[ "${1}" != "run" ] && { unshare -n "${0}" run; exit $?; }
16+
17+
# give other scripts a chance to finish - audit_logread sees all activity
18+
sleep 1
19+
1420
logfile=$(mktemp)
1521
rulefile=$(mktemp)
1622
echo "logging into $logfile"
@@ -93,6 +99,12 @@ do_test 'nft add counter t1 c1' \
9399
do_test 'nft add counter t2 c1; add counter t2 c2' \
94100
'table=t2 family=2 entries=2 op=nft_register_obj'
95101

102+
for ((i = 3; i <= 500; i++)); do
103+
echo "add counter t2 c$i"
104+
done >$rulefile
105+
do_test "nft -f $rulefile" \
106+
'table=t2 family=2 entries=498 op=nft_register_obj'
107+
96108
# adding/updating quotas
97109

98110
do_test 'nft add quota t1 q1 { 10 bytes }' \
@@ -101,6 +113,12 @@ do_test 'nft add quota t1 q1 { 10 bytes }' \
101113
do_test 'nft add quota t2 q1 { 10 bytes }; add quota t2 q2 { 10 bytes }' \
102114
'table=t2 family=2 entries=2 op=nft_register_obj'
103115

116+
for ((i = 3; i <= 500; i++)); do
117+
echo "add quota t2 q$i { 10 bytes }"
118+
done >$rulefile
119+
do_test "nft -f $rulefile" \
120+
'table=t2 family=2 entries=498 op=nft_register_obj'
121+
104122
# changing the quota value triggers obj update path
105123
do_test 'nft add quota t1 q1 { 20 bytes }' \
106124
'table=t1 family=2 entries=1 op=nft_register_obj'
@@ -150,6 +168,40 @@ done
150168
do_test 'nft reset set t1 s' \
151169
'table=t1 family=2 entries=3 op=nft_reset_setelem'
152170

171+
# resetting counters
172+
173+
do_test 'nft reset counter t1 c1' \
174+
'table=t1 family=2 entries=1 op=nft_reset_obj'
175+
176+
do_test 'nft reset counters t1' \
177+
'table=t1 family=2 entries=1 op=nft_reset_obj'
178+
179+
do_test 'nft reset counters t2' \
180+
'table=t2 family=2 entries=342 op=nft_reset_obj
181+
table=t2 family=2 entries=158 op=nft_reset_obj'
182+
183+
do_test 'nft reset counters' \
184+
'table=t1 family=2 entries=1 op=nft_reset_obj
185+
table=t2 family=2 entries=341 op=nft_reset_obj
186+
table=t2 family=2 entries=159 op=nft_reset_obj'
187+
188+
# resetting quotas
189+
190+
do_test 'nft reset quota t1 q1' \
191+
'table=t1 family=2 entries=1 op=nft_reset_obj'
192+
193+
do_test 'nft reset quotas t1' \
194+
'table=t1 family=2 entries=1 op=nft_reset_obj'
195+
196+
do_test 'nft reset quotas t2' \
197+
'table=t2 family=2 entries=315 op=nft_reset_obj
198+
table=t2 family=2 entries=185 op=nft_reset_obj'
199+
200+
do_test 'nft reset quotas' \
201+
'table=t1 family=2 entries=1 op=nft_reset_obj
202+
table=t2 family=2 entries=314 op=nft_reset_obj
203+
table=t2 family=2 entries=186 op=nft_reset_obj'
204+
153205
# deleting rules
154206

155207
readarray -t handles < <(nft -a list chain t1 c1 | \

0 commit comments

Comments
 (0)