Messages in this thread | | | Date | Tue, 8 Dec 2020 16:28:44 +0300 | From | Dan Carpenter <> | Subject | Re: sparse annotation for error types? |
| |
Hi Zhang,
Are you using Coccinelle to detect these bugs?
On Sat, Dec 05, 2020 at 02:32:50PM -0800, Jakub Kicinski wrote: > Hi! > > Recently we've been getting a steady stream of patches from Changzhong > to fix missing assignment to error variables before jumping to error > cases.
I've mucked about with this a little in Smatch trying to work out some heuristics to use. I added a warning for a NULL return followed by a goto. Then on Friday I added a warning for a _dev_err() print followed by a goto. But neither of those rules catches the bug fixed by commit 4de377b65903 ("net: marvell: prestera: Fix error return code in prestera_port_create()"), where the error was invalid data.
if (idx >= size) goto free_whatever;
I'm going to print a warning if the function ends in a cleanup block that can only be reached by gotos. We'll see how that works tomorrow.
static void match_return(struct statement *stmt) { struct sm_state *sm, *tmp; sval_t sval; char *name; bool is_last;
// Only complain if the function returns a variable if (!stmt->ret_value || stmt->ret_value->type != EXPR_SYMBOL) return;
// The function returns an int if (cur_func_return_type() != &int_ctype) return;
// It's only reachable via a goto if (get_state(my_id, "path", NULL) != &label) return;
// It returns a negative error code sm = get_extra_sm_state(stmt->ret_value); if (!sm || !estate_rl(sm->state) || !sval_is_negative(rl_min(estate_rl(sm->state)))) return;
FOR_EACH_PTR(sm->possible, tmp) { // There is at least one path where "ret" is zero if (estate_get_single_value(tmp->state, &sval) && sval.value == 0) goto warn; } END_FOR_EACH_PTR(tmp);
return; warn: // It's the last statement of a function is_last = is_last_stmt(stmt);
name = expr_to_str(stmt->ret_value); sm_warning("missing error code '%s' rl='%s' is_last=%d", name, sm->state->name, is_last); free_string(name); }
regards, dan carpenter
| |