[PATCH net v3 2/5] eth: fbnic: validate the Rx completion offset and length

[Yehyeong Lee]

fbnic_pkt_prepare() and fbnic_add_rx_frag() take the offset and length
from the completion descriptor (FBNIC_RCD_AL_BUFF_OFF_MASK and _LEN_MASK)
and use them to place data in the fragment page without checking them. A
device that reports an offset below the reserved headroom, or a length
that runs past the 4K fragment, produces a negative headroom and a frame
that extends beyond the page, which is then read out past the page into
adjacent memory.

Reject a header whose offset is below FBNIC_RX_HROOM or whose offset plus
length exceeds the fragment, and a payload whose offset plus length
exceeds the fragment, via the drop path. Clamp the derived frame end and
truesize to the fragment so a FIN descriptor cannot push them past the
page. A maximum-size header (offset 192, length 3584) stays inside the
fragment and is still accepted.

  BUG: unable to handle page fault for address: ffffed1021a8ee10
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  Oops: Oops: 0000 [#1] SMP KASAN NOPTI
  RIP: 0010:kasan_check_range+0x15e/0x1b0
  Call Trace:
   <TASK>
   __asan_memset+0x23/0x50
   __build_skb_around+0x222/0x320
   napi_build_skb+0x50/0x1e0
   xdp_build_skb_from_buff+0xda/0x8b0
   fbnic_run_xdp+0x29a/0x1040 [fbnic]

An offset and length that keep the frame inside the page but past the
valid data produce no splat: the read lands in adjacent page_pool memory,
which carries no KASAN redzone, and is copied into the skb silently.

Fixes: a29b8eb6e533 ("eth: fbnic: Add basic Rx handling")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx>
---
 drivers/net/ethernet/meta/fbnic/fbnic_txrx.c | 41 ++++++++++++++++++--
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c b/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
index 305fe6d2f511f..7dda59bba14ee 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
@@ -1037,13 +1037,34 @@ static void fbnic_pkt_prepare(struct fbnic_napi_vector *nv, u64 rcd,
 		return;
 	}
 
-	page = fbnic_page_pool_get_head(qt, hdr_pg_idx);
+	/* The completion describes one frame inside the one
+	 * FBNIC_BD_FRAG_SIZE fragment it names.  fbnic_hdr_pg_start() takes
+	 * the headroom back out of the offset, so an offset below the
+	 * headroom underflows it, and the frame data runs to
+	 * hdr_pg_off + len, which has to stay inside the fragment.
+	 */
+	if (unlikely(hdr_pg_off < FBNIC_RX_HROOM ||
+		     hdr_pg_off + len > FBNIC_BD_FRAG_SIZE)) {
+		pkt->add_frag_failed = true;
+		return;
+	}
 
 	/* Short-cut the end calculation if we know page is fully consumed */
 	hdr_pg_end = FIELD_GET(FBNIC_RCD_AL_PAGE_FIN, rcd) ?
 		     FBNIC_BD_FRAG_SIZE : fbnic_hdr_pg_end(hdr_pg_off, len);
 	hdr_pg_start = fbnic_hdr_pg_start(hdr_pg_off);
 
+	/* fbnic_hdr_pg_end() rounds the frame up to where the next header
+	 * would start, which can be past the end of the fragment when this
+	 * frame is the last one in it.  napi_build_skb() puts the shared
+	 * info at the end of the frame, so hold the frame inside the
+	 * fragment, which is what the FBNIC_RCD_AL_PAGE_FIN case above
+	 * already does.
+	 */
+	hdr_pg_end = min(hdr_pg_end, FBNIC_BD_FRAG_SIZE);
+
+	page = fbnic_page_pool_get_head(qt, hdr_pg_idx);
+
 	headroom = hdr_pg_off - hdr_pg_start + FBNIC_RX_PAD;
 	frame_sz = hdr_pg_end - hdr_pg_start;
 	xdp_init_buff(&pkt->buff, frame_sz, &qt->xdp_rxq);
@@ -1091,11 +1112,25 @@ static void fbnic_add_rx_frag(struct fbnic_napi_vector *nv, u64 rcd,
 		return;
 	}
 
-	netmem = fbnic_page_pool_get_data(qt, pg_idx);
-
 	truesize = FIELD_GET(FBNIC_RCD_AL_PAGE_FIN, rcd) ?
 		   FBNIC_BD_FRAG_SIZE - pg_off : ALIGN(len, 128);
 
+	/* The data has to stay inside the FBNIC_BD_FRAG_SIZE fragment the
+	 * completion names.
+	 */
+	if (unlikely(pg_off + len > FBNIC_BD_FRAG_SIZE)) {
+		pkt->add_frag_failed = true;
+		return;
+	}
+
+	/* ALIGN() can round the charged size past the end of the fragment
+	 * when this is the last fragment in it, so hold it inside, which is
+	 * what the FBNIC_RCD_AL_PAGE_FIN case above already does.
+	 */
+	truesize = min(truesize, FBNIC_BD_FRAG_SIZE - pg_off);
+
+	netmem = fbnic_page_pool_get_data(qt, pg_idx);
+
 	pg_off += (FBNIC_RCD_AL_BUFF_FRAG_MASK & rcd) *
 		  FBNIC_BD_FRAG_SIZE;
 
-- 
2.43.0