Re: smatch and tun.c

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

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