From 3f39ea88e592dba24d68db4abdf65b37eca2cb33 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Thu, 11 May 2017 12:14:57 +0300 Subject: [PATCH] return_to_param: don't modify memory on the stack The param_set/param_add code never tries to modify stack memory but when we use the param_long_to_short mapping then it can happen. What that looks like is this: ptr = foo->bar; frob(foo); The frob() function sets "foo->bar" and in the original code that would change "ptr" as well, but "ptr" is stored on the stack and should stay the same. For PARAM_LIMIT and PARAM_FILTER then we do want to update "ptr". That looks like this: ptr = foo->bar; if (frob(foo)) { ... Imagine that frob() only returns true if foo->bar is non-NULL then we know that ptr is non-NULL also. Signed-off-by: Dan Carpenter --- smatch.h | 1 + smatch_extra.c | 2 +- smatch_return_to_param.c | 25 +++++++++++++++++++------ 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/smatch.h b/smatch.h index 633af41c..0febeb4b 100644 --- a/smatch.h +++ b/smatch.h @@ -540,6 +540,7 @@ static const sval_t vmalloc_seg_max = { char *get_other_name_sym(const char *name, struct symbol *sym, struct symbol **new_sym); char *map_call_to_other_name_sym(const char *name, struct symbol *sym, struct symbol **new_sym); char *map_long_to_short_name_sym(const char *name, struct symbol *sym, struct symbol **new_sym); +char *map_long_to_short_name_sym_nostack(const char *name, struct symbol *sym, struct symbol **new_sym); #define STRLEN_MAX_RET 1010101 diff --git a/smatch_extra.c b/smatch_extra.c index 977a5a2e..9b566122 100644 --- a/smatch_extra.c +++ b/smatch_extra.c @@ -2199,7 +2199,7 @@ static void db_param_add_set(struct expression *expr, int param, char *key, char else new = rl_union(new, added); - tmp_name = map_long_to_short_name_sym(name, sym, &tmp_sym); + tmp_name = map_long_to_short_name_sym_nostack(name, sym, &tmp_sym); if (tmp_name && tmp_sym) { free_string(name); name = tmp_name; diff --git a/smatch_return_to_param.c b/smatch_return_to_param.c index f39ff94b..5a6b9748 100644 --- a/smatch_return_to_param.c +++ b/smatch_return_to_param.c @@ -65,7 +65,7 @@ char *map_call_to_other_name_sym(const char *name, struct symbol *sym, struct sy return alloc_string(buf); } -static char *map_my_state_long_to_short(struct sm_state *sm, const char *name, struct symbol *sym, struct symbol **new_sym) +static char *map_my_state_long_to_short(struct sm_state *sm, const char *name, struct symbol *sym, struct symbol **new_sym, bool stack) { int len; char buf[256]; @@ -78,12 +78,14 @@ static char *map_my_state_long_to_short(struct sm_state *sm, const char *name, s if (name[len] == '.') return NULL; + if (!stack && name[len] != '-') + return NULL; snprintf(buf, sizeof(buf), "%s%s", sm->name, name + len); *new_sym = sm->sym; return alloc_string(buf); } -static char *map_assignment_long_to_short(struct sm_state *sm, const char *name, struct symbol *sym, struct symbol **new_sym) +static char *map_assignment_long_to_short(struct sm_state *sm, const char *name, struct symbol *sym, struct symbol **new_sym, bool stack) { struct symbol *orig_sym; int len; @@ -104,6 +106,8 @@ static char *map_assignment_long_to_short(struct sm_state *sm, const char *name, if (name[len] == '.') return NULL; + if (!stack && name[len] != '-') + return NULL; snprintf(buf, sizeof(buf), "%s%s", sm->name, name + len); *new_sym = sm->sym; return alloc_string(buf); @@ -118,7 +122,7 @@ static char *map_assignment_long_to_short(struct sm_state *sm, const char *name, * which in turn updates the longer name. * */ -char *map_long_to_short_name_sym(const char *name, struct symbol *sym, struct symbol **new_sym) +static char *map_long_to_short_name_sym_helper(const char *name, struct symbol *sym, struct symbol **new_sym, bool stack) { char *ret; struct sm_state *sm; @@ -127,23 +131,32 @@ char *map_long_to_short_name_sym(const char *name, struct symbol *sym, struct sy FOR_EACH_SM(__get_cur_stree(), sm) { if (sm->owner == my_id) { - ret = map_my_state_long_to_short(sm, name, sym, new_sym); + ret = map_my_state_long_to_short(sm, name, sym, new_sym, stack); if (ret) return ret; continue; } if (sm->owner == check_assigned_expr_id) { - ret = map_assignment_long_to_short(sm, name, sym, new_sym); + ret = map_assignment_long_to_short(sm, name, sym, new_sym, stack); if (ret) return ret; continue; } - } END_FOR_EACH_SM(sm); return NULL; } +char *map_long_to_short_name_sym(const char *name, struct symbol *sym, struct symbol **new_sym) +{ + return map_long_to_short_name_sym_helper(name, sym, new_sym, 1); +} + +char *map_long_to_short_name_sym_nostack(const char *name, struct symbol *sym, struct symbol **new_sym) +{ + return map_long_to_short_name_sym_helper(name, sym, new_sym, 0); +} + char *map_call_to_param_name_sym(struct expression *expr, struct symbol **sym) { char *name; -- 2.11.4.GIT