tg3: Fix potential netpoll crash

Up until now the tg3 driver would call netif_napi_add() for the maximum
number of NAPI instances the driver could use.  The problem is that
netpoll could call tg3_poll() on instances that are not active.  The net
effect is that the driver will crash attempting to dereference
uninitialized pointers.

The fix is to only allocate as many NAPI instances as the driver would
use in tg3_open() and deleted them in tg3_close().

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Reviewed-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 54fff5bb6f9834f2e110ada31058f777335deb1d..982ed44f07521f0ab4b7cb92a737329f408e1db3 100644 (file)
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -752,42 +752,6 @@ static void tg3_int_reenable(struct tg3_napi *tnapi)
                     HOSTCC_MODE_ENABLE | tnapi->coal_now);
 }
 
-static void tg3_napi_disable(struct tg3 *tp)
-{
-       int i;
-
-       for (i = tp->irq_cnt - 1; i >= 0; i--)
-               napi_disable(&tp->napi[i].napi);
-}
-
-static void tg3_napi_enable(struct tg3 *tp)
-{
-       int i;
-
-       for (i = 0; i < tp->irq_cnt; i++)
-               napi_enable(&tp->napi[i].napi);
-}
-
-static inline void tg3_netif_stop(struct tg3 *tp)
-{
-       tp->dev->trans_start = jiffies; /* prevent tx timeout */
-       tg3_napi_disable(tp);
-       netif_tx_disable(tp->dev);
-}
-
-static inline void tg3_netif_start(struct tg3 *tp)
-{
-       /* NOTE: unconditional netif_tx_wake_all_queues is only
-        * appropriate so long as all callers are assured to
-        * have free tx slots (such as after tg3_init_hw)
-        */
-       netif_tx_wake_all_queues(tp->dev);
-
-       tg3_napi_enable(tp);
-       tp->napi[0].hw_status->status |= SD_STATUS_UPDATED;
-       tg3_enable_ints(tp);
-}
-
 static void tg3_switch_clocks(struct tg3 *tp)
 {
        u32 clock_ctrl;
@@ -4338,6 +4302,11 @@ static int tg3_setup_phy(struct tg3 *tp, int force_reset)
        return err;
 }
 
+static inline int tg3_irq_sync(struct tg3 *tp)
+{
+       return tp->irq_sync;
+}
+
 /* This is called whenever we suspect that the system chipset is re-
  * ordering the sequence of MMIO to the tx send mailbox. The symptom
  * is bogus tx completions. We try to recover by setting the
@@ -5083,6 +5052,59 @@ tx_recovery:
        return work_done;
 }
 
+static void tg3_napi_disable(struct tg3 *tp)
+{
+       int i;
+
+       for (i = tp->irq_cnt - 1; i >= 0; i--)
+               napi_disable(&tp->napi[i].napi);
+}
+
+static void tg3_napi_enable(struct tg3 *tp)
+{
+       int i;
+
+       for (i = 0; i < tp->irq_cnt; i++)
+               napi_enable(&tp->napi[i].napi);
+}
+
+static void tg3_napi_init(struct tg3 *tp)
+{
+       int i;
+
+       netif_napi_add(tp->dev, &tp->napi[0].napi, tg3_poll, 64);
+       for (i = 1; i < tp->irq_cnt; i++)
+               netif_napi_add(tp->dev, &tp->napi[i].napi, tg3_poll_msix, 64);
+}
+
+static void tg3_napi_fini(struct tg3 *tp)
+{
+       int i;
+
+       for (i = 0; i < tp->irq_cnt; i++)
+               netif_napi_del(&tp->napi[i].napi);
+}
+
+static inline void tg3_netif_stop(struct tg3 *tp)
+{
+       tp->dev->trans_start = jiffies; /* prevent tx timeout */
+       tg3_napi_disable(tp);
+       netif_tx_disable(tp->dev);
+}
+
+static inline void tg3_netif_start(struct tg3 *tp)
+{
+       /* NOTE: unconditional netif_tx_wake_all_queues is only
+        * appropriate so long as all callers are assured to
+        * have free tx slots (such as after tg3_init_hw)
+        */
+       netif_tx_wake_all_queues(tp->dev);
+
+       tg3_napi_enable(tp);
+       tp->napi[0].hw_status->status |= SD_STATUS_UPDATED;
+       tg3_enable_ints(tp);
+}
+
 static void tg3_irq_quiesce(struct tg3 *tp)
 {
        int i;
@@ -5096,11 +5118,6 @@ static void tg3_irq_quiesce(struct tg3 *tp)
                synchronize_irq(tp->napi[i].irq_vec);
 }
 
-static inline int tg3_irq_sync(struct tg3 *tp)
-{
-       return tp->irq_sync;
-}
-
 /* Fully shutdown all tg3 driver activity elsewhere in the system.
  * If irq_sync is non-zero, then the IRQ handler must be synchronized
  * with as well.  Most of the time, this is not necessary except when
@@ -8920,6 +8937,8 @@ static int tg3_open(struct net_device *dev)
        if (err)
                goto err_out1;
 
+       tg3_napi_init(tp);
+
        tg3_napi_enable(tp);
 
        for (i = 0; i < tp->irq_cnt; i++) {
@@ -9007,6 +9026,7 @@ err_out3:
 
 err_out2:
        tg3_napi_disable(tp);
+       tg3_napi_fini(tp);
        tg3_free_consistent(tp);
 
 err_out1:
@@ -9054,6 +9074,8 @@ static int tg3_close(struct net_device *dev)
        memcpy(&tp->estats_prev, tg3_get_estats(tp),
               sizeof(tp->estats_prev));
 
+       tg3_napi_fini(tp);
+
        tg3_free_consistent(tp);
 
        tg3_set_power_state(tp, PCI_D3hot);
@@ -14604,13 +14626,10 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
                tnapi->consmbox = rcvmbx;
                tnapi->prodmbox = sndmbx;
 
-               if (i) {
+               if (i)
                        tnapi->coal_now = HOSTCC_MODE_COAL_VEC1_NOW << (i - 1);
-                       netif_napi_add(dev, &tnapi->napi, tg3_poll_msix, 64);
-               } else {
+               else
                        tnapi->coal_now = HOSTCC_MODE_NOW;
-                       netif_napi_add(dev, &tnapi->napi, tg3_poll, 64);
-               }
 
                if (!(tp->tg3_flags & TG3_FLAG_SUPPORT_MSIX))
                        break;