[nf PATCH 0/3] Review nf_tables audit logging

[nf PATCH 0/3] Review nf_tables audit logging

[Phil Sutter]

When working on locking for reset commands, some audit log calls had to
be adjusted as well. This series deals with the "fallout" from adding
tests for the changed log calls, dealing with the uncovered issues and
adding more tests.

Patch 1 adds more testing to nft_audit.sh for commands which are
unproblematic.

Patch 2 deals with (likely) leftovers from audit log flood prevention in
commit c520292f29b80 ("audit: log nftables configuration change events
once per table").

Patch 3 changes logging for object reset requests to happen once per
table (if skb size is sufficient) and thereby aligns output with object
add requests. As a side-effect, logging is fixed to happen after the
actual reset has succeeded, not before.

NOTE: This whole series probably depends on the reset locking series[1]
submitted earlier, but there's no functional connection and reviews
should happen independently.

[1] https://lore.kernel.org/netfilter-devel/20230923013807.11398-1-phil@nwl.cc/

Phil Sutter (3):
  selftests: netfilter: Extend nft_audit.sh
  netfilter: nf_tables: Deduplicate nft_register_obj audit logs
  netfilter: nf_tables: Audit log object reset once per table

 net/netfilter/nf_tables_api.c                 |  95 +++++-----
 .../testing/selftests/netfilter/nft_audit.sh  | 163 ++++++++++++++++--
 2 files changed, 203 insertions(+), 55 deletions(-)

-- 
2.41.0

[nf PATCH 1/3] selftests: netfilter: Extend nft_audit.sh

[Phil Sutter]

Add tests for sets and elements and deletion of all kinds. Also
reorder rule reset tests: By moving the bulk rule add command up, the
two 'reset rules' tests become identical.

While at it, fix for a failing bulk rule add test's error status getting
lost due to its use in a pipe. Avoid this by using a temporary file.

Headings in diff output for failing tests contain no useful data, strip
them.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 .../testing/selftests/netfilter/nft_audit.sh  | 97 ++++++++++++++++---
 1 file changed, 81 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/netfilter/nft_audit.sh b/tools/testing/selftests/netfilter/nft_audit.sh
index 83c271b1c7352..0b3255e7b3538 100755
--- a/tools/testing/selftests/netfilter/nft_audit.sh
+++ b/tools/testing/selftests/netfilter/nft_audit.sh
@@ -12,10 +12,11 @@ nft --version >/dev/null 2>&1 || {
 }
 
 logfile=$(mktemp)
+rulefile=$(mktemp)
 echo "logging into $logfile"
 ./audit_logread >"$logfile" &
 logread_pid=$!
-trap 'kill $logread_pid; rm -f $logfile' EXIT
+trap 'kill $logread_pid; rm -f $logfile $rulefile' EXIT
 exec 3<"$logfile"
 
 do_test() { # (cmd, log)
@@ -26,12 +27,14 @@ do_test() { # (cmd, log)
 	res=$(diff -a -u <(echo "$2") - <&3)
 	[ $? -eq 0 ] && { echo "OK"; return; }
 	echo "FAIL"
-	echo "$res"
-	((RC++))
+	grep -v '^\(---\|+++\|@@\)' <<< "$res"
+	((RC--))
 }
 
 nft flush ruleset
 
+# adding tables, chains and rules
+
 for table in t1 t2; do
 	do_test "nft add table $table" \
 	"table=$table family=2 entries=1 op=nft_register_table"
@@ -62,6 +65,28 @@ for table in t1 t2; do
 	"table=$table family=2 entries=6 op=nft_register_rule"
 done
 
+for ((i = 0; i < 500; i++)); do
+	echo "add rule t2 c3 counter accept comment \"rule $i\""
+done >$rulefile
+do_test "nft -f $rulefile" \
+'table=t2 family=2 entries=500 op=nft_register_rule'
+
+# adding sets and elements
+
+settype='type inet_service; counter'
+setelem='{ 22, 80, 443 }'
+setblock="{ $settype; elements = $setelem; }"
+do_test "nft add set t1 s $setblock" \
+"table=t1 family=2 entries=4 op=nft_register_set"
+
+do_test "nft add set t1 s2 $setblock; add set t1 s3 { $settype; }" \
+"table=t1 family=2 entries=5 op=nft_register_set"
+
+do_test "nft add element t1 s3 $setelem" \
+"table=t1 family=2 entries=3 op=nft_register_setelem"
+
+# resetting rules
+
 do_test 'nft reset rules t1 c2' \
 'table=t1 family=2 entries=3 op=nft_reset_rule'
 
@@ -70,19 +95,6 @@ do_test 'nft reset rules table t1' \
 table=t1 family=2 entries=3 op=nft_reset_rule
 table=t1 family=2 entries=3 op=nft_reset_rule'
 
-do_test 'nft reset rules' \
-'table=t1 family=2 entries=3 op=nft_reset_rule
-table=t1 family=2 entries=3 op=nft_reset_rule
-table=t1 family=2 entries=3 op=nft_reset_rule
-table=t2 family=2 entries=3 op=nft_reset_rule
-table=t2 family=2 entries=3 op=nft_reset_rule
-table=t2 family=2 entries=3 op=nft_reset_rule'
-
-for ((i = 0; i < 500; i++)); do
-	echo "add rule t2 c3 counter accept comment \"rule $i\""
-done | do_test 'nft -f -' \
-'table=t2 family=2 entries=500 op=nft_register_rule'
-
 do_test 'nft reset rules t2 c3' \
 'table=t2 family=2 entries=189 op=nft_reset_rule
 table=t2 family=2 entries=188 op=nft_reset_rule
@@ -105,4 +117,57 @@ table=t2 family=2 entries=180 op=nft_reset_rule
 table=t2 family=2 entries=188 op=nft_reset_rule
 table=t2 family=2 entries=135 op=nft_reset_rule'
 
+# resetting sets and elements
+
+elem=(22 ,80 ,443)
+relem=""
+for i in {1..3}; do
+	relem+="${elem[((i - 1))]}"
+	do_test "nft reset element t1 s { $relem }" \
+	"table=t1 family=2 entries=$i op=nft_reset_setelem"
+done
+
+do_test 'nft reset set t1 s' \
+'table=t1 family=2 entries=3 op=nft_reset_setelem'
+
+# deleting rules
+
+readarray -t handles < <(nft -a list chain t1 c1 | \
+			 sed -n 's/.*counter.* handle \(.*\)$/\1/p')
+
+do_test "nft delete rule t1 c1 handle ${handles[0]}" \
+'table=t1 family=2 entries=1 op=nft_unregister_rule'
+
+cmd='delete rule t1 c1 handle'
+do_test "nft $cmd ${handles[1]}; $cmd ${handles[2]}" \
+'table=t1 family=2 entries=2 op=nft_unregister_rule'
+
+do_test 'nft flush chain t1 c2' \
+'table=t1 family=2 entries=3 op=nft_unregister_rule'
+
+do_test 'nft flush table t2' \
+'table=t2 family=2 entries=509 op=nft_unregister_rule'
+
+# deleting chains
+
+do_test 'nft delete chain t2 c2' \
+'table=t2 family=2 entries=1 op=nft_unregister_chain'
+
+# deleting sets and elements
+
+do_test 'nft delete element t1 s { 22 }' \
+'table=t1 family=2 entries=1 op=nft_unregister_setelem'
+
+do_test 'nft delete element t1 s { 80, 443 }' \
+'table=t1 family=2 entries=2 op=nft_unregister_setelem'
+
+do_test 'nft flush set t1 s2' \
+'table=t1 family=2 entries=3 op=nft_unregister_setelem'
+
+do_test 'nft delete set t1 s2' \
+'table=t1 family=2 entries=1 op=nft_unregister_set'
+
+do_test 'nft delete set t1 s3' \
+'table=t1 family=2 entries=1 op=nft_unregister_set'
+
 exit $RC
-- 
2.41.0

[nf PATCH 2/3] netfilter: nf_tables: Deduplicate nft_register_obj audit logs

[Phil Sutter]

When adding/updating an object, the transaction handler emits suitable
audit log entries already, the one in nft_obj_notify() is redundant. To
fix that (and retain the audit logging from objects' 'update' callback),
Introduce an "audit log free" variant for internal use.

Fixes: c520292f29b80 ("audit: log nftables configuration change events once per table")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 net/netfilter/nf_tables_api.c                 | 44 ++++++++++++-------
 .../testing/selftests/netfilter/nft_audit.sh  | 20 +++++++++
 2 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 0e5d9bdba82b8..48d50df950a18 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -8046,24 +8046,14 @@ static int nf_tables_delobj(struct sk_buff *skb, const struct nfnl_info *info,
 	return nft_delobj(&ctx, obj);
 }
 
-void nft_obj_notify(struct net *net, const struct nft_table *table,
-		    struct nft_object *obj, u32 portid, u32 seq, int event,
-		    u16 flags, int family, int report, gfp_t gfp)
+static void
+__nft_obj_notify(struct net *net, const struct nft_table *table,
+		 struct nft_object *obj, u32 portid, u32 seq, int event,
+		 u16 flags, int family, int report, gfp_t gfp)
 {
 	struct nftables_pernet *nft_net = nft_pernet(net);
 	struct sk_buff *skb;
 	int err;
-	char *buf = kasprintf(gfp, "%s:%u",
-			      table->name, nft_net->base_seq);
-
-	audit_log_nfcfg(buf,
-			family,
-			obj->handle,
-			event == NFT_MSG_NEWOBJ ?
-				 AUDIT_NFT_OP_OBJ_REGISTER :
-				 AUDIT_NFT_OP_OBJ_UNREGISTER,
-			gfp);
-	kfree(buf);
 
 	if (!report &&
 	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
@@ -8086,13 +8076,35 @@ void nft_obj_notify(struct net *net, const struct nft_table *table,
 err:
 	nfnetlink_set_err(net, portid, NFNLGRP_NFTABLES, -ENOBUFS);
 }
+
+void nft_obj_notify(struct net *net, const struct nft_table *table,
+		    struct nft_object *obj, u32 portid, u32 seq, int event,
+		    u16 flags, int family, int report, gfp_t gfp)
+{
+	struct nftables_pernet *nft_net = nft_pernet(net);
+	char *buf = kasprintf(gfp, "%s:%u",
+			      table->name, nft_net->base_seq);
+
+	audit_log_nfcfg(buf,
+			family,
+			obj->handle,
+			event == NFT_MSG_NEWOBJ ?
+				 AUDIT_NFT_OP_OBJ_REGISTER :
+				 AUDIT_NFT_OP_OBJ_UNREGISTER,
+			gfp);
+	kfree(buf);
+
+	__nft_obj_notify(net, table, obj, portid, seq, event,
+			 flags, family, report, gfp);
+}
 EXPORT_SYMBOL_GPL(nft_obj_notify);
 
 static void nf_tables_obj_notify(const struct nft_ctx *ctx,
 				 struct nft_object *obj, int event)
 {
-	nft_obj_notify(ctx->net, ctx->table, obj, ctx->portid, ctx->seq, event,
-		       ctx->flags, ctx->family, ctx->report, GFP_KERNEL);
+	__nft_obj_notify(ctx->net, ctx->table, obj, ctx->portid,
+			 ctx->seq, event, ctx->flags, ctx->family,
+			 ctx->report, GFP_KERNEL);
 }
 
 /*
diff --git a/tools/testing/selftests/netfilter/nft_audit.sh b/tools/testing/selftests/netfilter/nft_audit.sh
index 0b3255e7b3538..bb34329e02a7f 100755
--- a/tools/testing/selftests/netfilter/nft_audit.sh
+++ b/tools/testing/selftests/netfilter/nft_audit.sh
@@ -85,6 +85,26 @@ do_test "nft add set t1 s2 $setblock; add set t1 s3 { $settype; }" \
 do_test "nft add element t1 s3 $setelem" \
 "table=t1 family=2 entries=3 op=nft_register_setelem"
 
+# adding counters
+
+do_test 'nft add counter t1 c1' \
+'table=t1 family=2 entries=1 op=nft_register_obj'
+
+do_test 'nft add counter t2 c1; add counter t2 c2' \
+'table=t2 family=2 entries=2 op=nft_register_obj'
+
+# adding/updating quotas
+
+do_test 'nft add quota t1 q1 { 10 bytes }' \
+'table=t1 family=2 entries=1 op=nft_register_obj'
+
+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'
+
+# 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'
+
 # resetting rules
 
 do_test 'nft reset rules t1 c2' \
-- 
2.41.0

Re: [nf PATCH 2/3] netfilter: nf_tables: Deduplicate nft_register_obj audit logs

[Pablo Neira Ayuso]

Hi Phil,

On Sat, Sep 23, 2023 at 03:53:50AM +0200, Phil Sutter wrote:
> When adding/updating an object, the transaction handler emits suitable
> audit log entries already, the one in nft_obj_notify() is redundant. To
> fix that (and retain the audit logging from objects' 'update' callback),
> Introduce an "audit log free" variant for internal use.
> 
> Fixes: c520292f29b80 ("audit: log nftables configuration change events once per table")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  net/netfilter/nf_tables_api.c                 | 44 ++++++++++++-------
>  .../testing/selftests/netfilter/nft_audit.sh  | 20 +++++++++
>  2 files changed, 48 insertions(+), 16 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 0e5d9bdba82b8..48d50df950a18 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -8046,24 +8046,14 @@ static int nf_tables_delobj(struct sk_buff *skb, const struct nfnl_info *info,
>  	return nft_delobj(&ctx, obj);
>  }
>  
> -void nft_obj_notify(struct net *net, const struct nft_table *table,
> -		    struct nft_object *obj, u32 portid, u32 seq, int event,
> -		    u16 flags, int family, int report, gfp_t gfp)
> +static void
> +__nft_obj_notify(struct net *net, const struct nft_table *table,
> +		 struct nft_object *obj, u32 portid, u32 seq, int event,
> +		 u16 flags, int family, int report, gfp_t gfp)
>  {
>  	struct nftables_pernet *nft_net = nft_pernet(net);
>  	struct sk_buff *skb;
>  	int err;
> -	char *buf = kasprintf(gfp, "%s:%u",
> -			      table->name, nft_net->base_seq);
> -
> -	audit_log_nfcfg(buf,
> -			family,
> -			obj->handle,
> -			event == NFT_MSG_NEWOBJ ?
> -				 AUDIT_NFT_OP_OBJ_REGISTER :
> -				 AUDIT_NFT_OP_OBJ_UNREGISTER,
> -			gfp);
> -	kfree(buf);
>  
>  	if (!report &&
>  	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
> @@ -8086,13 +8076,35 @@ void nft_obj_notify(struct net *net, const struct nft_table *table,
>  err:
>  	nfnetlink_set_err(net, portid, NFNLGRP_NFTABLES, -ENOBUFS);
>  }
> +
> +void nft_obj_notify(struct net *net, const struct nft_table *table,
> +		    struct nft_object *obj, u32 portid, u32 seq, int event,
> +		    u16 flags, int family, int report, gfp_t gfp)
> +{
> +	struct nftables_pernet *nft_net = nft_pernet(net);
> +	char *buf = kasprintf(gfp, "%s:%u",
> +			      table->name, nft_net->base_seq);
> +
> +	audit_log_nfcfg(buf,
> +			family,
> +			obj->handle,
> +			event == NFT_MSG_NEWOBJ ?
> +				 AUDIT_NFT_OP_OBJ_REGISTER :
> +				 AUDIT_NFT_OP_OBJ_UNREGISTER,
> +			gfp);
> +	kfree(buf);
> +
> +	__nft_obj_notify(net, table, obj, portid, seq, event,
> +			 flags, family, report, gfp);
> +}
>  EXPORT_SYMBOL_GPL(nft_obj_notify);

OK, so nft_obj_notify() is called from nft_quota to notify that the
quota is depleted and the audit log is still there in this case.

>  static void nf_tables_obj_notify(const struct nft_ctx *ctx,
>  				 struct nft_object *obj, int event)
>  {
> -	nft_obj_notify(ctx->net, ctx->table, obj, ctx->portid, ctx->seq, event,
> -		       ctx->flags, ctx->family, ctx->report, GFP_KERNEL);
> +	__nft_obj_notify(ctx->net, ctx->table, obj, ctx->portid,
> +			 ctx->seq, event, ctx->flags, ctx->family,
> +			 ctx->report, GFP_KERNEL);
>  }

This function is called from the commit path to send the event
notification, and it should send the audit log?

Is this nf_tables_commit_audit_log() that provides the redundant log,
right?

>  /*
> diff --git a/tools/testing/selftests/netfilter/nft_audit.sh b/tools/testing/selftests/netfilter/nft_audit.sh
> index 0b3255e7b3538..bb34329e02a7f 100755
> --- a/tools/testing/selftests/netfilter/nft_audit.sh
> +++ b/tools/testing/selftests/netfilter/nft_audit.sh
> @@ -85,6 +85,26 @@ do_test "nft add set t1 s2 $setblock; add set t1 s3 { $settype; }" \
>  do_test "nft add element t1 s3 $setelem" \
>  "table=t1 family=2 entries=3 op=nft_register_setelem"
>  
> +# adding counters
> +
> +do_test 'nft add counter t1 c1' \
> +'table=t1 family=2 entries=1 op=nft_register_obj'
> +
> +do_test 'nft add counter t2 c1; add counter t2 c2' \
> +'table=t2 family=2 entries=2 op=nft_register_obj'
> +
> +# adding/updating quotas
> +
> +do_test 'nft add quota t1 q1 { 10 bytes }' \
> +'table=t1 family=2 entries=1 op=nft_register_obj'
> +
> +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'
> +
> +# 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'
> +
>  # resetting rules
>  
>  do_test 'nft reset rules t1 c2' \
> -- 
> 2.41.0
> 

Re: [nf PATCH 2/3] netfilter: nf_tables: Deduplicate nft_register_obj audit logs

[Phil Sutter]

Hi Pablo,

On Wed, Sep 27, 2023 at 09:41:53PM +0200, Pablo Neira Ayuso wrote:
> On Sat, Sep 23, 2023 at 03:53:50AM +0200, Phil Sutter wrote:
> > When adding/updating an object, the transaction handler emits suitable
> > audit log entries already, the one in nft_obj_notify() is redundant. To
> > fix that (and retain the audit logging from objects' 'update' callback),
> > Introduce an "audit log free" variant for internal use.
> > 
> > Fixes: c520292f29b80 ("audit: log nftables configuration change events once per table")
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  net/netfilter/nf_tables_api.c                 | 44 ++++++++++++-------
> >  .../testing/selftests/netfilter/nft_audit.sh  | 20 +++++++++
> >  2 files changed, 48 insertions(+), 16 deletions(-)
> > 
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index 0e5d9bdba82b8..48d50df950a18 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -8046,24 +8046,14 @@ static int nf_tables_delobj(struct sk_buff *skb, const struct nfnl_info *info,
> >  	return nft_delobj(&ctx, obj);
> >  }
> >  
> > -void nft_obj_notify(struct net *net, const struct nft_table *table,
> > -		    struct nft_object *obj, u32 portid, u32 seq, int event,
> > -		    u16 flags, int family, int report, gfp_t gfp)
> > +static void
> > +__nft_obj_notify(struct net *net, const struct nft_table *table,
> > +		 struct nft_object *obj, u32 portid, u32 seq, int event,
> > +		 u16 flags, int family, int report, gfp_t gfp)
> >  {
> >  	struct nftables_pernet *nft_net = nft_pernet(net);
> >  	struct sk_buff *skb;
> >  	int err;
> > -	char *buf = kasprintf(gfp, "%s:%u",
> > -			      table->name, nft_net->base_seq);
> > -
> > -	audit_log_nfcfg(buf,
> > -			family,
> > -			obj->handle,
> > -			event == NFT_MSG_NEWOBJ ?
> > -				 AUDIT_NFT_OP_OBJ_REGISTER :
> > -				 AUDIT_NFT_OP_OBJ_UNREGISTER,
> > -			gfp);
> > -	kfree(buf);
> >  
> >  	if (!report &&
> >  	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
> > @@ -8086,13 +8076,35 @@ void nft_obj_notify(struct net *net, const struct nft_table *table,
> >  err:
> >  	nfnetlink_set_err(net, portid, NFNLGRP_NFTABLES, -ENOBUFS);
> >  }
> > +
> > +void nft_obj_notify(struct net *net, const struct nft_table *table,
> > +		    struct nft_object *obj, u32 portid, u32 seq, int event,
> > +		    u16 flags, int family, int report, gfp_t gfp)
> > +{
> > +	struct nftables_pernet *nft_net = nft_pernet(net);
> > +	char *buf = kasprintf(gfp, "%s:%u",
> > +			      table->name, nft_net->base_seq);
> > +
> > +	audit_log_nfcfg(buf,
> > +			family,
> > +			obj->handle,
> > +			event == NFT_MSG_NEWOBJ ?
> > +				 AUDIT_NFT_OP_OBJ_REGISTER :
> > +				 AUDIT_NFT_OP_OBJ_UNREGISTER,
> > +			gfp);
> > +	kfree(buf);
> > +
> > +	__nft_obj_notify(net, table, obj, portid, seq, event,
> > +			 flags, family, report, gfp);
> > +}
> >  EXPORT_SYMBOL_GPL(nft_obj_notify);
> 
> OK, so nft_obj_notify() is called from nft_quota to notify that the
> quota is depleted and the audit log is still there in this case.

Exactly. I needed an internal __nft_obj_notify() which just serves
notify_list but not audit.

> >  static void nf_tables_obj_notify(const struct nft_ctx *ctx,
> >  				 struct nft_object *obj, int event)
> >  {
> > -	nft_obj_notify(ctx->net, ctx->table, obj, ctx->portid, ctx->seq, event,
> > -		       ctx->flags, ctx->family, ctx->report, GFP_KERNEL);
> > +	__nft_obj_notify(ctx->net, ctx->table, obj, ctx->portid,
> > +			 ctx->seq, event, ctx->flags, ctx->family,
> > +			 ctx->report, GFP_KERNEL);
> >  }
> 
> This function is called from the commit path to send the event
> notification, and it should send the audit log?
> 
> Is this nf_tables_commit_audit_log() that provides the redundant log,
> right?

It's easier to to make nf_tables_obj_notify() skip the audit log and
not introduce special casing in nf_tables_commit_audit_log() or call
nf_tables_commit_audit_collect() for all transaction entries but NEWOBJ
and DELOBJ.

Cheers, Phil

Re: [nf PATCH 2/3] netfilter: nf_tables: Deduplicate nft_register_obj audit logs

[Pablo Neira Ayuso]

On Thu, Sep 28, 2023 at 04:48:11PM +0200, Phil Sutter wrote:
> On Wed, Sep 27, 2023 at 09:41:53PM +0200, Pablo Neira Ayuso wrote:
> > OK, so nft_obj_notify() is called from nft_quota to notify that the
> > quota is depleted and the audit log is still there in this case.
> 
> Exactly. I needed an internal __nft_obj_notify() which just serves
> notify_list but not audit.

Patch LGTM, thanks for explaining!

Re: [nf PATCH 2/3] netfilter: nf_tables: Deduplicate nft_register_obj audit logs

[Richard Guy Briggs]

On 2023-09-23 03:53, Phil Sutter wrote:
> When adding/updating an object, the transaction handler emits suitable
> audit log entries already, the one in nft_obj_notify() is redundant. To
> fix that (and retain the audit logging from objects' 'update' callback),
> Introduce an "audit log free" variant for internal use.
> 
> Fixes: c520292f29b80 ("audit: log nftables configuration change events once per table")
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Reviewed-by: Richard Guy Briggs <rgb@redhat.com>

> ---
>  net/netfilter/nf_tables_api.c                 | 44 ++++++++++++-------
>  .../testing/selftests/netfilter/nft_audit.sh  | 20 +++++++++
>  2 files changed, 48 insertions(+), 16 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 0e5d9bdba82b8..48d50df950a18 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -8046,24 +8046,14 @@ static int nf_tables_delobj(struct sk_buff *skb, const struct nfnl_info *info,
>  	return nft_delobj(&ctx, obj);
>  }
>  
> -void nft_obj_notify(struct net *net, const struct nft_table *table,
> -		    struct nft_object *obj, u32 portid, u32 seq, int event,
> -		    u16 flags, int family, int report, gfp_t gfp)
> +static void
> +__nft_obj_notify(struct net *net, const struct nft_table *table,
> +		 struct nft_object *obj, u32 portid, u32 seq, int event,
> +		 u16 flags, int family, int report, gfp_t gfp)
>  {
>  	struct nftables_pernet *nft_net = nft_pernet(net);
>  	struct sk_buff *skb;
>  	int err;
> -	char *buf = kasprintf(gfp, "%s:%u",
> -			      table->name, nft_net->base_seq);
> -
> -	audit_log_nfcfg(buf,
> -			family,
> -			obj->handle,
> -			event == NFT_MSG_NEWOBJ ?
> -				 AUDIT_NFT_OP_OBJ_REGISTER :
> -				 AUDIT_NFT_OP_OBJ_UNREGISTER,
> -			gfp);
> -	kfree(buf);
>  
>  	if (!report &&
>  	    !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
> @@ -8086,13 +8076,35 @@ void nft_obj_notify(struct net *net, const struct nft_table *table,
>  err:
>  	nfnetlink_set_err(net, portid, NFNLGRP_NFTABLES, -ENOBUFS);
>  }
> +
> +void nft_obj_notify(struct net *net, const struct nft_table *table,
> +		    struct nft_object *obj, u32 portid, u32 seq, int event,
> +		    u16 flags, int family, int report, gfp_t gfp)
> +{
> +	struct nftables_pernet *nft_net = nft_pernet(net);
> +	char *buf = kasprintf(gfp, "%s:%u",
> +			      table->name, nft_net->base_seq);
> +
> +	audit_log_nfcfg(buf,
> +			family,
> +			obj->handle,
> +			event == NFT_MSG_NEWOBJ ?
> +				 AUDIT_NFT_OP_OBJ_REGISTER :
> +				 AUDIT_NFT_OP_OBJ_UNREGISTER,
> +			gfp);
> +	kfree(buf);
> +
> +	__nft_obj_notify(net, table, obj, portid, seq, event,
> +			 flags, family, report, gfp);
> +}
>  EXPORT_SYMBOL_GPL(nft_obj_notify);
>  
>  static void nf_tables_obj_notify(const struct nft_ctx *ctx,
>  				 struct nft_object *obj, int event)
>  {
> -	nft_obj_notify(ctx->net, ctx->table, obj, ctx->portid, ctx->seq, event,
> -		       ctx->flags, ctx->family, ctx->report, GFP_KERNEL);
> +	__nft_obj_notify(ctx->net, ctx->table, obj, ctx->portid,
> +			 ctx->seq, event, ctx->flags, ctx->family,
> +			 ctx->report, GFP_KERNEL);
>  }
>  
>  /*
> diff --git a/tools/testing/selftests/netfilter/nft_audit.sh b/tools/testing/selftests/netfilter/nft_audit.sh
> index 0b3255e7b3538..bb34329e02a7f 100755
> --- a/tools/testing/selftests/netfilter/nft_audit.sh
> +++ b/tools/testing/selftests/netfilter/nft_audit.sh
> @@ -85,6 +85,26 @@ do_test "nft add set t1 s2 $setblock; add set t1 s3 { $settype; }" \
>  do_test "nft add element t1 s3 $setelem" \
>  "table=t1 family=2 entries=3 op=nft_register_setelem"
>  
> +# adding counters
> +
> +do_test 'nft add counter t1 c1' \
> +'table=t1 family=2 entries=1 op=nft_register_obj'
> +
> +do_test 'nft add counter t2 c1; add counter t2 c2' \
> +'table=t2 family=2 entries=2 op=nft_register_obj'
> +
> +# adding/updating quotas
> +
> +do_test 'nft add quota t1 q1 { 10 bytes }' \
> +'table=t1 family=2 entries=1 op=nft_register_obj'
> +
> +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'
> +
> +# 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'
> +
>  # resetting rules
>  
>  do_test 'nft reset rules t1 c2' \
> -- 
> 2.41.0
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
Upstream IRC: SunRaycer
Voice: +1.613.860 2354 SMS: +1.613.518.6570

Re: [nf PATCH 2/3] netfilter: nf_tables: Deduplicate nft_register_obj audit logs

[Paul Moore]

On Fri, Sep 22, 2023 at 9:53 PM Phil Sutter <phil@nwl.cc> wrote:
>
> When adding/updating an object, the transaction handler emits suitable
> audit log entries already, the one in nft_obj_notify() is redundant. To
> fix that (and retain the audit logging from objects' 'update' callback),
> Introduce an "audit log free" variant for internal use.
>
> Fixes: c520292f29b80 ("audit: log nftables configuration change events once per table")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  net/netfilter/nf_tables_api.c                 | 44 ++++++++++++-------
>  .../testing/selftests/netfilter/nft_audit.sh  | 20 +++++++++
>  2 files changed, 48 insertions(+), 16 deletions(-)

Thanks for working on this Phil, it looks good to me from an audit perspective.

Acked-by: Paul Moore <paul@paul-moore.com> (Audit)

-- 
paul-moore.com

[nf PATCH 3/3] netfilter: nf_tables: Audit log object reset once per table

[Phil Sutter]

When resetting multiple objects at once (via dump request), emit a log
message per table (or filled skb) and resurrect the 'entries' parameter
to contain the number of objects being logged for.

With the above in place, all audit logs for op=nft_register_obj have a
predictable value in 'entries', so drop the value zeroing for them in
audit_logread.c.

To test the skb exhaustion path, perform some bulk counter and quota
adds in the kselftest.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 net/netfilter/nf_tables_api.c                 | 51 ++++++++++---------
 .../testing/selftests/netfilter/nft_audit.sh  | 46 +++++++++++++++++
 2 files changed, 74 insertions(+), 23 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 48d50df950a18..e04ef2c451be4 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -7733,6 +7733,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_dump_ctx {
 	unsigned int	s_idx;
 	char		*table;
@@ -7748,8 +7758,10 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
 	int family = nfmsg->nfgen_family;
 	struct nftables_pernet *nft_net;
 	const struct nft_table *table;
+	unsigned int entries = 0;
 	struct nft_object *obj;
 	unsigned int idx = 0;
+	int rc = 0;
 
 	rcu_read_lock();
 	nft_net = nft_pernet(net);
@@ -7759,6 +7771,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;
@@ -7769,34 +7782,27 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
 			if (ctx->type != NFT_OBJECT_UNSPEC &&
 			    obj->ops->type->type != ctx->type)
 				goto cont;
-			if (ctx->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, ctx->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, ctx->reset);
+			if (rc < 0)
+				break;
 
+			entries++;
 			nl_dump_check_consistent(cb, nlmsg_hdr(skb));
 cont:
 			idx++;
 		}
+		if (ctx->reset && entries)
+			audit_log_obj_reset(table, nft_net->base_seq, entries);
+		if (rc < 0)
+			break;
 	}
-done:
 	rcu_read_unlock();
 
 	ctx->s_idx = idx;
@@ -7977,8 +7983,7 @@ static int nf_tables_getobj_reset(struct sk_buff *skb,
 		return PTR_ERR(skb2);
 
 	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);
+	audit_log_nfcfg(buf, family, 1, AUDIT_NFT_OP_OBJ_RESET, GFP_ATOMIC);
 	kfree(buf);
 
 	return nfnetlink_unicast(skb2, net, portid);
diff --git a/tools/testing/selftests/netfilter/nft_audit.sh b/tools/testing/selftests/netfilter/nft_audit.sh
index bb34329e02a7f..e94a80859bbdb 100755
--- a/tools/testing/selftests/netfilter/nft_audit.sh
+++ b/tools/testing/selftests/netfilter/nft_audit.sh
@@ -93,6 +93,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 }' \
@@ -101,6 +107,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'
@@ -150,6 +162,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 | \
-- 
2.41.0

Re: [nf PATCH 3/3] netfilter: nf_tables: Audit log object reset once per table

[Richard Guy Briggs]

On 2023-09-23 03:53, Phil Sutter wrote:
> When resetting multiple objects at once (via dump request), emit a log
> message per table (or filled skb) and resurrect the 'entries' parameter
> to contain the number of objects being logged for.
> 
> With the above in place, all audit logs for op=nft_register_obj have a
> predictable value in 'entries', so drop the value zeroing for them in
> audit_logread.c.
> 
> To test the skb exhaustion path, perform some bulk counter and quota
> adds in the kselftest.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

(Resend, forgot to include other addressees.)
Reviewed-by: Richard Guy Briggs <rgb@redhat.com>

> ---
>  net/netfilter/nf_tables_api.c                 | 51 ++++++++++---------
>  .../testing/selftests/netfilter/nft_audit.sh  | 46 +++++++++++++++++
>  2 files changed, 74 insertions(+), 23 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 48d50df950a18..e04ef2c451be4 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -7733,6 +7733,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_dump_ctx {
>  	unsigned int	s_idx;
>  	char		*table;
> @@ -7748,8 +7758,10 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
>  	int family = nfmsg->nfgen_family;
>  	struct nftables_pernet *nft_net;
>  	const struct nft_table *table;
> +	unsigned int entries = 0;
>  	struct nft_object *obj;
>  	unsigned int idx = 0;
> +	int rc = 0;
>  
>  	rcu_read_lock();
>  	nft_net = nft_pernet(net);
> @@ -7759,6 +7771,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;
> @@ -7769,34 +7782,27 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
>  			if (ctx->type != NFT_OBJECT_UNSPEC &&
>  			    obj->ops->type->type != ctx->type)
>  				goto cont;
> -			if (ctx->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, ctx->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, ctx->reset);
> +			if (rc < 0)
> +				break;
>  
> +			entries++;
>  			nl_dump_check_consistent(cb, nlmsg_hdr(skb));
>  cont:
>  			idx++;
>  		}
> +		if (ctx->reset && entries)
> +			audit_log_obj_reset(table, nft_net->base_seq, entries);
> +		if (rc < 0)
> +			break;
>  	}
> -done:
>  	rcu_read_unlock();
>  
>  	ctx->s_idx = idx;
> @@ -7977,8 +7983,7 @@ static int nf_tables_getobj_reset(struct sk_buff *skb,
>  		return PTR_ERR(skb2);
>  
>  	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);
> +	audit_log_nfcfg(buf, family, 1, AUDIT_NFT_OP_OBJ_RESET, GFP_ATOMIC);
>  	kfree(buf);
>  
>  	return nfnetlink_unicast(skb2, net, portid);
> diff --git a/tools/testing/selftests/netfilter/nft_audit.sh b/tools/testing/selftests/netfilter/nft_audit.sh
> index bb34329e02a7f..e94a80859bbdb 100755
> --- a/tools/testing/selftests/netfilter/nft_audit.sh
> +++ b/tools/testing/selftests/netfilter/nft_audit.sh
> @@ -93,6 +93,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 }' \
> @@ -101,6 +107,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'
> @@ -150,6 +162,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 | \
> -- 
> 2.41.0
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
Upstream IRC: SunRaycer
Voice: +1.613.860 2354 SMS: +1.613.518.6570

Re: [nf PATCH 3/3] netfilter: nf_tables: Audit log object reset once per table

[Paul Moore]

On Fri, Sep 22, 2023 at 9:53 PM Phil Sutter <phil@nwl.cc> wrote:
>
> When resetting multiple objects at once (via dump request), emit a log
> message per table (or filled skb) and resurrect the 'entries' parameter
> to contain the number of objects being logged for.
>
> With the above in place, all audit logs for op=nft_register_obj have a
> predictable value in 'entries', so drop the value zeroing for them in
> audit_logread.c.
>
> To test the skb exhaustion path, perform some bulk counter and quota
> adds in the kselftest.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  net/netfilter/nf_tables_api.c                 | 51 ++++++++++---------
>  .../testing/selftests/netfilter/nft_audit.sh  | 46 +++++++++++++++++
>  2 files changed, 74 insertions(+), 23 deletions(-)

Thanks Phil.

Acked-by: Paul Moore <paul@paul-moore.com> (Audit)

-- 
paul-moore.com

Re: [nf PATCH 3/3] netfilter: nf_tables: Audit log object reset once per table

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> When resetting multiple objects at once (via dump request), emit a log
> message per table (or filled skb) and resurrect the 'entries' parameter
> to contain the number of objects being logged for.
> 
> With the above in place, all audit logs for op=nft_register_obj have a
> predictable value in 'entries', so drop the value zeroing for them in
> audit_logread.c.
> 
> To test the skb exhaustion path, perform some bulk counter and quota
> adds in the kselftest.

This patch doesnt apply to nf.git, can you rebase it?

Re: [nf PATCH 0/3] Review nf_tables audit logging

[Paul Moore]

On Fri, Sep 22, 2023 at 9:53 PM Phil Sutter <phil@nwl.cc> wrote:
>
> When working on locking for reset commands, some audit log calls had to
> be adjusted as well. This series deals with the "fallout" from adding
> tests for the changed log calls, dealing with the uncovered issues and
> adding more tests.
>
> Patch 1 adds more testing to nft_audit.sh for commands which are
> unproblematic.
>
> Patch 2 deals with (likely) leftovers from audit log flood prevention in
> commit c520292f29b80 ("audit: log nftables configuration change events
> once per table").
>
> Patch 3 changes logging for object reset requests to happen once per
> table (if skb size is sufficient) and thereby aligns output with object
> add requests. As a side-effect, logging is fixed to happen after the
> actual reset has succeeded, not before.
>
> NOTE: This whole series probably depends on the reset locking series[1]
> submitted earlier, but there's no functional connection and reviews
> should happen independently.
>
> [1] https://lore.kernel.org/netfilter-devel/20230923013807.11398-1-phil@nwl.cc/
>
> Phil Sutter (3):
>   selftests: netfilter: Extend nft_audit.sh
>   netfilter: nf_tables: Deduplicate nft_register_obj audit logs
>   netfilter: nf_tables: Audit log object reset once per table
>
>  net/netfilter/nf_tables_api.c                 |  95 +++++-----
>  .../testing/selftests/netfilter/nft_audit.sh  | 163 ++++++++++++++++--
>  2 files changed, 203 insertions(+), 55 deletions(-)

Hi Phil,

Thanks for continuing to work on this, my network access is limited at
the moment but I hope to be able to review this next week.

-- 
paul-moore.com