linux-kernel - Re: [ovs-dev] [PATCH net-next] net: openvswitch: allow providing upcall pid for the 'execute' command

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1039a336-5f4e-4197-a27d-f91c58aa5104@ovn.org>
Date: Thu, 3 Jul 2025 13:15:17 +0200
From: Ilya Maximets <i.maximets@....org>
To: Flavio Leitner <fbl@...close.org>
Cc: i.maximets@....org, netdev@...r.kernel.org, dev@...nvswitch.org,
 linux-kernel@...r.kernel.org, Eric Dumazet <edumazet@...gle.com>,
 Simon Horman <horms@...nel.org>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, "David S. Miller" <davem@...emloft.net>
Subject: Re: [ovs-dev] [PATCH net-next] net: openvswitch: allow providing
 upcall pid for the 'execute' command

On 7/3/25 1:04 PM, Flavio Leitner wrote:
> On Thu, 3 Jul 2025 10:38:49 +0200
> Ilya Maximets <i.maximets@....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.

Best regards, Ilya Maximets.