Re: [PATCH net v5] openvswitch: cap upcall PID array size and pre-size vport replies

[Ilya Maximets <i.maximets@ovn.org>]

On 4/16/26 4:46 AM, Weiming Shi wrote:
> The vport netlink reply helpers allocate a fixed-size skb with
> nlmsg_new(NLMSG_DEFAULT_SIZE, ...) but serialize the full upcall PID
> array via ovs_vport_get_upcall_portids().  Since
> ovs_vport_set_upcall_portids() accepts any non-zero multiple of
> sizeof(u32) with no upper bound, a CAP_NET_ADMIN user can install a PID
> array large enough to overflow the reply buffer, causing nla_put() to
> fail with -EMSGSIZE and hitting BUG_ON(err < 0).  On systems with
> unprivileged user namespaces enabled (e.g., Ubuntu default), this is
> reachable via unshare -Urn since OVS vport mutation operations use
> GENL_UNS_ADMIN_PERM.
> 
>  kernel BUG at net/openvswitch/datapath.c:2414!
>  Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
>  CPU: 1 UID: 0 PID: 65 Comm: poc Not tainted 7.0.0-rc7-00195-geb216e422044 #1
>  RIP: 0010:ovs_vport_cmd_set+0x34c/0x400
>  Call Trace:
>   <TASK>
>   genl_family_rcv_msg_doit (net/netlink/genetlink.c:1116)
>   genl_rcv_msg (net/netlink/genetlink.c:1194)
>   netlink_rcv_skb (net/netlink/af_netlink.c:2550)
>   genl_rcv (net/netlink/genetlink.c:1219)
>   netlink_unicast (net/netlink/af_netlink.c:1344)
>   netlink_sendmsg (net/netlink/af_netlink.c:1894)
>   __sys_sendto (net/socket.c:2206)
>   __x64_sys_sendto (net/socket.c:2209)
>   do_syscall_64 (arch/x86/entry/syscall_64.c:63)
>   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
>   </TASK>
>  Kernel panic - not syncing: Fatal exception
> 
> Reject attempts to set more PIDs than nr_cpu_ids in
> ovs_vport_set_upcall_portids(), and pre-compute the worst-case reply
> size in ovs_vport_cmd_msg_size() based on that bound, similar to the
> existing ovs_dp_cmd_msg_size().  nr_cpu_ids matches the cap already
> used by the per-CPU dispatch configuration on the datapath side
> (ovs_dp_cmd_fill_info() serialises at most nr_cpu_ids PIDs), so the
> two sides stay consistent.
> 
> Fixes: 5cd667b0a456 ("openvswitch: Allow each vport to have an array of 'port_id's.")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Assisted-by: Claude:claude-opus-4-6
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---

sashiko AI review is concerned about a few things:

1. Potentially higher memory usage for these messages on systems with
   high core count.  This is not really a problem as we have similarly
   sized datapath info messages and even larger flow messages.

2. Technically a uAPI change since we're limiting the number of PIDs
   that can be supplied.  But, as I said before, this number should be
   limited by some value regardless, it's not good to have it unbounded,
   and also it is generally not a good idea to configure more handlers
   than cores, as it will worsen the "thundering herd" issue the
   per-vport dispatch method has.  Existing userspace by default sizes
   the number of threads to be lower than the number of cores.

In practice, per-vport dispatch is only used as a fallback in all modern
versions of Open vSwitch (since 2021).  So, we should also, probably,
explore the ways to deprecate it and eventually remove, which would likely
require at least removing the warning for when the dp->upcall_portids
array is smaller than the cpu_id, so the users are not always required to
supply the full array, e.g. for tests, but that's a separate topic.

For this particular patch:

Reviewed-by: Ilya Maximets <i.maximets@ovn.org>