Re: [RFC v2] mac80211: fix possible null-pointer dereference

[Dan Carpenter <error27@gmail.com>]

On Thu, Sep 30, 2010 at 06:52:30PM +0200, Christian Lamparter wrote:
> On Thursday 30 September 2010 18:27:08 Bob Copeland wrote:
> > On Fri, Sep 24, 2010 at 6:02 PM, Christian Lamparter
> > <chunkeey@googlemail.com> wrote:
> > 
> > > hard to say, smatch must see the null dereference, when
> > > we receive an action action frame where ftype is PLINK_OPEN
> > > and the mesh_matches_local(&elems, sdata) check fail, but why
> > > doesn't it complain about the "spin_lock_bh(&sta->lock)"?
> > 
> > Smatch just does pattern matching right?
> Uhh, I guess that's a question for Dan.
> 
> The README-smatch sums it up as:
> "It's basically a state machine that tracks the flow of code."
> 
> (I think coccicheck,  is the "pattern matching" checker, right?)
> > Maybe smatch doesn't assume you are actually using
> > the pointer in spin_lock_bh().
> > 
> > I.e. it is ok to do "&null_ptr->member", offsetof() basically
> > does that; but not "null_ptr->member".
> 

Yes.  You are right.

This is from check_check_deref.c which handles this as a special case
because people quite often do:

	struct foo *bar = &x->y;

	if (!x)
		return;

If you comment out the "if (getting_address())" check then it will
complain.

> net/mac80211/mesh_plink.c +574 mesh_rx_plink_frame(168)
> error: we previously assumed 'sta' could be null.
> 
> 574:        switch (sta->plink_state) {
> 
> Smatch is definitely following code paths. Is there a switch
> to make it more verbose (e.g.: comment on about the conditions
> about the objected code - branch)?

There is a --debug but I suspect it's way more verbose than what you
want.

You could also hack the net/mac80211/mesh_plink.c source file and add
an "#include /path/to/smatch/check_debug.h" and sprinkle the code with
calls to __smatch_cur_slist() which will make it dump all the current
states at that point.

regards,
dan carpenter