Re: [PATCH] net/openvswitch: fix trigger-able BUG_ON after ovs_vport_cmd_fill_info

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

On 3/23/26 8:14 AM, sunichi wrote:
> ovs_vport_set_upcall_portids() does not validate the length of the
> user-supplied OVS_VPORT_ATTR_UPCALL_PID netlink attribute. A
> sufficiently large portid list can overflow the reply skb allocated
> with NLMSG_DEFAULT_SIZE in causing ovs_vport_cmd_fill_info()
> to return -EMSGSIZE and triggering the unconditional BUG_ON(),
> which panics the kernel on most distributions.
> 
> Any local user with CAP_NET_ADMIN (or an equivalent unprivileged
> namespace capability where applicable) can exploit this to perform a
> denial-of-service against the host.
> 
> Replace BUG_ON with WARN_ON_ONCE to prevent kernel panic.
> 
> Signed-off-by: sunichi <sunyiqixm@gmail.com>
> ---
>  net/openvswitch/datapath.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index e209099218b4..50c2945081a1 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -2202,7 +2202,8 @@ struct sk_buff *ovs_vport_cmd_build_info(struct vport *vport, struct net *net,
>  
>  	retval = ovs_vport_cmd_fill_info(vport, skb, net, portid, seq, 0, cmd,
>  					 GFP_KERNEL);
> -	BUG_ON(retval < 0);
> +	if (WARN_ON_ONCE(retval < 0))
> +		return ERR_PTR(-EMSGSIZE);

Hi, sunichi.  Thanks for the patch!  Though I don't think this is the right
solution.  Instead of just failing the request, we should allocate appropriate
amount of memory for it instead.  The fact that the array is sort of unbounded
today is also a problem.

So, what I'd suggest is, let's limit the number of upcall pids for a vport with
the number of CPUs as it is done for the upcall_pids array on the datapath level.
This will give us some reasonable upper value as there is no point for the
application to have more handlers than there are CPUs, and the existing userspace
never does that, so the limit should be safe.  Next we can create a new function
ovs_vport_cmd_msg_size() similar to the existing ovs_dp_cmd_msg_size() that would
calculate and allocate the appropriate message size, so the allocation is always
correct.

P.S.: This patch also needs a Fixes tag and should be targeted for the 'net'
tree, i.e. have [PATCH net] as a subject prefix.  Also, IIRC, kernel requires
a full name in the sign-off tag.

AI review also points out a memory leak in case we just return without freeing
and potentially leaving half- or even fully configured port while returning an
error to the userspace.

Best regards, Ilya Maximets.