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 <[email protected]> > --- > 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. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
