netdev - [PATCH] tg3: driver sleeps indefinitely when EEH errors exceed eeh_max

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20200615190119.382589-1-drc@linux.vnet.ibm.com>
Date:   Mon, 15 Jun 2020 12:01:19 -0700
From:   David Christensen <drc@...ux.vnet.ibm.com>
To:     netdev@...r.kernel.org,
        Siva Reddy Kallam <siva.kallam@...adcom.com>,
        Prashant Sreedharan <prashant@...adcom.com>,
        Michael Chan <mchan@...adcom.com>
Cc:     linux-kernel@...r.kernel.org,
        David Christensen <drc@...ux.vnet.ibm.com>
Subject: [PATCH] tg3: driver sleeps indefinitely when EEH errors exceed eeh_max_freezes

The driver function tg3_io_error_detected() calls napi_disable twice,
without an intervening napi_enable, when the number of EEH errors exceeds
eeh_max_freezes, resulting in an indefinite sleep while holding rtnl_lock.

The function is called once with the PCI state pci_channel_io_frozen and
then called again with the state pci_channel_io_perm_failure when the
number of EEH failures in an hour exceeds eeh_max_freezes.

Protecting the calls to napi_enable/napi_disable with a new state
variable prevents the long sleep.

Signed-off-by: David Christensen <drc@...ux.vnet.ibm.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 18 +++++++++++++++---
 drivers/net/ethernet/broadcom/tg3.h |  1 +
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 7a3b22b35238..953f535e0ceb 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -7373,22 +7373,34 @@ static void tg3_napi_disable(struct tg3 *tp)
 {
 	int i;
 
+	if (!tp->napi_enabled)
+		return;
+
 	for (i = tp->irq_cnt - 1; i >= 0; i--)
 		napi_disable(&tp->napi[i].napi);
+
+	tp->napi_enabled = false;
 }
 
 static void tg3_napi_enable(struct tg3 *tp)
 {
 	int i;
 
+	if (tp->napi_enabled)
+		return;
+
 	for (i = 0; i < tp->irq_cnt; i++)
 		napi_enable(&tp->napi[i].napi);
+
+	tp->napi_enabled = true;
 }
 
 static void tg3_napi_init(struct tg3 *tp)
 {
 	int i;
 
+	tp->napi_enabled = false;
+
 	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);
@@ -7400,6 +7412,8 @@ static void tg3_napi_fini(struct tg3 *tp)
 
 	for (i = 0; i < tp->irq_cnt; i++)
 		netif_napi_del(&tp->napi[i].napi);
+
+	tp->napi_enabled = false;
 }
 
 static inline void tg3_netif_stop(struct tg3 *tp)
@@ -18194,10 +18208,8 @@ static pci_ers_result_t tg3_io_error_detected(struct pci_dev *pdev,
 
 done:
 	if (state == pci_channel_io_perm_failure) {
-		if (netdev) {
-			tg3_napi_enable(tp);
+		if (netdev)
 			dev_close(netdev);
-		}
 		err = PCI_ERS_RESULT_DISCONNECT;
 	} else {
 		pci_disable_device(pdev);
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index 6953d0546acb..0681f4b9ec79 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -3430,6 +3430,7 @@ struct tg3 {
 	u32                             ape_hb;
 	unsigned long                   ape_hb_interval;
 	unsigned long                   ape_hb_jiffies;
+	bool				napi_enabled;
 };
 
 /* Accessor macros for chip and asic attributes
-- 
2.18.2