The kernel allows netlink messages that use the legacy NFT_CT_SRC and NFT_CT_DST keys in inet tables without an accompanying nft_meta expression specifying NFT_META_NFPROTO. This results in ambiguous conntrack expressions that cannot be reliably evaluated during packet processing. When that happens, the register size calculation defaults to IPv6 (16 bytes) regardless of the actual packet family. This causes two issues: 1. For IPv4 packets, only 4 bytes contain valid address data while 12 bytes contain uninitialized memory during comparison. 2. nft userspace cannot properly display these rules ([invalid type]). The bug is not reproducible through standard nft commands, which use NFT_CT_SRC_IP(6) and NFT_CT_DST_IP(6) keys when NFT_META_NFPROTO is not defined. Fix by adding a pointer to the parent nft_rule in nft_expr, allowing iteration over preceding expressions to ensure that an nft_meta with NFT_META_NFPROTO has been defined. Signed-off-by: Nikolaos Gkarlis --- Adding a pointer from nft_expr to nft_rule may be controversial, but it was the only approach I could come up with that provides context about preceding expressions when validating an expression. I am not certain if this is the best way to handle it, but I am sending the patch for review. Let me know if you would rather this handled another way. include/net/netfilter/nf_tables.h | 2 ++ include/net/netfilter/nft_meta.h | 2 ++ net/netfilter/nf_tables_api.c | 10 +++++++--- net/netfilter/nft_ct.c | 21 +++++++++++++++++++++ net/netfilter/nft_meta.c | 3 ++- 5 files changed, 34 insertions(+), 4 deletions(-) diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h index 891e43a01bdc..4beb3aa46fe0 100644 --- a/include/net/netfilter/nf_tables.h +++ b/include/net/netfilter/nf_tables.h @@ -403,10 +403,12 @@ struct nft_set_estimate { * struct nft_expr - nf_tables expression * * @ops: expression ops + * @rule: rule this expression is contained in * @data: expression private data */ struct nft_expr { const struct nft_expr_ops *ops; + const struct nft_rule *rule; unsigned char data[] __attribute__((aligned(__alignof__(u64)))); }; diff --git a/include/net/netfilter/nft_meta.h b/include/net/netfilter/nft_meta.h index d602263590fe..93d987de78a6 100644 --- a/include/net/netfilter/nft_meta.h +++ b/include/net/netfilter/nft_meta.h @@ -51,4 +51,6 @@ void nft_meta_inner_eval(const struct nft_expr *expr, struct nft_regs *regs, const struct nft_pktinfo *pkt, struct nft_inner_tun_ctx *tun_ctx); +extern const struct nft_expr_ops nft_meta_get_ops; + #endif diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 58c5425d61c2..eab26b33d80e 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -3500,6 +3500,7 @@ int nft_expr_inner_parse(const struct nft_ctx *ctx, const struct nlattr *nla, } static int nf_tables_newexpr(const struct nft_ctx *ctx, + const struct nft_rule *rule, const struct nft_expr_info *expr_info, struct nft_expr *expr) { @@ -3507,6 +3508,7 @@ static int nf_tables_newexpr(const struct nft_ctx *ctx, int err; expr->ops = ops; + expr->rule = rule; if (ops->init) { err = ops->init(ctx, expr, (const struct nlattr **)expr_info->tb); if (err < 0) @@ -3516,6 +3518,7 @@ static int nf_tables_newexpr(const struct nft_ctx *ctx, return 0; err1: expr->ops = NULL; + expr->rule = NULL; return err; } @@ -3530,6 +3533,7 @@ static void nf_tables_expr_destroy(const struct nft_ctx *ctx, } static struct nft_expr *nft_expr_init(const struct nft_ctx *ctx, + const struct nft_rule *rule, const struct nlattr *nla) { struct nft_expr_info expr_info; @@ -3550,7 +3554,7 @@ static struct nft_expr *nft_expr_init(const struct nft_ctx *ctx, if (expr == NULL) goto err_expr_stateful; - err = nf_tables_newexpr(ctx, &expr_info, expr); + err = nf_tables_newexpr(ctx, rule, &expr_info, expr); if (err < 0) goto err_expr_new; @@ -4339,7 +4343,7 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info, expr = nft_expr_first(rule); for (i = 0; i < n; i++) { - err = nf_tables_newexpr(&ctx, &expr_info[i], expr); + err = nf_tables_newexpr(&ctx, rule, &expr_info[i], expr); if (err < 0) { NL_SET_BAD_ATTR(extack, expr_info[i].attr); goto err_release_rule; @@ -6681,7 +6685,7 @@ struct nft_expr *nft_set_elem_expr_alloc(const struct nft_ctx *ctx, struct nft_expr *expr; int err; - expr = nft_expr_init(ctx, attr); + expr = nft_expr_init(ctx, NULL, attr); if (IS_ERR(expr)) return expr; diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c index d526e69a2a2b..b18bd5705872 100644 --- a/net/netfilter/nft_ct.c +++ b/net/netfilter/nft_ct.c @@ -22,6 +22,7 @@ #include #include #include +#include struct nft_ct_helper_obj { struct nf_conntrack_helper *helper4; @@ -440,6 +441,26 @@ static int nft_ct_get_init(const struct nft_ctx *ctx, break; case NFPROTO_IPV6: case NFPROTO_INET: + const struct nft_expr *curr, *last; + bool meta_nfproto = false; + if (!expr->rule) + return -EINVAL; + + nft_rule_for_each_expr(curr, last, expr->rule) { + if (curr == expr) + break; + + if (curr->ops == &nft_meta_get_ops) { + const struct nft_meta *meta = nft_expr_priv(curr); + if (meta->key == NFT_META_NFPROTO) { + meta_nfproto = true; + break; + } + } + } + if (!meta_nfproto) + return -EINVAL; + len = sizeof_field(struct nf_conntrack_tuple, src.u3.ip6); break; diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c index 05cd1e6e6a2f..aa6bf05e3907 100644 --- a/net/netfilter/nft_meta.c +++ b/net/netfilter/nft_meta.c @@ -767,7 +767,7 @@ bool nft_meta_get_reduce(struct nft_regs_track *track, } EXPORT_SYMBOL_GPL(nft_meta_get_reduce); -static const struct nft_expr_ops nft_meta_get_ops = { +const struct nft_expr_ops nft_meta_get_ops = { .type = &nft_meta_type, .size = NFT_EXPR_SIZE(sizeof(struct nft_meta)), .eval = nft_meta_get_eval, @@ -777,6 +777,7 @@ static const struct nft_expr_ops nft_meta_get_ops = { .validate = nft_meta_get_validate, .offload = nft_meta_get_offload, }; +EXPORT_SYMBOL_GPL(nft_meta_get_ops); static bool nft_meta_set_reduce(struct nft_regs_track *track, const struct nft_expr *expr) -- 2.34.1