Subject: Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL

From: Al Viro <[email-masked]>
Date: Mon, 27 Aug 2018 05:04:23 +0100

On Sun, Aug 26, 2018 at 11:35:17PM -0400, Julia Lawall wrote:

> * x = \(kmalloc\|kzalloc\|devm_kmalloc\|devm_kzalloc\)(...)

I can name several you've missed right off the top of my head -
vmalloc, kvmalloc, kmem_cache_alloc, kmem_cache_zalloc, variants
with _trace slapped on, and that is not to mention the things like
get_free_page or

void *my_k3wl_alloc(u64 n) // 'cause all artificial limits suck, that's why
{
	lots and lots of home-grown stats collection
	some tracepoints thrown in just for fun
	return kmalloc(n);
}

(and no, I'm not implying that net/sched folks had done anything of that
sort; I have seen that and worse in drivers, though)

> The * at the beginning of the line means to highlight what you are looking
> for, which is done by making a diff in which the highlighted line
> appears to be removed.

Umm...  Does that cover return, BTW?  Or something like
	T *barf;
	extern void foo(T *p);
	foo(kmalloc(sizeof(*barf)));

> The limitation is the ability to figure out the type of x.  If it is a
> local variable, Coccinelle should have no problem.  If it is a structure
> field, it may be necessary to provide command line arguments like
> 
> --all-includes --include-headers-for-types
> 
> --all-includes means to try to find all include files that are mentioned
> in the .c file.  The next stronger option is --recursive includes, which
> means include what all of the mentioned files include as well,
> recursively.  This tends to cause a major performance hit, because a lot
> of code is being parsed.  --include-headers-for-types heals a bit with
> that, as it only considers the header files when computing type
> information, and now when applying the rules.
> 
> With respect to ifdefs around variable declarations and structure field
> declaration, in these cases Coccinelle considers that it cannot make the
> ifdef have an if-like control flow, and so if considers the #ifdef, #else
> and #endif to be comments.  Thus it takes into account only the last type
> provided for a given variable.

[snip]

What about several variants of structure definition?  Because ifdefs around
includes do occur in the wild...