Re: [PATCH nf] netfilter: x_tables: add late validate callback for nft_compat sake

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Sun, Apr 19, 2026 at 12:59:23PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > On Sun, Apr 19, 2026 at 12:45:05PM +0200, Florian Westphal wrote:
> > > > x_tables and nftables are fundamentally different.
> > > > In x_tables, one gets the full ruleset graph via setsockopt().
> > > > ->checkentry() gets called at ruleset validation time.
> > > > 
> > > > In nf_tables, you get a transactional request (rule add in this case)
> > > > in netlink format.  At this time, it is not yet knowm from which
> > > > basechain(s) the new expression is reachable.
> > > >
> > > > In nf_tables, there is one final hook validation pass right before the
> > > > point-of-no-return when the new state is fully known.
> > > >
> > > > However, nft_compat calls the x_tables checkentry functions way too
> > > > early, at expression instantiation time, when we have the netlink
> > > > info available but not the base chain info (not yet known).
> > > 
> > > There used to be full validation of the table in each transaction in
> > > nf_tables.
> > > 
> > > What happened?
> > 
> > As far as I can see this never worked correctly.
> > 
> > A few matches/targets perform hook_mask checks in ->checkentry(), but
> > nft_compat calls ->checkentry() at expression init stage, which is too
> > early and only catches problems if the target/match is called from
> > basechain.
> 
> There is nft_{match,target}_validate() which check against
> {match,target}->hooks and select_ops sets ops->validate accordingly.

This problem is not related to target->hooks, which is 0 for xt_TCPMSS.

Have a look at tcpmss_tg6_check().  I don't think this ever worked
for nft_compat.

 367         ret = nft_chain_validate_hooks(ctx->chain,
 368                                        (1 << NF_INET_PRE_ROUTING) |
 369                                        (1 << NF_INET_LOCAL_IN) |
 370                                        (1 << NF_INET_FORWARD) |
 371                                        (1 << NF_INET_LOCAL_OUT) |
 372                                        (1 << NF_INET_POST_ROUTING));
 373         if (ret)
 374                 return ret;
 375

I don't see what this validates.  Can probable be axed?
(but unrelated).

 376         if (nft_is_base_chain(ctx->chain)) {
 377                 const struct nft_base_chain *basechain =
 378                                                 nft_base_chain(ctx->chain);
 379                 const struct nf_hook_ops *ops = &basechain->ops;
 380 
 381                 hook_mask = 1 << ops->hooknum;
 382                 if (target->hooks && !(hook_mask & target->hooks))
 383                         return -EINVAL;

target->hooks is 0, so nothing is validated.
xt_FOO that set .hooks to a nonzero value are handled correctly, even
before this patch.