netdev - Re: [RFC/T] [NET] make pskb_expand_head warn when called with invalid state

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:	Mon, 05 May 2008 18:01:14 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	"David S. Miller" <davem@...emloft.net>
Cc:	netdev <netdev@...r.kernel.org>,
	Herbert Xu <herbert@...dor.apana.org.au>
Subject: Re: [RFC/T] [NET] make pskb_expand_head warn when called with
	invalid state

On Mon, 2008-05-05 at 17:41 +0200, Johannes Berg wrote:
> On Sun, 2008-05-04 at 20:16 +0200, Johannes Berg wrote:
> > This makes pskb_expand_head warn when called when a socket is attached.
>  
> > +	WARN_ON((nhead || ntail) && skb->sk);
> > +
> 
> This is triggering now. I think it's a false positive, trace below.
> 
> [23194.608011] Badness at net/core/skbuff.c:726


> [23194.608077] [ccf9bba0] [c02735a0] pskb_expand_head+0x58/0x1f8 (unreliable)
> [23194.608082] [ccf9bbc0] [c02737a4] __pskb_pull_tail+0x64/0x374

It's actually not really a false positive. What is happening is that
__pskb_pull_tail does (follow 'eat'):

        /* If skb has not enough free space at tail, get new one
         * plus 128 bytes for future expansions. If we have enough
         * room at tail, reallocate without expansion only if skb is cloned.
         */
        int i, k, eat = (skb->tail + delta) - skb->end;

        if (eat > 0 || skb_cloned(skb)) {
                if (pskb_expand_head(skb, 0, eat > 0 ? eat + 128 : 0,
                                     GFP_ATOMIC))
                        return NULL;
        }

which of course changes the true size of the skb without accounting it
to the socket. Now, the reason this hasn't been known before is that the
data size doesn't change because the stuff that is copied into the
header is removed from the data_len... or something like that, I think.

johannes

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)