Re: mac80211 truesize bugs

Johannes Berg
2008-05-03T23:43:46Z

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

> > > > + if (unlikely((nhead || ntail) && skb->sk)) {
> > > > + printk(KERN_ERR "SKB BUG: Illegal pskb expand (%d:%d) "
> > > > + "with socket attached\n",
> > > > + nhead, ntail);
> > > > + }
> > > > +
> > > > size = SKB_DATA_ALIGN(size);
> > >
> > > Ok I think I'm starting to understand this a little better. However,
> > > shouldn't this function update skb->truesize so if the skb is later
> > > attached to a different socket again it has the right size?
> >
> > Judging from some of the callers, the caller should. Why?!
>
> Relax :-)

:)

Hm. The only caller that does seem to do it seems to be in af_netlink.c.

> We certainly could check that there is no socket attached here,
> and make the truesize adjustment right at this spot.
>
> It just never happened before in practice in a way that matters.
>
> That's why we have the truesize assertion, to discover situations
> like this and thus be able to fix it.

Except unfortunately the truesize assertion is rather useless since you
have no idea where it comes from. FWIW, some caller that does the
adjustment must be going wrong, whenever I start vpnc I get a single one
like this:

[ 162.108556] SKB BUG: Invalid truesize (408) size=432,
sizeof(sk_buff)=176

This is again without the patch to pskb_expand_head that did the
truesize adjustment, I only put in a WARN_ON (similar to the code you
had above but is, I think, more useful since it has a stack dump and
other useful info)

Right now I think I'm too lazy to dig into where this happens. I don't
hit the warning in pskb_expand_head so it must be one of the other
20-odd places where truesize is adjusted. Maybe I'll just make each of
them print out the info.