From: "Bruno Prémont" <bonbons@linux-vserver.org>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [2.6.33-rc5 regression] NULL pointer dereference in vlan_skb_recv - probably introduced by commit 9793241fe92f7d9303fb221e43fc598eb065f267
Date: Sun, 24 Jan 2010 16:02:28 +0100 [thread overview]
Message-ID: <20100124160228.366f4e72@neptune.home> (raw)
In-Reply-To: <4B5C4E5E.2010507@gmail.com>
On Sun, 24 January 2010 Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le 23/01/2010 22:31, Bruno Prémont a écrit :
> >> Above part of code did change between 2.6.32 and 2.6.33-rc5 with
> >> commit 9793241fe92f7d9303fb221e43fc598eb065f267 (vlan: Precise RX
> >> stats accounting)
> >> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=9793241fe92f7d9303fb221e43fc598eb065f267
> >
> > Reverting just that commit gets the system running correctly.
> >
> > Bruno
>
> I have no idea how this patch can break vlan networking.
>
> Your disassembly and .config seems to show your machine is not SMP
>
> Maybe something is broken on UP and alloc_percpu() ?
>
> Could you add a debug in vlan_dev_init()
>
>
> vlan_dev_info(dev)->vlan_rx_stats = alloc_percpu(struct
> vlan_rx_stats); if (!vlan_dev_info(dev)->vlan_rx_stats)
> return -ENOMEM;
> + pr_err("vlan_dev_init() rx_stats=%p\n",
> vlan_dev_info(dev)->vlan_rx_stats);
>
>
> This make sure vlan_dev_init() is called and percpu allocation is
> properly done.
>
> Thanks
Readding your "precise RX stats" commit with following additional changes I get
(first hunk to avoid crash):
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index b788978..9216a98 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -165,8 +165,11 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
rx_stats = per_cpu_ptr(vlan_dev_info(dev)->vlan_rx_stats,
smp_processor_id());
- rx_stats->rx_packets++;
- rx_stats->rx_bytes += skb->len;
+ if (rx_stats) {
+ rx_stats->rx_packets++;
+ rx_stats->rx_bytes += skb->len;
+ } else
+ pr_err("vlan_skb_recv() rx_stats=%p -> %p\n",
vlan_dev_info(dev)->vlan_rx_stats, rx_stats);
skb_pull_rcsum(skb, VLAN_HLEN);
@@ -738,6 +741,7 @@ static int vlan_dev_init(struct net_device *dev)
vlan_dev_info(dev)->vlan_rx_stats = alloc_percpu(struct
vlan_rx_stats); if (!vlan_dev_info(dev)->vlan_rx_stats)
return -ENOMEM;
+ pr_err("vlan_dev_init() rx_stats=%p\n",
vlan_dev_info(dev)->vlan_rx_stats);
return 0;
}
...
[ 13.877610] Ending clean XFS mount for filesystem: loop0
[ 15.795601] Adding 1004020k swap on /dev/sda5. Priority:-1 extents:1 across:1004020k
[ 16.612143] ADDRCONF(NETDEV_UP): lan: link is not ready
[ 16.851846] 802.1Q VLAN Support v1.8 Ben Greear <greearb@candelatech.com>
[ 16.851856] All bugs added by David S. Miller <davem@redhat.com>
[ 16.878049] vlan_dev_init() rx_stats=dceae290
[ 20.040395] b44: lan: Link is up at 100 Mbps, full duplex.
[ 20.040404] b44: lan: Flow control is off for TX and off for RX.
[ 20.040535] ADDRCONF(NETDEV_CHANGE): lan: link becomes ready
[ 25.019941] RPC: Registered udp transport module.
[ 25.019950] RPC: Registered tcp transport module.
[ 25.019956] RPC: Registered tcp NFSv4.1 backchannel transport module.
[ 25.382400] svc: failed to register lockdv1 RPC service (errno 97).
[ 25.385278] mount.nfs used greatest stack depth: 1012 bytes left
[ 25.872973] squashfs: version 4.0 (2009/01/31) Phillip Lougher
[ 26.740561] vlan_skb_recv() rx_stats=(null) -> (null)
[ 26.775071] vlan_skb_recv() rx_stats=(null) -> (null)
[ 27.050223] lan.658: no IPv6 routers present
[ 58.357052] vlan_skb_recv() rx_stats=(null) -> (null)
[ 63.350334] vlan_skb_recv() rx_stats=(null) -> (null)
[ 91.767589] vlan_skb_recv() rx_stats=(null) -> (null)
[ 91.932344] vlan_skb_recv() rx_stats=(null) -> (null)
[ 91.933053] vlan_skb_recv() rx_stats=(null) -> (null)
[ 91.998628] vlan_skb_recv() rx_stats=(null) -> (null)
[ 92.002752] vlan_skb_recv() rx_stats=(null) -> (null)
[ 92.007918] vlan_skb_recv() rx_stats=(null) -> (null)
[ 92.009644] vlan_skb_recv() rx_stats=(null) -> (null)
[ 92.016789] vlan_skb_recv() rx_stats=(null) -> (null)
[ 92.018520] vlan_skb_recv() rx_stats=(null) -> (null)
[ 92.041737] vlan_skb_recv() rx_stats=(null) -> (null)
...
So during vlan_dev_init() rx_stats is properly initialized, but when
packets reach the vlan dev later on exactly that same pointer is NULL...
So question is, who did reset vlan_dev_info(dev)->vlan_rx_stats to NULL?
Thanks,
Bruno
next prev parent reply other threads:[~2010-01-24 15:11 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-23 15:56 [2.6.33-rc5 regression] NULL pointer dereference in vlan_skb_recv - probably introduced by commit 9793241fe92f7d9303fb221e43fc598eb065f267 Bruno Prémont
2010-01-23 21:31 ` Bruno Prémont
2010-01-24 13:42 ` Eric Dumazet
2010-01-24 15:02 ` Bruno Prémont [this message]
2010-01-24 15:25 ` Bruno Prémont
2010-01-24 16:25 ` Américo Wang
2010-01-24 16:27 ` Eric Dumazet
2010-01-24 16:26 ` Eric Dumazet
2010-01-24 19:18 ` Bruno Prémont
2010-01-24 19:43 ` [PATCH] vlan: fix vlan_skb_recv() Eric Dumazet
2010-01-25 3:52 ` David Miller
2010-01-24 7:12 ` [2.6.33-rc5 regression] NULL pointer dereference in vlan_skb_recv - probably introduced by commit 9793241fe92f7d9303fb221e43fc598eb065f267 Américo Wang
2010-01-24 7:17 ` Alexey Dobriyan
2010-01-24 7:30 ` Américo Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100124160228.366f4e72@neptune.home \
--to=bonbons@linux-vserver.org \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).