| Message ID | 20210606200551.1609521-1-olek2@wp.pl (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] lantiq: net: fix duplicated skb in rx descriptor ring | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 5 of 5 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 39 this patch: 39 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | fail | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 22 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 32 this patch: 32 |
| netdev/header_inline | success | Link |
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(-)