Fix error from runc run on noexec fs #3522
Conversation
kolyshkin
added
kind/bug
backport/1.1-todo
|
@opencontainers/runc-maintainers do we ever use |
A newer runc (v1.1.x) changed the error message again, so simplify the check to look for a common denominator of all these errors. Note that due to an entirely different issue [1] the test still fails when using runc, but this is being fixed elsewhere ([2], [3]). This will presumably be fixed in runc 1.1.4 and runc 1.2.0. [1] opencontainers/runc#3520 [2] opencontainers/runc#3522 [3] https://go-review.googlesource.com/c/go/+/414824 Signed-off-by: Kir Kolyshkin <[email protected]>
There was a problem hiding this comment.
Sorry NOTLGTM, as potentially somebody might be using runc with setuid bit
(I know colima and Rancher Desktop launch nerdctl with setuid bit. AFAICS they do not launch runc with setuid bit, but I assume there are some similar projects and some of them may launch runc with setuid bit)
Thanks! I explained above this is not a problem -- we just need a more complex check for this case. Working on it. |
When starting a new container, and the very last step of executing of a user process fails (last lines of (*linuxStandardInit).Init), it is too late to print a proper error since both the log pipe and the init pipe are closed. This is partially mitigated by using exec.LookPath() which is supposed to say whether we will be able to execute or not. Alas, it fails to do so when the binary to be executed resides on a filesystem mounted with noexec flag. A workaround would be to use access(2) with X_OK flag. Alas, it is not working when runc itself is a setuid (or setgid) binary. In this case, faccessat2(2) with AT_EACCESS can be used, but it is only available since Linux v5.8. So, use faccessat2(2) with AT_EACCESS if available. If not, fall back to access(2) for non-setuid runc, and do nothing for setuid runc (as there is nothing we can do). Note that this check if in addition to whatever exec.LookPath does. Fixes opencontainers#3520 Signed-off-by: Kir Kolyshkin <[email protected]>
|
@AkihiroSuda added suid runc support; PTAL |
|
@cyphar LGTY? |
kolyshkin
added
backport/1.1-done
|
Can we revert this? For |
|
@AkihiroSuda from what I see, we need to add a check for CAP_DAC_OVERRIDE -- it it is set, than the lack of x bit can be ignored. I will look at it tomorrow. |
When starting a new container, and the very last step of executing of a
user process fails (last lines of
(*linuxStandardInit).Init), it is toolate to print a proper error since both the log pipe and the init pipe
are closed.
This is partially mitigated by using exec.LookPath() which is supposed
to say whether we will be able to execute or not. Alas, it fails to do
so when the binary to be executed resides on a filesystem mounted with
noexec flag.
A workaround is to use access(2) with X_OK flag. Alas, it is not
working when runc itself is a setuid (or setgid) binary. In this case,
faccessat2(2) with AT_EACCESS can be used, but it is only available
since Linux v5.8.
So, use faccessat2(2) with AT_EACCESS if available. If not, fall back to
access(2) for non-setuid runc, and do nothing for setuid runc (as there
is nothing we can do). Note that this check if in addition to whatever
exec.LookPath does.
The proper fix is something like https://go-review.googlesource.com/c/go/+/414824 but it won't be available for a while.
Fixes #3520