netdev - Re: [PATCH] Fix up truesize after pskb_expand

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]

Message-Id: <1231161379.3334.14.camel@johannes>
Date:	Mon, 05 Jan 2009 14:16:19 +0100
From:	Johannes Berg <johannes@...solutions.net>
To:	Andi Kleen <andi@...stfloor.org>
Cc:	linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
	linville@...driver.com, davem <davem@...emloft.net>
Subject: Re: [PATCH] Fix up truesize after pskb_expand_head() in wireless
	stack

On Mon, 2009-01-05 at 14:21 +0100, Andi Kleen wrote:

> > This statement is incompatible with your patch when you think about the
> > exact definition of truesize and the (unconditional!) adjustments your
> > patch makes.
> 
> __alloc_skb does 
> 
>         size = SKB_DATA_ALIGN(size);
>         skb->truesize = size + sizeof(struct sk_buff);
> 
> [ BTW it would be probably better if alloc_skb() just asked
> slab what the truesize is for kmalloc instead of guessing wrong like this.
> But that's a different topic]
> 
> and SKB_DATA_ALIGN is 
> 
> #define SKB_DATA_ALIGN(X)       (((X) + (SMP_CACHE_BYTES - 1)) & \
>                                  ~(SMP_CACHE_BYTES - 1))
> 
> and on my configuration SMP_CACHE_BYTES is 64
> 
> skb_truesize_check does 
> 
>  int len = sizeof(struct sk_buff) + skb->len;
> 
>         if (unlikely((int)skb->truesize < len))
>                 skb_truesize_bug(skb);
> 
> This means if the change is less than the 64byte cache alignment 
> (or more commonly 128 bytes on GENERIC_CPU distro kernels)
> it won't be reported. To my knowledge header adjustments are usually
> smaller and that is what pskb_expand_head() is usually used for.

Ah, indeed, I thought I remembered that truesize had to match exactly,
but obviously that wasn't done either. We should experiment with that
though, and let pskb_expand_head() adjust things.

> I didn't use any monitoring with this. No tcpdump, no wireless
> sniffer tools or anything. It happened all the time during
> normal operation.

You did have a monitor interface up though, didn't you? If nothing
actually used those skbs then it's likely that the warning didn't result
in any corruption at all.

johannes

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