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

[Julia Lawall]

On Mon, 27 Aug 2018, Al Viro wrote:

> 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

OK, maybe for a given type the set of functions would be smaller.

>
> 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)));

It only covers the pattern that is shown, ie an assignment.  For this,
another pattern would be needed.  It would be necessary to match first the
call that one is concerned with and then go find the function definition
or prototype to find the type of the associated parameter.  It is possible
to count the offset of the kmalloc call in the argument list and then get
the type at the corresponding offset in the parameter list of the function
declaration or prototype.

>
>
> > 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...

Such ifdefs would be ignored completely.  I suspect that only the last
definition of the structure would be taken into account.

julia