RE: [PATCH net 3/4] tipc: prevent snt_unacked underflow on CONN_ACK

[Tung Quang Nguyen]

>Subject: [PATCH net 3/4] tipc: prevent snt_unacked underflow on CONN_ACK
>
>tipc_sk_conn_proto_rcv() subtracts the peer-supplied connection ack count
>from the unsigned 16-bit send counter snt_unacked without checking that it
>does not exceed the number of messages actually
>outstanding:
>
>	tsk->snt_unacked -= msg_conn_ack(hdr);
>
>msg_conn_ack() is read straight from a received CONN_MANAGER/CONN_ACK
>message. If the ack count is larger than snt_unacked the subtraction wraps to a
>near-maximum value, leaving tsk_conn_cong() permanently true and starving
>the connection of further transmits.
>
>Cap the ack to the outstanding count before subtracting. A peer (or, for a local
>connection, the connected peer socket) can otherwise wedge a TIPC
>connection's send side by sending an oversized connection ack.
>
>Fixes: 10724cc7bb78 ("tipc: redesign connection-level flow control")
>Assisted-by: Claude:claude-opus-4-7
>Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
>---
> net/tipc/socket.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/net/tipc/socket.c b/net/tipc/socket.c index
>9329919fb07f0..9c739a3cea126 100644
>--- a/net/tipc/socket.c
>+++ b/net/tipc/socket.c
>@@ -1362,9 +1362,16 @@ static void tipc_sk_conn_proto_rcv(struct tipc_sock
>*tsk, struct sk_buff *skb,
> 			__skb_queue_tail(xmitq, skb);
> 		return;
> 	} else if (mtyp == CONN_ACK) {
>+		u16 conn_ack = msg_conn_ack(hdr);
>+
> 		was_cong = tsk_conn_cong(tsk);
> 		tipc_sk_push_backlog(tsk, msg_nagle_ack(hdr));
>-		tsk->snt_unacked -= msg_conn_ack(hdr);
>+		/* Cap a peer-supplied ack so a forged value cannot underflow
>+		 * the unsigned counter and wedge connection flow control.
>+		 */
>+		if (conn_ack > tsk->snt_unacked)

This handling of an invalid field in the message header is not correct.
Please validate this field at the beginning of the body block and drop the message if it is invalid.
 
>+			conn_ack = tsk->snt_unacked;
>+		tsk->snt_unacked -= conn_ack;
> 		if (tsk->peer_caps & TIPC_BLOCK_FLOWCTL)
> 			tsk->snd_win = msg_adv_win(hdr);
> 		if (was_cong && !tsk_conn_cong(tsk))
>--
>2.53.0
>