RE: [PATCH net] tipc: bound the device MTU accepted for L2 bearers

Subject: [PATCH net] tipc: bound the device MTU accepted for L2 bearers

struct tipc_bearer keeps the bearer MTU in a u32 while struct tipc_link keeps
the link MTU in a u16.  tipc_enable_l2_media() and the NETDEV_CHANGEMTU
notifier both do b->mtu = dev->mtu, tipc_link_create() assigns that into l->mtu,
and tipc_link_set_queue_limits() divides by it:

int max_bulk = TIPC_MAX_PUBL / (l->mtu / ITEM_SIZE);

ITEM_SIZE is 20, so a device MTU of 65536 truncates to 0 and the division
faults.

tipc_mtu_bad() is the only check on either assignment and tests a lower bound
only.  The truncation defeats even that: a device MTU of 65556 passes it, yet
the link comes out with l->mtu = 20, below the TIPC_MIN_BEARER_MTU the
check exists to enforce, and tipc_link_mss() then evaluates 20 - INT_H_SIZE -
EMSG_OVERHEAD as an int and stores it in the u32 tipc_link_entry::mtu.  Read
back over TIPC_NL_LINK_GET, every device MTU above 65535 leaves the link
with a wrong MTU: 20 at 65556,
464 at 66000, 32768 at 98304.

The bound added in commit 9f29cd8a8e79 ("tipc: fix u16 MTU truncation in
media and bearer MTU validation") applies to TIPC_NLA_PROP_MTU in
tipc_nl_prop_policy, which covers TIPC_NL_MEDIA_SET and
TIPC_NL_BEARER_SET.  A device MTU never passes through that policy.

dummy sets max_mtu to 0, and dev_validate_mtu() applies its ceiling only
when max_mtu is non-zero, so the device accepts 65536; a macvlan inherits
max_mtu from its lower device, and two macvlans in bridge mode on one
dummy carry each other's TIPC discovery frames, so the bearer reaches
tipc_node_check_dest() and
tipc_link_create() with b->mtu = 65536.  TIPC_NL_BEARER_ENABLE is
GENL_UNS_ADMIN_PERM, so an unprivileged user can do this in a user
namespace of their own, on a kernel that has TIPC loaded.

 Oops: divide error: 0000 [#1] SMP KASAN NOPTI
 RIP: 0010:tipc_link_create (net/tipc/link.c:2531 net/tipc/link.c:520)
 Call Trace:
  <IRQ>
  tipc_node_check_dest (net/tipc/node.c:1284)
  tipc_disc_rcv (net/tipc/discover.c:252)
  tipc_rcv (net/tipc/node.c:2134)
  tipc_l2_rcv_msg (net/tipc/bearer.c:669)
  __netif_receive_skb_one_core (net/core/dev.c:6216)
  process_backlog (net/core/dev.c:6680)
  __napi_poll (net/core/dev.c:7739)
  net_rx_action (net/core/dev.c:7802)
  handle_softirqs (kernel/softirq.c:622)
  </IRQ>
 Kernel panic - not syncing: Fatal exception in interrupt

Give tipc_mtu_bad() the matching upper bound, so the limit sits next to the
existing minimum and one check covers both assignments.  A device MTU of
65535 is still accepted and the link still comes up, with the
65532 that TIPC rounds it to.

The u32 max_pkt fields became u16 mtu and advertised_mtu in commit
ed193ece2649 ("tipc: simplify link mtu negotiation"), which also removed
link_init_max_pkt(); that had clamped the bearer MTU to MAX_MSG_SIZE
before it could reach this division.

Fixes: ed193ece2649 ("tipc: simplify link mtu negotiation")
Reported-by: Yunpeng Tian <redacted>
Reported-by: Gongming Wang <redacted>
Reported-by: Mingda Zhang <redacted>
Reported-by: Xiaoyu Zhou <redacted>
Reported-by: Qinrun Dai <redacted>
Cc: stable@vger.kernel.org # v4.1
Signed-off-by: Yunpeng Tian <redacted>

Next time:
1. Please use correct "Reported-by". Five reporters for a small issue looks weird.
2. Person in Signed-off-by should not be in Reported-by.

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