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
| ||
|
Message-ID: <20100124162523.GC11037@hack> Date: Mon, 25 Jan 2010 00:25:23 +0800 From: Américo Wang <xiyou.wangcong@...il.com> To: Bruno Prémont <bonbons@...ux-vserver.org> Cc: Eric Dumazet <eric.dumazet@...il.com>, "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [2.6.33-rc5 regression] NULL pointer dereference in vlan_skb_recv - probably introduced by commit 9793241fe92f7d9303fb221e43fc598eb065f267 On Sun, Jan 24, 2010 at 04:25:49PM +0100, Bruno Prémont wrote: >On Sun, 24 January 2010 Eric Dumazet <eric.dumazet@...il.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 > >Exact > >> Maybe something is broken on UP and alloc_percpu() ? > >Apparently not, see below and previous mail > >> Could you add a debug in vlan_dev_init() > >In addition to previous mail, I'm also dumping the result of >vlan_dev_info(dev) shows that the returned pointer is not the same >during vlan_dev_init() and vlan_skb_recv() ... > >diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c >index b788978..f370ce1 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()); I am thinking if vlan_dev_info(dev) here should be vlan_dev_info(skb->dev)... >- 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() %p->rx_stats=%p -> %p\n", vlan_dev_info(dev), 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() %p->rx_stats=%p\n", vlan_dev_info(dev), vlan_dev_info(dev)->vlan_rx_stats); > > return 0; > } > >This slightly adjusted change produces the following output: >... >[ 1192.257978] ADDRCONF(NETDEV_UP): lan: link is not ready >[ 1192.399059] 802.1Q VLAN Support v1.8 Ben Greear <greearb@...delatech.com> >[ 1192.399063] All bugs added by David S. Miller <davem@...hat.com> >[ 1192.404475] vlan_dev_init() da4ff360->rx_stats=dbd5a340 > ^^^^^^^^ >[ 1196.000225] b44: lan: Link is up at 100 Mbps, full duplex. >[ 1196.000234] b44: lan: Flow control is off for TX and off for RX. >[ 1196.000379] ADDRCONF(NETDEV_CHANGE): lan: link becomes ready >[ 1203.160226] lan.658: no IPv6 routers present >[ 1212.760561] vlan_skb_recv() ddbb8b60->rx_stats=(null) -> (null) > ^^^^^^^^ >[ 1212.794961] vlan_skb_recv() ddbb8b60->rx_stats=(null) -> (null) >[ 1219.247018] svc: failed to register lockdv1 RPC service (errno 97). >[ 1219.249919] mount.nfs used greatest stack depth: 1008 bytes left >[ 1221.388602] vlan_skb_recv() ddbb8b60->rx_stats=(null) -> (null) >[ 1221.388690] vlan_skb_recv() ddbb8b60->rx_stats=(null) -> (null) >[ 1278.793350] vlan_skb_recv() ddbb8b60->rx_stats=(null) -> (null) >[ 1283.750363] vlan_skb_recv() ddbb8b60->rx_stats=(null) -> (null) >... > >This might explain the NULL rx_stats pointer, but why do there exist >two distinct vlan_dev_info(dev)? (unless in one case dev would be >the physical network device and in the other case it would be vlan device? >that is lan versus lan.658 in my case...) > >Thanks, >Bruno >-- >To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >the body of a message to majordomo@...r.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html >Please read the FAQ at http://www.tux.org/lkml/ -- Live like a child, think like the god. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists