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

[Tung Quang Nguyen]

>Subject: [PATCH net v4] tipc: fix broadcast sender hang on last acker departure
>
>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@xxxxxxxxxxxxxxx
>Fixes: 2f487712b893 ("tipc: guarantee that group broadcast doesn't bypass
>group unicast")
>Signed-off-by: Henry Martin <bsdhenrymartin@xxxxxxxxx>

Reviewed-by: Tung Nguyen <tung.quang.nguyen@xxxxxxxx>