[PATCH net v3] tipc: fix broadcast sender hang on last acker departure
flat view
DORMANTno replies
From: Henry Martin <hidden>
Date: 2026-10-15 02:20:33
Also in:
lkml, stable
Subsystem:
networking [general], the rest, tipc network layer · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds, Jon Maloy, Tung Quang Nguyen
TIPC group broadcast senders are blocked while a replicast still has
outstanding member acks: tipc_group_bc_cong() returns true as long as
grp->bc_ackers is non-zero, and clears *grp->open, which is what
tipc_poll() uses to report EPOLLOUT (via tsk->group_is_open).
When the last outstanding acker departs before acking,
tipc_group_delete_member() decrements grp->bc_ackers to zero but
never restores *grp->open. As a result poll(EPOLLOUT) never fires
again. A writer blocked in sendmsg() is only released if some other
wakeup happens after the deletion: with TIPC_GROUP_MEMBER_EVTS the
leave event delivery calls sk_data_ready() and wakes it, but without
member events nothing does and it sleeps indefinitely, since the
default send timeout is MAX_SCHEDULE_TIMEOUT.
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.
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.
Also make the ack 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), and the GRP_ACK_MSG handler accepted
an ack from any member whose bc_acked differed from the expected
value. Either could zero bc_ackers on behalf of a member that owes
nothing and silently void an ack still owed by 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 use it both to accept an ack and
to decide on deletion. bc_acked has no remaining user and is
removed. 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 <redacted>
---
v3: Correct the changelog: tipc_group_bc_cong() only writes *grp->open,
and a blocked sendmsg() hangs only when nothing wakes it after the
deletion (e.g. member events disabled); also require bc_pending
in the GRP_ACK_MSG handler so an uncounted member cannot decrement
bc_ackers, and drop the now unused bc_acked field.
v2: Subject no longer claims a leak; 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, groups joined
without member events; 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, v1 of 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 | 20 +++++++++++---------
net/tipc/socket.c | 12 +++++++++++-
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/net/tipc/group.c b/net/tipc/group.c
index 74f6d3dac0784..b76edd5001ffa 100644
--- a/net/tipc/group.c
+++ b/net/tipc/group.c@@ -74,7 +74,7 @@ struct tipc_member { u16 window; u16 bc_rcv_nxt; u16 bc_syncpt; - u16 bc_acked; + bool bc_pending; }; struct tipc_group {
@@ -316,7 +316,6 @@ static struct tipc_member *tipc_group_create_member(struct tipc_group *grp, m->node = node; m->port = port; m->instance = instance; - m->bc_acked = grp->bc_snd_nxt - 1; ret = tipc_group_add_to_tree(grp, m); if (ret < 0) { kfree(m);
@@ -340,9 +339,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);
@@ -394,7 +396,6 @@ void tipc_group_update_member(struct tipc_member *m, int len) void tipc_group_update_bc_members(struct tipc_group *grp, int len, bool ack) { - u16 prev = grp->bc_snd_nxt - 1; struct tipc_member *m; struct rb_node *n; u16 ackers = 0;
@@ -403,9 +404,10 @@ void tipc_group_update_bc_members(struct tipc_group *grp, int len, bool ack) m = container_of(n, struct tipc_member, tree_node); if (tipc_group_is_receiver(m)) { tipc_group_update_member(m, len); - m->bc_acked = prev; ackers++; } + if (ack) + m->bc_pending = tipc_group_is_receiver(m); } /* Mark number of acknowledges to expect, if any */
@@ -800,9 +802,9 @@ void tipc_group_proto_rcv(struct tipc_group *grp, bool *usr_wakeup, if (!m || !grp->bc_ackers) return; acked = msg_grp_bc_acked(hdr); - if (acked != grp->bc_snd_nxt || m->bc_acked == acked) + if (acked != grp->bc_snd_nxt || !m->bc_pending) 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