Harshit was trying to debug a check where he wanted to warn about if you
pass an uninitialized list to list_del(). In this situation the ->next
and ->prev pointers are NULL.
Harshit was like "Just solve this and email the list! I don't want any
more excuses!" ;) That's how Harshit rolls. (I asked him if I could
post this example to the list).
It turns out there was an ordering issue where the DEREFERENCE
information from the DB was processed first so by the time his check was
run the pointer was marked as non-NULL. It can't have been NULL if we
dereferenced it and the kernel didn't crash.
I have created a couple new _early() hooks, add_function_hook_early()
and add_function_param_key_hook_early() so that checks can look at
parameters before the return states are processed.
This kind of check is pretty easy to write using the param/key API.
Just register the param/key. The function is list_del, 0 is the param,
and "$->next" is the key.
add_function_param_key_hook_early("list_del", &check_non_null, 0, "$->next", NULL);
Then get the SMATCH_EXTRA state and see if there is a state where the
parameter is set to NULL:
static void check_non_null(struct expression *expr, const char *name, struct symbol *sym, void *data)
{
struct sm_state *sm, *tmp;
sval_t sval;
sm = get_sm_state(SMATCH_EXTRA, name, sym);
if (!sm)
return;
FOR_EACH_PTR(sm->possible, tmp) {
if (!estate_get_single_value(tmp->state, &sval) ||
sval.value != 0)
continue;
sm_warning("%s potentially NULL", name);
return;
} END_FOR_EACH_PTR(tmp);
}
If you wanted, intead of checking for if it has one state which is
specifically NULL, you could check that each state is definitely
non-NULL but that's probably going to get a ton of false positives.
I also wrote a similar check because I saw a bug the other day which
did:
unsigned long bitmap;
if (something)
set_bit(5, &bitmap);
Setting bit 5 is nonsense when the whoule bitmap is uninitialized. It
worked for the person testing the code because most people are
automatically zeroing stack variables these days, but it's still a bug.
Register the param/key. The function is "set_bit", the param is 1, and
"*$" is the key.
add_function_param_key_hook_early("set_bit", &check_initialized, 1, "*$", NULL);
Then use the data from check_uninitialized.c and complain if there is
no way that &bitmap has been initialized.
static void check_initialized(struct expression *expr, const char *name, struct symbol *sym, void *data)
{
static int uninitialized_id = -1;
struct smatch_state *state;
if (uninitialized_id == -1)
uninitialized_id = id_from_name("check_uninitialized");
state = get_state(uninitialized_id, name, sym);
if (!state)
return;
if (strcmp(state->name, "uninitialized") != 0)
return;
sm_msg("passing uninitialized variable '%s'", name);
}
In this case, maybe it should look for if it's possibly uninitialized
instead of definitely uninitialized. Something to try perhaps.
I've attached both checks. Neither one is tested, but presumably
Harshit is planning to review the potential NULL dereference bugs.
If everyone could just keep an eye out for other NULL dereferences which
have caused us problems in the past we can add those param/keys to the
list as well.
regards,
dan carpenter
/*
* Copyright (C) 2023 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_slist.h"
static int my_id;
static void check_initialized(struct expression *expr, const char *name, struct symbol *sym, void *data)
{
static int uninitialized_id = -1;
struct smatch_state *state;
if (uninitialized_id == -1)
uninitialized_id = id_from_name("check_uninitialized");
state = get_state(uninitialized_id, name, sym);
if (!state)
return;
if (strcmp(state->name, "uninitialized") != 0)
return;
sm_msg("passing uninitialized variable '%s'", name);
}
void check_passing_uninitialized(int id)
{
my_id = id;
add_function_param_key_hook_early("set_bit", &check_initialized, 1, "*$", NULL);
add_function_param_key_hook_early("__set_bit", &check_initialized, 1, "*$", NULL);
add_function_param_key_hook_early("set_bit_le", &check_initialized, 1, "*$", NULL);
add_function_param_key_hook_early("__set_bit_le", &check_initialized, 1, "*$", NULL);
add_function_param_key_hook_early("test_and_set_bit_le", &check_initialized, 1, "*$", NULL);
add_function_param_key_hook_early("__test_and_set_bit_le", &check_initialized, 1, "*$", NULL);
add_function_param_key_hook_early("clear_bit", &check_initialized, 1, "*$", NULL);
add_function_param_key_hook_early("clear_bit_le", &check_initialized, 1, "*$", NULL);
add_function_param_key_hook_early("__clear_bit", &check_initialized, 1, "*$", NULL);
add_function_param_key_hook_early("__clear_bit_le", &check_initialized, 1, "*$", NULL);
add_function_param_key_hook_early("__test_and_clear_bit_le", &check_initialized, 1, "*$", NULL);
}
/*
* Copyright (C) 2023 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_slist.h"
static int my_id;
static void check_non_null(struct expression *expr, const char *name, struct symbol *sym, void *data)
{
struct sm_state *sm, *tmp;
sval_t sval;
sm = get_sm_state(SMATCH_EXTRA, name, sym);
if (!sm)
return;
FOR_EACH_PTR(sm->possible, tmp) {
if (!estate_get_single_value(tmp->state, &sval) ||
sval.value != 0)
continue;
sm_warning("%s potentially NULL", name);
return;
} END_FOR_EACH_PTR(tmp);
}
void check_passing_possible_null(int id)
{
my_id = id;
add_function_param_key_hook_early("list_del", &check_non_null, 0, "$->next", NULL);
}
