Re: [PATCH 2.5.70] e100 initialize fields prior to register_netdevice

["Feldman, Scott" <scott.feldman@xxxxxxxxx>]

On Thu, 29 May 2003, Jeff Garzik wrote:

> 
> Stephen Hemminger wrote:
[cut]
> > This patch moves the initialization earlier before registration.  It
> > probably closes other races as well, where some program could access 
> > the e100 driver before the pointers were setup.
> 
> 
> Agreed.  I consider this a rather serious bug, that could cause crashes
> in multiple corner cases; sysfs being only one of those cases.
> 
> I'm going to go ahead and apply this to 2.5.
> 
> Stephen, could I get you to send me a similar patch for 2.4?
> (it doesn't apply cleanly)  It's very much a bug in 2.4 also.
> 
>         Jeff

Wait!  More work is required, otherwise VLANs, checksum offloading and SG
support are broken.  Here's a patch that applies on top of Stephen's to
address this.  USE_IPCB needs to be defined before filling out
netdev->features.

Again, not an issue for 2.4 e100 driver.

-scott

--- linux-2.5.70/drivers/net/e100/e100_main.c.orig      2003-05-29 
16:40:00.000000000 -0700
+++ linux-2.5.70/drivers/net/e100/e100_main.c   2003-05-29 16:40:27.000000000 
-0700
@@ -614,26 +614,6 @@
                goto err_dealloc;
        }
 
-       dev->vlan_rx_register = e100_vlan_rx_register;
-       dev->vlan_rx_add_vid = e100_vlan_rx_add_vid;
-       dev->vlan_rx_kill_vid = e100_vlan_rx_kill_vid;
-       dev->irq = pcid->irq;
-       dev->open = &e100_open;
-       dev->hard_start_xmit = &e100_xmit_frame;
-       dev->stop = &e100_close;
-       dev->change_mtu = &e100_change_mtu;
-       dev->get_stats = &e100_get_stats;
-       dev->set_multicast_list = &e100_set_multi;
-       dev->set_mac_address = &e100_set_mac;
-       dev->do_ioctl = &e100_ioctl;
-       if (bdp->flags & USE_IPCB)
-               dev->features = NETIF_F_SG | NETIF_F_HW_CSUM |
-                               NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
-
-       if ((rc = register_netdev(dev)) != 0) {
-               goto err_pci;
-       }
-
        if (((bdp->pdev->device > 0x1030)
               && (bdp->pdev->device < 0x103F))
            || ((bdp->pdev->device >= 0x1050)
@@ -657,6 +637,27 @@
        } else {
                bdp->rfd_size = 16;
        }
+
+       dev->vlan_rx_register = e100_vlan_rx_register;
+       dev->vlan_rx_add_vid = e100_vlan_rx_add_vid;
+       dev->vlan_rx_kill_vid = e100_vlan_rx_kill_vid;
+       dev->irq = pcid->irq;
+       dev->open = &e100_open;
+       dev->hard_start_xmit = &e100_xmit_frame;
+       dev->stop = &e100_close;
+       dev->change_mtu = &e100_change_mtu;
+       dev->get_stats = &e100_get_stats;
+       dev->set_multicast_list = &e100_set_multi;
+       dev->set_mac_address = &e100_set_mac;
+       dev->do_ioctl = &e100_ioctl;
+       if (bdp->flags & USE_IPCB)
+               dev->features = NETIF_F_SG | NETIF_F_HW_CSUM |
+                               NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
+
+       if ((rc = register_netdev(dev)) != 0) {
+               goto err_pci;
+       }
+
        e100_check_options(e100nics, bdp);
 
        if (!e100_init(bdp)) {