core/execute: refine the conditions to ignore -ENOANO #10115
core/execute: refine the conditions to ignore -ENOANO #10115yuwata wants to merge 3 commits intosystemd:masterfrom
Conversation
…turns -ENOANO Without this, log shows meaningless error message 'No anode', e.g., === Failed to unshare the mount namespace: Operation not permitted foo.service: Failed to set up mount namespacing: No anode foo.service: Failed at step NAMESPACE spawning /usr/bin/test: No anode === Follow-up for 1beab8b.
Setting DynamicUser= may introduce some bind mounts from RuntimeDirectory= and request mount namespacing. However, such implied bind mounts are not critical if creating mount namespace is failed.
|
I can't say I like this. The code is nice, even very nice, this is not the problem. But the idea of saying that some mount namespacing operations are "important" and others not seems wrong. In particular, ReadWritePaths, ReadOnlyPaths, InaccessiblePaths, PrivateTmp, PrivateDevices, Protect* are all things that I'd rate "critical". Then we're maybe left with some minor bits that are "non-critical", but then this doesn't make much sense. I think it's better to keep things more understandable by avoiding magic and just failing if the configuration cannot be executed. (This is different then stuff like seccomp filters, which are necessarilly architecture dependent and hard to get 100% complete. Namespacing operations are a core linux concept, and I expect the same behaviour everywhere.) |
|
@keszybz Thank you for the comment. Actually, making so many settings 'non-critical' is bad, and I made the 'critical' region too narrow. But, some implied settings seem not critical for me, e.g., implied bind mounts by RuntimeDirectory= + DynamicUser=, or implied private tmp by DynamicUser=. I feel DynamicUser= implies too much... |
yuwata
added
the
reviewed/needs-rework 🔨
|
hmm, so i missed the discussion around this one. I think the PR actually made a ton of sense. And yes, I think ReadOnlyPaths= is not as critical as BindPaths=, and the inability of having namespace support should be a problem for the latter but not for the former. Note that we don't blindly ignore errors when ReadOnlyPaths= fails, we only do that on EACCESS, EPERM, ENOSYS, EOPNOTSUPP, i.e. the errors typically used when some functionality is turned for us (either in the kernel or through seccomp or something like that). If somebody does that for us, then I think we can take that as a safe hint for "don't bother with sandboxing". i.e. if this priv is taken away from us then I think it's entirely OK to revert to sysv behaviour: i.e. not sandbox things like this. This is very different from BindPaths= that actually remaps stuff. If we'd silently ignore failures around that we'd break things beyond just making sandboxing ineffective, as file paths that should map to something specific suddenly would map somewhere else. So yes, I think namespacing failures are safe to gracefully ignore under the following two conditions:
|
3 participants
Setting
DynamicUser=may introduce some bind mounts fromRuntimeDirectory=and request mount namespacing. However, such implied bind mounts are not critical if creating mount namespace is failed.C.f. #10025.
cc @sourcejedi.
(Note that this includes the commits in #10114. But I hope GitHub treats them nicely.)