SKB BUG: Invalid truesize

SKB BUG: Invalid truesize

[Beschorner Daniel]

Does it harm?

SKB BUG: Invalid truesize (380) len=1383, sizeof(sk_buff)=156
SKB BUG: Invalid truesize (316) len=1383, sizeof(sk_buff)=156
SKB BUG: Invalid truesize (348) len=1383, sizeof(sk_buff)=156
SKB BUG: Invalid truesize (316) len=1383, sizeof(sk_buff)=156
SKB BUG: Invalid truesize (380) len=1383, sizeof(sk_buff)=156

I found it in the log of a 2.6.17 box using IPSEC tunnels.

Re: SKB BUG: Invalid truesize

[Auke Kok]

Beschorner Daniel wrote:
> Does it harm?
> 
> SKB BUG: Invalid truesize (380) len=1383, sizeof(sk_buff)=156
> SKB BUG: Invalid truesize (316) len=1383, sizeof(sk_buff)=156
> SKB BUG: Invalid truesize (348) len=1383, sizeof(sk_buff)=156
> SKB BUG: Invalid truesize (316) len=1383, sizeof(sk_buff)=156
> SKB BUG: Invalid truesize (380) len=1383, sizeof(sk_buff)=156
> 
> I found it in the log of a 2.6.17 box using IPSEC tunnels.

it does harm, we need to know which network driver(s) are/were loaded when 
this happened, for starters.

cheers,

Auke

[IPCOMP]: Fix truesize after decompression

[Herbert Xu]

On Thu, Jul 06, 2006 at 12:53:45PM +0000, Beschorner Daniel wrote:
> Does it harm?
> 
> SKB BUG: Invalid truesize (380) len=1383, sizeof(sk_buff)=156
> SKB BUG: Invalid truesize (316) len=1383, sizeof(sk_buff)=156
> SKB BUG: Invalid truesize (348) len=1383, sizeof(sk_buff)=156
> SKB BUG: Invalid truesize (316) len=1383, sizeof(sk_buff)=156
> SKB BUG: Invalid truesize (380) len=1383, sizeof(sk_buff)=156
> 
> I found it in the log of a 2.6.17 box using IPSEC tunnels.

It's not fatal, but it does stuff up socket accounting.  Unfortunately
getting totally accurate truesizes is not easy due to the large numbers
of pskb_expand_head calls scattered around the stack.

[IPCOMP]: Fix truesize after decompression

The truesize check has uncovered the fact that we forgot to update truesize
after pskb_expand_head.  Unfortunately pskb_expand_head can't update it for
us because it's used in all sorts of different contexts, some of which would
not allow truesize to be updated by itself.

So the solution for now is to simply update it in IPComp.

