Re: [PATCH nf] netfilter: nft_fib: reject fib expression on the netdev egress hook

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

Hi,

Comments below.

On Thu, Jun 25, 2026 at 03:28:34PM +0300, Theodor Arsenij Larionov-Trichkine wrote:
> A fib expression in a netdev egress base chain dereferences the input
> device nft_in(pkt), which is NULL on the transmit path, causing a
> NULL-ptr-deref at eval. nft_fib_validate() gates the chain hook with
> NF_INET_* masks, but a netdev chain's hook numbers come from a separate
> enum that aliases them (NF_NETDEV_EGRESS == NF_INET_LOCAL_IN == 1), so a
> netdev egress chain passes validation and then faults. Add a dedicated
> nft_fib_netdev_validate() restricting the netdev fib expression to
> NF_NETDEV_INGRESS, the only netdev hook with an input device, and
> restrict nft_fib_validate() to NFPROTO_IPV4/IPV6/INET so its NF_INET_*
> hook masks are never applied to another family's hook numbering.
>
> Fixes: 42df6e1d221d ("netfilter: Introduce egress hook")
> Link: https://lore.kernel.org/netfilter-devel/ajxsjcDOnwllMfoR@strlen.de/
> Signed-off-by: Theodor Arsenij Larionov-Trichkine <theodorlarionov@gmail.com>
> ---
>  net/netfilter/nft_fib.c        | 9 +++++++++
>  net/netfilter/nft_fib_netdev.c | 8 +++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nft_fib.c b/net/netfilter/nft_fib.c
> index e048f05694cd..89555380f1c5 100644
> --- a/net/netfilter/nft_fib.c
> +++ b/net/netfilter/nft_fib.c
> @@ -31,6 +31,15 @@ int nft_fib_validate(const struct nft_ctx *ctx, const struct nft_expr *expr)
>  	const struct nft_fib *priv = nft_expr_priv(expr);
>  	unsigned int hooks;
>  
> +	switch (ctx->family) {
> +	case NFPROTO_IPV4:
> +	case NFPROTO_IPV6:
> +	case NFPROTO_INET:
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
>  	switch (priv->result) {
>  	case NFT_FIB_RESULT_OIF:
>  	case NFT_FIB_RESULT_OIFNAME:
> diff --git a/net/netfilter/nft_fib_netdev.c b/net/netfilter/nft_fib_netdev.c
> index 3f3478abd845..c855c5dccd41 100644
> --- a/net/netfilter/nft_fib_netdev.c
> +++ b/net/netfilter/nft_fib_netdev.c
> @@ -50,6 +50,12 @@ static void nft_fib_netdev_eval(const struct nft_expr *expr,
>  	regs->verdict.code = NFT_BREAK;
>  }
>  
> +static int nft_fib_netdev_validate(const struct nft_ctx *ctx,
> +				   const struct nft_expr *expr)
> +{

Maybe this is missing:

        switch (priv->result) {
        case NFT_FIB_RESULT_OIF:
        case NFT_FIB_RESULT_OIFNAME:
                hooks = (1 << NF_NETDEV_INGRESS);
                break;
        case NFT_FIB_RESULT_ADDRTYPE:
                if (priv->flags & NFTA_FIB_F_IIF)
                        hooks = (1 << NF_NETDEV_INGRESS);
                else if (priv->flags & NFTA_FIB_F_OIF)
                        hooks = (1 << NF_NETDEV_EGRESS);
                else
                        hooks = (1 << NF_NETDEV_INGRESS) |
                                (1 << NF_NETDEV_EGRESS);
                break;
        default:
                return -EINVAL;
        }

> +	return nft_chain_validate_hooks(ctx->chain, 1 << NF_NETDEV_INGRESS);
> +}
> +
>  static struct nft_expr_type nft_fib_netdev_type;
>  static const struct nft_expr_ops nft_fib_netdev_ops = {
>  	.type		= &nft_fib_netdev_type,
> @@ -57,7 +63,7 @@ static const struct nft_expr_ops nft_fib_netdev_ops = {
>  	.eval		= nft_fib_netdev_eval,
>  	.init		= nft_fib_init,
>  	.dump		= nft_fib_dump,
> -	.validate	= nft_fib_validate,
> +	.validate	= nft_fib_netdev_validate,
>  };
>  
>  static struct nft_expr_type nft_fib_netdev_type __read_mostly = {
> -- 
> 2.34.1
>