RE: [PATCH net] tipc: validate Gap ACK blocks header before parsing
From: Tung Quang Nguyen <hidden>
Date: 2026-07-24 06:24:01
Also in:
lkml, stable
Subject: [PATCH net] tipc: validate Gap ACK blocks header before parsing An established TIPC peer that negotiated TIPC_GAP_ACK_BLOCK can send a STATE_MSG with fewer than the four bytes required for struct tipc_gap_ack_blks. Basic TIPC message validation accepts such a message, including one with an empty data area, because its declared size still contains a complete protocol header. tipc_get_gap_ack_blks() then dereferences the record to read len, ugack_cnt, and bgack_cnt. Both callers compare the returned record size with the declared message data length, but only after these fields have already been read. The late checks therefore cannot protect the fixed record header access. This was reproduced on current net-next with two live TIPC nodes over a veth bearer. Replaying an established peer's STATE_MSG with its message size reduced from 44 to 40 bytes produced:
It is not recommended to use TIPC in insecure environment and create fake protocol messages because TIPC cannot handle this. You need to use IPSec (UDP bearer) or Encryption (Ethernet bearer) in insecure environment. See the requirement below: https://datatracker.ietf.org/doc/html/draft-maloy-tipc-01.txt#section-6
quoted hunk ↗ jump to hunk
BUG: KMSAN: uninit-value in tipc_get_gap_ack_blks tipc_get_gap_ack_blks tipc_bcast_sync_rcv tipc_node_bc_sync_rcv tipc_rcv tipc_l2_rcv_msg Uninit was created at: __alloc_skb alloc_skb_with_frags sock_alloc_send_pskb packet_sendmsg The uninitialized record length then reached tipc_bcast_sync_rcv(), where it was used to decide whether the STATE_MSG should be dropped. Supplying four initialized bytes after the declared end of otherwise identical messages also changed that decision: a trailing len of zero continued processing, while a trailing len of four dropped the message. Protocol processing therefore depends on bytes that are not part of the declared TIPC message. The KMSAN finding was reproduced for declared data lengths zero through three. After adding the minimum-length check, none of those four inputs reported an uninitialized value in tipc_get_gap_ack_blks() or its caller, and bytes after the declared message no longer affected the decision. Check that the fixed record header is present before parsing it. The callers' existing size checks continue to validate the complete variable-length record. Fixes: d7626b5acff9 ("tipc: introduce Gap ACK blocks for broadcast link") Cc: stable@vger.kernel.org Signed-off-by: Sangho Lee <redacted> --- net/tipc/link.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)diff --git a/net/tipc/link.c b/net/tipc/link.c index 49dfc098d89b..8cc30ebdf575100644--- a/net/tipc/link.c +++ b/net/tipc/link.c@@ -1418,7 +1418,8 @@ u16 tipc_get_gap_ack_blks(struct tipc_gap_ack_blks**ga, struct tipc_link *l, u16 sz = 0; /* Does peer support the Gap ACK blocks feature? */ - if (l->peer_caps & TIPC_GAP_ACK_BLOCK) { + if ((l->peer_caps & TIPC_GAP_ACK_BLOCK) && + msg_data_sz(hdr) >= sizeof(*p)) {
It does not make sense to have this check because when data size can be modified, there is no guarantee that fields in struct tipc_gap_ack_blks are correct.
p = (struct tipc_gap_ack_blks *)msg_data(hdr); sz = ntohs(p->len); /* Sanity check */ -- 2.43.0