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

[Adriel Santos]

ath12k_wifi7_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,
ath12k_wifi7_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
  or peer teardown) frees the skb a second time;
- if the remaining fragments of the MPDU arrive first, the bitmap is
  complete, ath12k_wifi7_dp_rx_h_defrag() walks the list with the freed
  skb, and ath12k_wifi7_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, on ath11k 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.

ath11k has the same code and gets the same fix.

Found during an LLM-assisted review of the RX defragmentation path; the
fix was LLM-assisted as well.

Compile tested only.

Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Signed-off-by: Adriel Santos <adriel at adriel.eu>
---

Notes:
    Found while fixing the same bug in ath11k (previous patch), which was
    tested on IPQ5018 with forced kmemdup() failures. No ath12k hardware was
    available: this patch is build-tested only (arm64, W=1, no new
    warnings), and no reproducer was run on ath12k.

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

diff --git a/drivers/net/wireless/ath/ath12k/wifi7/dp_rx.c b/drivers/net/wireless/ath/ath12k/wifi7/dp_rx.c
index a32c80440..d84cbb8df 100644
--- a/drivers/net/wireless/ath/ath12k/wifi7/dp_rx.c
+++ b/drivers/net/wireless/ath/ath12k/wifi7/dp_rx.c
@@ -1227,6 +1227,17 @@ static int ath12k_wifi7_dp_rx_frag_h_mpdu(struct ath12k_pdev_dp *dp_pdev,
 		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
@@ -1236,18 +1247,9 @@ static int ath12k_wifi7_dp_rx_frag_h_mpdu(struct ath12k_pdev_dp *dp_pdev,
 	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)
 		ath12k_wifi7_dp_rx_link_desc_return(dp, &ring_desc->buf_addr_info,
 						    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