netdev - [PATCH] net: netsec: replace cpu_relax() with timeout handling for register checks

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]

Date: Fri, 17 Nov 2023 17:10:02 +0900
From: Ryosuke Saito <ryosuke.saito@...aro.org>
To: jaswinder.singh@...aro.org,
	ilias.apalodimas@...aro.org,
	davem@...emloft.net,
	edumazet@...gle.com,
	kuba@...nel.org,
	pabeni@...hat.com,
	masahisa.kojima@...aro.org
Cc: netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] net: netsec: replace cpu_relax() with timeout handling for register checks

The cpu_relax() loops have the potential to hang if the specified
register bits are not met on condition. The patch replaces it with
usleep_range() and netsec_wait_while_busy() which includes timeout
logic.

Additionally, if the error condition is met during interrupting DMA
transfer, there's no recovery mechanism available. In that case, any
frames being sent or received will be discarded, which leads to
potential frame loss as indicated in the comments.

Signed-off-by: Ryosuke Saito <ryosuke.saito@...aro.org>
---
 drivers/net/ethernet/socionext/netsec.c | 35 ++++++++++++++++---------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 0dcd6a568b06..6f9127d30a9a 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -1410,21 +1410,28 @@ static int netsec_reset_hardware(struct netsec_priv *priv,
 		netsec_write(priv, NETSEC_REG_DMA_MH_CTRL,
 			     NETSEC_DMA_CTRL_REG_STOP);
 
-		while (netsec_read(priv, NETSEC_REG_DMA_HM_CTRL) &
-		       NETSEC_DMA_CTRL_REG_STOP)
-			cpu_relax();
-
-		while (netsec_read(priv, NETSEC_REG_DMA_MH_CTRL) &
-		       NETSEC_DMA_CTRL_REG_STOP)
-			cpu_relax();
+		if (netsec_wait_while_busy(priv, NETSEC_REG_DMA_HM_CTRL,
+					   NETSEC_DMA_CTRL_REG_STOP) ||
+		    netsec_wait_while_busy(priv, NETSEC_REG_DMA_MH_CTRL,
+					   NETSEC_DMA_CTRL_REG_STOP)) {
+			dev_warn(priv->dev,
+				 "%s: DMA transfer cannot be stopped.\n",
+				 __func__);
+			/* There is no recovery mechanism in place if this
+			 * error occurs. Frames may be lost.
+			 */
+		}
 	}
 
 	netsec_write(priv, NETSEC_REG_SOFT_RST, NETSEC_SOFT_RST_REG_RESET);
 	netsec_write(priv, NETSEC_REG_SOFT_RST, NETSEC_SOFT_RST_REG_RUN);
 	netsec_write(priv, NETSEC_REG_COM_INIT, NETSEC_COM_INIT_REG_ALL);
 
-	while (netsec_read(priv, NETSEC_REG_COM_INIT) != 0)
-		cpu_relax();
+	if (netsec_wait_while_busy(priv, NETSEC_REG_COM_INIT, 1)) {
+		dev_err(priv->dev,
+			"%s: failed to reset NETSEC.\n", __func__);
+		return -ETIMEDOUT;
+	}
 
 	/* set desc_start addr */
 	netsec_write(priv, NETSEC_REG_NRM_RX_DESC_START_UP,
@@ -1476,9 +1483,13 @@ static int netsec_reset_hardware(struct netsec_priv *priv,
 	netsec_write(priv, NETSEC_REG_DMA_MH_CTRL, MH_CTRL__MODE_TRANS);
 	netsec_write(priv, NETSEC_REG_PKT_CTRL, value);
 
-	while ((netsec_read(priv, NETSEC_REG_MODE_TRANS_COMP_STATUS) &
-		NETSEC_MODE_TRANS_COMP_IRQ_T2N) == 0)
-		cpu_relax();
+	usleep_range(100000, 120000);
+
+	if ((netsec_read(priv, NETSEC_REG_MODE_TRANS_COMP_STATUS) &
+			 NETSEC_MODE_TRANS_COMP_IRQ_T2N) == 0) {
+		dev_warn(priv->dev,
+			 "%s: trans comp timeout.\n", __func__);
+	}
 
 	/* clear any pending EMPTY/ERR irq status */
 	netsec_write(priv, NETSEC_REG_NRM_TX_STATUS, ~0);
-- 
2.34.1