From ee12c54ac0b358f8817edbbea560cb80d30ca320 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 14 Mar 2018 16:20:08 +0300 Subject: [PATCH] function_hooks: don't be too ambitious faking parameter assignments The story here is you have code like kfifo_out_locked() where it takes the whole macro and passes it through the __kfifo_uint_must_check_helper() function just to get the __must_check annotation. So it returns $0. We should parse this correctly, really, but we don't. First of all in that case e should be faking that "len = __ret;" but we instead fake the *whole* huge expression statement. I just thought of that now, writing this changelog. But also there is something wrong with how we parse it and it ends up causing locking false positives because we end up parsing the lock and unlock twice. I don't know exactly why this is... I am a bad person for not investigating fully. Perhaps it has to do with nesting levels and when we give up. And, finally, there is another issue with parsing code twice which has side effects so we really shouldn't be doing it. I looked at changing how fake assignments are handled in smatch_flow but it turned out to be a bit subtle so I gave up. The bottom line is, it easiest to only try faking it for straight forward arguments and not very involved ones like expression statements. Also we should only be faking vanilla assignments, not += and -= etc. Signed-off-by: Dan Carpenter --- smatch_function_hooks.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/smatch_function_hooks.c b/smatch_function_hooks.c index 473113d7..766012da 100644 --- a/smatch_function_hooks.c +++ b/smatch_function_hooks.c @@ -760,8 +760,9 @@ static void fake_a_param_assignment(struct expression *expr, const char *return_ char *p; int param; char buf[256]; + char *str; - if (expr->type != EXPR_ASSIGNMENT) + if (expr->type != EXPR_ASSIGNMENT || expr->op != '=') return; left = expr->left; right = expr->right; @@ -795,6 +796,15 @@ static void fake_a_param_assignment(struct expression *expr, const char *return_ arg = get_argument_from_call_expr(right->args, param); if (!arg) return; + /* + * This is a sanity check to prevent side effects from evaluating stuff + * twice. + */ + str = expr_to_chunk_sym_vsl(arg, NULL, NULL); + if (!str) + return; + free_string(str); + right = gen_expression_from_key(arg, buf); if (!right) /* Mostly fails for binops like [$0 + 4032] */ return; -- 2.11.4.GIT