[PATCH net v2] tipc: fix broadcast sender hang on last acker departure

[Henry Martin <bsdhenrymartin@gmail.com>]

TIPC group broadcast senders are blocked while a replicast still has
outstanding member acks: poll(EPOLLOUT) only fires when
tsk->group_is_open, and writers keep re-sleeping on every unrelated
data-ready wakeup because tipc_group_bc_cong() keeps seeing
*grp->open == false (the default sk_sndtimeo is MAX_SCHEDULE_TIMEOUT,
so there is no bounded send timeout either).

When the last outstanding acker departs before acking,
tipc_group_delete_member() decrements grp->bc_ackers but never
restores *grp->open, and the blocked senders are never woken.
Commit 99cc2a62e07a ("tipc: reject invalid and unexpected
GRP_ACK_MSG to prevent bc_ackers underflow") already calls this out
in its changelog as a pre-existing issue.

Reproduced on a two-node TIPC group cluster (v7.3-rc5, two QEMU
guests over a UDP bearer): with one member's ack outstanding and
that member then departing, poll(POLLOUT) on the sender never
fired and a blocking sendmsg() never returned; with the fix
applied both are released when the leave event is delivered (log
below the fold).

Restore the open state exactly on the 1->0 transition in the delete
path, mirroring the GRP_ACK_MSG completion path, and invoke
sk->sk_write_space() from tipc_sk_filter_rcv() when the group just
became writable again.

While at it, make the accounting precise: the delete path used to
test less(m->bc_acked, grp->bc_snd_nxt - 1), which also matches
members that were never counted for the current replicast
(JOINING/LEAVING members keep a stale bc_acked).  Zeroing
bc_ackers on such a member could silently void a still-outstanding
ack from a real acker.  Track whether a member owes an ack for the
current replicast in a new per-member bc_pending flag, set when the
member is counted and cleared when its ack arrives, and test that
in the delete path instead.  Suggested by Tung Quang Nguyen.

This issue was discovered by Tencent CodeBuddy Security.

Cc: stable@vger.kernel.org
Fixes: 2f487712b893 ("tipc: guarantee that group broadcast doesn't bypass group unicast")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---

v2: Subject no longer claims a leak; describe the actual wakeup
    mechanics and add the reproduction; the delete path now
    discriminates members that were actually counted for the
    current replicast via a new per-member bc_pending flag instead
    of comparing a possibly stale bc_acked (Tung Quang Nguyen).

Reproduction (two QEMU guests, TIPC over UDP bearer; the acking
peer withheld GRP_ACK_MSG via a test-only knob to emulate the
lossy peer, the sender's kernel ran unmodified TIPC):

    UNPATCHED (#88/#90, fix absent):
      sender:  NO POLLOUT for 30s after member departed -> STALL
      sender2: R2 NEVER returned in 32s (blocked writer not woken)
    PATCHED (#89, this patch):
      sender:  POLLOUT at t=18.5s, when the leave event was delivered
      sender2: R2 returned at t=18.6s, EHOSTUNREACH on now-empty group

 net/tipc/group.c  | 13 ++++++++++---
 net/tipc/socket.c | 12 +++++++++++-
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/net/tipc/group.c b/net/tipc/group.c
index 74f6d3dac0784..e9c77f12c9819 100644
--- a/net/tipc/group.c
+++ b/net/tipc/group.c
@@ -75,6 +75,7 @@ struct tipc_member {
 	u16 bc_rcv_nxt;
 	u16 bc_syncpt;
 	u16 bc_acked;
+	bool bc_pending;
 };
 
 struct tipc_group {
@@ -340,9 +341,12 @@ static void tipc_group_delete_member(struct tipc_group *grp,
 	rb_erase(&m->tree_node, &grp->members);
 	grp->member_cnt--;
 
-	/* Check if we were waiting for replicast ack from this member */
-	if (grp->bc_ackers && less(m->bc_acked, grp->bc_snd_nxt - 1))
-		grp->bc_ackers--;
+	/* If this member owed an ack for the current replicast and it
+	 * was the last one, restore the open state so blocked broadcast
+	 * senders can proceed.
+	 */
+	if (grp->bc_ackers && m->bc_pending && !--grp->bc_ackers)
+		*grp->open = true;
 
 	list_del_init(&m->list);
 	list_del_init(&m->small_win);
@@ -406,6 +410,8 @@ void tipc_group_update_bc_members(struct tipc_group *grp, int len, bool ack)
 			m->bc_acked = prev;
 			ackers++;
 		}
+		if (ack)
+			m->bc_pending = tipc_group_is_receiver(m);
 	}
 
 	/* Mark number of acknowledges to expect, if any */
@@ -803,6 +809,7 @@ void tipc_group_proto_rcv(struct tipc_group *grp, bool *usr_wakeup,
 		if (acked != grp->bc_snd_nxt || m->bc_acked == acked)
 			return;
 		m->bc_acked = acked;
+		m->bc_pending = false;
 		if (--grp->bc_ackers)
 			return;
 		list_del_init(&m->small_win);
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index d5d70eb230b56..65cc0c4964b31 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2356,8 +2356,18 @@ static void tipc_sk_filter_rcv(struct sock *sk, struct sk_buff *skb,
 	if (unlikely(!msg_isdata(hdr)))
 		tipc_sk_proto_rcv(sk, &inputq, xmitq);
 
-	if (unlikely(grp))
+	if (unlikely(grp)) {
+		bool was_open = tsk->group_is_open;
+
 		tipc_group_filter_msg(grp, &inputq, xmitq);
+		/* A departing member may have held the last outstanding
+		 * replicast ack, which can restore the open state inside
+		 * tipc_group_filter_msg(); wake up senders blocked in
+		 * poll()/write() if the group just became writable.
+		 */
+		if (!was_open && tsk->group_is_open)
+			sk->sk_write_space(sk);
+	}
 
 	if (unlikely(!grp) && mtyp == TIPC_MCAST_MSG)
 		tipc_mcast_filter_msg(net, &tsk->mc_method.deferredq, &inputq);
-- 
2.43.7