Re: [PATCH net v2] tipc: fix NULL deref in tipc_named_node_up() on empty publication list

[Weiming Shi]

Simon Horman <horms@xxxxxxxxxx> 于2026年7月13日周一 21:46写道：
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> tipc: fix NULL deref in tipc_named_node_up() on empty publication list
>
> This commit fixes a NULL pointer dereference in tipc_named_node_up() that
> occurs when the cluster_scope publication list is empty. The fix introduces
> an early return to avoid calling named_distribute() with an empty list.
>
> > diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
> > index ba4f4906e13b7..60ccaa862162c 100644
> > --- a/net/tipc/name_distr.c
> > +++ b/net/tipc/name_distr.c
> > @@ -218,6 +218,10 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities)
> >       spin_unlock_bh(&tn->nametbl_lock);
> >
> >       read_lock_bh(&nt->cluster_scope_lock);
> > +     if (list_empty(&nt->cluster_scope)) {
> > +             read_unlock_bh(&nt->cluster_scope_lock);
> > +             return;
> > +     }
>
> [Severity: Critical]
> Does this early return prevent the transmission of the last_bulk
> synchronization message to the peer node?
>
> In tipc_named_dequeue(), the peer node sets *open = true only when receiving
> a message where msg_is_last_bulk(hdr) is true:
>
> net/tipc/name_distr.c:tipc_named_dequeue() {
>         ...
>         if (*open && (*rcv_nxt == seqno)) {
>                 (*rcv_nxt)++;
>                 __skb_unlink(skb, namedq);
>                 ...
>                 return skb;
>         }
>         ...
> }
>
> If *open is false, all subsequent non-bulk name publications will fail this
> sequence check and remain in the namedq indefinitely.
>
> Without sending the last_bulk message, could this cause a permanent stall
> of name table updates and an unbounded memory leak of socket buffers on the
> peer node?
>
> >       named_distribute(net, &head, dnode, &nt->cluster_scope, seqno);
>
> [Severity: High]
> This isn't a bug introduced by this patch, but looking at named_distribute()
> called here, what happens if buffer allocation fails during the loop?
>
> Inside named_distribute():
>
> net/tipc/name_distr.c:named_distribute() {
>         ...
>         if (!skb) {
>                 skb = named_prepare_buf(net, PUBLICATION, msg_rem, dnode);
>                 if (!skb) {
>                         pr_warn("Bulk publication failure\n");
>                         return;
>                 }
>         ...
> }
>
> If the function returns early due to allocation failure, the messages
> already added to the queue are transmitted by tipc_node_xmit(), but none
> of them will have the last_bulk flag set since that happens at the very end
> of the function.
>
> Could this also prevent the receiving node from transitioning its named_open
> state to true, leading to the exact same permanent stall and memory leak of
> subsequent normal name publications?
>
> >       tipc_node_xmit(net, &head, dnode, 0);
> >       read_unlock_bh(&nt->cluster_scope_lock);
> --
> pw-bot: changes-requested


Hi ,
Thank you for the review and feedback. v3 sent.