On 3/24/26 1:01 PM, Ilya Maximets wrote: > 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.
Thanks for the detailed review, Ilya! Agreed on all points. For v2 patch submit later, would: 1. Add ovs_vport_cmd_msg_size() to calculate the correct allocation size upfront, eliminating the EMSGSIZE path entirely. 2. Fix the memory leak on the error path. > 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. > Respectfully, sunichi. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
