netdev - [PATCH net 1/7] net: stmmac: fix error in updating rx tail pointer to last free entry

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-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 14 Jan 2020 10:01:10 +0800
From:   Ong Boon Leong <boon.leong.ong@...el.com>
To:     netdev@...r.kernel.org
Cc:     Jose Abreu <Jose.Abreu@...opsys.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        "David S . Miller" <davem@...emloft.net>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Ong Boon Leong <boon.leong.ong@...el.com>,
        Tan Tee Min <tee.min.tan@...el.com>,
        Voon Weifeng <weifeng.voon@...el.com>,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: [PATCH net 1/7] net: stmmac: fix error in updating rx tail pointer to last free entry

DMA_CH(#i)_RxDesc_Tail_Pointer points to an offset from the base and
indicates the location of the last valid descriptor.

The change introduced by "net: stmmac: Update RX Tail Pointer to last
free entry" incorrectly updates the RxDesc_Tail_Pointer and causess
Rx operation to freeze in corner case. The issue is explained as
follow:-

Say, cur_rx=1 and dirty_rx=0, then we have dirty=1 and entry=0 before
the while (dirty-- > 0) loop of stmmac_rx_refill() is entered. When
the while loop is 1st entered, Rx buffer[entry=0] is refilled and after
entry++, then, entry=1. Now, the while loop condition check "dirty-- > 0"
and the while loop bails out because dirty=0. Up to this point, the
driver code works correctly.

However, the current implementation sets the Rx Tail Pointer to the
location pointed by dirty_rx, just updated to the value of entry(=1).
This is incorrect because the last Rx buffer that is refileld with empty
buffer is with entry=0. In another words, the current logics always sets
Rx Tail Pointer to the next Rx buffer to be refilled (too early).

So, we fix this by tracking the index of the most recently refilled Rx
buffer by using "last_refill" and use "last_refill" to update the Rx Tail
Pointer instead of using "entry" which points to the next dirty_rx to be
refilled in future.

Fixes: 858a31ffc3d9 ("net: stmmac: Update RX Tail Pointer to last free entry")

Signed-off-by: Ong Boon Leong <boon.leong.ong@...el.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 80d59b7..a317f67 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3417,6 +3417,7 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
 	struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
 	int len, dirty = stmmac_rx_dirty(priv, queue);
 	unsigned int entry = rx_q->dirty_rx;
+	unsigned int last_refill = entry;

 	len = DIV_ROUND_UP(priv->dma_buf_sz, PAGE_SIZE) * PAGE_SIZE;

@@ -3471,12 +3472,13 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)

 		dma_wmb();
 		stmmac_set_rx_owner(priv, p, use_rx_wd);
-
+		last_refill = entry;
 		entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
 	}
+
 	rx_q->dirty_rx = entry;
 	rx_q->rx_tail_addr = rx_q->dma_rx_phy +
-			    (rx_q->dirty_rx * sizeof(struct dma_desc));
+			     (last_refill * sizeof(struct dma_desc));
 	stmmac_set_rx_tail_ptr(priv, priv->ioaddr, rx_q->rx_tail_addr, queue);
 }

-- 
2.7.4