[nf-next PATCH v3 0/4] nf_tables: Export rule optimizer results to user space

[nf-next PATCH v3 0/4] nf_tables: Export rule optimizer results to user space

[Phil Sutter]

Changes since v2:
- New patches 1 and 2

Changes since v1:
- Fixed two bugs in patch 2.

While transforming rules into binary blob, code checks if certain
expressions may be omitted. Any bugs in this code might lead to very
subtle breakage of firewall rulesets, so a way of asserting optimizer
correctness is highly necessary.

This series achieves this in the most minimal way by annotating omitted
expressions with a flag. Integrated into libnftnl print output,
testsuites in user space may verify optimizer effect and assert
correctness.

First patch prepares for a blob-specific variant of struct nft_expr
which is smaller than the original. Second patch introduces this
variant. Third patch extends structy nft_expr by a new field and finally
fourth patch populates it.

Phil Sutter (4):
  netfilter: nf_tables: Store net size in nft_expr_ops::size
  netfilter: nf_tables: Introduce struct nft_expr_dp
  netfilter: nf_tables: Introduce expression flags
  netfilter: nf_tables: Annotate reduced expressions

 include/net/netfilter/nf_tables.h        | 13 ++++++--
 include/uapi/linux/netfilter/nf_tables.h |  8 +++++
 net/netfilter/nf_tables_api.c            | 42 ++++++++++++++++--------
 3 files changed, 48 insertions(+), 15 deletions(-)

-- 
2.34.1

[nf-next PATCH v3 1/4] netfilter: nf_tables: Store net size in nft_expr_ops::size

[Phil Sutter]

Prepare for expressions of different size in ruleset blob by storing
only the per-expression payload in struct nft_expr_ops' size field
instead of a value depending on size of struct nft_expr.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/net/netfilter/nf_tables.h |  3 ++-
 net/netfilter/nf_tables_api.c     | 23 +++++++++++++----------
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 20af9d3557b9d..4308e38df8e7a 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -335,7 +335,8 @@ struct nft_set_estimate {
 };
 
 #define NFT_EXPR_MAXATTR		16
