Re: [PATCH 2/4] ACPI: sysfs: Fix a potential out-of-bound write in create_of_modalias()

[Dan Carpenter]

On Tue, Oct 24, 2023 at 09:08:26AM +0300, Dan Carpenter wrote:
> So I had a Smatch check for this kind of stuff but it was pretty junk.
> It also only looked for "modalias + len" and here the code is doing
> "&modalias[len]".
> 
> I can fix it up a bit today and look again at the warnings.  Here is the
> the check and the warnings as-is.

Alright.  Here is the new version.  :)

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_slist.h"
#include "smatch_extra.h"

static int my_id;

STATE(plus_equal);
STATE(minus_equal);

static bool is_plus_address(struct expression *expr)
{
	sval_t sval;

	if (expr->type != EXPR_PREOP ||
	    expr->op != '&')
		return false;

	expr = expr->unop;
	if (expr->type != EXPR_PREOP ||
	    expr->op != '*')
		return false;

	expr = expr->unop;
	if (expr->type != EXPR_BINOP ||
	    expr->op != '+')
		return false;

	if (get_implied_value(expr->right, &sval) && sval.value == 0)
		return false;

	return true;
}

static bool is_plus(struct expression *expr)
{
	struct expression *tmp;
	sval_t sval;

	tmp = get_assigned_expr(expr);
	if (tmp)
		expr = tmp;
	expr = strip_expr(expr);
	if (expr->type == EXPR_BINOP && expr->op == '+') {
		if (get_implied_value(expr->right, &sval) && sval.value == 0)
			return false;
		return true;
	}

	if (is_plus_address(expr))
		return true;

	if (get_state_expr(my_id, expr) == &plus_equal)
		return true;

	return false;
}

static bool is_minus(struct expression *expr)
{
	struct expression *tmp;

	tmp = get_assigned_expr(expr);
	if (tmp)
		expr = tmp;
	expr = strip_expr(expr);
	if (expr->type == EXPR_BINOP && expr->op == '-')
		return true;
	/*
	 * If, after calling strip_expr, the expression is still () that means
	 * it is an EXPR_STATEMENT and some kind of complicated macro.
	 */
	if (expr->type == EXPR_PREOP && expr->op == '(')
		return true;
	if (get_state_expr(my_id, expr) == &minus_equal)
		return true;
	return false;
}

static void match_snprintf(const char *fn, struct expression *expr, void *unused)
{
	struct expression *dest, *limit;
	char *name;

	dest = get_argument_from_call_expr(expr->args, 0);
	limit = get_argument_from_call_expr(expr->args, 1);
	dest = strip_expr(dest);
	limit = strip_expr(limit);
	if (!dest || !limit)
		return;

	if (!is_plus(dest))
		return;

	if (is_minus(limit))
		return;

	name = expr_to_str(limit);
	sm_warning("expected subtract in snprintf limit '%s'", name);
	free_string(name);
}

static void match_assign(struct expression *expr)
{
	if (expr->op == SPECIAL_ADD_ASSIGN)
		set_state_expr(my_id, expr->left, &plus_equal);

	if (expr->op == SPECIAL_SUB_ASSIGN)
		set_state_expr(my_id, expr->left, &minus_equal);
}

void check_snprintf_no_minus(int id)
{
	my_id = id;

	add_function_hook("snprintf", &match_snprintf, NULL);
	add_function_hook("scnprintf", &match_snprintf, NULL);

	add_hook(&match_assign, ASSIGNMENT_HOOK);
}