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

[Tung Quang Nguyen]

>Subject: [PATCH net v2 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.
>
>Validate the ACK count at the start of the CONN_ACK block and drop the
>message if it acknowledges more messages than are outstanding. 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>
>---
>v2:
>- Validate msg_conn_ack() at the beginning of the CONN_ACK block and
>  drop invalid messages instead of capping the peer-supplied value.
>
> net/tipc/socket.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/net/tipc/socket.c b/net/tipc/socket.c index
>9329919fb07f0..80ad973cda16e 100644
>--- a/net/tipc/socket.c
>+++ b/net/tipc/socket.c
>@@ -1362,9 +1362,14 @@ 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);

It is not necessary to declare a local variable here.
We can do it in a simple way:

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 9329919fb07f..f64f7a35b5c9 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1362,6 +1362,9 @@ 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) {
+               if (tsk->snt_unacked < msg_conn_ack(hdr))
+                       goto exit;

>+
>+		if (conn_ack > tsk->snt_unacked)
>+			goto exit;
>+
> 		was_cong = tsk_conn_cong(tsk);
> 		tipc_sk_push_backlog(tsk, msg_nagle_ack(hdr));
>-		tsk->snt_unacked -= msg_conn_ack(hdr);
>+		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