[PATCH net v2] tipc: fix memory leaks in bundle and fragment paths

[PATCH net v2] tipc: fix memory leaks in bundle and fragment paths

[xiaoshoukui]

From: xiaoshoukui <xiaoshoukui@gmail.com>

In tipc_data_input(), recognized internal control message users (such
as MSG_BUNDLER, TUNNEL_PROTOCOL, MSG_FRAGMENTER, and BCAST_PROTOCOL)
return false WITHOUT releasing the skb via kfree_skb().

However, callers in the bundle extraction and fragment reassembly
receive paths currently ignore the return value of tipc_data_input().
This leads to memory leaks in the following scenarios:

1. Nested Bundle Path:
   When an outer MSG_BUNDLER message contains a structurally valid
   inner MSG_BUNDLER payload, tipc_msg_extract() allocates and
   extracts the inner skb. tipc_data_input() inspects the inner
   header, sees MSG_BUNDLER, and returns false without consuming or
   freeing the skb. Because the extraction loop ignores the return
   value, the inner skb is leaked.

2. Fragment Reassembly Path:
   When reassembly completes in tipc_buf_append(), the reassembled
   skb may carry msg_user == MSG_FRAGMENTER. tipc_data_input()
   inspects the reassembled skb, sees MSG_FRAGMENTER, and returns
   false without consuming or freeing it. The fragment completion path
   ignores the return value, leaking the reassembled skb.

Fix this by checking the return value of tipc_data_input() with
unlikely() in both receive paths and dropping unhandled skbs via
kfree_skb().

Hot-path disassembly verification confirms that adding unlikely()
preserves ideal branch layout. Benchmark evaluations under strict CPU
core isolation show no statistically significant throughput variation.

Fixes: c637c1035534 ("tipc: resolve race problem at unicast message reception")
Signed-off-by: xiaoshoukui <xiaoshoukui@gmail.com>
---
v1 -> v2:
 - Added unlikely() annotations to low-probability error paths.
 - Reverted to kfree_skb() to align with existing TIPC module conventions
   and avoid 80-column line wrapping / checkpatch warnings.
 - Re-evaluated benchmarks with strict CPU affinity (taskset).

Link: https://lore.kernel.org/netdev/20260916061220.798324-1-xiaoshoukui@gmail.com/

 net/tipc/link.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 49dfc098d89b..dec72a2b113e 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1306,15 +1306,18 @@ static int tipc_link_input(struct tipc_link *l, struct sk_buff *skb,
 		skb_queue_head_init(&tmpq);
 		l->stats.recv_bundles++;
 		l->stats.recv_bundled += msg_msgcnt(hdr);
-		while (tipc_msg_extract(skb, &iskb, &pos))
-			tipc_data_input(l, iskb, &tmpq);
+		while (tipc_msg_extract(skb, &iskb, &pos)) {
+			if (unlikely(!tipc_data_input(l, iskb, &tmpq)))
+				kfree_skb(iskb);
+		}
 		tipc_skb_queue_splice_tail(&tmpq, inputq);
 		return 0;
 	} else if (usr == MSG_FRAGMENTER) {
 		l->stats.recv_fragments++;
 		if (tipc_buf_append(reasm_skb, &skb)) {
 			l->stats.recv_fragmented++;
-			tipc_data_input(l, skb, inputq);
+			if (unlikely(!tipc_data_input(l, skb, inputq)))
+				kfree_skb(skb);
 		} else if (!*reasm_skb && !link_is_bc_rcvlink(l)) {
 			pr_warn_ratelimited("Unable to build fragment list\n");
 			return tipc_link_fsm_evt(l, LINK_FAILURE_EVT);
-- 
2.34.1

Re: [PATCH net v2] tipc: fix memory leaks in bundle and fragment paths

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

RE: [PATCH net v2] tipc: fix memory leaks in bundle and fragment paths

[Tung Quang Nguyen]

>Subject: [PATCH net v2] tipc: fix memory leaks in bundle and fragment paths
>
>From: xiaoshoukui <xiaoshoukui@gmail.com>
>
>In tipc_data_input(), recognized internal control message users (such as
>MSG_BUNDLER, TUNNEL_PROTOCOL, MSG_FRAGMENTER, and
>BCAST_PROTOCOL) return false WITHOUT releasing the skb via kfree_skb().
>
>However, callers in the bundle extraction and fragment reassembly receive
>paths currently ignore the return value of tipc_data_input().
>This leads to memory leaks in the following scenarios:
>
>1. Nested Bundle Path:
>   When an outer MSG_BUNDLER message contains a structurally valid
>   inner MSG_BUNDLER payload, tipc_msg_extract() allocates and
>   extracts the inner skb. tipc_data_input() inspects the inner
>   header, sees MSG_BUNDLER, and returns false without consuming or
>   freeing the skb. Because the extraction loop ignores the return
>   value, the inner skb is leaked.
>
>2. Fragment Reassembly Path:
>   When reassembly completes in tipc_buf_append(), the reassembled
>   skb may carry msg_user == MSG_FRAGMENTER. tipc_data_input()
>   inspects the reassembled skb, sees MSG_FRAGMENTER, and returns
>   false without consuming or freeing it. The fragment completion path
>   ignores the return value, leaking the reassembled skb.
>

This cannot happen as I explained.
We do not accept a bogus fix that causes regression.
Please drop your patch.

--
pw-bot: rejected

Re: [PATCH net v2] tipc: fix memory leaks in bundle and fragment paths

[xiaoshoukui]

Hi Eric,

Since you previously provided helpful feedback on this thread, I would
appreciate your guidance on the appropriate way to address this TIPC
receive-path issue.

To summarize the current situation:

1. Demonstrated issue:
   Streaming 30,000 crafted TIPC UDP packets to a 4 GB VM reliably triggers
   unconsumed skb memory leaks originating from
   tipc_buf_acquire() -> tipc_msg_extract() -> tipc_link_input() when
   tipc_data_input() returns false. The memory growth eventually results in
   an OOM kernel panic in 14 seconds.

2. Benchmark results:
   I re-ran the benchmark for 10 back-to-back rounds with strict CPU
   affinity. The results did not show any measurable throughput regression
   between the pre-patch and post-patch versions with unlikely().

3. Current review concern:
   Tung rejected the v2 implementation because of the concern that adding
   caller-side cleanup checks may introduce overhead on the fast path, and
   because these malformed inputs are not generated by compliant TIPC
   senders.

Given that the remote memory exhaustion issue is reliably reproducible, 
I would appreciate your thoughts on the preferred netdev/TIPC approach:

- Should the TIPC receive path defensively handle such malformed wire inputs
  to prevent unbounded skb memory consumption and remote DoS?
- If the current caller-side cleanup is not the preferred approach, what would
  be the cleanest way to ensure that the unconsumed skbs are released without
  adding meaningful overhead to the normal receive path?

I am not wedded to the current v2 implementation. I would be happy to explore 
a cleaner, more effective fix based on the maintainers' guidance.

Thanks,
Xiaoshoukui