mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v4] tipc: raise the minimum bearer MTU to fit tunnel and crypto overhead
@ 2026-10-28 23:17 Joas Antonio dos Santos
  2026-10-30  7:14 ` Tung Quang Nguyen
  0 siblings, 1 reply; 4+ messages in thread
From: Joas Antonio dos Santos @ 2026-10-28 23:17 UTC (permalink / raw)
  To: Jon Maloy, Tung Quang Nguyen
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netdev, tipc-discussion, linux-kernel

A peer can lower the link MTU to TIPC_MIN_BEARER_MTU (100 bytes, plus
the UDP encapsulation header) with the max_pkt field of a RESET or
ACTIVATE message; tipc_link_proto_rcv() only checks the lower bound
returned by tipc_bearer_min_mtu().  The same bound is applied to the
device MTU when a bearer is enabled.

That bound predates two later reductions of the link MSS:

  tipc_link_mss() = mtu - INT_H_SIZE                    (tunnel header)
                         - EMSG_OVERHEAD               (CONFIG_TIPC_CRYPTO)

With CONFIG_TIPC_CRYPTO=y, which is the default, an MTU of 100 gives an
MSS of 28.  Once the link comes up, tipc_named_node_up() calls
named_distribute(), which computes

  msg_dsz = ((tipc_node_get_mtu() - INT_H_SIZE) / ITEM_SIZE) * ITEM_SIZE;

For an MSS of 28 the subtraction is negative and the result truncates to
0xfffffff0; for an MSS between 40 and 59 it is 0.  named_prepare_buf()
then allocates a buffer with no room for any item (the u32 size wraps),
and msg_rem never reaches zero, so every cluster-scope publication on
the node is written with publ_to_item() past the end of that one skb,
over skb_shared_info and into adjacent memory.

Without crypto the MSS is at least 60, which still does not hold the
first fragment built by tipc_msg_build(): INT_H_SIZE fragment header
plus a message header of up to MAX_H_SIZE.  With pktmax below that, the
header copies run past the pktmax bytes allocated for the fragment.

Raise TIPC_MIN_BEARER_MTU to MAX_H_SIZE + 2 * INT_H_SIZE so the MSS can
always hold INT_H_SIZE + MAX_H_SIZE, which also leaves room for name
table items, and add tipc_bearer_mtu_floor(), which adds EMSG_OVERHEAD
on top for CONFIG_TIPC_CRYPTO=y.  Use it for the device MTU check
(tipc_mtu_bad()), the UDP MTU check, the netlink MTU setting and
tipc_bearer_min_mtu(), which also bounds the max_pkt negotiation.

Found by manual code review, assisted by an LLM, and reproduced with
KASAN using two network namespaces joined by a veth pair (see below).

Fixes: fc1b6d6de220 ("tipc: introduce TIPC encryption & authentication")
Fixes: 2320bcdae628 ("tipc: fix changeover issues due to large packet")
Suggested-by: Tung Nguyen <tung.quang.nguyen@est.tech>
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Assisted-by: Claude:claude-opus-5-5
---
Changes in v4:
- Rebased on net/main, on top of 592d0fdb02df ("tipc: bound the device
  MTU accepted for L2 bearers"); tipc_mtu_bad() keeps the new
  TIPC_MAX_BEARER_MTU upper bound and only the lower bound changes.
  No functional change otherwise.

Changes in v3:
- Keep the CONFIG_TIPC_CRYPTO dependency in one helper,
  tipc_bearer_mtu_floor(), instead of #ifdefs at each call site, as
  suggested by Paolo.  tipc_mtu_bad() and tipc_udp_mtu_bad() use it, so
  tipc_mtu_bad_crypto() and the crypto.h include in udp_media.h are gone.
  The bounds are unchanged: 172 with crypto, 140 without.

Changes in v2:
- Derive the crypto bound from EMSG_OVERHEAD instead of a hard-coded 32:
  TIPC_MIN_BEARER_MTU_CRYPTO in crypto.h and tipc_mtu_bad_crypto(), as
  suggested by Tung.  Drop the BUILD_BUG_ON().
- Apply the crypto bound to the UDP MTU check and the netlink MTU
  setting too.

v4 builds cleanly on net/main with W=1, with and without
CONFIG_TIPC_CRYPTO.  The runtime results below are from v3, retested on
602042bf29f6 (arm64, QEMU, KASAN); the rebase does not change the
bounds.  With the reproducer
below (node A with 40 cluster-scope publications and MTU 1500, node B
enabling its bearer on a veth with the given MTU):

  config            peer MTU   bearer on B         link   KASAN
  unpatched            100     enabled              up      slab-out-of-bounds
                                                            in publ_to_item()
  TIPC_CRYPTO=y        100     refused (too low)    -       none
  TIPC_CRYPTO=y        171     refused (too low)    -       none
  TIPC_CRYPTO=y        172     enabled              up      none
  TIPC_CRYPTO=y       1500     enabled              up      none
  TIPC_CRYPTO=n        100     refused (too low)    -       none
  TIPC_CRYPTO=n        139     refused (too low)    -       none
  TIPC_CRYPTO=n        140     enabled              up      none
  TIPC_CRYPTO=n       1500     enabled              up      none

The reproducer exercises the bearer-enable check; the max_pkt
negotiation in tipc_link_proto_rcv() uses the same bound through
tipc_bearer_min_mtu() but was not hit separately with forged messages.

Reproducer (as root; build with "gcc -Wall -O2 -o pub pub.c", run
"sh tipc_repro.sh [peer_mtu]"):

pub.c:

  /* Bind N TIPC services with cluster scope, then sleep. */
  #include <linux/tipc.h>
  #include <stdio.h>
  #include <stdlib.h>
  #include <sys/socket.h>
  #include <unistd.h>

  int main(int argc, char **argv)
  {
  	int n = argc > 1 ? atoi(argv[1]) : 40;
  	for (int i = 0; i < n; i++) {
  		struct sockaddr_tipc a = {
  			.family = AF_TIPC,
  			.addrtype = TIPC_SERVICE_RANGE,
  			.scope = TIPC_CLUSTER_SCOPE,
  			.addr.nameseq = { .type = 1000 + i, .lower = 0, .upper = 0 },
  		};
  		int s = socket(AF_TIPC, SOCK_RDM, 0);
  		if (s < 0 || bind(s, (struct sockaddr *)&a, sizeof(a))) {
  			perror("tipc bind");
  			return 1;
  		}
  	}
  	printf("pub: bound %d cluster-scope services\n", n);
  	fflush(stdout);
  	pause();
  	return 0;
  }

tipc_repro.sh:

  #!/bin/sh
  # TIPC named_distribute() overflow reproducer.
  # Needs root, CONFIG_TIPC=y, CONFIG_TIPC_CRYPTO=y (default), KASAN recommended.
  # Usage: ./tipc_repro.sh [peer_mtu]   (default 100; 100..131 trigger it)
  PEER_MTU=${1:-100}

  ip netns add A
  ip netns add B
  ip link add va type veth peer name vb
  ip link set va netns A
  ip link set vb netns B
  ip -n A link set lo up
  ip -n B link set lo up
  ip -n A link set va mtu 1500 up
  ip -n B link set vb mtu "$PEER_MTU" up

  # node A: 40 cluster-scope publications (any number >= 1 overflows;
  # more publications write further past the buffer)
  ip netns exec A ./pub 40 &
  sleep 2

  # node A uses MTU 1500, node B uses PEER_MTU; the link MTU is
  # negotiated down to PEER_MTU, the link MSS becomes PEER_MTU - 72
  ip netns exec A tipc bearer enable media eth dev va
  ip netns exec B tipc bearer enable media eth dev vb

  # the overflow happens in tipc_named_node_up() as soon as the link
  # comes up (a few seconds)
  sleep 20
  dmesg | grep -A30 -E "KASAN|BUG:|Oops"

 net/tipc/bearer.c    | 17 +++++++++++++++--
 net/tipc/bearer.h    |  9 ++++++---
 net/tipc/udp_media.h |  2 +-
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 774f68d..8b82101 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -546,9 +546,22 @@ int tipc_bearer_mtu(struct net *net, u32 bearer_id)
 	return mtu;
 }
 
+/* tipc_bearer_mtu_floor - smallest bearer MTU, excluding the media
+ * encapsulation header, that leaves room for a fragment header plus a
+ * full message header after the link MSS reductions (see tipc_link_mss())
+ */
+u32 tipc_bearer_mtu_floor(void)
+{
+#ifdef CONFIG_TIPC_CRYPTO
+	return TIPC_MIN_BEARER_MTU + EMSG_OVERHEAD;
+#else
+	return TIPC_MIN_BEARER_MTU;
+#endif
+}
+
 int tipc_bearer_min_mtu(struct net *net, u32 bearer_id)
 {
-	int mtu = TIPC_MIN_BEARER_MTU;
+	int mtu = tipc_bearer_mtu_floor();
 	struct tipc_bearer *b;
 
 	rcu_read_lock();
@@ -1164,7 +1177,7 @@ int __tipc_nl_bearer_set(struct sk_buff *skb, struct genl_info *info)
 			}
 #ifdef CONFIG_TIPC_MEDIA_UDP
 			if (nla_get_u32(props[TIPC_NLA_PROP_MTU]) <
-			    b->encap_hlen + TIPC_MIN_BEARER_MTU) {
+			    b->encap_hlen + tipc_bearer_mtu_floor()) {
 				NL_SET_ERR_MSG(info->extack,
 					       "MTU value is out-of-range");
 				return -EINVAL;
diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
index f9bfcc9..72abfca 100644
--- a/net/tipc/bearer.h
+++ b/net/tipc/bearer.h
@@ -60,8 +60,10 @@
 #define TIPC_MEDIA_TYPE_IB	2
 #define TIPC_MEDIA_TYPE_UDP	3
 
-/* Minimum bearer MTU */
-#define TIPC_MIN_BEARER_MTU	(MAX_H_SIZE + INT_H_SIZE)
+/* Minimum bearer MTU, not counting the crypto overhead, see
+ * tipc_bearer_mtu_floor()
+ */
+#define TIPC_MIN_BEARER_MTU	(MAX_H_SIZE + 2 * INT_H_SIZE)
 
 /* Maximum bearer MTU
  *
@@ -241,6 +243,7 @@ void tipc_bearer_cleanup(void);
 void tipc_bearer_stop(struct net *net);
 int tipc_bearer_mtu(struct net *net, u32 bearer_id);
 int tipc_bearer_min_mtu(struct net *net, u32 bearer_id);
+u32 tipc_bearer_mtu_floor(void);
 bool tipc_bearer_bcast_support(struct net *net, u32 bearer_id);
 void tipc_bearer_xmit_skb(struct net *net, u32 bearer_id,
 			  struct sk_buff *skb,
@@ -265,7 +268,7 @@ static inline void tipc_loopback_trace(struct net *net,
 /* check if device MTU is usable for tipc bearers */
 static inline bool tipc_mtu_bad(struct net_device *dev)
 {
-	if (dev->mtu >= TIPC_MIN_BEARER_MTU &&
+	if (dev->mtu >= tipc_bearer_mtu_floor() &&
 	    dev->mtu <= TIPC_MAX_BEARER_MTU)
 		return false;
 	netdev_warn(dev, "MTU not usable for tipc bearer\n");
diff --git a/net/tipc/udp_media.h b/net/tipc/udp_media.h
index e7455cc..030e00a 100644
--- a/net/tipc/udp_media.h
+++ b/net/tipc/udp_media.h
@@ -48,7 +48,7 @@ int tipc_udp_nl_dump_remoteip(struct sk_buff *skb, struct netlink_callback *cb);
 /* check if configured MTU is too low for tipc headers */
 static inline bool tipc_udp_mtu_bad(u32 mtu)
 {
-	if (mtu >= (TIPC_MIN_BEARER_MTU + sizeof(struct iphdr) +
+	if (mtu >= (tipc_bearer_mtu_floor() + sizeof(struct iphdr) +
 	    sizeof(struct udphdr)))
 		return false;
 
-- 
2.39.5

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH net v4] tipc: raise the minimum bearer MTU to fit tunnel and crypto overhead
  2026-10-28 23:17 [PATCH net v4] tipc: raise the minimum bearer MTU to fit tunnel and crypto overhead Joas Antonio dos Santos
@ 2026-10-30  7:14 ` Tung Quang Nguyen
  2026-10-30 17:24   ` Joas Antonio dos Santos
  0 siblings, 1 reply; 4+ messages in thread
From: Tung Quang Nguyen @ 2026-10-30  7:14 UTC (permalink / raw)
  To: Joas Antonio dos Santos
  Cc: David S.Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netdev, tipc-discussion, linux-kernel, Jon Maloy

>Subject: [PATCH net v4] tipc: raise the minimum bearer MTU to fit tunnel and
>crypto overhead
>
>A peer can lower the link MTU to TIPC_MIN_BEARER_MTU (100 bytes, plus the
>UDP encapsulation header) with the max_pkt field of a RESET or ACTIVATE
>message; tipc_link_proto_rcv() only checks the lower bound returned by
>tipc_bearer_min_mtu().  The same bound is applied to the device MTU when a
>bearer is enabled.
>
>That bound predates two later reductions of the link MSS:
>
>  tipc_link_mss() = mtu - INT_H_SIZE                    (tunnel header)
>                         - EMSG_OVERHEAD               (CONFIG_TIPC_CRYPTO)
>
>With CONFIG_TIPC_CRYPTO=y, which is the default, an MTU of 100 gives an
>MSS of 28.  Once the link comes up, tipc_named_node_up() calls
>named_distribute(), which computes
>
>  msg_dsz = ((tipc_node_get_mtu() - INT_H_SIZE) / ITEM_SIZE) * ITEM_SIZE;
>
>For an MSS of 28 the subtraction is negative and the result truncates to
>0xfffffff0; for an MSS between 40 and 59 it is 0.  named_prepare_buf() then
>allocates a buffer with no room for any item (the u32 size wraps), and
>msg_rem never reaches zero, so every cluster-scope publication on the node is
>written with publ_to_item() past the end of that one skb, over
>skb_shared_info and into adjacent memory.
>
>Without crypto the MSS is at least 60, which still does not hold the first
>fragment built by tipc_msg_build(): INT_H_SIZE fragment header plus a
>message header of up to MAX_H_SIZE.  With pktmax below that, the header
>copies run past the pktmax bytes allocated for the fragment.
>
>Raise TIPC_MIN_BEARER_MTU to MAX_H_SIZE + 2 * INT_H_SIZE so the MSS
>can always hold INT_H_SIZE + MAX_H_SIZE, which also leaves room for name
>table items, and add tipc_bearer_mtu_floor(), which adds EMSG_OVERHEAD
>on top for CONFIG_TIPC_CRYPTO=y.  Use it for the device MTU check
>(tipc_mtu_bad()), the UDP MTU check, the netlink MTU setting and
>tipc_bearer_min_mtu(), which also bounds the max_pkt negotiation.
>
>Found by manual code review, assisted by an LLM, and reproduced with KASAN
>using two network namespaces joined by a veth pair (see below).
>

Can you check this failure ?
https://netdev-ctrl.bots.linux.dev/logview.html?f=/logs/build/1195789/14922723/checkpatch/stdout



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v4] tipc: raise the minimum bearer MTU to fit tunnel and crypto overhead
  2026-10-30  7:14 ` Tung Quang Nguyen
