linux-kernel - Re: smatch and tun.c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 26 Jul 2009 23:40:57 +0200
From:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To:	Dan Carpenter <error27@...il.com>
Cc:	linux-kernel@...r.kernel.org, corbet@....net, eteo@...hat.com
Subject: Re: smatch and tun.c

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/