Bug 679407

Description of problem:

There is no synchronization to protect NIU's get_stats method from
seeing a NULL pointer in either np->rx_rings or np->tx_rings.  In fact,
as far as ->ndo_get_stats is concerned, these values are set completely
asynchronously.

The lack of synchronization causes this patch below:
Unable to handle kernel NULL pointer dereference at 0000000000000058 RIP: 
 [<ffffffff8827f10c>] :niu:niu_get_stats+0x2f/0xa4
PGD 3fbc60067 PUD 3fbc57067 PMD 0 
Oops: 0000 [1] SMP 
last sysfs file: /class/net/eth0/address
CPU 5 
Modules linked in: 8021q iptable_mangle ip_tables x_tables bonding autofs4 hidp rfcomm l2cap bluetooth lockd sunrpc cpufreq_ondemand acpi_cpufreq freq_table mperf loop dm_multipath scsi_dh video backlight sbs power_meter hwmon i2c_ec dell_wmi wmi button battery asus_acpi acpi_memhotplug ac ipv6 xfrm_nalgo crypto_api parport_pc lp parport tpm_infineon joydev sr_mod cdrom i5000_edac tpm_tis i2c_i801 tpm edac_mc tpm_bios i2c_core niu e1000e pcspkr sg serio_raw dm_raid45 dm_message dm_region_hash dm_mem_cache dm_snapshot dm_zero dm_mirror dm_log dm_mod usb_storage ahci ata_piix libata shpchp mptsas mptscsih mptbase scsi_transport_sas sd_mod scsi_mod ext3 jbd uhci_hcd ohci_hcd ehci_hcd
Pid: 6532, comm: ifconfig Not tainted 2.6.18-238.el5.0.DEBUG00413622 #1
RIP: 0010:[<ffffffff8827f10c>]  [<ffffffff8827f10c>] :niu:niu_get_stats+0x2f/0xa4
RSP: 0018:ffff8103fbcf3e90  EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff810418a84000 RCX: 0000000000000001
RDX: ffff810418a84500 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffff810418851740 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000004 R11: 0000000000000000 R12: 000000000000068c
R13: 0000000000001000 R14: 0000000000000000 R15: 00002b7b99cc0000
FS:  00002b7b99cce5e0(0000) GS:ffff81041fc7b4c0(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000000058 CR3: 00000003fbc66000 CR4: 00000000000006e0
Process ifconfig (pid: 6532, threadinfo ffff8103fbcf2000, task ffff8103fc873820)
Stack:  ffffffff802335ed ffff810418851740 ffff810418851740 ffff810418a84000
 ffffffff8003fa30 ffff8103fbcf3f50 ffff8103fce17a80 ffff810418851770
 000000000000000d 000000000000000c ffff8103fce17a80 0000000000001000
Call Trace:
 [<ffffffff802335ed>] dev_seq_show+0x35/0xd0
 [<ffffffff8003fa30>] seq_read+0x1b8/0x28c
 [<ffffffff8000b787>] vfs_read+0xcb/0x171
 [<ffffffff80011c9a>] sys_read+0x45/0x6e
 [<ffffffff8005e28d>] tracesys+0xd5/0xe0


Code: 48 03 78 58 48 03 70 60 4c 03 48 68 4c 03 40 70 44 39 d1 7c 
RIP  [<ffffffff8827f10c>] :niu:niu_get_stats+0x2f/0xa4
 RSP <ffff8103fbcf3e90>

Version-Release number of selected component (if applicable):
kernel-2.6.18-238.el5

How reproducible:
Frequently

Steps to Reproduce:
1. run 'cat /proc/net/dev' in a busy loop to see the stats
2. then run ifup <niu_iface>
  
Actual results:
kernel panic

Additional info:
This has been worked upstream in the following threads:
http://www.spinics.net/lists/netdev/msg154375.html
http://www.spinics.net/lists/netdev/msg154405.html

The result patch is merged upstream and works out for the customer.

commit 9690c636ac118b6662f28308bee817343d9932d8
Author: David S. Miller <davem>
Date:   Thu Feb 3 16:12:50 2011 -0800

    niu: Fix races between up/down and get_stats.
    
    As reported by Flavio Leitner, there is no synchronization to protect
    NIU's get_stats method from seeing a NULL pointer in either
    np->rx_rings or np->tx_rings.  In fact, as far as ->ndo_get_stats
    is concerned, these values are set completely asynchronously.
    
    Flavio attempted to fix this using a RW semaphore, which in fact
    works most of the time.  However, dev_get_stats() can be invoked
    from non-sleepable contexts in some cases, so this fix doesn't
    work in all cases.
    
    So instead, control the visibility of the np->{rx,tx}_ring pointers
    when the device is being brough up, and use properties of the device
    down sequence to our advantage.
    
    In niu_get_stats(), return immediately if netif_running() is false.
    The device shutdown sequence first marks the device as not running (by
    clearing the __LINK_STATE_START bit), then it performans a
    synchronize_rcu() (in dev_deactive_many()), and then finally it
    invokes the driver ->ndo_stop() method.
    
    This guarentees that all invocations of niu_get_stats() either see
    netif_running() as false, or they see the channel pointers before
    ->ndo_stop() clears them out.
    
    If netif_running() is true, protect against startup races by loading
    the np->{rx,tx}_rings pointer into a local variable, and punting if
    it is NULL.  Use ACCESS_ONCE to prevent the compiler from reloading
    the pointer on us.
    
    Also, during open, control the order in which the pointers and the
    ring counts become visible globally using SMP write memory barriers.
    We make sure the np->num_{rx,tx}_rings value is stable and visible
    before np->{rx,tx}_rings is.
    
    Such visibility control is not necessary on the niu_free_channels()
    side because of the RCU sequencing that happens during device down as
    described above.  We are always guarenteed that all niu_get_stats
    calls are finished, or will see netif_running() false, by the time
    ->ndo_stop is invoked.
    
    Reported-by: Flavio Leitner <fleitner>
    Signed-off-by: David S. Miller <davem>