@ 2026-10-30 17:24   ` Joas Antonio dos Santos
  2026-10-30 22:07     ` Jakub Kicinski
  0 siblings, 1 reply; 4+ messages in thread
From: Joas Antonio dos Santos @ 2026-10-30 17:24 UTC (permalink / raw)
  To: Tung Quang Nguyen
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jon Maloy, netdev, tipc-discussion, linux-kernel

Hi Tung,

The only items in that checkpatch run are about the Assisted-by trailer:

  WARNING: Non-standard signature: Assisted-by:
  ERROR: Unrecognized email address: 'Claude:claude-opus-5-5'

That line is there on purpose: it discloses that an AI coding
assistant was used, in the "Assisted-by: AGENT:MODEL" form.  checkpatch
does not know the tag yet and parses its value as an email address.
There is nothing reported against the code or the rest of the commit
message.

Thanks,
Joas

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v4] tipc: raise the minimum bearer MTU to fit tunnel and crypto overhead
  2026-10-30 17:24   ` Joas Antonio dos Santos
@ 2026-10-30 22:07     ` Jakub Kicinski
  0 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2026-10-30 22:07 UTC (permalink / raw)
  To: Joas Antonio dos Santos
  Cc: Tung Quang Nguyen, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Jon Maloy, netdev, tipc-discussion, linux-kernel

On Fri, 30 Oct 2026 14:24:15 -0300 Joas Antonio dos Santos wrote:
> The only items in that checkpatch run are about the Assisted-by trailer:
> 
>   WARNING: Non-standard signature: Assisted-by:
>   ERROR: Unrecognized email address: 'Claude:claude-opus-5-5'
> 
> That line is there on purpose: it discloses that an AI coding
> assistant was used, in the "Assisted-by: AGENT:MODEL" form.  checkpatch
> does not know the tag yet and parses its value as an email address.
> There is nothing reported against the code or the rest of the commit
> message.

Hi dear bot,

The Assisted-by tags are deprecated.
See commit 1e035297ffda ("docs: remove Assisted-by references")

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-30 22:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28 23:17 [PATCH net v4] tipc: raise the minimum bearer MTU to fit tunnel and crypto overhead Joas Antonio dos Santos
2026-10-30  7:14 ` Tung Quang Nguyen
2026-10-30 17:24   ` Joas Antonio dos Santos
2026-10-30 22:07     ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®