[Ksummit-2012-discuss] Smatch for static analysis testing

Dan Carpenter dan.carpenter at oracle.com
Fri Jun 22 11:13:11 UTC 2012


I'm coming late to the discussion.  Guenter Roeck mentioned static
checking in a previous message:
http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/000166.html

Smatch actually found the bug mentioned in the email.

I go through Smatch warnings for every linux-next release.  Starting
from the last kernel summit I would have emailed the submitter and
the OCFS list.  Even back in May 2011 I probably would have fixed
the bug except that my laptop had just been stolen a few weeks prior
and I was unemployed.  These days I'm rolling in cash and laptops so
I'm confident I would have reported it.

I only check allmodconfig warnings on x86_64.

I only look at new warnings.  1)  I've looked at most of the old
warnings before.  2)  We tend to fix bugs and leave the false
positives so the old warnings are mostly false positives.  3)  For
the old stuff the patch submitter has long since moved on and no one
cares.

The most common kinds of bugs I fix are:
    * Not unlocking on an error path
    * Inconsistent NULL checking
    * Checking for NULL instead of IS_ERR()
But there are a variety of other bugs as well.  Most of my kernel
patches are Smatch fixes:  git log --author=Carpenter

My Smatch output looks far different from the official Smatch output
because I am more willing to endure false positives.  For example,
I consider all data that is not CPU endian to be user controlled.  I
was able to fix two integer overflow bugs with that last week.

I also audit every time someone introduces a new "array[foo - 1]"
because maybe "foo" is zero.  I've found 3 bugs with that.  I check
when people introduce a new "for (i = 0; i < x - 1; i++) {" because
why are we skipping the last element?  I've found a couple bugs that
way.  These are not stuff I would push though because of the false
positives.

Smatch has improved a lot since last year.  The new cross function
analysis is very promising.  I've sent instructions to lkml on how
to set that up.  https://lkml.org/lkml/2012/3/4/55

There are a couple maintainers run Smatch on their code.  I know
people do get frustrated with the false positives.

I would like to hear ideas about what kinds of bugs people want
detected.  If you have a specific patch and you are wondering if
Smatch could detect it automatically then send that to me at
smatch at vger.kernel.org.

Since last year, I've moved from fixing bugs to just reporting them.
If I don't understand the code then I normally don't report it.
For example in  sound/drivers/serial-u16550.c:
   442                  byte = (0 & UART_IER_RDI)       /* Disable Receiver data interrupt */
   443                      |(0 & UART_IER_THRI)        /* Disable Transmitter holding register empty interrupt */
   444                      ;
Surely, the author understands that he is setting "byte = 0;" right?
Looking at it now, I suppose it's possible that bitwise negate was
intended.  This is old so I wouldn't report it for that reason as
well.

regards,
dan carpenter


More information about the Ksummit-2012-discuss mailing list