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@xxxxxx>
> ---
> 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
>
