• David Arinzon's avatar
    net: ena: Fix DMA syncing in XDP path when SWIOTLB is on · d7601170
    David Arinzon authored
    This patch fixes two issues:
    
    Issue 1
    -------
    Description
    ```````````
    Current code does not call dma_sync_single_for_cpu() to sync data from
    the device side memory to the CPU side memory before the XDP code path
    uses the CPU side data.
    This causes the XDP code path to read the unset garbage data in the CPU
    side memory, resulting in incorrect handling of the packet by XDP.
    
    Solution
    ````````
    1. Add a call to dma_sync_single_for_cpu() before the XDP code starts to
       use the data in the CPU side memory.
    2. The XDP code verdict can be XDP_PASS, in which case there is a
       fallback to the non-XDP code, which also calls
       dma_sync_single_for_cpu().
       To avoid calling dma_sync_single_for_cpu() twice:
    2.1. Put the dma_sync_single_for_cpu() in the code in such a place where
         it happens before XDP and non-XDP code.
    2.2. Remove the calls to dma_sync_single_for_cpu() in the non-XDP code
         for the first buffer only (rx_copybreak and non-rx_copybreak
         cases), since the new call that was added covers these cases.
         The call to dma_sync_single_for_cpu() for the second buffer and on
         stays because only the first buffer is handled by the newly added
         dma_sync_single_for_cpu(). And there is no need for special
         handling of the second buffer and on for the XDP path since
         currently the driver supports only single buffer packets.
    
    Issue 2
    -------
    Description
    ```````````
    In case the XDP code forwarded the packet (ENA_XDP_FORWARDED),
    ena_unmap_rx_buff_attrs() is called with attrs set to 0.
    This means that before unmapping the buffer, the internal function
    dma_unmap_page_attrs() will also call dma_sync_single_for_cpu() on
    the whole buffer (not only on the data part of it).
    This sync is both wasteful (since a sync was already explicitly
    called before) and also causes a bug, which will be explained
    using the below diagram.
    
    The following diagram shows the flow of events causing the bug.
    The order of events is (1)-(4) as shown in the diagram.
    
    CPU side memory area
    
         (3)convert_to_xdp_frame() initializes the
            headroom with xdpf metadata
                          ||
                          \/
              ___________________________________
             |                                   |
     0       |                                   V                       4K
     ---------------------------------------------------------------------
     | xdpf->data      | other xdpf       |   < data >   | tailroom ||...|
     |                 | fields           |              | GARBAGE  ||   |
     ---------------------------------------------------------------------
    
                       /\                        /\
                       ||                        ||
       (4)ena_unmap_rx_buff_attrs() calls     (2)dma_sync_single_for_cpu()
          dma_sync_single_for_cpu() on the       copies data from device
          whole buffer page, overwriting         side to CPU side memory
          the xdpf->data with GARBAGE.           ||
     0                                                                   4K
     ---------------------------------------------------------------------
     | headroom                           |   < data >   | tailroom ||...|
     | GARBAGE                            |              | GARBAGE  ||   |
     ---------------------------------------------------------------------
    
    Device side memory area                      /\
                                                 ||
                                   (1) device writes RX packet data
    
    After the call to ena_unmap_rx_buff_attrs() in (4), the xdpf->data
    becomes corrupted, and so when it is later accessed in
    ena_clean_xdp_irq()->xdp_return_frame(), it causes a page fault,
    crashing the kernel.
    
    Solution
    ````````
    Explicitly tell ena_unmap_rx_buff_attrs() not to call
    dma_sync_single_for_cpu() by passing it the ENA_DMA_ATTR_SKIP_CPU_SYNC
    flag.
    
    Fixes: f7d625ad ("net: ena: Add dynamic recycling mechanism for rx buffers")
    Signed-off-by: default avatarArthur Kiyanovski <akiyano@amazon.com>
    Signed-off-by: default avatarDavid Arinzon <darinzon@amazon.com>
    Link: https://lore.kernel.org/r/20231211062801.27891-4-darinzon@amazon.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    d7601170
ena_netdev.c 129 KB