lkml.org 
[lkml]   [2010]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [2.6.33-rc5 regression] NULL pointer dereference in vlan_skb_recv - probably introduced by commit 9793241fe92f7d9303fb221e43fc598eb065f267
    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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-01-24 16:13    [W:0.028 / U:0.340 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site