Re: [PATCH nf] netfilter: xt_TCPMSS: check skb_dst before path-MTU clamping

[Florian Westphal <fw@strlen.de>]

Florian Westphal <fw@strlen.de> wrote:
> Weiming Shi <bestswngs@gmail.com> wrote:
> > When TCPMSS with CLAMP_PMTU is used via nft_compat in a non-base
> > chain, par->hook_mask is set to 0, bypassing the checkentry hook
> > validation. The target can then run at PRE_ROUTING where skb_dst is
> > NULL, causing a null-ptr-deref in tcpmss_mangle_packet():
> > 
> >  KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> >  RIP: 0010:tcpmss_mangle_packet (include/net/dst.h:219 net/netfilter/xt_TCPMSS.c:105)
> >   tcpmss_tg4 (net/netfilter/xt_TCPMSS.c:202)
> >   nft_target_eval_xt (net/netfilter/nft_compat.c:87)
> >   nft_do_chain (net/netfilter/nf_tables_core.c:287)
> >   nf_hook_slow (net/netfilter/core.c:623)
> > 
> > Check skb_dst() for NULL before calling dst_mtu().
> 
> FWIW I will apply this patch even though its wrong.
> 
> nft_compat.c is just too broken, I don't see how it can be
> fixed in any reasonable amount of time.

net/netfilter/xt_TCPMSS.c:          (par->hook_mask & ~((1 << NF_INET_FORWARD) |
net/netfilter/xt_addrtype.c:    if (par->hook_mask & ((1 << NF_INET_PRE_ROUTING) |
net/netfilter/xt_devgroup.c:        par->hook_mask & ~((1 << NF_INET_PRE_ROUTING) |
net/netfilter/xt_physdev.c:         par->hook_mask & (1 << NF_INET_LOCAL_OUT)) {
net/netfilter/xt_policy.c:      if (par->hook_mask & ((1 << NF_INET_POST_ROUTING) |
net/netfilter/xt_set.c:              (par->hook_mask & ~(1 << NF_INET_FORWARD |

Look at this I don't see an alternative to mixing nft specific bits into
x_tables, i.e.:

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -187,6 +187,8 @@ struct xt_target {
        /* Should return 0 on success or an error code otherwise (-Exxxx). */
        int (*checkentry)(const struct xt_tgchk_param *);
 
+       int (*nft_validate_chain)(const void *targinfo, unsigned int hook_mask);
+
        /* Called when entry of this type deleted. */
        void (*destroy)(const struct xt_tgdtor_param *);
 #ifdef CONFIG_NETFILTER_XTABLES_COMPAT

.. and then call that from nft_compat.c for TCPSS.
Same for matches.