From 427a05c53ddd678fd6e9a137fa4173d0a7807335 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Thu, 25 Jan 2018 17:34:44 +0300 Subject: [PATCH] user_data2: do a hack around in the pre_merge_hook() I've been running with this code for a while... This change has to do with the interactions between smatch_capped.c and check_user_data3.c. Smatch capped records that we capped a variable against a trusted but unknown value. The problem is you have code like this: if (user_var > unknown_trusted_variable) user_var = unknown_trusted_variable; On the true side, then we set user_data to the empty state because the user_var gets overwritten. On the false side, we record that user_var is in the 0-u32max user controlled range, but capped. Then when we merge them together, we say that it's not capped and 0-32max which is bad and useless. Signed-off-by: Dan Carpenter --- check_user_data2.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/check_user_data2.c b/check_user_data2.c index b8b795f1..fbb9b585 100644 --- a/check_user_data2.c +++ b/check_user_data2.c @@ -80,13 +80,39 @@ static void pre_merge_hook(struct sm_state *sm) struct smatch_state *extra; struct range_list *rl; sval_t dummy; + sval_t sval_100 = { + .type = &int_ctype, + .value = 100, + }; + user = get_state(my_id, sm->name, sm->sym); + if (!user) + return; + if (!estate_rl(sm->state)) { + /* + * If the one side is capped and the other side is empty then + * let's just mark it as not-user data because the information + * isn't going to be useful. How this looks is: + * + * if (user_var > trusted) + * user_var = trusted; <-- empty state + * else + * <-- capped + * + * The problem is that sometimes things are capped to a literal + * and we'd like to keep the state in that case... Ugh. I've + * added a check which assumes that everything less than 100 is + * probably capped against a literal. + * + */ + if (is_capped_var_sym(sm->name, sm->sym) && + sval_cmp(estate_max(user), sval_100) > 0) + set_state(my_id, sm->name, sm->sym, alloc_estate_empty()); + return; + } extra = get_state(SMATCH_EXTRA, sm->name, sm->sym); if (!extra || !estate_rl(extra)) return; - user = get_state(my_id, sm->name, sm->sym); - if (!user || !estate_rl(user)) - return; rl = rl_intersection(estate_rl(user), estate_rl(extra)); if (rl_to_sval(rl, &dummy)) rl = NULL; -- 2.11.4.GIT