From 3387938185fed2c36fb53c93afe3cdf7b2977ef0 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 17 Jan 2018 16:14:44 +0300 Subject: [PATCH] check_atomic_inc_dec: track atomic_inc() and atomic_dec() I had some ideas that I would find leaked atomic_inc() overflows with this where we increment a counter to show that something is in use but forget to decrement it on some error paths. That project turned out harder to implement than I had hoped. But I still want to commit this because I use the information for other checks. Signed-off-by: Dan Carpenter --- check_atomic_inc_dec.c | 246 +++++++++++++++++++++++++++++++++++++++++ smatch.h | 1 + smatch_data/db/fixup_kernel.sh | 13 +++ 3 files changed, 260 insertions(+) create mode 100644 check_atomic_inc_dec.c diff --git a/check_atomic_inc_dec.c b/check_atomic_inc_dec.c new file mode 100644 index 00000000..cd3fe5ed --- /dev/null +++ b/check_atomic_inc_dec.c @@ -0,0 +1,246 @@ +/* + * Copyright (C) 2016 Oracle. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt + */ + +#include "smatch.h" +#include "smatch_extra.h" +#include "smatch_slist.h" + +static int my_id; + +STATE(inc); +STATE(orig); +STATE(dec); + +static void db_inc_dec(struct expression *expr, int param, const char *key, const char *value, int inc_dec) +{ + struct smatch_state *start_state; + struct expression *arg; + char *name; + struct symbol *sym; + + while (expr->type == EXPR_ASSIGNMENT) + expr = strip_expr(expr->right); + if (expr->type != EXPR_CALL) + return; + + arg = get_argument_from_call_expr(expr->args, param); + if (!arg) + return; + + name = get_variable_from_key(arg, key, &sym); + if (!name || !sym) + goto free; + + start_state = get_state(my_id, name, sym); + + if (inc_dec == ATOMIC_INC) { +// if (start_state == &inc) +// sm_msg("error: XXX double increment '%s'", name); + set_state(my_id, name, sym, &inc); + } else { +// if (start_state == &dec) +// sm_msg("error: XXX double decrement '%s'", name); + if (start_state == &inc) + set_state(my_id, name, sym, &orig); + else + set_state(my_id, name, sym, &dec); + } + +free: + free_string(name); +} + +static void db_inc(struct expression *expr, int param, char *key, char *value) +{ + db_inc_dec(expr, param, key, value, ATOMIC_INC); +} + +static void db_dec(struct expression *expr, int param, char *key, char *value) +{ + db_inc_dec(expr, param, key, value, ATOMIC_DEC); +} + +static void match_atomic_inc(const char *fn, struct expression *expr, void *_unused) +{ + db_inc_dec(expr, 0, "$->counter", "", ATOMIC_INC); +} + +static void match_atomic_dec(const char *fn, struct expression *expr, void *_unused) +{ + db_inc_dec(expr, 0, "$->counter", "", ATOMIC_DEC); +} + +static void match_atomic_add(const char *fn, struct expression *expr, void *_unused) +{ + struct expression *amount; + sval_t sval; + + amount = get_argument_from_call_expr(expr->args, 0); + if (get_implied_value(amount, &sval) && sval_is_negative(sval)) { + db_inc_dec(expr, 1, "$->counter", "", ATOMIC_DEC); + return; + } + + db_inc_dec(expr, 1, "$->counter", "", ATOMIC_INC); +} + +static void match_atomic_sub(const char *fn, struct expression *expr, void *_unused) +{ + db_inc_dec(expr, 1, "$->counter", "", ATOMIC_DEC); +} + +static void refcount_inc(const char *fn, struct expression *expr, void *param) +{ + db_inc_dec(expr, PTR_INT(param), "$->ref.counter", "", ATOMIC_INC); +} + +static void refcount_dec(const char *fn, struct expression *expr, void *param) +{ + db_inc_dec(expr, PTR_INT(param), "$->ref.counter", "", ATOMIC_DEC); +} + +static void match_return_info(int return_id, char *return_ranges, struct expression *expr) +{ + struct sm_state *sm; + const char *param_name; + int param; + + FOR_EACH_MY_SM(my_id, __get_cur_stree(), sm) { + if (sm->state != &inc && + sm->state != &dec) + continue; + param = get_param_num_from_sym(sm->sym); + if (param < 0) + continue; + param_name = get_param_name(sm); + if (!param_name) + continue; + sql_insert_return_states(return_id, return_ranges, + (sm->state == &inc) ? ATOMIC_INC : ATOMIC_DEC, + param, param_name, ""); + } END_FOR_EACH_SM(sm); +} + +enum { + NEGATIVE, ZERO, POSITIVE, +}; + +static int success_fail_positive(struct range_list *rl) +{ + if (!rl) + return ZERO; + + if (sval_is_negative(rl_min(rl))) + return NEGATIVE; + + if (rl_min(rl).value == 0) + return ZERO; + + return POSITIVE; +} + +static void check_counter(const char *name, struct symbol *sym) +{ + struct range_list *inc_lines = NULL; + struct range_list *dec_lines = NULL; + int inc_buckets[3] = {}; + struct stree *stree; + struct sm_state *return_sm; + struct sm_state *sm; + sval_t line = sval_type_val(&int_ctype, 0); + + FOR_EACH_PTR(get_all_return_strees(), stree) { + return_sm = get_sm_state_stree(stree, RETURN_ID, "return_ranges", NULL); + if (!return_sm) + continue; + line.value = return_sm->line; + + sm = get_sm_state_stree(stree, my_id, name, sym); + if (!sm) + continue; + + if (sm->state != &inc && sm->state != &dec) + continue; + + if (sm->state == &inc) { + add_range(&inc_lines, line, line); + inc_buckets[success_fail_positive(estate_rl(return_sm->state))] = 1; + } + if (sm->state == &dec) + add_range(&dec_lines, line, line); + } END_FOR_EACH_PTR(stree); + + if (inc_buckets[NEGATIVE] && + inc_buckets[ZERO]) { + // sm_msg("warn: XXX '%s' not decremented on lines: %s.", name, show_rl(inc_lines)); + } + +} + +static void match_check_missed(struct symbol *sym) +{ + struct sm_state *sm; + + FOR_EACH_MY_SM(my_id, get_all_return_states(), sm) { + check_counter(sm->name, sm->sym); + } END_FOR_EACH_SM(sm); +} + +int on_atomic_dec_path(void) +{ + struct sm_state *sm; + + FOR_EACH_MY_SM(my_id, __get_cur_stree(), sm) { + if (sm->state == &dec) + return 1; + } END_FOR_EACH_SM(sm); + + return 0; +} + +int was_inced(const char *name, struct symbol *sym) +{ + return get_state(my_id, name, sym) == &inc; +} + +void check_atomic_inc_dec(int id) +{ + my_id = id; + + if (option_project != PROJ_KERNEL) + return; + + select_return_states_hook(ATOMIC_INC, &db_inc); + select_return_states_hook(ATOMIC_DEC, &db_dec); + add_function_hook("atomic_inc_return", &match_atomic_inc, NULL); + add_function_hook("atomic_add_return", &match_atomic_add, NULL); + add_function_hook("atomic_sub_return", &match_atomic_sub, NULL); + add_function_hook("atomic_sub_and_test", &match_atomic_sub, NULL); + add_function_hook("atomic_dec_and_test", &match_atomic_dec, NULL); + add_function_hook("atomic_dec", &match_atomic_dec, NULL); + add_function_hook("atomic_long_inc", &match_atomic_inc, NULL); + add_function_hook("atomic_long_dec", &match_atomic_dec, NULL); + add_function_hook("atomic_inc", &match_atomic_inc, NULL); + add_function_hook("atomic_sub", &match_atomic_sub, NULL); + add_split_return_callback(match_return_info); + + add_function_hook("refcount_add_not_zero", &refcount_inc, INT_PTR(1)); + add_function_hook("refcount_inc_not_zero", &refcount_inc, INT_PTR(0)); + add_function_hook("refcount_sub_and_test", &refcount_dec, INT_PTR(1)); + + add_hook(&match_check_missed, END_FUNC_HOOK); +} diff --git a/smatch.h b/smatch.h index 96559db8..c75afb9a 100644 --- a/smatch.h +++ b/smatch.h @@ -1026,6 +1026,7 @@ int db_var_is_array_limit(struct expression *array, const char *name, struct var struct stree *get_all_return_states(void); struct stree_stack *get_all_return_strees(void); +int on_atomic_dec_path(void); int was_inced(const char *name, struct symbol *sym); /* smatch_constraints.c */ diff --git a/smatch_data/db/fixup_kernel.sh b/smatch_data/db/fixup_kernel.sh index 553362a8..6c5f3edb 100755 --- a/smatch_data/db/fixup_kernel.sh +++ b/smatch_data/db/fixup_kernel.sh @@ -156,6 +156,19 @@ delete from return_states where function = "__write_once_size"; update return_states set value = "s32min-s32max[\$1]" where function = 'atomic_set' and parameter = 0 and type = 1025; +/* handled in the check itself */ +delete from return_states where function = 'atomic_inc_return' and (type = 8023 or type = 8024); +delete from return_states where function = 'atomic_add_return' and (type = 8023 or type = 8024); +delete from return_states where function = 'atomic_sub_return' and (type = 8023 or type = 8024); +delete from return_states where function = 'atomic_sub_and_test' and (type = 8023 or type = 8024); +delete from return_states where function = 'atomic_dec_and_test' and (type = 8023 or type = 8024); +delete from return_states where function = 'atomic_dec' and (type = 8023 or type = 8024); +delete from return_states where function = 'atomic_inc' and (type = 8023 or type = 8024); +delete from return_states where function = 'atomic_sub' and (type = 8023 or type = 8024); +delete from return_states where function = 'refcount_add_not_zero' and (type = 8023 or type = 8024); +delete from return_states where function = 'refcount_inc_not_zero' and (type = 8023 or type = 8024); +delete from return_states where function = 'refcount_sub_and_test' and (type = 8023 or type = 8024); + update return_states set return = '0-32,2147483648-2147483690' where function = '_parse_integer' and return = '0'; update return_states set value = '0-u64max' where function = '_parse_integer' and type = 1025 and parameter = 2 and key = '*$'; -- 2.11.4.GIT