Re: [PATCH net v2] tipc: fix out-of-bounds read in broadcast Gap ACK blocks

[Sam P]

On 25/06/2026 10:23, Tung Quang Nguyen wrote:
> > diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c index
> > 76a1585d3f6b..08637c3c9db0 100644
> > --- a/net/tipc/bcast.c
> > +++ b/net/tipc/bcast.c
> > @@ -497,11 +497,12 @@ void tipc_bcast_ack_rcv(struct net *net, struct
> > tipc_link *l,
> >   */
> > int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l,
> > 			struct tipc_msg *hdr,
> > -			struct sk_buff_head *retrq)
> > +			struct sk_buff_head *retrq, bool *valid)
> > {
> > 	struct sk_buff_head *inputq = &tipc_bc_base(net)->inputq;
> > 	struct tipc_gap_ack_blks *ga;
> > 	struct sk_buff_head xmitq;
> > +	u16 glen;
>
> Move this variable declaration to the bottom to follow reverse xmas tree style.
>
> > 	int rc = 0;
> >
> > 	__skb_queue_head_init(&xmitq);
> > @@ -510,13 +511,18 @@ int tipc_bcast_sync_rcv(struct net *net, struct
> > tipc_link *l,
> > 	if (msg_type(hdr) != STATE_MSG) {
> > 		tipc_link_bc_init_rcv(l, hdr);
> > 	} else if (!msg_bc_ack_invalid(hdr)) {
> > -		tipc_get_gap_ack_blks(&ga, l, hdr, false);
> > -		if (!sysctl_tipc_bc_retruni)
> > -			retrq = &xmitq;
> > -		rc = tipc_link_bc_ack_rcv(l, msg_bcast_ack(hdr),
> > -					  msg_bc_gap(hdr), ga, &xmitq,
> > -					  retrq);
> > -		rc |= tipc_link_bc_sync_rcv(l, hdr, &xmitq);
> > +		glen = tipc_get_gap_ack_blks(&ga, l, hdr, false);
> > +		if (glen > msg_data_sz(hdr)) {
> > +			/* Malformed Gap ACK blocks; caller drops the msg */
> > +			*valid = false;
> > +		} else {
> > +			if (!sysctl_tipc_bc_retruni)
> > +				retrq = &xmitq;
> > +			rc = tipc_link_bc_ack_rcv(l, msg_bcast_ack(hdr),
> > +						  msg_bc_gap(hdr), ga, &xmitq,
> > +						  retrq);
> > +			rc |= tipc_link_bc_sync_rcv(l, hdr, &xmitq);
> > +		}
> > 	}
> > 	tipc_bcast_unlock(net);
> >
> > diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h index
> > 2d9352dc7b0e..55d17b5413e1 100644
> > --- a/net/tipc/bcast.h
> > +++ b/net/tipc/bcast.h
> > @@ -97,7 +97,7 @@ void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l,
> > 			struct tipc_msg *hdr);
> > int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l,
> > 			struct tipc_msg *hdr,
> > -			struct sk_buff_head *retrq);
> > +			struct sk_buff_head *retrq, bool *valid);
> > int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg,
> > 			struct tipc_link *bcl);
> > int tipc_nl_bc_link_set(struct net *net, struct nlattr *attrs[]); diff --git
> > a/net/tipc/node.c b/net/tipc/node.c index 97aa970a0d83..2887f94ee28f
> > 100644
> > --- a/net/tipc/node.c
> > +++ b/net/tipc/node.c
> > @@ -1831,12 +1831,13 @@ static void tipc_node_mcast_rcv(struct tipc_node
> > *n)  }
> >
> > static void tipc_node_bc_sync_rcv(struct tipc_node *n, struct tipc_msg *hdr,
> > -				  int bearer_id, struct sk_buff_head *xmitq)
> > +				  int bearer_id, struct sk_buff_head *xmitq,
> > +				  bool *valid)
> > {
> > 	struct tipc_link *ucl;
> > 	int rc;
> >
> > -	rc = tipc_bcast_sync_rcv(n->net, n->bc_entry.link, hdr, xmitq);
> > +	rc = tipc_bcast_sync_rcv(n->net, n->bc_entry.link, hdr, xmitq, valid);
>
> 'valid' needs to be checked after this call. Then, return immediately if it is false.
>
> > 	if (rc & TIPC_LINK_DOWN_EVT) {
> > 		tipc_node_reset_links(n);
> > @@ -2140,12 +2141,18 @@ void tipc_rcv(struct net *net, struct sk_buff *skb,
> > struct tipc_bearer *b)
> >
> > 	/* Ensure broadcast reception is in synch with peer's send state */
> > 	if (unlikely(usr == LINK_PROTOCOL)) {
> > +		bool valid = true;
> > +
> > 		if (unlikely(skb_linearize(skb))) {
> > 			tipc_node_put(n);
> > 			goto discard;
> > 		}
> > 		hdr = buf_msg(skb);
> > -		tipc_node_bc_sync_rcv(n, hdr, bearer_id, &xmitq);
> > +		tipc_node_bc_sync_rcv(n, hdr, bearer_id, &xmitq, &valid);
> > +		if (!valid) {
> > +			tipc_node_put(n);
> > +			goto discard;
> > +		}
> > 	} else if (unlikely(tipc_link_acked(n->bc_entry.link) != bc_ack)) {
> > 		tipc_bcast_ack_rcv(net, n->bc_entry.link, hdr);
> > 	}
> >
> > base-commit: a986fde914d88af47eb78fd29c5d1af7952c3500
> > --
> > 2.54.0

Thanks for the review, I'll address this in a v3.