| Message ID | 20090213121424.GA8717@ff.dom.local |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
Jarek Poplawski wrote: > On 11-02-2009 16:55, Krzysztof Oledzki wrote: >> On Wed, 11 Feb 2009, Marco Berizzi wrote: >> >>> Hi Folks, >>> >>> I'm getting this error on 2.6.28.4 when I run tcpdump on >>> the interface where ipsec packets are enc/decrypted. >>> >>> TIA >>> >>>> Feb 11 10:53:55 Pleiadi kernel: ------------[ cut here ]------------ >>>> Feb 11 10:53:55 Pleiadi kernel: WARNING: at net/core/skbuff.c:154 skb_truesize_bug+0x2e/0x33() >>>> Feb 11 10:53:55 Pleiadi kernel: SKB BUG: Invalid truesize (268) len=134, sizeof(sk_buff)=172 >> <CUT> >> >> This annoying problem is quite old (appeared in 2.6.25) and already known: >> http://bugzilla.kernel.org/show_bug.cgi?id=10996 >> >> Sadly, no one is interested in fixing it. :( > > Here is a debugging patch doing these checks a bit earlier, so maybe > we get something new and interesting. ;) > > Thanks, > Jarek P. > --- > I did notice that pskb_expand_head() doesn't change the skb->truesize even though it could grow the skb. I saw this problem with tcpdump while experimenting with some SCTP code. This is not to say that it is the problem in this case, but it's one of them that I've seen. -vlad -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 13, 2009 at 01:56:01PM -0500, Vlad Yasevich wrote: ... > I did notice that pskb_expand_head() doesn't change the skb->truesize even > though it could grow the skb. I saw this problem with tcpdump while > experimenting with some SCTP code. > > This is not to say that it is the problem in this case, but it's one of > them that I've seen. Yes, I've read Herbert Xu's message pointing especially to xfrm_state_check_space(). So I would like to make sure if there is no other reason it triggers in packet_recvmsg() on these several reports. If af_packet code is OK, I guess we could update truesize for it: there is no reason to warn here about bugs from other, well known places. Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jarek Poplawski wrote: > On Fri, Feb 13, 2009 at 01:56:01PM -0500, Vlad Yasevich wrote: > ... >> I did notice that pskb_expand_head() doesn't change the skb->truesize even >> though it could grow the skb. I saw this problem with tcpdump while >> experimenting with some SCTP code. >> >> This is not to say that it is the problem in this case, but it's one of >> them that I've seen. > > Yes, I've read Herbert Xu's message pointing especially to > xfrm_state_check_space(). So I would like to make sure if there is no > other reason it triggers in packet_recvmsg() on these several reports. > > If af_packet code is OK, I guess we could update truesize for it: > there is no reason to warn here about bugs from other, well known > places. > Personally, I think pskb_expand_head should fix the skb->truesize. This way any subsequent clones will not trigger this warning. Another alternative is to audit the pskb_expand_head() usages and adjust truesize in each case needed, which is just ugly. -vlad > Jarek P. > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 13, 2009 at 02:42:07PM -0500, Vlad Yasevich wrote: ... > Personally, I think pskb_expand_head should fix the skb->truesize. This > way any subsequent clones will not trigger this warning. Personally, I think there is no reason to call skb_truesize_bug() in anything but some #ifdef CONFIG_XX_DEBUG, if we ignore these reports for so long. > Another alternative is to audit the pskb_expand_head() usages and adjust > truesize in each case needed, which is just ugly. I guess, it's a lot of work to do it right (if it's possible at all). Yes, I think about something really ugly here. ;-) Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jarek Poplawski <jarkao2@gmail.com> Date: Fri, 13 Feb 2009 21:00:14 +0100 > On Fri, Feb 13, 2009 at 02:42:07PM -0500, Vlad Yasevich wrote: > ... > > Personally, I think pskb_expand_head should fix the skb->truesize. This > > way any subsequent clones will not trigger this warning. > > Personally, I think there is no reason to call skb_truesize_bug() in > anything but some #ifdef CONFIG_XX_DEBUG, if we ignore these reports > for so long. If skb->sk is non-NULL, fixing up the truesize will corrupt socket memory accounting. Anyways, Herbert and I have talked about %100 removing the warning. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index cf2cb50..20c3182 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -418,12 +418,16 @@ extern void skb_under_panic(struct sk_buff *skb, int len, void *here); extern void skb_truesize_bug(struct sk_buff *skb); -static inline void skb_truesize_check(struct sk_buff *skb) +static inline int skb_truesize_check(struct sk_buff *skb) { int len = sizeof(struct sk_buff) + skb->len; - if (unlikely((int)skb->truesize < len)) + if (unlikely((int)skb->truesize < len)) { skb_truesize_bug(skb); + return 1; + } + + return 0; } extern int skb_append_datato_frags(struct sock *sk, struct sk_buff *skb, diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 1fc4a78..08200be 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -307,6 +307,9 @@ static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev, struct if (dev_net(dev) != sock_net(sk)) goto out; + if (skb_truesize_check(skb)) + goto out; + if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL) goto oom; @@ -495,6 +498,9 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev, struct packet if (dev_net(dev) != sock_net(sk)) goto drop; + if (skb_truesize_check(skb)) + goto drop; + skb->dev = dev; if (dev->header_ops) { @@ -617,6 +623,9 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, struct packe if (dev_net(dev) != sock_net(sk)) goto drop; + if (skb_truesize_check(skb)) + goto drop; + if (dev->header_ops) { if (sk->sk_type != SOCK_DGRAM) skb_push(skb, skb->data - skb_mac_header(skb));