Re: [PATCH net] lantiq: net: fix duplicated skb in rx descriptor ring
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: 2021-06-06 22:25:44
Also in:
lkml
On 6/6/21 10:05 PM, Aleksander Jan Bajkowski wrote:
quoted hunk ↗ jump to hunk
The previous commit didn't fix the bug properly. By mistake, it replaces the pointer of the next skb in the descriptor ring instead of the current one. As a result, the two descriptors are assigned the same SKB. The error is seen during the iperf test when skb_put tries to insert a second packet and exceeds the available buffer. Fixes: c7718ee96dbc ("net: lantiq: fix memory corruption in RX ring ") Signed-off-by: Aleksander Jan Bajkowski <redacted> --- drivers/net/ethernet/lantiq_xrx200.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c index 36dc3e5f6218..e710f83b3700 100644 --- a/drivers/net/ethernet/lantiq_xrx200.c +++ b/drivers/net/ethernet/lantiq_xrx200.c@@ -193,17 +193,18 @@ static int xrx200_hw_receive(struct xrx200_chan *ch) int ret; ret = xrx200_alloc_skb(ch); - - ch->dma.desc++; - ch->dma.desc %= LTQ_DESC_NUM; - if (ret) { ch->skb[ch->dma.desc] = skb; net_dev->stats.rx_dropped++; + ch->dma.desc++; + ch->dma.desc %= LTQ_DESC_NUM;
I would prefer if you handle this problem in the xrx200_alloc_skb() function. You could also provide the skb to xrx200_alloc_skb() and use it in case a problem occurs, you would loose the current received packet, but the DMA ring stays valid. An other solution would be to not set LTQ_DMA_OWN, then the DMA hardware will not use this memory, but we have to handle it somewhere else. I am also not sure if the wmb() is needed, the desc_base was allocated with dma_alloc_coherent(), I assume that the changes are directly written to the memory and not reordered.
netdev_err(net_dev, "failed to allocate new rx buffer\n"); return ret; } + ch->dma.desc++; + ch->dma.desc %= LTQ_DESC_NUM; + skb_put(skb, len); skb->protocol = eth_type_trans(skb, net_dev); netif_receive_skb(skb);
Attachments
- OpenPGP_signature [application/pgp-signature] 488 bytes