On Tue, Jun 01, 2021 at 11:50:06PM +0300, Dan Carpenter wrote: > On Tue, Jun 01, 2021 at 10:51:23PM +0300, Dan Carpenter wrote: > > The other thing which might be interesting is if you pass a NULL > > to IS_ERR() and then dereference the NULL then print a warning about > > that. This has a lot of overlaps with some of my existing checks, but > > it's still a new idea so it belongs in a separate check. It's fine and > > good even if one bug triggers a lot of different warnings. I'll write > > that, hang on, brb. > > 100% untested. :) I'll test it tonight. Ha, you make it look easy. Let me know if I can help with testing Should I just add below to my smatch and recompile, or is there an experimental branch to build from? > > There are a bunch of ways to write a check like this. This test is > based on copy and paste, guess work, and instinct. I normally just > start writing the simplest check I can and test that, then I refine it > based on whatever the common false postives are. > > In this code, do I need to have a modification hook? Probably not, but > it was in the original code I copy and pasted and it seemed harmless. > Slightly ugly perhaps? > > I knew from experience that I want to check if it's an explicit NULL > pointer passed to IS_ERR(). There are a few ways to write that. I > could have looked at the values or I could have looked at the ->possible > values. I probably should have looked at the values instead... > > The __in_fake_assign assignment is copy and paste. I shoud probably > delete that but it's harmless and a potential speed up. It was in the > check_check_deref.c and I don't remember why. Probably it's essential. > > I'm not happy with the DEREF_HOOK api. I've been planning to re-write > that but I haven't yet. > > regards, > dan carpenter > > /* > * Copyright (C) 2021 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(null); > > static void is_ok(struct sm_state *sm, struct expression *mod_expr) > { > set_state(my_id, sm->name, sm->sym, &undefined); > } > > /* > * The expr_has_null_exact() function means that it was explicitly > * assigned NULL, not just that it is potentially NULL. > */ > static bool expr_has_null_exact(struct expression *expr) > { > struct sm_state *sm, *tmp; > sval_t sval; > > sm = get_sm_state_expr(SMATCH_EXTRA, expr); > if (!sm) > return false; > > FOR_EACH_PTR(sm->possible, tmp) { > if (!estate_get_single_value(tmp->state, &sval)) > continue; > if (sval.value == 0) > return true; > } END_FOR_EACH_PTR(tmp); > > return false; > } > > static void match_is_err(const char *fn, struct expression *expr, void *unused) > { > struct expression *arg; > > arg = get_argument_from_call_expr(expr->args, 0); > if (!expr_has_null_exact(arg)) > return; > set_state_expr(my_id, arg, &null); > } > > static void check_dereference(struct expression *expr) > { > char *name; > > if (__in_fake_assign) > return; > > if (get_state_expr(my_id, expr) != &null) > return; > if (implied_not_equal(expr, 0)) > return; > > name = expr_to_str(expr); > sm_error("potential NULL dereference '%s'", name); > free_string(name); > } > > static void match_dereferences(struct expression *expr) > { > if (expr->type != EXPR_PREOP) > return; > check_dereference(expr->unop); > } > > void check_null_deref_after_IS_ERR(int id) > { > my_id = id; > > if (option_project != PROJ_KERNEL) > return; > > add_function_hook("IS_ERR", &match_is_err, NULL); > add_hook(&match_dereferences, DEREF_HOOK); > > add_modification_hook(my_id, &is_ok); > } > >
