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

[Weiming Shi]

Tung Quang Nguyen <tung.quang.nguyen@xxxxxxxx> 于2026年7月16日周四 17:28写道：
>
> >named_distribute() ends by stamping the last_bulk flag on the tail skb via
> >buf_msg(skb_peek_tail(list)). When the publication list is empty no skb is
> >enqueued, skb_peek_tail() returns NULL, and buf_msg(NULL) is dereferenced.
> >
> >tipc_named_node_up() runs this on &nt->cluster_scope. With a node-id
> >configuration cluster_scope is populated only later by tipc_net_finalize(), so a
> >peer link that comes up first reaches named_distribute() with an empty list. It
> >is reachable by an unprivileged user (TIPC genl ops use
> >GENL_UNS_ADMIN_PERM) over a UDP bearer in a user+net namespace:
> >
> > KASAN: null-ptr-deref in range [0x00000000000000d8-0x00000000000000df]
> > RIP: 0010:tipc_named_node_up (net/tipc/name_distr.c:196)
> >  tipc_named_node_up (net/tipc/name_distr.c:196 net/tipc/name_distr.c:221)
> >  tipc_node_write_unlock (net/tipc/node.c:428)
> >  tipc_rcv (net/tipc/node.c:2185)
> >  tipc_udp_recv (net/tipc/udp_media.c:392)  Kernel panic - not syncing: Fatal
> >exception in interrupt
> >
> >The peer holds back this node's later name updates until it sees a bulk with the
> >last_bulk flag, so simply skipping the empty bulk would stall it.
> >Emit an item-less bulk when the list is empty, and break out of the build loop
> >on allocation failure instead of returning, so the last_bulk flag is applied to the
> >last queued skb.
> >
> >Fixes: cad2929dc432 ("tipc: update a binding service via broadcast")
> >Reported-by: Xiang Mei <xmei5@xxxxxxx>
> >Assisted-by: Claude:claude-opus-4-8
> >Signed-off-by: Weiming Shi <bestswngs@xxxxxxxxx>
> >---
> > net/tipc/name_distr.c | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> >diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index
> >ba4f4906e13b..dbcfa965de34 100644
> >--- a/net/tipc/name_distr.c
> >+++ b/net/tipc/name_distr.c
> >@@ -165,7 +165,7 @@ static void named_distribute(struct net *net, struct
> >sk_buff_head *list,
> >                                               dnode);
> >                       if (!skb) {
> >                               pr_warn("Bulk publication failure\n");
> >-                              return;
> >+                              break;
> >                       }
> >                       hdr = buf_msg(skb);
> >                       msg_set_bc_ack_invalid(hdr, true);
> >@@ -192,6 +192,20 @@ static void named_distribute(struct net *net, struct
> >sk_buff_head *list,
> >               skb_trim(skb, INT_H_SIZE + (msg_dsz - msg_rem));
> >               __skb_queue_tail(list, skb);
> >       }
> >+
> >+      if (skb_queue_empty(list)) {
> >+              skb = named_prepare_buf(net, PUBLICATION, 0, dnode);
> >+              if (!skb) {
> >+                      pr_warn("Bulk publication failure\n");
> >+                      return;
> >+              }
>
> This approach is wrong because:
> 1. When 'list' is empty, it is caused by memory allocation failure before. So, it is likely that 'skb' could be NULL again because of memory allocation failure.
> 2. Even if 'skb' is not NULL, allocation of non-data (zero-in-size) message will break the receiving peer when it handles this message.
>
> >+              hdr = buf_msg(skb);
> >+              msg_set_bc_ack_invalid(hdr, true);
> >+              msg_set_bulk(hdr);
> >+              msg_set_non_legacy(hdr);
> >+              __skb_queue_tail(list, skb);
> >+      }
> >+
> >       hdr = buf_msg(skb_peek_tail(list));
> >       msg_set_last_bulk(hdr);
> >       msg_set_named_seqno(hdr, seqno);
> >--
> >2.43.0
>

Thanks for your review. Fixed and v4 sent.