RE: [PATCH net] tipc: reject name table updates with invalid origin node
Tung Quang Nguyen <[email protected]>
| Newsgroups | org.kernel.vger.netdev,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <GV1P189MB19887144315DA4A6AF16FE02C6D52@GV1P189MB1988.EURP189.PROD.OUTLOOK.COM> |
>Subject: [PATCH net] tipc: reject name table updates with invalid origin node > >From: Jun Yang <[email protected]> > >tipc_update_nametbl() trusts the origin node carried in a received >NAME_DISTRIBUTOR message. For a WITHDRAWAL it removes and frees the >matching publication with tipc_nametbl_remove_publ() (a {key,ref} wildcard >match that ignores the node), then calls >tipc_node_unsubscribe() to unlink the publication from the advertising node's >publ_list. > >tipc_node_unsubscribe() is a no-op when in_own_node(net, node) is true, >and in_own_node() treats node 0 as "own" (addr == own || !addr). So a >WITHDRAWAL with orignode == 0 frees the publication via kfree_rcu() while >the paired unsubscribe silently does nothing, leaving the freed publication >linked on the owning peer's publ_list through its binding_node. > >A subsequent legitimate WITHDRAWAL for a neighbouring publication that the >same peer advertised then performs list_del() across the freed object, writing >a kernel pointer into freed memory (use-after-free >write) and corrupting the live publ_list head. The whole sequence is attacker- >driven from received packets and is reachable by an unprivileged local user, >who can gain CAP_NET_ADMIN in a new network namespace via >unshare(CLONE_NEWUSER|CLONE_NEWNET), enable a TIPC UDP bearer, and >emulate a peer over loopback UDP. Please do not create fake TIPC protocol message in insecure environment. In insecure environment, IPSec needs to be used: https://datatracker.ietf.org/doc/html/draft-maloy-tipc-01.txt#section-6 > >A name table update must always originate from a real peer node, never from >our own address or from node 0. Reject such updates up front: >in_own_node() already covers both cases, so a single guard at the top of >tipc_update_nametbl() drops the malformed update before any publication is >removed or freed. > >Fixes: 37922ea4a310 ("tipc: permit overlapping service ranges in name table") >Cc: [email protected] >Reported-by: TencentOS Corvus AI <[email protected]> >Signed-off-by: Jun Yang <[email protected]> >--- > net/tipc/name_distr.c | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index >ba4f4906e13b..a496e2e9ef62 100644 >--- a/net/tipc/name_distr.c >+++ b/net/tipc/name_distr.c >@@ -286,6 +286,9 @@ static bool tipc_update_nametbl(struct net *net, struct >distr_item *i, > u32 key = ntohl(i->key); > struct tipc_uaddr ua; > >+ if (in_own_node(net, node)) >+ return false; This is redundant because ' tipc_update_nametbl()' is called when receiving messages from other nodes. >+ > /* A peer-advertised binding with lower > upper can never be >matched > * or withdrawn and would leak the publication; the local bind path > * rejects such ranges, so reject ranges learned from the network too. >-- >2.55.0 >