This patch also changes skb_put to __skb_put since we've just expanded
tailroom by exactly that amount so we know it's there (but gcc does not).

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/ipv4/ipcomp.c b/net/ipv4/ipcomp.c
index 8e03748..8a8b5cf 100644
--- a/net/ipv4/ipcomp.c
+++ b/net/ipv4/ipcomp.c
@@ -70,7 +70,8 @@ static int ipcomp_decompress(struct xfrm
 	if (err)
 		goto out;
 		
-	skb_put(skb, dlen - plen);
+	skb->truesize += dlen - plen;
+	__skb_put(skb, dlen - plen);
 	memcpy(skb->data, scratch, dlen);
 out:	
 	put_cpu();
diff --git a/net/ipv6/ipcomp6.c b/net/ipv6/ipcomp6.c
index b285b03..7e4d1c1 100644
--- a/net/ipv6/ipcomp6.c
+++ b/net/ipv6/ipcomp6.c
@@ -109,7 +109,8 @@ static int ipcomp6_input(struct xfrm_sta
 		goto out_put_cpu;
 	}
 
-	skb_put(skb, dlen - plen);
+	skb->truesize += dlen - plen;
+	__skb_put(skb, dlen - plen);
 	memcpy(skb->data, scratch, dlen);
 	err = ipch->nexthdr;
 

Re: [IPCOMP]: Fix truesize after decompression

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 11 Jul 2006 21:45:44 +1000

> [IPCOMP]: Fix truesize after decompression
> 
> The truesize check has uncovered the fact that we forgot to update truesize
> after pskb_expand_head.  Unfortunately pskb_expand_head can't update it for
> us because it's used in all sorts of different contexts, some of which would
> not allow truesize to be updated by itself.
> 
> So the solution for now is to simply update it in IPComp.
> 
> This patch also changes skb_put to __skb_put since we've just expanded
> tailroom by exactly that amount so we know it's there (but gcc does not).
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied.

I think it is possible cover a certain class of these situations
from within pskb_expand_head.  For example, if skb->sk is NULL
we can prove that updating skb->truesize is safe since no
socket's buffer accounting can possible depend upon the truesize
value of this skb.

That way, we don't need to dilly-dally around spotting all the
cases like these two decompression bits where we need to add
the truesize adjustment by-hand.

This got me thinking... we can probably handle the skb->sk != NULL
case with some minor surgery.  If we could give some more information
to the skb->destructor callback, it could be used for truesize
adjustments.

The only remaining issue is context within which the destructor
is called for truesize changes.  There are some messy assumptions
out that, such as sk_stream_rfree(), which work on the basis
that the SKB is always freed up from the socket code with the
socket locked.  They use this assumption in order to safely
be able to adjust sk->sk_forward_alloc without any explicit
locking.

So it gets a little messy, but I think it's doable in the end.

Re: [IPCOMP]: Fix truesize after decompression

[Herbert Xu]

On Tue, Jul 11, 2006 at 01:55:53PM -0700, David Miller wrote:
> 
> I think it is possible cover a certain class of these situations
> from within pskb_expand_head.  For example, if skb->sk is NULL
> we can prove that updating skb->truesize is safe since no
> socket's buffer accounting can possible depend upon the truesize
> value of this skb.

Yes that's certainly possible.  However, we'll need to audit the few
spots (e.g., ATM) that use truesize without setting skb->sk.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [IPCOMP]: Fix truesize after decompression

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 12 Jul 2006 09:12:51 +1000

> On Tue, Jul 11, 2006 at 01:55:53PM -0700, David Miller wrote:
> > 
> > I think it is possible cover a certain class of these situations
> > from within pskb_expand_head.  For example, if skb->sk is NULL
> > we can prove that updating skb->truesize is safe since no
> > socket's buffer accounting can possible depend upon the truesize
> > value of this skb.
> 
> Yes that's certainly possible.  However, we'll need to audit the few
> spots (e.g., ATM) that use truesize without setting skb->sk.

Yuck, I'd forgotten about that usage....

What ATM is doing here is charging the SKB to the virtual circuit
sockets.  At least in the few cases I've looked at just now, the
skb is some private ATM level signalling message, and not part of
a normal transmit/receive packet from the normal networking stack.

Therefore, there seems to be no reason why ATM can't use the
destructor callback of the SKB to manage this accounting just like the
rest of the kernel does.

Re: [IPCOMP]: Fix truesize after decompression

[Herbert Xu]

On Tue, Jul 11, 2006 at 04:22:17PM -0700, David Miller wrote:
> 
> What ATM is doing here is charging the SKB to the virtual circuit
> sockets.  At least in the few cases I've looked at just now, the
> skb is some private ATM level signalling message, and not part of
> a normal transmit/receive packet from the normal networking stack.

Indeed, at least they do have an sk to charge things to :)
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: SKB BUG: Invalid truesize

[Beschorner Daniel]

>> Does it harm?
>> 
>> SKB BUG: Invalid truesize (380) len=1383, sizeof(sk_buff)=156
>> SKB BUG: Invalid truesize (316) len=1383, sizeof(sk_buff)=156
>> SKB BUG: Invalid truesize (348) len=1383, sizeof(sk_buff)=156
>> SKB BUG: Invalid truesize (316) len=1383, sizeof(sk_buff)=156
>> SKB BUG: Invalid truesize (380) len=1383, sizeof(sk_buff)=156
>> 
>> I found it in the log of a 2.6.17 box using IPSEC tunnels.

> it does harm, we need to know which network driver(s) are/were loaded when

> this happened, for starters.

It happened only once during quick mode of an IPSEC tunnel.
Two 3c905C cards, both with 3c59x, non-module build.

SKB BUG: Invalid truesize

[Ernst Herzberg]

moin,

this BUG fills my syslogs since 2.6.25 (or .24? can't remeber),
but since 2.6.28.? it gives a nice calltrace .. and fills it even more;-)

Machine is used as a adsl router, pppoe with a traffic shaper, messages
are only written if 'iftop' http://www.ex-parrot.com/~pdw/iftop/ is running.

