db: include the arguments in the db_api_rule_list struct

Instead of dynamically allocating a variable number of arguments,
include an array of ARG_COUNT_MAX elements directly in the struct.
Also perform a number of simplifications to the code with the
understanding that ARG_COUNT_MAX is an ABI independent value that
isn't variable.

Signed-off-by: Paul Moore <paul@paul-moore.com>

7 files changed, 44 insertions, 101 deletions

diff --git a/src/arch-s390.c b/src/arch-s390.c
index 2e79670..0d83e97 100644
--- a/src/arch-s390.c
+++ b/src/arch-s390.c
@@ -209,7 +209,6 @@ int s390_rule_add(struct db_filter_col *col, struct db_filter *db, bool strict,
 {
 	int rc;
 	unsigned int iter;
-	size_t args_size;
 	int sys = rule->syscall;
 	int sys_a, sys_b;
 	struct db_api_rule_list *rule_a, *rule_b;
@@ -219,7 +218,7 @@ int s390_rule_add(struct db_filter_col *col, struct db_filter *db, bool strict,
 		   (359 to 373)   : direct socket syscalls, Linux 4.3+ */
 
 		/* strict check for the multiplexed socket syscalls */
-		for (iter = 0; iter < rule->args_cnt; iter++) {
+		for (iter = 0; iter < ARG_COUNT_MAX; iter++) {
 			if ((rule->args[iter].valid != 0) && (strict))
 				return -EINVAL;
 		}
@@ -249,19 +248,9 @@ int s390_rule_add(struct db_filter_col *col, struct db_filter *db, bool strict,
 		} else {
 			/* need two rules, dup the first and link together */
 			rule_a = rule;
-			rule_b = malloc(sizeof(*rule_b));
+			rule_b = db_rule_dup(rule_a);
 			if (rule_b == NULL)
 				return -ENOMEM;
-			args_size = sizeof(*rule_b->args) * rule_a->args_cnt;
-			rule_b->args = malloc(args_size);
-			if (rule_b->args == NULL) {
-				free(rule_b);
-				return -ENOMEM;
-			}
-			rule_b->action = rule_a->action;
-			rule_b->syscall = rule_a->syscall;
-			rule_b->args_cnt = rule_a->args_cnt;
-			memcpy(rule_b->args, rule_a->args, args_size);
 			rule_b->prev = rule_a;
 			rule_b->next = NULL;
 			rule_a->next = rule_b;
diff --git a/src/arch-s390x.c b/src/arch-s390x.c
index 4de1a1b..fcfd5e5 100644
--- a/src/arch-s390x.c
+++ b/src/arch-s390x.c
@@ -209,7 +209,6 @@ int s390x_rule_add(struct db_filter_col *col, struct db_filter *db, bool strict,
 {
 	int rc;
 	unsigned int iter;
-	size_t args_size;
 	int sys = rule->syscall;
 	int sys_a, sys_b;
 	struct db_api_rule_list *rule_a, *rule_b;
@@ -219,7 +218,7 @@ int s390x_rule_add(struct db_filter_col *col, struct db_filter *db, bool strict,
 		   (359 to 373)   : direct socket syscalls, Linux 4.3+ */
 
 		/* strict check for the multiplexed socket syscalls */
-		for (iter = 0; iter < rule->args_cnt; iter++) {
+		for (iter = 0; iter < ARG_COUNT_MAX; iter++) {
 			if ((rule->args[iter].valid != 0) && (strict))
 				return -EINVAL;
 		}
@@ -249,19 +248,9 @@ int s390x_rule_add(struct db_filter_col *col, struct db_filter *db, bool strict,
 		} else {
 			/* need two rules, dup the first and link together */
 			rule_a = rule;
-			rule_b = malloc(sizeof(*rule_b));
+			rule_b = db_rule_dup(rule_a);
 			if (rule_b == NULL)
 				return -ENOMEM;
-			args_size = sizeof(*rule_b->args) * rule_a->args_cnt;
-			rule_b->args = malloc(args_size);
-			if (rule_b->args == NULL) {
-				free(rule_b);
-				return -ENOMEM;
-			}
-			rule_b->action = rule_a->action;
-			rule_b->syscall = rule_a->syscall;
-			rule_b->args_cnt = rule_a->args_cnt;
-			memcpy(rule_b->args, rule_a->args, args_size);
 			rule_b->prev = rule_a;
 			rule_b->next = NULL;
 			rule_a->next = rule_b;
diff --git a/src/arch-x86.c b/src/arch-x86.c
index 8542079..39c2f09 100644
--- a/src/arch-x86.c
+++ b/src/arch-x86.c
@@ -225,7 +225,6 @@ int x86_rule_add(struct db_filter_col *col, struct db_filter *db, bool strict,
 {
 	int rc;
 	unsigned int iter;
-	size_t args_size;
 	int sys = rule->syscall;
 	int sys_a, sys_b;
 	struct db_api_rule_list *rule_a, *rule_b;
@@ -235,7 +234,7 @@ int x86_rule_add(struct db_filter_col *col, struct db_filter *db, bool strict,
 		   (359 to 373)   : direct socket syscalls, Linux 4.3+ */
 
 		/* strict check for the multiplexed socket syscalls */
-		for (iter = 0; iter < rule->args_cnt; iter++) {
+		for (iter = 0; iter < ARG_COUNT_MAX; iter++) {
 			if ((rule->args[iter].valid != 0) && (strict))
 				return -EINVAL;
 		}
@@ -265,19 +264,9 @@ int x86_rule_add(struct db_filter_col *col, struct db_filter *db, bool strict,
 		} else {
 			/* need two rules, dup the first and link together */
 			rule_a = rule;
-			rule_b = malloc(sizeof(*rule_b));
+			rule_b = db_rule_dup(rule_a);
 			if (rule_b == NULL)
 				return -ENOMEM;
-			args_size = sizeof(*rule_b->args) * rule_a->args_cnt;
-			rule_b->args = malloc(args_size);
-			if (rule_b->args == NULL) {
-				free(rule_b);
-				return -ENOMEM;
-			}
-			rule_b->action = rule_a->action;
-			rule_b->syscall = rule_a->syscall;
-			rule_b->args_cnt = rule_a->args_cnt;
-			memcpy(rule_b->args, rule_a->args, args_size);
 			rule_b->prev = rule_a;
 			rule_b->next = NULL;
 			rule_a->next = rule_b;
diff --git a/src/arch.c b/src/arch.c
index f5a898d..0332fa1 100644
--- a/src/arch.c
+++ b/src/arch.c
@@ -46,8 +46,6 @@
 #include "db.h"
 #include "system.h"
 
-#define default_arg_count_max		6
-
 #define default_arg_offset(x)		(offsetof(struct seccomp_data, args[x]))
 
 #if __i386__
@@ -213,19 +211,6 @@ const struct arch_def *arch_def_lookup_name(const char *arch_name)
 }
 
 /**
- * Determine the maximum number of syscall arguments
- * @param arch the architecture definition
- *
- * Determine the maximum number of syscall arguments for the given architecture.
- * Returns the number of arguments on success, negative values on failure.
- *
- */
-int arch_arg_count_max(const struct arch_def *arch)
-{
-	return (arch_valid(arch->token) == 0 ? default_arg_count_max : -EDOM);
-}
-
-/**
  * Determine the argument offset for the lower 32 bits
  * @param arch the architecture definition
  * @param arg the argument number
@@ -414,10 +399,9 @@ int arch_syscall_rewrite(const struct arch_def *arch, int *syscall)
  */
 int arch_filter_rule_add(struct db_filter_col *col, struct db_filter *db,
 			 bool strict, uint32_t action, int syscall,
-			 unsigned int chain_len, struct db_api_arg *chain)
+			 struct db_api_arg *chain)
 {
 	int rc;
-	size_t chain_size = sizeof(*chain) * chain_len;
 	struct db_api_rule_list *rule, *rule_tail;
 
 	/* ensure we aren't using any reserved syscall values */
@@ -433,15 +417,9 @@ int arch_filter_rule_add(struct db_filter_col *col, struct db_filter *db,
 	rule = malloc(sizeof(*rule));
 	if (rule == NULL)
 		return -ENOMEM;
-	rule->args = malloc(chain_size);
-	if (rule->args == NULL) {
-		free(rule);
-		return -ENOMEM;
-	}
 	rule->action = action;
 	rule->syscall = syscall;
-	rule->args_cnt = chain_len;
-	memcpy(rule->args, chain, chain_size);
+	memcpy(rule->args, chain, sizeof(*chain) * ARG_COUNT_MAX);
 	rule->prev = NULL;
 	rule->next = NULL;
 
@@ -479,7 +457,6 @@ rule_add_failure:
 	do {
 		rule_tail = rule;
 		rule = rule->next;
-		free(rule_tail->args);
 		free(rule_tail);
 	} while (rule);
 	return rc;
diff --git a/src/arch.h b/src/arch.h
index 8077e8b..63f0d7f 100644
--- a/src/arch.h
+++ b/src/arch.h
@@ -77,8 +77,6 @@ int arch_valid(uint32_t arch);
 const struct arch_def *arch_def_lookup(uint32_t token);
 const struct arch_def *arch_def_lookup_name(const char *arch_name);
 
-int arch_arg_count_max(const struct arch_def *arch);
-
 int arch_arg_offset_lo(const struct arch_def *arch, unsigned int arg);
 int arch_arg_offset_hi(const struct arch_def *arch, unsigned int arg);
 int arch_arg_offset(const struct arch_def *arch, unsigned int arg);
@@ -91,6 +89,6 @@ int arch_syscall_rewrite(const struct arch_def *arch, int *syscall);
 
 int arch_filter_rule_add(struct db_filter_col *col, struct db_filter *db,
 			 bool strict, uint32_t action, int syscall,
-			 unsigned int chain_len, struct db_api_arg *chain);
+			 struct db_api_arg *chain);
 
 #endif
diff --git a/src/db.c b/src/db.c
index 1a85b1f..37f013c 100644
--- a/src/db.c
+++ b/src/db.c
@@ -381,7 +381,6 @@ static void _db_reset(struct db_filter *db)
 		r_iter = db->rules;
 		while (r_iter != NULL) {
 			db->rules = r_iter->next;
-			free(r_iter->args);
 			free(r_iter);
 			r_iter = db->rules;
 		}
@@ -513,6 +512,29 @@ static int _db_syscall_priority(struct db_filter *db,
 }
 
 /**
+ * Duplicate an existing filter rule
+ * @param src the rule to duplicate
+ *
+ * This function makes an exact copy of the given rule, but does not add it
+ * to any lists.  Returns a pointer to the new rule on success, NULL on
+ * failure.
+ *
+ */
+struct db_api_rule_list *db_rule_dup(const struct db_api_rule_list *src)
+{
+	struct db_api_rule_list *dest;
+
+	dest = malloc(sizeof(*dest));
+	if (dest == NULL)
+		return NULL;
+	memcpy(dest, src, sizeof(*dest));
+	dest->prev = NULL;
+	dest->next = NULL;
+
+	return dest;
+}
+
+/**
  * Free and reset the seccomp filter collection
  * @param col the seccomp filter collection
  * @param def_action the default filter action
@@ -989,10 +1011,9 @@ static void _db_node_mask_fixup(struct db_arg_chain_tree *node)
 static struct db_sys_list *_db_rule_gen_64(const struct arch_def *arch,
 					   uint32_t action,
 					   unsigned int syscall,
-					   struct db_api_arg *chain)
+					   const struct db_api_arg *chain)
 {
 	unsigned int iter;
-	int chain_len_max;
 	struct db_sys_list *s_new;
 	struct db_arg_chain_tree *c_iter_hi = NULL, *c_iter_lo = NULL;
 	struct db_arg_chain_tree *c_prev_hi = NULL, *c_prev_lo = NULL;
@@ -1005,10 +1026,7 @@ static struct db_sys_list *_db_rule_gen_64(const struct arch_def *arch,
 	s_new->num = syscall;
 	s_new->valid = true;
 	/* run through the argument chain */
-	chain_len_max = arch_arg_count_max(arch);
-	if (chain_len_max < 0)
-		goto gen_64_failure;
-	for (iter = 0; iter < chain_len_max; iter++) {
+	for (iter = 0; iter < ARG_COUNT_MAX; iter++) {
 		if (chain[iter].valid == 0)
 			continue;
 
@@ -1129,10 +1147,9 @@ gen_64_failure:
 static struct db_sys_list *_db_rule_gen_32(const struct arch_def *arch,
 					   uint32_t action,
 					   unsigned int syscall,
-					   struct db_api_arg *chain)
+					   const struct db_api_arg *chain)
 {
 	unsigned int iter;
-	int chain_len_max;
 	struct db_sys_list *s_new;
 	struct db_arg_chain_tree *c_iter = NULL, *c_prev = NULL;
 	bool tf_flag;
@@ -1144,10 +1161,7 @@ static struct db_sys_list *_db_rule_gen_32(const struct arch_def *arch,
 	s_new->num = syscall;
 	s_new->valid = true;
 	/* run through the argument chain */
-	chain_len_max = arch_arg_count_max(arch);
-	if (chain_len_max < 0)
-		goto gen_32_failure;
-	for (iter = 0; iter < chain_len_max; iter++) {
+	for (iter = 0; iter < ARG_COUNT_MAX; iter++) {
 		if (chain[iter].valid == 0)
 			continue;
 
@@ -1238,7 +1252,7 @@ int db_rule_add(struct db_filter *db, const struct db_api_rule_list *rule)
 	int rc = -ENOMEM;
 	int syscall = rule->syscall;
 	uint32_t action = rule->action;
-	struct db_api_arg *chain = rule->args;
+	const struct db_api_arg *chain = rule->args;
 	struct db_sys_list *s_new, *s_iter, *s_prev = NULL;
 	struct db_arg_chain_tree *c_iter = NULL, *c_prev = NULL;
 	struct db_arg_chain_tree *ec_iter;
@@ -1568,15 +1582,13 @@ int db_col_rule_add(struct db_filter_col *col,
 {
 	int rc = 0, rc_tmp;
 	unsigned int iter;
-	unsigned int chain_len;
 	unsigned int arg_num;
 	size_t chain_size;
 	struct db_api_arg *chain = NULL;
 	struct scmp_arg_cmp arg_data;
 
 	/* collect the arguments for the filter rule */
-	chain_len = ARG_COUNT_MAX;
-	chain_size = sizeof(*chain) * chain_len;
+	chain_size = sizeof(*chain) * ARG_COUNT_MAX;
 	chain = malloc(chain_size);
 	if (chain == NULL)
 		return -ENOMEM;
@@ -1584,7 +1596,7 @@ int db_col_rule_add(struct db_filter_col *col,
 	for (iter = 0; iter < arg_cnt; iter++) {
 		arg_data = arg_array[iter];
 		arg_num = arg_data.arg;
-		if (arg_num < chain_len && chain[arg_num].valid == 0) {
+		if (arg_num < ARG_COUNT_MAX && chain[arg_num].valid == 0) {
 			chain[arg_num].valid = 1;
 			chain[arg_num].arg = arg_num;
 			chain[arg_num].op = arg_data.op;
@@ -1616,8 +1628,7 @@ int db_col_rule_add(struct db_filter_col *col,
 
 	for (iter = 0; iter < col->filter_cnt; iter++) {
 		rc_tmp = arch_filter_rule_add(col, col->filters[iter], strict,
-					      action, syscall,
-					      chain_len, chain);
+					      action, syscall, chain);
 		if (rc == 0 && rc_tmp < 0)
 			rc = rc_tmp;
 	}
@@ -1639,7 +1650,6 @@ add_return:
 int db_col_transaction_start(struct db_filter_col *col)
 {
 	unsigned int iter;
-	size_t args_size;
 	struct db_filter_snap *snap;
 	struct db_filter *filter_o, *filter_s;
 	struct db_api_rule_list *rule_o, *rule_s;
@@ -1673,19 +1683,9 @@ int db_col_transaction_start(struct db_filter_col *col)
 			continue;
 		do {
 			/* copy the rule */
-			rule_s = malloc(sizeof(*rule_s));
+			rule_s = db_rule_dup(rule_o);
 			if (rule_s == NULL)
 				goto trans_start_failure;
-			args_size = sizeof(*rule_s->args) * rule_o->args_cnt;
-			rule_s->args = malloc(args_size);
-			if (rule_s->args == NULL) {
-				free(rule_s);
-				goto trans_start_failure;
-			}
-			rule_s->action = rule_o->action;
-			rule_s->syscall = rule_o->syscall;
-			rule_s->args_cnt = rule_o->args_cnt;
-			memcpy(rule_s->args, rule_o->args, args_size);
 			if (filter_s->rules != NULL) {
 				rule_s->prev = filter_s->rules->prev;
 				rule_s->next = filter_s->rules;
diff --git a/src/db.h b/src/db.h
index 1f67b5d..687e777 100644
--- a/src/db.h
+++ b/src/db.h
@@ -43,8 +43,7 @@ struct db_api_arg {
 struct db_api_rule_list {
 	uint32_t action;
 	int syscall;
-	struct db_api_arg *args;
-	unsigned int args_cnt;
+	struct db_api_arg args[ARG_COUNT_MAX];
 
 	struct db_api_rule_list *prev, *next;
 };
@@ -189,6 +188,8 @@ struct db_filter_col {
 
 int db_action_valid(uint32_t action);
 
+struct db_api_rule_list *db_rule_dup(const struct db_api_rule_list *src);
+
 struct db_filter_col *db_col_init(uint32_t def_action);
 int db_col_reset(struct db_filter_col *col, uint32_t def_action);
 void db_col_release(struct db_filter_col *col);