[PATCH ath-next 1/2] wifi: ath11k: copy the first fragment's descriptor before queuing it

[Adriel Santos]

ath11k_dp_rx_frag_h_mpdu() queues a received fragment on rx_tid->rx_frags
and sets its bit in rx_frag_bitmap before, for fragment 0, it copies the
REO destination descriptor with kmemdup(GFP_ATOMIC). If that copy fails,
the function returns -ENOMEM and its caller,
ath11k_dp_process_rx_err_buf(), frees the msdu and returns its link
descriptor while the msdu is still on rx_frags:

- the next flush of rx_frags (a new sequence number, the fragment timer,
  peer teardown or a key install) frees the skb a second time;
- if the remaining fragments of the MPDU arrive first, the bitmap is
  complete, ath11k_dp_rx_h_defrag() walks the list with the freed skb,
  and ath11k_dp_rx_h_defrag_reo_reinject() dereferences the NULL
  rx_tid->dst_ring_desc.

This has not been seen in the field; it was reproduced only by forcing
the allocation to fail.

Copy the descriptor before the fragment is queued, so a failure leaves
nothing of this fragment in rx_tid and the caller's free is the only
one. Fragments other than 0 still return their link descriptor after
being queued.

Found during an LLM-assisted review of the RX defragmentation path; the
fix and its testing were LLM-assisted as well.

Tested-on: IPQ5018 hw1.0 AHB WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1

Fixes: 243874c64c81 ("ath11k: handle RX fragments")
Signed-off-by: Adriel Santos <adriel at adriel.eu>
---

Notes:
    How this was tested, and what was not done:
    
    - Found while debugging fragment reception on an IPQ5018 board; the
      failure needs a 64-byte GFP_ATOMIC allocation to fail, so it was never
      seen in normal use. Under Documentation/process/threat-model.rst it is
      a regular bug.
    - To exercise the error path, a test-only module parameter made the
      kmemdup() fail for every Nth first fragment (N = 1 or 3), with a
      client sending 1000-byte pings and TCP uploads fragmented at 256 bytes
      on 2.4 GHz (host data path), over two days:
      - without this patch, the board crashed and rebooted within seconds,
        4 times out of 4; in the two runs that sent one ping at a time, at
        the first forced failure. There is no serial console, so no oops
        trace was captured.
      - with it, 9365 forced failures over 22 minutes: each affected frame
        was dropped, with no warning, SUnreclaim flat, and normal reception
        once the parameter was cleared.
    - The tests ran on OpenWrt's backports of this driver (with downstream
      patches; NSS offload disabled for the radio), not on ath-next. The
      function and its caller are the same there; on ath-next the patch is
      build-tested only (arm64, W=1, no new warnings).
    - That tree also carries a not-yet-submitted change that programs the
      IPQ5018 RX fragment destination in REO1_MISC_CTL, which ath-next does
      not write for this chip. Without it, fragments may not reach this
      function on IPQ5018 at all, so ath-next may not take the tested path
      on that chip.
    - ath12k has the same order in ath12k/wifi7/dp_rx.c; no ath12k hardware
      was available to test a fix.

 drivers/net/wireless/ath/ath11k/dp_rx.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index 6d95f7b34..789a56488 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -3674,6 +3674,17 @@ static int ath11k_dp_rx_frag_h_mpdu(struct ath11k *ar,
 		goto out_unlock;
 	}
 
+	/* On failure the caller frees msdu, so it must not be queued yet */
+	if (frag_no == 0) {
+		rx_tid->dst_ring_desc = kmemdup(ring_desc,
+						sizeof(*rx_tid->dst_ring_desc),
+						GFP_ATOMIC);
+		if (!rx_tid->dst_ring_desc) {
+			ret = -ENOMEM;
+			goto out_unlock;
+		}
+	}
+
 	if (!rx_tid->rx_frag_bitmap || (frag_no > __fls(rx_tid->rx_frag_bitmap)))
 		__skb_queue_tail(&rx_tid->rx_frags, msdu);
 	else
@@ -3683,18 +3694,9 @@ static int ath11k_dp_rx_frag_h_mpdu(struct ath11k *ar,
 	if (!more_frags)
 		rx_tid->last_frag_no = frag_no;
 
-	if (frag_no == 0) {
-		rx_tid->dst_ring_desc = kmemdup(ring_desc,
-						sizeof(*rx_tid->dst_ring_desc),
-						GFP_ATOMIC);
-		if (!rx_tid->dst_ring_desc) {
-			ret = -ENOMEM;
-			goto out_unlock;
-		}
-	} else {
+	if (frag_no != 0)
 		ath11k_dp_rx_link_desc_return(ab, ring_desc,
 					      HAL_WBM_REL_BM_ACT_PUT_IN_IDLE);
-	}
 
 	if (!rx_tid->last_frag_no ||
 	    rx_tid->rx_frag_bitmap != GENMASK(rx_tid->last_frag_no, 0)) {
-- 
2.47.3