Re: smatch and tun.c

From: Bartlomiej Zolnierkiewicz
Date: Sun Jul 26 2009 - 17:43:33 EST


On Saturday 25 July 2009 14:21:54 Dan Carpenter wrote:
> Jonathan Corbet from lwn.net suggested that I write a smatch
> (http://repo.or.cz/w/smatch.git) script to prevent the tun.c type bugs
> from happening again.
>
> I wrote two scripts. One just looks for bugs where a variable is
> dereferenced while initializing a local variable and checked for NULL like
> this:
> int x = foo->bar;
> BUG_ON(!foo);
> I have a theory that sometimes peoople are sloppy when they initialize
> variables. This script printed 96 warnings. The other script prints
> those messages along with every other time a variable is dereferenced and
> then checked for NULL. This script printed 685 messages.
>
> Smatch scripts tends to have a lot of false positives. The script that
> only printed problems with variable initialization was maybe 40% actual
> bugs but the one that checked everything was maybe 15% actual bugs. I
> guess my theory is correct.
>
> One thing that causes false positives is that some macros need to
> check for NULL pointers. Also smatch doesn't handle loops correctly yet.
> Sometimes variables get changed inside another function and that isn't
> handled correctly yet.

Indeed there is still a lot of false positives but the functionality
is already very useful so big thanks for doing it!

> By bugs, I don't mean security bugs, I mean picky little things. Quite
> often places have useless checks for NULL. But still it's worth fixing.

Yes, moreover they are a "noise" which makes discovery of real issues more
time consuming and difficult.

> I feel bad for just sending this email instead of proper bug reports and
> patches, but the truth is that I'm cycling through Africa on a bycicle. I
> sleep in a tent. It took me days to scrape together enough electricity
> and internet to send this one email...

You've just became my personal when it comes to the best use of available
resources! ;)

> I have attached a zip file with the warnings from 2.6.31-rc3 allmodconfig.
> Here are the instructions if you want to use a different config.
> git clone git://repo.or.cz/smatch.git
> cd smatch
> make
> cd /usr/src/linux
> make C=1 CHECK=/path/to/smatch modules bzImage | tee warns.txt
> egrep '(warn|error):' warns.txt | egrep '(before|initializer)'

The last line needs to be:

egrep '(warn|warning|error):' warns.txt | egrep '(before|initializer)'

to make it work, also if somebody wants to see the same list but against
current -next kernel it is here:

http://www.kernel.org/pub/linux/kernel/people/bart/null-pointer-err-list-next-0724.gz

[ some issues have been fixed already and some line numbers have changed ]

PS I'll also post a "filtered" list (without false positives) in a few days
(I've verified ~50% of entries till now).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/