Skip to content

Commit

Permalink
net: enetc: fix buffer leaks with XDP_TX enqueue rejections
Browse files Browse the repository at this point in the history
If the TX ring is congested, enetc_xdp_tx() returns false for the
current XDP frame (represented as an array of software BDs).

This array of software TX BDs is constructed in enetc_rx_swbd_to_xdp_tx_swbd
from software BDs freshly cleaned from the RX ring. The issue is that we
scrub the RX software BDs too soon, more precisely before we know that
we can enqueue the TX BDs successfully into the TX ring.

If we can't enqueue them (and enetc_xdp_tx returns false), we call
enetc_xdp_drop which attempts to recycle the buffers held by the RX
software BDs. But because we scrubbed those RX BDs already, two things
happen:

(a) we leak their memory
(b) we populate the RX software BD ring with an all-zero rx_swbd
    structure, which makes the buffer refill path allocate more memory.

enetc_refill_rx_ring
-> if (unlikely(!rx_swbd->page))
   -> enetc_new_page

That is a recipe for fast OOM.

Fixes: 7ed2bc8 ("net: enetc: add support for XDP_TX")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Vladimir Oltean authored and David S. Miller committed Apr 17, 2021
1 parent 975acc8 commit 92ff9a6
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions drivers/net/ethernet/freescale/enetc/enetc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1175,9 +1175,7 @@ static void enetc_build_xdp_buff(struct enetc_bdr *rx_ring, u32 bd_status,
}

/* Convert RX buffer descriptors to TX buffer descriptors. These will be
* recycled back into the RX ring in enetc_clean_tx_ring. We need to scrub the
* RX software BDs because the ownership of the buffer no longer belongs to the
* RX ring, so enetc_refill_rx_ring may not reuse rx_swbd->page.
* recycled back into the RX ring in enetc_clean_tx_ring.
*/
static int enetc_rx_swbd_to_xdp_tx_swbd(struct enetc_tx_swbd *xdp_tx_arr,
struct enetc_bdr *rx_ring,
Expand All @@ -1199,7 +1197,6 @@ static int enetc_rx_swbd_to_xdp_tx_swbd(struct enetc_tx_swbd *xdp_tx_arr,
tx_swbd->is_dma_page = true;
tx_swbd->is_xdp_tx = true;
tx_swbd->is_eof = false;
memset(rx_swbd, 0, sizeof(*rx_swbd));
}

/* We rely on caller providing an rx_ring_last > rx_ring_first */
Expand Down Expand Up @@ -1317,6 +1314,17 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring,
tx_ring->stats.xdp_tx += xdp_tx_bd_cnt;
rx_ring->xdp.xdp_tx_in_flight += xdp_tx_bd_cnt;
xdp_tx_frm_cnt++;
/* The XDP_TX enqueue was successful, so we
* need to scrub the RX software BDs because
* the ownership of the buffers no longer
* belongs to the RX ring, and we must prevent
* enetc_refill_rx_ring() from reusing
* rx_swbd->page.
*/
while (orig_i != i) {
rx_ring->rx_swbd[orig_i].page = NULL;
enetc_bdr_idx_inc(rx_ring, &orig_i);
}
}
break;
case XDP_REDIRECT:
Expand Down

0 comments on commit 92ff9a6

Please sign in to comment.