-#define NFT_EXPR_SIZE(size)		(sizeof(struct nft_expr) + \
+#define NFT_EXPR_SIZE(size)		size
+#define NFT_EXPR_FULL_SIZE(size)	(sizeof(struct nft_expr) + \
 					 ALIGN(size, __alignof__(struct nft_expr)))
 
 /**
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index f3ad02a399f8a..609fc9137ac01 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2876,7 +2876,8 @@ static struct nft_expr *nft_expr_init(const struct nft_ctx *ctx,
 		goto err1;
 
 	err = -ENOMEM;
-	expr = kzalloc(expr_info.ops->size, GFP_KERNEL_ACCOUNT);
+	expr = kzalloc(NFT_EXPR_FULL_SIZE(expr_info.ops->size),
+		       GFP_KERNEL_ACCOUNT);
 	if (expr == NULL)
 		goto err2;
 
@@ -2907,7 +2908,7 @@ int nft_expr_clone(struct nft_expr *dst, struct nft_expr *src)
 		if (err < 0)
 			return err;
 	} else {
-		memcpy(dst, src, src->ops->size);
+		memcpy(dst, src, NFT_EXPR_FULL_SIZE(src->ops->size));
 	}
 
 	__module_get(src->ops->type->owner);
@@ -3468,7 +3469,7 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
 				NL_SET_BAD_ATTR(extack, tmp);
 				goto err_release_expr;
 			}
-			size += expr_info[n].ops->size;
+			size += NFT_EXPR_FULL_SIZE(expr_info[n].ops->size);
 			n++;
 		}
 	}
@@ -5526,7 +5527,8 @@ int nft_set_elem_expr_clone(const struct nft_ctx *ctx, struct nft_set *set,
 	int err, i, k;
 
 	for (i = 0; i < set->num_exprs; i++) {
-		expr = kzalloc(set->exprs[i]->ops->size, GFP_KERNEL_ACCOUNT);
+		expr = kzalloc(NFT_EXPR_FULL_SIZE(set->exprs[i]->ops->size),
+			       GFP_KERNEL_ACCOUNT);
 		if (!expr)
 			goto err_expr;
 
@@ -5562,7 +5564,7 @@ static int nft_set_elem_expr_setup(struct nft_ctx *ctx,
 		if (err < 0)
 			goto err_elem_expr_setup;
 
-		elem_expr->size += expr_array[i]->ops->size;
+		elem_expr->size += NFT_EXPR_FULL_SIZE(expr_array[i]->ops->size);
 		nft_expr_destroy(ctx, expr_array[i]);
 		expr_array[i] = NULL;
 	}
@@ -5929,7 +5931,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 
 	if (num_exprs) {
 		for (i = 0; i < num_exprs; i++)
-			size += expr_array[i]->ops->size;
+			size += NFT_EXPR_FULL_SIZE(expr_array[i]->ops->size);
 
 		nft_set_ext_add_length(&tmpl, NFT_SET_EXT_EXPRESSIONS,
 				       sizeof(struct nft_set_elem_expr) +
@@ -8356,9 +8358,9 @@ static bool nft_expr_reduce(struct nft_regs_track *track,
 
 static int nf_tables_commit_chain_prepare(struct net *net, struct nft_chain *chain)
 {
+	unsigned int size, expr_size, data_size;
 	const struct nft_expr *expr, *last;
 	struct nft_regs_track track = {};
-	unsigned int size, data_size;
 	void *data, *data_boundary;
 	struct nft_rule_dp *prule;
 	struct nft_rule *rule;
@@ -8404,11 +8406,12 @@ static int nf_tables_commit_chain_prepare(struct net *net, struct nft_chain *cha
 				continue;
 			}
 
-			if (WARN_ON_ONCE(data + expr->ops->size > data_boundary))
+			expr_size = NFT_EXPR_FULL_SIZE(expr->ops->size);
+			if (WARN_ON_ONCE(data + expr_size > data_boundary))
 				return -ENOMEM;
 
-			memcpy(data + size, expr, expr->ops->size);
-			size += expr->ops->size;
+			memcpy(data + size, expr, expr_size);
+			size += expr_size;
 		}
 		if (WARN_ON_ONCE(size >= 1 << 12))
 			return -ENOMEM;
-- 
2.34.1

[nf-next PATCH v3 2/4] netfilter: nf_tables: Introduce struct nft_expr_dp

[Phil Sutter]

This is a minimal variant of struct nft_expr for use in ruleset blob.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/net/netfilter/nf_tables.h |  9 ++++++++-
 net/netfilter/nf_tables_api.c     | 11 ++++++++---
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 4308e38df8e7a..708593dd4142e 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -338,6 +338,8 @@ struct nft_set_estimate {
 #define NFT_EXPR_SIZE(size)		size
 #define NFT_EXPR_FULL_SIZE(size)	(sizeof(struct nft_expr) + \
 					 ALIGN(size, __alignof__(struct nft_expr)))
+#define NFT_EXPR_DP_SIZE(size)		(sizeof(struct nft_expr_dp) + \
+					 ALIGN(size, __alignof__(struct nft_expr_dp)))
 
 /**
  *	struct nft_expr - nf_tables expression
@@ -992,12 +994,17 @@ static inline void nft_set_elem_update_expr(const struct nft_set_ext *ext,
 
 #define NFT_CHAIN_POLICY_UNSET		U8_MAX
 
+struct nft_expr_dp {
+	const struct nft_expr_ops	*ops;
+	unsigned char			data[] __aligned(__alignof__(u64));
+};
+
 struct nft_rule_dp {
 	u64				is_last:1,
 					dlen:12,
 					handle:42;	/* for tracing */
 	unsigned char			data[]