------------[ cut here ]------------
WARNING: at net/core/skbuff.c:150 sock_rfree+0x63/0x70()
SKB BUG: Invalid truesize (1632) len=1422, sizeof(sk_buff)=224
Modules linked in: cls_fw sch_sfq sch_htb xt_connbytes xt_mark xt_length xt_dscp xt_multiport xt_MARK xt_TCPMSS xt_tcpudp iptable_filter iptable_mangle iptable_nat nf_nat 
nf_conntrack_ipv4 nf_conntrack nf_defrag_ipv4 ip_tables x_tables pppoe pppox ppp_generic slhc tun it87 hwmon_vid ppdev ehci_hcd pcspkr k8temp firewire_ohci firewire_core 
crc_itu_t ohci_hcd usbcore i2c_nforce2 parport_pc parport evdev
Pid: 9718, comm: iftop Not tainted 2.6.28.6 #1
Call Trace:
 [<ffffffff8022e6ed>] warn_slowpath+0xcd/0x110
 [<ffffffff8025c2fb>] get_page_from_freelist+0x2db/0x460
 [<ffffffff802295f5>] update_curr+0x55/0xb0
 [<ffffffff8022988d>] dequeue_entity+0x1d/0xb0
 [<ffffffff80229805>] set_next_entity+0x25/0x50
 [<ffffffff804aceb4>] thread_return+0x30/0x1bc
 [<ffffffff804ad1e5>] schedule_timeout+0x95/0xd0
 [<ffffffff8043f023>] sock_rfree+0x63/0x70
 [<ffffffff804423e7>] skb_release_head_state+0x37/0xb0
 [<ffffffff80442c29>] skb_release_all+0x9/0x20
 [<ffffffff804422e9>] __kfree_skb+0x9/0x80
 [<ffffffff8044536c>] skb_free_datagram+0xc/0x40
 [<ffffffff804a1223>] packet_recvmsg+0x1b3/0x1e0
 [<ffffffff802295f5>] update_curr+0x55/0xb0
 [<ffffffff8043bf75>] sock_recvmsg+0xd5/0x100
 [<ffffffff8022a5c1>] enqueue_task_fair+0x21/0x40
 [<ffffffff802414b0>] autoremove_wake_function+0x0/0x30
 [<ffffffff80228f50>] __wake_up_common+0x50/0x80
 [<ffffffff80232869>] ns_to_timeval+0x9/0x40
 [<ffffffff8043b781>] sockfd_lookup_light+0x41/0x80
 [<ffffffff8043d172>] sys_recvfrom+0xe2/0x190
 [<ffffffff8043d9e4>] sock_ioctl+0x54/0x240
 [<ffffffff8028ad7b>] vfs_ioctl+0x1b/0x70
 [<ffffffff8028ae6c>] do_vfs_ioctl+0x7c/0x430
 [<ffffffff8020b45b>] system_call_fastpath+0x16/0x1b
---[ end trace 855e82560af6b46f ]---
------------[ cut here ]------------
WARNING: at net/core/skbuff.c:150 sock_rfree+0x63/0x70()
SKB BUG: Invalid truesize (416) len=198, sizeof(sk_buff)=224
Modules linked in: cls_fw sch_sfq sch_htb xt_connbytes xt_mark xt_length xt_dscp xt_multiport xt_MARK xt_TCPMSS xt_tcpudp iptable_filter iptable_mangle iptable_nat nf_nat 
nf_conntrack_ipv4 nf_conntrack nf_defrag_ipv4 ip_tables x_tables pppoe pppox ppp_generic slhc tun it87 hwmon_vid ppdev ehci_hcd pcspkr k8temp firewire_ohci firewire_core 
crc_itu_t ohci_hcd usbcore i2c_nforce2 parport_pc parport evdev
Pid: 9718, comm: iftop Tainted: G        W  2.6.28.6 #1
Call Trace:
 [<ffffffff8022e6ed>] warn_slowpath+0xcd/0x110
 [<ffffffff8025c2fb>] get_page_from_freelist+0x2db/0x460
 [<ffffffff802295f5>] update_curr+0x55/0xb0
 [<ffffffff8022988d>] dequeue_entity+0x1d/0xb0
 [<ffffffff80229805>] set_next_entity+0x25/0x50
 [<ffffffff804aceb4>] thread_return+0x30/0x1bc
 [<ffffffff804ad1e5>] schedule_timeout+0x95/0xd0
 [<ffffffff8043f023>] sock_rfree+0x63/0x70
 [<ffffffff804423e7>] skb_release_head_state+0x37/0xb0
 [<ffffffff80442c29>] skb_release_all+0x9/0x20
 [<ffffffff804422e9>] __kfree_skb+0x9/0x80
 [<ffffffff8044536c>] skb_free_datagram+0xc/0x40
 [<ffffffff804a1223>] packet_recvmsg+0x1b3/0x1e0
 [<ffffffff802295f5>] update_curr+0x55/0xb0
 [<ffffffff8043bf75>] sock_recvmsg+0xd5/0x100
 [<ffffffff8022a5c1>] enqueue_task_fair+0x21/0x40
 [<ffffffff802414b0>] autoremove_wake_function+0x0/0x30
 [<ffffffff80228f50>] __wake_up_common+0x50/0x80
 [<ffffffff80232869>] ns_to_timeval+0x9/0x40
 [<ffffffff8043b781>] sockfd_lookup_light+0x41/0x80
 [<ffffffff8043d172>] sys_recvfrom+0xe2/0x190
 [<ffffffff8043d9e4>] sock_ioctl+0x54/0x240
 [<ffffffff8028ad7b>] vfs_ioctl+0x1b/0x70
 [<ffffffff8028ae6c>] do_vfs_ioctl+0x7c/0x430
 [<ffffffff8020b45b>] system_call_fastpath+0x16/0x1b
---[ end trace 855e82560af6b46f ]---

earny