Re: [RFC PATCH] check_freeing_devm: Also track erroneous usage of kfree when the pointer has been re-assigned

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

On Thu, May 26, 2022 at 09:15:16AM +0200, Christophe JAILLET wrote:
> When some memory is allocated, a common pattern is to use a tmp variable
> to check if the allocation has succeeded or not. This tmp variable is then
> stored in another variable for future use.
> 
>    ptr = devm_kmalloc(...);
>    if (!ptr)
>       return -ENOMEM;
>    another_val = ptr;
> 
> So trying to see if this 'alias' is incorrectly freed with kfree makes
> sense.
> 
> To do that, add a new hook that check, when an assignment is detected if
> the right part of the expression is already recorded.
> If so, also record the right part, so that bogus kfree can check both
> reference.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Thanks! Applied.

> ---
> This is my first 'real' code proposal for smatch.
> It is likely to be wrong/incomplete/imperfect. Maybe this functionality
> is already implemented somewhere else and I just try to duplicate it.

No, it's fine.  There are a lot of different ways to write any check.
I wrote this check nine years ago.  If I were to write it now, I probably
would only hook into the free functions and not have any states.

	orig = get_assigned_expr_recurse();
	if (!orig || orig->type != EXPR_CALL)
		return;

	str = expr_to_str(orig);
	if (!str)
		return;
	if (strstr(str, "devm_"))
		sm_warning("devm_ data freed");
	free_string(str);

But, whatever, the check works fine as-is.  It's not like I have run out
of other work to do.

The other idea that I have, is to look at struct member types.  Make a
list of "(struct foo)->bar" was allocated with devm_ so that type cannot
be freed using kfree() within the same file.  It's not tied to the
variables at all, only to the struct member names.

That check would print many of the same warnings as this check but it's
fine to print duplicate warnings since the heuristic is different.

> 
> All I know is that it seams to work for me, even if it has not detected any
> issue yet :) (compiling takes SO MUCH time on my machine)

Yeah. :/  How big is your smatch_db.sqlite file?

I have been trying to speed things up on my system these past two weeks.

regards,
dan carpenter