Re: [PATCH] mm: hugetlb: checking for IS_ERR() instead of NULL

[Dan Carpenter <dan.carpenter@oracle.com>]

On Tue, Jun 01, 2021 at 01:54:11PM -0400, Nigel Christian wrote:
> On Tue, Jun 01, 2021 at 12:25:40PM +0300, Dan Carpenter wrote:
> > The alloc_migrate_huge_page() doesn't return error pointers, it returns
> > NULL.
> 
> Hi Dan,
> 
> I'm trying to start using smatch. Is this in the warns report?
> Wasn't able to find using smatch_scripts/kchecker mm/hugetlb.c (T_T)
> 

No, it's not.  I was never able to make the check work well enough to
publish.  To many false positives, because some functions return error
pointers depending on the .config.  Also I should have modified it to
ignore paramaters and only look at returned pointers.

The test is from 12 years ago and it's really bad...  :P  Also it
references some debugfs rules which changed a many years ago.  I have
to go for a call but it would probably take 15 minutes to write it
correctly these days.

regards,
dan carpenter

/*
 * Copyright (C) 2009 Dan Carpenter.
 *
 * 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"
#include "smatch_extra.h"

static int my_id;

STATE(is_null);
STATE(ok);

static void ok_to_use(struct sm_state *sm, struct expression *mod_expr)
{
	set_state(my_id, sm->name, sm->sym, &ok);
}

static bool is_basically_unknown(struct expression *expr)
{
	struct sm_state *sm, *tmp;
	bool known = false;

	sm = get_extra_sm_state(expr);
	if (!sm)
		return false;

	FOR_EACH_PTR(sm->possible, tmp) {
		if (!is_whole_rl(estate_rl(tmp->state)))
			known = true;
	} END_FOR_EACH_PTR(tmp);

	return !known;
}

static int rl_is_null_and_valid(struct range_list *rl)
{
	struct data_range *range;
	int i;

	i = -1;
	FOR_EACH_PTR(rl, range) {
		i++;
		if (i == 0) {
			if (range->min.value != 0 || range->max.value != 0)
				return 0;
			continue;
		}
		if (i == 1) {
			if (range->min.value == valid_ptr_min && range->max.value == valid_ptr_max)
				return 1;
		}
		return 0;
	} END_FOR_EACH_PTR(range);
	return 0;
}

static struct expression *get_call(struct expression *pointer)
{
	struct expression *assigned;

	assigned = get_assigned_expr(pointer);
	if (!assigned)
		return NULL;
	if (assigned->type != EXPR_CALL)
		return NULL;
	return assigned;
}

static int is_a_debugfs_thing(struct expression *call)
{
	if (!call)
		return 0;
	if (call->fn->type != EXPR_SYMBOL)
		return 0;
	if (strstr(call->fn->symbol_name->name, "debugfs"))
		return 1;
	return 0;
}

static int returns_err_ptr_sometimes(struct expression *call)
{
	struct range_list *rl;

	if (!call)
		return 0;
	/* get_implied_rl() should normally return success here. */
	if (!get_implied_rl(call, &rl))
		return 0;
	if (rl_max(rl).uvalue >= (unsigned long long)-4095)
		return 1;
	return 0;
}

static void match_is_err(const char *fn, struct expression *expr, void *unused)
{
	struct expression *arg;
	struct expression *call;
	struct range_list *rl;
	char *name;

	arg = get_argument_from_call_expr(expr->args, 0);
	if (!get_implied_rl(arg, &rl))
		return;
	if (is_basically_unknown(arg))
		return;
	if (rl_is_null_and_valid(rl))
		set_state_expr(my_id, arg, &is_null);
	if (rl_max(rl).uvalue >= (unsigned long long)-4095)
		return;
	call = get_call(arg);
	if (is_a_debugfs_thing(call))
		return;
	if (returns_err_ptr_sometimes(call))
		return;
	name = expr_to_str(arg);
	sm_msg("warn: '%s' isn't an ERR_PTR", name);
	free_string(name);
}

static void match_dereferences(struct expression *expr)
{
	struct sm_state *sm;
	char *name;

	expr = strip_expr(expr->unop);
	sm = get_sm_state_expr(my_id, expr);
	if (!sm)
		return;
	if (!slist_has_state(sm->possible, &is_null))
		return;
	if (implied_not_equal(expr, 0))
		return;
	name = expr_to_str(expr);
	sm_msg("warn: possible NULL dereference of '%s'", name);
	free_string(name);
}

void check_err_ptr_vs_null(int id)
{
	if (option_project != PROJ_KERNEL)
		return;

	my_id = id;
	add_function_hook("IS_ERR", &match_is_err, NULL);
	add_hook(&match_dereferences, DEREF_HOOK);
	add_modification_hook(my_id, &ok_to_use);
}