Re: [PATCH] staging: vt6655: fix potential memory leak

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

[-- Attachment #1: Type: text/plain, Size: 1814 bytes --]

On Mon, Sep 12, 2022 at 05:17:28PM +0300, Dan Carpenter wrote:
> On Fri, Sep 09, 2022 at 04:42:05PM +0200, Nam Cao wrote:
> > I did not realize this initially, but this bug can cause more serious problem
> > than just a memory leak. In the case that kzalloc fails right from the
> > beginning with i=0; then in the while loop, "i" will wrap around and the code
> > will access priv->apTD0Rings[4294967295] which is obviously not good.
> > 
> 
> True.  You probably want to resend with that added to the commit
> message.  I will create a Smatch check to prevent these in the future.
> 

I created a static checker warning for these and it found a bunch more
bugs in the same file.

drivers/staging/vt6655/device_main.c:635 device_init_rd0_ring() warn: potential decrement underflow 'i' rl='s32min-(-1)' (iterator)
drivers/staging/vt6655/device_main.c:636 device_init_rd0_ring() warn: using underflowed offset 'i'
drivers/staging/vt6655/device_main.c:681 device_init_rd1_ring() warn: potential decrement underflow 'i' rl='s32min-(-1)' (iterator)
drivers/staging/vt6655/device_main.c:682 device_init_rd1_ring() warn: using underflowed offset 'i'
drivers/staging/vt6655/device_main.c:746 device_init_td0_ring() warn: potential decrement underflow 'i' rl='s32min-(-1)' (iterator)
drivers/staging/vt6655/device_main.c:747 device_init_td0_ring() warn: using underflowed offset 'i'
drivers/staging/vt6655/device_main.c:786 device_init_td1_ring() warn: potential decrement underflow 'i' rl='s32min-(-1)' (iterator)
drivers/staging/vt6655/device_main.c:787 device_init_td1_ring() warn: using underflowed offset 'i'

Could you fix those as well?  They're all the same bug and the same file
so I would do it all at once.  They were introduced in the same patch
as well so only one Fixes tag required.

regards,
dan carpenter

[-- Attachment #2: check_decrement_underflow.c --]
[-- Type: text/x-csrc, Size: 2137 bytes --]

/*
 * Copyright (C) 2022 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(decremented);

static struct expression *prev;
static void match_condition(struct expression *expr)
{
	struct expression *orig = expr;
	struct statement *stmt;
	struct range_list *rl;
	const char *iterator = "";
	char *name;

	if (expr == prev)
		return;

	if (expr->type != EXPR_PREOP || expr->op != SPECIAL_DECREMENT)
		return;

	expr = strip_expr(expr->unop);
	get_absolute_rl(expr, &rl);
	if (rl_min(rl).value > 0)
		return;

	set_state_expr(my_id, expr, &decremented);

	stmt = get_parent_stmt(orig);
	if (stmt && stmt->type == STMT_ITERATOR)
		iterator = " (iterator)";

	prev = orig;

	name = expr_to_str(expr);
	sm_warning("potential decrement underflow '%s' rl='%s'%s", name,
		   show_rl(rl), iterator);
	free_string(name);
}

static void array_check(struct expression *expr)
{
	struct expression *array, *offset;
	struct range_list *rl;
	char *name;

	expr = strip_expr(expr);
	array = get_array_base(expr);
	offset = get_array_offset(expr);

	if (!array || !offset)
		return;

	if (!get_state_expr(my_id, offset))
		return;

	get_absolute_rl(offset, &rl);
	if (rl_min(rl).value >= 0)
		return;

	name = expr_to_str(offset);
	sm_warning("using underflowed offset '%s'", name);
	free_string(name);
}


void check_decrement_underflow(int id)
{
	my_id = id;

	add_hook(&match_condition, CONDITION_HOOK);

	add_hook(&array_check, OP_HOOK);
}