Thread (5 messages) 5 messages, 2 authors, 10d ago

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

From: Weiming Shi <hidden>
Date: 2026-07-16 19:07:17
Also in: lkml

Tung Quang Nguyen [off-list ref] 于2026年7月16日周四 17:28写道:
quoted
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 <redacted>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <redacted>
---
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.
quoted
+              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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help