| Message ID | 20210606200551.1609521-1-olek2@wp.pl |
|---|---|
| State | New |
| Headers | show |
| Series | [net] lantiq: net: fix duplicated skb in rx descriptor ring | expand |
On 6/6/21 10:05 PM, Aleksander Jan Bajkowski wrote: > 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 <olek2@wp.pl> > --- > 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); >
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; 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);
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 <olek2@wp.pl> --- drivers/net/ethernet/lantiq_xrx200.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)