-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UDP socket control #10
Comments
I found out that In general, server can be used to respond promptly to requests from different clients and it would be unreasonable to establish pseudo-connection for every client. What do you think about this case? |
Do you mean
If a request is received by a server, wouldn't it be possible to just call |
They use
Yes, it can be rewritten using pre-connected mechanism, but i don't know how this will affect performance. In some cases, server could send only 1-2 ACK packets to the client, so it may be unreasonable to call
Server can use single socket both for receiving and responding to messages. So, it must specify the destination address for each client via |
Btw we may have some issues with UDP multicast sockets. From man connect(2):
This means that restricted sandbox can only use |
Yes, we'll need to support |
Hi Mickaël, I've read the linked discussion threads, and I can try working on a first shot at the problem if that's still ok with your roadmap, e.g. considering #16 ?
About performance: I haven't had time to look at AFS, but more generally if anyone has potential applications (clients or servers) that could have issues with such a patch, I'd be interested to benchmark it. I'll read up on general kernel dev in parallel, worst that can happen is I realize how much complexity is hidden within a few weeks and come back empty handed. Cheers :) |
Hi Matthieu!
Sure! #16 is not a blocker.
Yes, that's the idea.
We could indeed rely on caching, but let's ignore this optimization for now. 😉 About the performance impact, I think we should start with a first implementation and do some benchmarks to see the impact. I think it should be OK for most use cases, and controlling UDP (like other Landlock restrictions) will always be optional anyway. @sm1ling-knight is working on #24, which should help benchmark worse cases. A simple optimization for pseudo-connected UDP sockets would be to tag the sockets when calling This makes me think about more appropriate access rights:
I think it should be a good first contribution. We now have all the mechanic in place. The main new think that we may need would be a generic socket blob (i.e. there is no field dedicated to
Thinking more about this new access rights, here are more thoughts: With TCP (i.e. connected protocol), we can deny arbitrary peer from sending data to a sandboxed process if we forbid With UDP (and other unconnected protocol), the local port can be set with a
If this is correct, we should think about a complementary access right to control automatic binding with What do you think? |
Hi Mickaël, Just a follow-up as I now have a few bits of answers and I see time flew since my last message.
An implicit call equivalent to
While working on a first patch for this model, I found two usecases in which the results are sub-optimal:
Since |
Thanks for the heads up.
Good point.
OK, so similarly we could also have a These two use cases are good examples and should be documented in a dedicated patch.
I think with these four new UDP access rights we should be fine. The documentation should highlight that Socket tagging would be an optimization for when About tagging:
The infrastructure management of the sock security landed in LSM next, so it should be part of Linux 5.12 and we can use this blob in Landlock. I guess it will not be needed though, nor |
[ Upstream commit a699781 ] A sysfs reader can race with a device reset or removal, attempting to read device state when the device is not actually present. eg: [exception RIP: qed_get_current_link+17] #8 [ffffb9e4f2907c48] qede_get_link_ksettings at ffffffffc07a994a [qede] #9 [ffffb9e4f2907cd8] __rh_call_get_link_ksettings at ffffffff992b01a3 #10 [ffffb9e4f2907d38] __ethtool_get_link_ksettings at ffffffff992b04e4 #11 [ffffb9e4f2907d90] duplex_show at ffffffff99260300 #12 [ffffb9e4f2907e38] dev_attr_show at ffffffff9905a01c #13 [ffffb9e4f2907e50] sysfs_kf_seq_show at ffffffff98e0145b #14 [ffffb9e4f2907e68] seq_read at ffffffff98d902e3 #15 [ffffb9e4f2907ec8] vfs_read at ffffffff98d657d1 #16 [ffffb9e4f2907f00] ksys_read at ffffffff98d65c3f #17 [ffffb9e4f2907f38] do_syscall_64 at ffffffff98a052fb crash> struct net_device.state ffff9a9d21336000 state = 5, state 5 is __LINK_STATE_START (0b1) and __LINK_STATE_NOCARRIER (0b100). The device is not present, note lack of __LINK_STATE_PRESENT (0b10). This is the same sort of panic as observed in commit 4224cfd ("net-sysfs: add check for netdevice being present to speed_show"). There are many other callers of __ethtool_get_link_ksettings() which don't have a device presence check. Move this check into ethtool to protect all callers. Fixes: d519e17 ("net: export device speed and duplex via sysfs") Fixes: 4224cfd ("net-sysfs: add check for netdevice being present to speed_show") Signed-off-by: Jamie Bainbridge <[email protected]> Link: https://patch.msgid.link/8bae218864beaa44ed01628140475b9bf641c5b0.1724393671.git.jamie.bainbridge@gmail.com Signed-off-by: Jakub Kicinski <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
We can now control TCP actions (
bind(2)
andconnect(2)
), and it would be useful to have a similar semantic for UDP. It's a bit tricky because of the datagram nature of UDP though.However, it should not be an issue to check
sendto(2)
andrecvfrom(2)
for UDP because performant-sensitive applications should useconnect/bind
andsend/recv
instead, and we can checkconnect/bind
without checkingsend/recv
. Indeed,bind
andconnect
can be used to configure an UDP socket, even if it is not a connected protocol. This approach enables to limit the number of kernel checks and copies.We first need to check real use cases to validate these assumptions...
See https://lore.kernel.org/all/[email protected]/
Cc @BoardzMaster
The text was updated successfully, but these errors were encountered: