Skip to content

Commit

Permalink
Merge tag 'nf-23-10-18' of https://git.kernel.org/pub/scm/linux/kerne…
Browse files Browse the repository at this point in the history 
…l/git/netfilter/nf

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>
  • Loading branch information
Jakub Kicinski committed Oct 19, 2023
2 parents 88343fb + f86fb94 commit 9b9ac46
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 26 deletions.
55 changes: 29 additions & 26 deletions net/netfilter/nf_tables_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7607,6 +7607,16 @@ static int nf_tables_fill_obj_info(struct sk_buff *skb, struct net *net,
return -1;
}

static void audit_log_obj_reset(const struct nft_table *table,
unsigned int base_seq, unsigned int nentries)
{
char *buf = kasprintf(GFP_ATOMIC, "%s:%u", table->name, base_seq);

audit_log_nfcfg(buf, table->family, nentries,
AUDIT_NFT_OP_OBJ_RESET, GFP_ATOMIC);
kfree(buf);
}

struct nft_obj_filter {
char *table;
u32 type;
Expand All @@ -7621,8 +7631,10 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
struct net *net = sock_net(skb->sk);
int family = nfmsg->nfgen_family;
struct nftables_pernet *nft_net;
unsigned int entries = 0;
struct nft_object *obj;
bool reset = false;
int rc = 0;

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

entries = 0;
list_for_each_entry_rcu(obj, &table->objects, list) {
if (!nft_is_active(net, obj))
goto cont;
Expand All @@ -7650,34 +7663,27 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
filter->type != NFT_OBJECT_UNSPEC &&
obj->ops->type->type != filter->type)
goto cont;
if (reset) {
char *buf = kasprintf(GFP_ATOMIC,
"%s:%u",
table->name,
nft_net->base_seq);

audit_log_nfcfg(buf,
family,
obj->handle,
AUDIT_NFT_OP_OBJ_RESET,
GFP_ATOMIC);
kfree(buf);
}

if (nf_tables_fill_obj_info(skb, net, NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
NFT_MSG_NEWOBJ,
NLM_F_MULTI | NLM_F_APPEND,
table->family, table,
obj, reset) < 0)
goto done;
rc = nf_tables_fill_obj_info(skb, net,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
NFT_MSG_NEWOBJ,
NLM_F_MULTI | NLM_F_APPEND,
table->family, table,
obj, reset);
if (rc < 0)
break;

entries++;
nl_dump_check_consistent(cb, nlmsg_hdr(skb));
cont:
idx++;
}
if (reset && entries)
audit_log_obj_reset(table, nft_net->base_seq, entries);
if (rc < 0)
break;
}
done:
rcu_read_unlock();

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

audit_log_nfcfg(buf,
family,
obj->handle,
1,
AUDIT_NFT_OP_OBJ_RESET,
GFP_ATOMIC);
kfree(buf);
Expand Down Expand Up @@ -10339,10 +10345,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
break;
}
te = (struct nft_trans_elem *)trans->data;
if (!te->set->ops->abort ||
nft_setelem_is_catchall(te->set, &te->elem))
nft_setelem_remove(net, te->set, &te->elem);

nft_setelem_remove(net, te->set, &te->elem);
if (!nft_setelem_is_catchall(te->set, &te->elem))
atomic_dec(&te->set->nelems);

Expand Down
2 changes: 2 additions & 0 deletions net/netfilter/nft_set_rbtree.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,8 @@ static void *nft_rbtree_deactivate(const struct net *net,
nft_rbtree_interval_end(this)) {
parent = parent->rb_right;
continue;
} else if (nft_set_elem_expired(&rbe->ext)) {
break;
} else if (!nft_set_elem_active(&rbe->ext, genmask)) {
parent = parent->rb_left;
continue;
Expand Down
52 changes: 52 additions & 0 deletions tools/testing/selftests/netfilter/nft_audit.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ nft --version >/dev/null 2>&1 || {
exit $SKIP_RC
}

# Run everything in a separate network namespace
[ "${1}" != "run" ] && { unshare -n "${0}" run; exit $?; }

# give other scripts a chance to finish - audit_logread sees all activity
sleep 1

logfile=$(mktemp)
rulefile=$(mktemp)
echo "logging into $logfile"
Expand Down Expand Up @@ -93,6 +99,12 @@ do_test 'nft add counter t1 c1' \
do_test 'nft add counter t2 c1; add counter t2 c2' \
'table=t2 family=2 entries=2 op=nft_register_obj'

for ((i = 3; i <= 500; i++)); do
echo "add counter t2 c$i"
done >$rulefile
do_test "nft -f $rulefile" \
'table=t2 family=2 entries=498 op=nft_register_obj'

# adding/updating quotas

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

for ((i = 3; i <= 500; i++)); do
echo "add quota t2 q$i { 10 bytes }"
done >$rulefile
do_test "nft -f $rulefile" \
'table=t2 family=2 entries=498 op=nft_register_obj'

# changing the quota value triggers obj update path
do_test 'nft add quota t1 q1 { 20 bytes }' \
'table=t1 family=2 entries=1 op=nft_register_obj'
Expand Down Expand Up @@ -150,6 +168,40 @@ done
do_test 'nft reset set t1 s' \
'table=t1 family=2 entries=3 op=nft_reset_setelem'

# resetting counters

do_test 'nft reset counter t1 c1' \
'table=t1 family=2 entries=1 op=nft_reset_obj'

do_test 'nft reset counters t1' \
'table=t1 family=2 entries=1 op=nft_reset_obj'

do_test 'nft reset counters t2' \
'table=t2 family=2 entries=342 op=nft_reset_obj
table=t2 family=2 entries=158 op=nft_reset_obj'

do_test 'nft reset counters' \
'table=t1 family=2 entries=1 op=nft_reset_obj
table=t2 family=2 entries=341 op=nft_reset_obj
table=t2 family=2 entries=159 op=nft_reset_obj'

# resetting quotas

do_test 'nft reset quota t1 q1' \
'table=t1 family=2 entries=1 op=nft_reset_obj'

do_test 'nft reset quotas t1' \
'table=t1 family=2 entries=1 op=nft_reset_obj'

do_test 'nft reset quotas t2' \
'table=t2 family=2 entries=315 op=nft_reset_obj
table=t2 family=2 entries=185 op=nft_reset_obj'

do_test 'nft reset quotas' \
'table=t1 family=2 entries=1 op=nft_reset_obj
table=t2 family=2 entries=314 op=nft_reset_obj
table=t2 family=2 entries=186 op=nft_reset_obj'

# deleting rules

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

0 comments on commit 9b9ac46

Please sign in to comment.