On Thu, 3 Jul 2025 13:15:17 +0200 Ilya Maximets <i.maxim...@ovn.org> wrote:
> On 7/3/25 1:04 PM, Flavio Leitner wrote: > > On Thu, 3 Jul 2025 10:38:49 +0200 > > Ilya Maximets <i.maxim...@ovn.org> wrote: > > > >> On 7/3/25 1:08 AM, Flavio Leitner wrote: > >>>>>> @@ -651,6 +654,10 @@ static int ovs_packet_cmd_execute(struct sk_buff > >>>>>> *skb, struct genl_info *info) !!(hash & OVS_PACKET_HASH_L4_BIT)); > >>>>>> } > >>>>>> > >>>>>> + if (a[OVS_PACKET_ATTR_UPCALL_PID]) > >>>>>> + upcall_pid = > >>>>>> nla_get_u32(a[OVS_PACKET_ATTR_UPCALL_PID]); > >>>>>> + OVS_CB(packet)->upcall_pid = upcall_pid; > >>> > >>> Since this is coming from userspace, does it make sense to check if the > >>> upcall_pid is one of the pids in the dp->upcall_portids array? > >> > >> Not really. IMO, this would be an unnecessary artificial restriction. > >> We're not concerned about security here since OVS_PACKET_CMD_EXECUTE > >> requires the same privileges as the OVS_DP_CMD_NEW or the > >> OVS_DP_CMD_SET. > > > > What if the userspace is buggy or compromised? > > It seems netlink API will return -ECONNREFUSED and the upcall is dropped. > > Therefore, we would be okay either way, correct? > > If the userspace is compromised, it can overwrite the upcall_portids > and do many other things, since the userspace application here has a > CAP_NET_ADMIN. And if it's buggy, then the packet will be just dropped > on validation or on the upcall, there isn't much difference. > > It's a responsibility of the userspace application to make sure these > sockets exist before passing PIDs into the kernel. From the kernel's > perspective dropping the upcall is completely fine. So, yes, we should > be OK. ack, thanks! -- Flavio Leitner _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
