RE: [PATCH net v5] tipc: raise the minimum bearer MTU to fit tunnel and crypto overhead

[Tung Quang Nguyen]

>Subject: [PATCH net v5] 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 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@xxxxxxxx>
>Signed-off-by: Joas Antonio dos Santos <joasantonio108@xxxxxxxxx>
>---

Reviewed-by: Tung Nguyen <tung.quang.nguyen@xxxxxxxx>