Re: [PATCH] xfs: fix possible NULL dereference

Dave Jones
2013-10-25T09:16:17Z

On Wed, Oct 23, 2013 at 09:02:54AM +1100, Dave Chinner wrote:

> > it looks suspicious to pretty much anyone. I don't think Geyslan
> > sent it to shut Coverity up, he sent it because it looked like
> > a bug worth fixing (after Coverity spotted it).
> >
> > Let's not be too hard on him for trying; I appreciate it more
> > than spelling fixes and whitespace cleanups. ;)
>
> True, point taken.

So another reason you're seeing an uptick in coverity reports lately
is that back in June they gave me admin rights for the project at scan.coverity.com
so I've been doing daily builds since then. (Previously they only did one per point release).

The Coverity guys did a write-up on this thread at http://security.coverity.com/blog/2013/Oct/deliberate-null-pointer-dereferences-in-the-linux-kernel.html
The point about modelling is the pertinent part. I'm still trying to get my
head around a lot of how that stuff works, but that's the sort of thing
that I have rights to do on their site too.

If you or anyone else wants access to their bugs, I can approve that
easily enough. I've been going through and trying to filter out as many of
the intentional[*] issues as possible, and do things like sorting into components
so that you're able to look at just XFS bugs for eg.

I know Eric has been looking at their bugs when he has had time, but if there's
something I can do to make things easier for you guys, let me know.
(I could email you new issue reports as they come in for eg)

To end on a high note, XFS is actually one of the better subsystems from the
POV of number of issues they've found. Only 38 'New' issues right now, which
given the complexity in XFS, is pretty darn good, and I bet a bunch of those
are actually non-issues too. The painful part is going through and sorting
through the non-issues to get to the real meaty bugs, which is what I've slowly
been doing over the last couple months. (Down from 5900 or so, to 5305,
thanks to help from others)

Dave

[*] From what I've seen so far, a lot of issues it finds are the checker
getting tricked by idioms we use in the kernel rather than actual "false positives"
(in terms of "this is a bug in the checker"). As the url above points out,
sometimes we can help the checker out through modelling, but some of the code
I've seen it get tripped up is hard enough for a human to parse, so I don't
really blame the checker for getting confused ;)