| From: Anssi Hannula <anssi.hannula@bitwise.fi> |
| Date: Mon, 17 Dec 2018 15:05:41 +0200 |
| Subject: net: macb: add missing barriers when reading descriptors |
| |
| commit 6e0af298066f3b6d99f58989bb0dca6f764b4c6d upstream. |
| |
| When reading buffer descriptors on RX or on TX completion, an |
| RX_USED/TX_USED bit is checked first to ensure that the descriptors have |
| been populated, i.e. the ownership has been transferred. However, there |
| are no memory barriers to ensure that the data protected by the |
| RX_USED/TX_USED bit is up-to-date with respect to that bit. |
| |
| Specifically: |
| |
| - TX timestamp descriptors may be loaded before ctrl is loaded for the |
| TX_USED check, which is racy as the descriptors may be updated between |
| the loads, causing old timestamp descriptor data to be used. |
| |
| - RX ctrl may be loaded before addr is loaded for the RX_USED check, |
| which is racy as a new frame may be written between the loads, causing |
| old ctrl descriptor data to be used. |
| This issue exists for both macb_rx() and gem_rx() variants. |
| |
| Fix the races by adding DMA read memory barriers on those paths and |
| reordering the reads in macb_rx(). |
| |
| I have not observed any actual problems in practice caused by these |
| being missing, though. |
| |
| Tested on a ZynqMP based system. |
| |
| Fixes: 89e5785fc8a6 ("[PATCH] Atmel MACB ethernet driver") |
| Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi> |
| Cc: Nicolas Ferre <nicolas.ferre@microchip.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| [bwh: Backported to 3.16: |
| - Use rmb() instead of dma_rmb() |
| - Drop PTP changes |
| - Adjust filename, context] |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| --- a/drivers/net/ethernet/cadence/macb.c |
| +++ b/drivers/net/ethernet/cadence/macb.c |
| @@ -691,11 +691,15 @@ static int gem_rx(struct macb *bp, int b |
| rmb(); |
| |
| addr = desc->addr; |
| - ctrl = desc->ctrl; |
| |
| if (!(addr & MACB_BIT(RX_USED))) |
| break; |
| |
| + /* Ensure ctrl is at least as up-to-date as rxused */ |
| + rmb(); |
| + |
| + ctrl = desc->ctrl; |
| + |
| bp->rx_tail++; |
| count++; |
| |
| @@ -838,11 +842,15 @@ static int macb_rx(struct macb *bp, int |
| rmb(); |
| |
| addr = desc->addr; |
| - ctrl = desc->ctrl; |
| |
| if (!(addr & MACB_BIT(RX_USED))) |
| break; |
| |
| + /* Ensure ctrl is at least as up-to-date as addr */ |
| + rmb(); |
| + |
| + ctrl = desc->ctrl; |
| + |
| if (ctrl & MACB_BIT(RX_SOF)) { |
| if (first_frag != -1) |
| discard_partial_frame(bp, first_frag, tail); |