-		__attribute__((aligned(__alignof__(struct nft_expr))));
+		__aligned(__alignof__(struct nft_expr_dp));
 };
 
 struct nft_rule_blob {
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 609fc9137ac01..ba2f712823776 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -8363,6 +8363,7 @@ static int nf_tables_commit_chain_prepare(struct net *net, struct nft_chain *cha
 	struct nft_regs_track track = {};
 	void *data, *data_boundary;
 	struct nft_rule_dp *prule;
+	struct nft_expr_dp *pexpr;
 	struct nft_rule *rule;
 
 	/* already handled or inactive chain? */
@@ -8372,7 +8373,9 @@ static int nf_tables_commit_chain_prepare(struct net *net, struct nft_chain *cha
 	data_size = 0;
 	list_for_each_entry(rule, &chain->rules, list) {
 		if (nft_is_active_next(net, rule)) {
-			data_size += sizeof(*prule) + rule->dlen;
+			data_size += sizeof(*prule);
+			nft_rule_for_each_expr(expr, last, rule)
+				data_size += NFT_EXPR_DP_SIZE(expr->ops->size);
 			if (data_size > INT_MAX)
 				return -ENOMEM;
 		}
@@ -8406,11 +8409,13 @@ static int nf_tables_commit_chain_prepare(struct net *net, struct nft_chain *cha
 				continue;
 			}
 
-			expr_size = NFT_EXPR_FULL_SIZE(expr->ops->size);
+			expr_size = NFT_EXPR_DP_SIZE(expr->ops->size);
 			if (WARN_ON_ONCE(data + expr_size > data_boundary))
 				return -ENOMEM;
 
-			memcpy(data + size, expr, expr_size);
+			pexpr = (struct nft_expr_dp *)(data + size);
+			pexpr->ops = expr->ops;
+			memcpy(pexpr->data, expr->data, expr->ops->size);
 			size += expr_size;
 		}
 		if (WARN_ON_ONCE(size >= 1 << 12))
-- 
2.34.1

[nf-next PATCH v3 3/4] netfilter: nf_tables: Introduce expression flags

[Phil Sutter]

Allow dumping some info bits about expressions to user space.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/net/netfilter/nf_tables.h        | 1 +
 include/uapi/linux/netfilter/nf_tables.h | 1 +
 net/netfilter/nf_tables_api.c            | 4 ++++
 3 files changed, 6 insertions(+)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 708593dd4142e..16f6f36073522 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -349,6 +349,7 @@ struct nft_set_estimate {
  */
 struct nft_expr {
 	const struct nft_expr_ops	*ops;
+	u32				flags;
 	unsigned char			data[]
 		__attribute__((aligned(__alignof__(u64))));
 };
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 466fd3f4447c2..36bf019322a44 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -518,6 +518,7 @@ enum nft_expr_attributes {
 	NFTA_EXPR_UNSPEC,
 	NFTA_EXPR_NAME,
 	NFTA_EXPR_DATA,
+	NFTA_EXPR_FLAGS,
 	__NFTA_EXPR_MAX
 };
 #define NFTA_EXPR_MAX		(__NFTA_EXPR_MAX - 1)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index ba2f712823776..608c5e684dff7 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2731,6 +2731,7 @@ static const struct nft_expr_type *nft_expr_type_get(struct net *net,
 static const struct nla_policy nft_expr_policy[NFTA_EXPR_MAX + 1] = {
 	[NFTA_EXPR_NAME]	= { .type = NLA_STRING,
 				    .len = NFT_MODULE_AUTOLOAD_LIMIT },
+	[NFTA_EXPR_FLAGS]	= { .type = NLA_U32 },
 	[NFTA_EXPR_DATA]	= { .type = NLA_NESTED },
 };
 
@@ -2740,6 +2741,9 @@ static int nf_tables_fill_expr_info(struct sk_buff *skb,
 	if (nla_put_string(skb, NFTA_EXPR_NAME, expr->ops->type->name))
 		goto nla_put_failure;
 
+	if (nla_put_u32(skb, NFTA_EXPR_FLAGS, expr->flags))
+		goto nla_put_failure;
+
 	if (expr->ops->dump) {
 		struct nlattr *data = nla_nest_start_noflag(skb,
 							    NFTA_EXPR_DATA);
-- 
2.34.1

[nf-next PATCH v3 4/4] netfilter: nf_tables: Annotate reduced expressions

[Phil Sutter]

Introduce NFTA_EXPR_FLAG_REDUCED and set it for expressions which were
omitted from the rule blob due to being redundant. This allows user
space to verify the rule optimizer's results.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Keep pointers in struct nft_regs_track const and avoid assigning from
  track.cur to expr instead.
- Fix for situations where nft_expr_reduce() causes skipping of multiple
  expressions (payload + bitwise for instance).
---
 include/uapi/linux/netfilter/nf_tables.h | 7 +++++++
 net/netfilter/nf_tables_api.c            | 8 ++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 36bf019322a44..1da84ebc3f27a 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -523,6 +523,13 @@ enum nft_expr_attributes {
 };
 #define NFTA_EXPR_MAX		(__NFTA_EXPR_MAX - 1)
 
+/**
+ * NFTA_EXPR_FLAGS values
+ *
+ * @NFTA_EXPR_FLAG_REDUCED: redundant expression omitted from blob
+ */
+#define NFTA_EXPR_FLAG_REDUCED	(1 << 0)
+
 /**
  * enum nft_immediate_attributes - nf_tables immediate expression netlink attributes
  *
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 608c5e684dff7..01141412cb052 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -8363,8 +8363,8 @@ static bool nft_expr_reduce(struct nft_regs_track *track,
 static int nf_tables_commit_chain_prepare(struct net *net, struct nft_chain *chain)
 {
 	unsigned int size, expr_size, data_size;
-	const struct nft_expr *expr, *last;
 	struct nft_regs_track track = {};
+	struct nft_expr *expr, *last;
 	void *data, *data_boundary;
 	struct nft_rule_dp *prule;
 	struct nft_expr_dp *pexpr;
@@ -8409,7 +8409,11 @@ static int nf_tables_commit_chain_prepare(struct net *net, struct nft_chain *cha
 			track.cur = expr;
 
 			if (nft_expr_reduce(&track, expr)) {
-				expr = track.cur;
+				expr->flags |= NFTA_EXPR_FLAG_REDUCED;
+				while (expr != track.cur) {
+					expr = nft_expr_next(expr);
+					expr->flags |= NFTA_EXPR_FLAG_REDUCED;
+				}
 				continue;
 			}
 
-- 
2.34.1