[PATCH net 1/4] sctp: purge abandoned I-DATA fragments by stream and MID, not by TSN

[Tim Budding <tim.budding@gmail.com>]

When an I-FORWARD-TSN is received, sctp_report_iftsn() calls
sctp_intl_reasm_flushtsn(), which frees every fragment in reasm and
reasm_uo with a TSN at or below the New Cumulative TSN. That is only
correct for DATA chunks, where the fragments of a message use
consecutive TSNs. With I-DATA, fragments of messages on different
streams can be interleaved, and RFC 8260 section 2.2.3 says the receiver
"MUST NOT make any assumption about the TSN assignments of the sender".

So when a PR-SCTP message is abandoned while a large message on another
stream is still being received, the fragments of the large message below
the abandoned TSN are freed as well. They have already been acked and
are not retransmitted, so the large message is never delivered, its
remaining fragments stay queued until a later I-FORWARD-TSN covers them
or the association goes away, and on an ordered stream every later
message is blocked behind it. Also, received fragments of the abandoned
message above the New Cumulative TSN are only freed if a later
I-FORWARD-TSN happens to cover them.

The skip entries of the I-FORWARD-TSN already identify the abandoned
messages by stream, U bit and MID. Free the fragments from
sctp_intl_skip() based on those instead. An entry carries the largest
skipped MID for its stream and U bit (RFC 8260 section 2.3.1), and a
sender fragments only one message per stream at a time (section 2.2.2),
so all fragments on that stream with a MID at or below it can be freed.
The reassembly queues are sorted by stream and MID, so the walk stops
once it is past them. Fragments that no skip entry names, such as a late
copy of an abandoned fragment, are no longer freed by an unrelated
I-FORWARD-TSN; they stay until the association is freed. The association
reset paths that also call sctp_report_iftsn() are not affected: they
pass the highest TSN seen, so sctp_intl_abort_pd() still flushes the
whole ulpq for them.

Observed in real use as unexpected message loss when sending messages of
mixed sizes over one association; the cause was found by AI-assisted
code inspection. Reproduced on an unmodified kernel with the ordered and
unordered tests of the sctp_ifwdtsn.sh selftest added later in this
series, which pass with this fix.

Fixes: 47b20a88566f ("sctp: implement report_ftsn for sctp_stream_interleave")
Cc: stable@vger.kernel.org
Signed-off-by: Tim Budding <tim.budding@gmail.com>
---
 net/sctp/stream_interleave.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index 840f24045ae2..f8c818a5533c 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -1203,28 +1203,21 @@ static void sctp_report_fwdtsn(struct sctp_ulpq *ulpq, __u32 ftsn)
 	sctp_ulpq_abort_pd(ulpq, GFP_ATOMIC);
 }
 
-static void sctp_intl_reasm_flushtsn(struct sctp_ulpq *ulpq, __u32 ftsn)
+static void sctp_intl_reasm_flush_mid(struct sk_buff_head *reasm, __u16 sid,
+				      __u32 mid)
 {
 	struct sk_buff *pos, *tmp;
 
-	skb_queue_walk_safe(&ulpq->reasm, pos, tmp) {
+	skb_queue_walk_safe(reasm, pos, tmp) {
 		struct sctp_ulpevent *event = sctp_skb2event(pos);
-		__u32 tsn = event->tsn;
 
-		if (TSN_lte(tsn, ftsn)) {
-			__skb_unlink(pos, &ulpq->reasm);
-			sctp_ulpevent_free(event);
-		}
-	}
-
-	skb_queue_walk_safe(&ulpq->reasm_uo, pos, tmp) {
-		struct sctp_ulpevent *event = sctp_skb2event(pos);
-		__u32 tsn = event->tsn;
+		if (event->stream < sid)
+			continue;
+		if (event->stream > sid || MID_lt(mid, event->mid))
+			break;
 
-		if (TSN_lte(tsn, ftsn)) {
-			__skb_unlink(pos, &ulpq->reasm_uo);
-			sctp_ulpevent_free(event);
-		}
+		__skb_unlink(pos, reasm);
+		sctp_ulpevent_free(event);
 	}
 }
 
@@ -1232,8 +1225,6 @@ static void sctp_report_iftsn(struct sctp_ulpq *ulpq, __u32 ftsn)
 {
 	/* Move the Cumulattive TSN Ack ahead. */
 	sctp_tsnmap_skip(&ulpq->asoc->peer.tsn_map, ftsn);
-	/* purge the fragmentation queue */
-	sctp_intl_reasm_flushtsn(ulpq, ftsn);
 	/* abort only when it's for all data */
 	if (ftsn == sctp_tsnmap_get_max_tsn_seen(&ulpq->asoc->peer.tsn_map))
 		sctp_intl_abort_pd(ulpq, GFP_ATOMIC);
@@ -1260,6 +1251,7 @@ static void sctp_intl_skip(struct sctp_ulpq *ulpq, __u16 sid, __u32 mid,
 			sctp_intl_stream_abort_pd(ulpq, sid, mid, 0x1,
 						  GFP_ATOMIC);
 		}
+		sctp_intl_reasm_flush_mid(&ulpq->reasm_uo, sid, mid);
 		return;
 	}
 
@@ -1271,6 +1263,7 @@ static void sctp_intl_skip(struct sctp_ulpq *ulpq, __u16 sid, __u32 mid,
 		sctp_intl_stream_abort_pd(ulpq, sid, mid, 0x0, GFP_ATOMIC);
 	}
 
+	sctp_intl_reasm_flush_mid(&ulpq->reasm, sid, mid);
 	sctp_mid_skip(stream, in, sid, mid);
 
 	sctp_intl_reap_ordered(ulpq, sid);
-- 
2.34.1