Re: [nf-next PATCH v2] netfilter: nf_tables: Fix for extra data in delete notifications

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Jun 12, 2025 at 08:30:24PM +0200, Phil Sutter wrote:
> All routines modified in this patch conditionally return early depending
> on event value (and other criteria, i.e., chain/flowtable updates).
> These checks were defeated by an upfront modification of that variable
> for use in nfnl_msg_put(). Restore functionality by avoiding the
> modification.

Thanks for fixing this.

> This change is particularly important for user space to distinguish
> between a chain/flowtable update removing a hook and full deletion.
> 
> Fixes: 28339b21a365 ("netfilter: nf_tables: do not send complete notification of deletions")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Channeling this through -next despite it being a fix since unpatched
> nft monitor chokes on the shortened delete flowtable notifications.

I am afraid this patch will end up in -stable, breaking userspace, how
bad is the choking? Maybe 28339b21a365 needs to be reverted, then fix
userspace to prepare for it and re-add it in nf-next?

Not sure what path to follow with this.

> Changes since v1:
> - User space depends on NFTA_OBJ_TYPE for proper deserialization, do not
>   skip that attribute.
> ---
>  net/netfilter/nf_tables_api.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 8952b50b0224..9bf003797355 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -1153,9 +1153,9 @@ static int nf_tables_fill_table_info(struct sk_buff *skb, struct net *net,
>  {
>  	struct nlmsghdr *nlh;
>  
> -	event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
> -	nlh = nfnl_msg_put(skb, portid, seq, event, flags, family,
> -			   NFNETLINK_V0, nft_base_seq(net));
> +	nlh = nfnl_msg_put(skb, portid, seq,
> +			   nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event),
> +			   flags, family, NFNETLINK_V0, nft_base_seq(net));
>  	if (!nlh)
>  		goto nla_put_failure;
>  
> @@ -2020,9 +2020,9 @@ static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net,
>  {
>  	struct nlmsghdr *nlh;
>  
> -	event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
> -	nlh = nfnl_msg_put(skb, portid, seq, event, flags, family,
> -			   NFNETLINK_V0, nft_base_seq(net));
> +	nlh = nfnl_msg_put(skb, portid, seq,
> +			   nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event),
> +			   flags, family, NFNETLINK_V0, nft_base_seq(net));
>  	if (!nlh)
>  		goto nla_put_failure;
>  
> @@ -4851,9 +4851,10 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
>  	u32 seq = ctx->seq;
>  	int i;
>  
> -	event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
> -	nlh = nfnl_msg_put(skb, portid, seq, event, flags, ctx->family,
> -			   NFNETLINK_V0, nft_base_seq(ctx->net));
> +	nlh = nfnl_msg_put(skb, portid, seq,
> +			   nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event),
> +			   flags, ctx->family, NFNETLINK_V0,
> +			   nft_base_seq(ctx->net));
>  	if (!nlh)
>  		goto nla_put_failure;
>  
> @@ -8346,14 +8347,15 @@ static int nf_tables_fill_obj_info(struct sk_buff *skb, struct net *net,
>  {
>  	struct nlmsghdr *nlh;
>  
> -	event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
> -	nlh = nfnl_msg_put(skb, portid, seq, event, flags, family,
> -			   NFNETLINK_V0, nft_base_seq(net));
> +	nlh = nfnl_msg_put(skb, portid, seq,
> +			   nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event),
> +			   flags, family, NFNETLINK_V0, nft_base_seq(net));
>  	if (!nlh)
>  		goto nla_put_failure;
>  
>  	if (nla_put_string(skb, NFTA_OBJ_TABLE, table->name) ||
>  	    nla_put_string(skb, NFTA_OBJ_NAME, obj->key.name) ||
> +	    nla_put_be32(skb, NFTA_OBJ_TYPE, htonl(obj->ops->type->type)) ||
>  	    nla_put_be64(skb, NFTA_OBJ_HANDLE, cpu_to_be64(obj->handle),
>  			 NFTA_OBJ_PAD))
>  		goto nla_put_failure;
> @@ -8363,8 +8365,7 @@ static int nf_tables_fill_obj_info(struct sk_buff *skb, struct net *net,
>  		return 0;
>  	}
>  
> -	if (nla_put_be32(skb, NFTA_OBJ_TYPE, htonl(obj->ops->type->type)) ||
> -	    nla_put_be32(skb, NFTA_OBJ_USE, htonl(obj->use)) ||
> +	if (nla_put_be32(skb, NFTA_OBJ_USE, htonl(obj->use)) ||
>  	    nft_object_dump(skb, NFTA_OBJ_DATA, obj, reset))
>  		goto nla_put_failure;
>  
> @@ -9391,9 +9392,9 @@ static int nf_tables_fill_flowtable_info(struct sk_buff *skb, struct net *net,
>  	struct nft_hook *hook;
>  	struct nlmsghdr *nlh;
>  
> -	event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
> -	nlh = nfnl_msg_put(skb, portid, seq, event, flags, family,
> -			   NFNETLINK_V0, nft_base_seq(net));
> +	nlh = nfnl_msg_put(skb, portid, seq,
> +			   nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event),
> +			   flags, family, NFNETLINK_V0, nft_base_seq(net));
>  	if (!nlh)
>  		goto nla_put_failure;
>  
> -- 
> 2.49.0
>