[nf,4/5] netfilter: nf_tables: use chain info to validate type and hook.

Message ID 20180515122401.29480-1-ap420073@gmail.com
State RFC
Delegated to: Pablo Neira
Headers show
Series
  • netfilter: nf_tables: add validate non-basechain ruleset routine
Related show

Commit Message

Taehee Yoo May 15, 2018, 12:24 p.m.
After this patch, the nft_chain_validate_dependency and
nft_chain_validate_hooks use chain information array.
so that these functions can validate both basechain and non-basechain.

Now expr->ops->validate should be called in the nf_tables_validate because
that uses chain information that is allocated in the nf_tables_validate.
But exceptionally, the nf_tables_check_loops can call
that if ops is "immediate".

Now, nft_compat.c uses common validate routine instead of
the nft_compat_chain_validate_dependency.

Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
 net/netfilter/nf_tables_api.c | 51 +++++++++++-------------------
 net/netfilter/nft_compat.c    | 73 ++++++++++++++-----------------------------
 2 files changed, 42 insertions(+), 82 deletions(-)

Patch

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 36d8fba..d902ef9 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1899,26 +1899,13 @@  static int nf_tables_newexpr(const struct nft_ctx *ctx,
 	expr->ops = ops;
 	if (ops->init) {
 		err = ops->init(ctx, expr, (const struct nlattr **)info->tb);
-		if (err < 0)
-			goto err1;
-	}
-
-	if (ops->validate) {
-		const struct nft_data *data = NULL;
-
-		err = ops->validate(ctx, expr, &data);
-		if (err < 0)
-			goto err2;
+		if (err < 0) {
+			expr->ops = NULL;
+			return err;
+		}
 	}
 
 	return 0;
-
-err2:
-	if (ops->destroy)
-		ops->destroy(ctx, expr);
-err1:
-	expr->ops = NULL;
-	return err;
 }
 
 static void nf_tables_expr_destroy(const struct nft_ctx *ctx,
@@ -6397,13 +6384,12 @@  static const struct nfnetlink_subsystem nf_tables_subsys = {
 int nft_chain_validate_dependency(const struct nft_ctx *ctx,
 				  enum nft_chain_types type)
 {
-	const struct nft_base_chain *basechain;
+	struct net *net = ctx->net;
+	struct nft_chain *chain = ctx->chain;
+	struct nft_chain_info *cinfo = nft_get_chain_info(net, chain);
 
-	if (nft_is_base_chain(ctx->chain)) {
-		basechain = nft_base_chain(ctx->chain);
-		if (basechain->type->type != type)
-			return -EOPNOTSUPP;
-	}
+	if (cinfo->type && cinfo->type != type)
+		return -EOPNOTSUPP;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(nft_chain_validate_dependency);
@@ -6411,17 +6397,14 @@  EXPORT_SYMBOL_GPL(nft_chain_validate_dependency);
 int nft_chain_validate_hooks(const struct nft_ctx *ctx,
 			     unsigned int hook_flags)
 {
-	struct nft_base_chain *basechain;
-
-	if (nft_is_base_chain(ctx->chain)) {
-		basechain = nft_base_chain(ctx->chain);
-
-		if ((1 << basechain->ops.hooknum) & hook_flags)
-			return 0;
+	struct net *net = ctx->net;
+	struct nft_chain *chain = ctx->chain;
+	struct nft_chain_info *cinfo = nft_get_chain_info(net, chain);
 
+	if (!hook_flags)
+		return 0;
+	if (cinfo->hooknum & ~hook_flags)
 		return -EOPNOTSUPP;
-	}
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(nft_chain_validate_hooks);
@@ -6479,12 +6462,14 @@  static int nf_tables_check_loops(const struct nft_ctx *ctx,
 
 			if (!expr->ops->validate)
 				continue;
+			if (strcmp(expr->ops->type->name, "immediate"))
+				continue;
 
 			err = expr->ops->validate(ctx, expr, &data);
 			if (err < 0)
 				return err;
 
-			if (data == NULL)
+			if (!data)
 				continue;
 
 			switch (data->verdict.code) {
diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 1d99a1ef..c7aad9c 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -54,23 +54,6 @@  static bool nft_xt_put(struct nft_xt *xt)
 	return false;
 }
 
-static int nft_compat_chain_validate_dependency(const char *tablename,
-						const struct nft_chain *chain)
-{
-	const struct nft_base_chain *basechain;
-
-	if (!tablename ||
-	    !nft_is_base_chain(chain))
-		return 0;
-
-	basechain = nft_base_chain(chain);
-	if (strcmp(tablename, "nat") == 0 &&
-	    basechain->type->type != NFT_CHAIN_T_NAT)
-		return -EINVAL;
-
-	return 0;
-}
-
 union nft_entry {
 	struct ipt_entry e4;
 	struct ip6t_entry e6;
@@ -311,24 +294,20 @@  static int nft_target_validate(const struct nft_ctx *ctx,
 			       const struct nft_data **data)
 {
 	struct xt_target *target = expr->ops->data;
-	unsigned int hook_mask = 0;
-	int ret;
-
-	if (nft_is_base_chain(ctx->chain)) {
-		const struct nft_base_chain *basechain =
-						nft_base_chain(ctx->chain);
-		const struct nf_hook_ops *ops = &basechain->ops;
+	enum nft_chain_types type;
+	int err;
 
-		hook_mask = 1 << ops->hooknum;
-		if (target->hooks && !(hook_mask & target->hooks))
-			return -EINVAL;
+	if (!target->table)
+		return 0;
+	if (!strcmp(target->table, "nat"))
+		type = NFT_CHAIN_T_NAT;
+	else
+		type = NFT_CHAIN_T_DEFAULT;
 
-		ret = nft_compat_chain_validate_dependency(target->table,
-							   ctx->chain);
-		if (ret < 0)
-			return ret;
-	}
-	return 0;
+	err = nft_chain_validate_dependency(ctx, type);
+	if (err < 0)
+		return err;
+	return nft_chain_validate_hooks(ctx, target->hooks);
 }
 
 static void __nft_match_eval(const struct nft_expr *expr,
@@ -558,24 +537,20 @@  static int nft_match_validate(const struct nft_ctx *ctx,
 			      const struct nft_data **data)
 {
 	struct xt_match *match = expr->ops->data;
-	unsigned int hook_mask = 0;
-	int ret;
-
-	if (nft_is_base_chain(ctx->chain)) {
-		const struct nft_base_chain *basechain =
-						nft_base_chain(ctx->chain);
-		const struct nf_hook_ops *ops = &basechain->ops;
+	enum nft_chain_types type;
+	int err;
 
-		hook_mask = 1 << ops->hooknum;
-		if (match->hooks && !(hook_mask & match->hooks))
-			return -EINVAL;
+	if (!match->table)
+		return 0;
+	if (!strcmp(match->table, "nat"))
+		type = NFT_CHAIN_T_NAT;
+	else
+		type = NFT_CHAIN_T_DEFAULT;
 
-		ret = nft_compat_chain_validate_dependency(match->table,
-							   ctx->chain);
-		if (ret < 0)
-			return ret;
-	}
-	return 0;
+	err = nft_chain_validate_dependency(ctx, type);
+	if (err < 0)
+		return err;
+	return nft_chain_validate_hooks(ctx, match->hooks);
 }
 
 static int