From e3addce3794ddb6dc174d429da055296282df0e6 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 17 Feb 2017 12:15:20 -0500 Subject: 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 --- src/arch-s390.c | 15 ++----------- src/arch-s390x.c | 15 ++----------- src/arch-x86.c | 15 ++----------- src/arch.c | 27 ++---------------------- src/arch.h | 4 +--- src/db.c | 64 ++++++++++++++++++++++++++++---------------------------- src/db.h | 5 +++-- 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__ @@ -212,19 +210,6 @@ const struct arch_def *arch_def_lookup_name(const char *arch_name) return NULL; } -/** - * 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 @@ -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; } @@ -512,6 +511,29 @@ static int _db_syscall_priority(struct db_filter *db, return 0; } +/** + * 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 @@ -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); -- cgit v1.2.1