libct: switch to new mount api #5378
Conversation
ef4fecf to
3630a53
Compare
3630a53 to
6473ca6
Compare
6473ca6 to
dced1a2
Compare
dced1a2 to
381ad37
Compare
381ad37 to
f64ccc8
Compare
f64ccc8 to
30c74ea
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
libcontainer/rootfs_linux.go:1366
- sharedMaskSrc is always created as mountSourcePlain from sharedMaskFile, but when maskDir() returns a non-nil fd it is a detached mount fd (open_tree/fsmount). Detached mount fds cannot be used as a bind-mount source via mount(2) (/proc/.../fd/N), so later mountViaFds(..., sharedMaskSrc, ..., MS_BIND, ...) can fail or bind the wrong thing. To keep shared masking working, always reopen the mounted directory (reopenAfterMount) to get an O_PATH handle suitable for mountSourcePlain, and close any detached mount fd returned by maskDir().
sharedMaskSrc = &mountSource{Type: mountSourcePlain, file: sharedMaskFile}
| fd, err := unix.OpenTree(-int(unix.EBADF), "/", unix.OPEN_TREE_CLOEXEC) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| _ = unix.Close(fd) |
There was a problem hiding this comment.
This function is copied from github.com/cyphar/filepath-securejoin:
https://github.com/cyphar/filepath-securejoin/blob/main/pathrs-lite/internal/linux/mount_linux.go#L23-L47
kolyshkin
left a comment
kolyshkin
left a comment
There was a problem hiding this comment.
For the first commit, I'd like to add the following into the commit message (assuming it's true).
No functional change, just moving the code around.
| func checkProcMount(rootfs, dest string, m mountEntry) error { | ||
| func checkProcMount(rootfs, dest string, m *mountEntry) error { |
There was a problem hiding this comment.
What is the reason for this change? Just unification?
There was a problem hiding this comment.
Yes, it's not necessary to make this change, so reverted it now.
mount_entry has become complex and requires further refactoring. Extract it into a separate file to improve modularity and maintainability. No functional change, just moving the code around. Signed-off-by: lifubang <lifubang@acmcoder.com>
This change avoids duplicate fd closing and lays the groundwork for migrating to the new mount API by centralizing resource cleanup logic. Signed-off-by: lifubang <lifubang@acmcoder.com>
354f77f to
0024b4b
Compare
Adds a regression test to guarantee mount labels are respected for all supported mount types. Signed-off-by: lifubang <lifubang@acmcoder.com>
As documented in mount(2): if MS_BIND is included in mountflags, the filesystem type and mount data arguments are ignored. Therefore, keep mountLabel empty for bind mounts. Note: mount data is also blocked in the config validator: https://github.com/opencontainers/runc/blob/main/libcontainer/configs/validate/validator.go#L373-L375 Signed-off-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: lifubang <lifubang@acmcoder.com>
Signed-off-by: lifubang <lifubang@acmcoder.com>
0024b4b to
69bb33b
Compare
|
Add a commit libct: always ignore mount label for bind mounts to align the legacy mount(2) behavior with the new mount API for bind mounts. |
| // file descriptor fsfd using fsconfig(2). The options are expected to be a | ||
| // comma-separated list of key=value pairs or flags. | ||
| func fsconfigApplyOptions(fsfd int, opts string) error { | ||
| fields := strings.SplitSeq(opts, ",") |
There was a problem hiding this comment.
On another note, since unix.FsconfigSet(String/Flag) requires individual calls, we need to re-parse back the value of m.Data. It might be better to pass the original array directly instead of joining it here:
https://github.com/opencontainers/runc/blob/main/libcontainer/specconv/spec_linux.go#L1181-L1185
However, configs.Mount is widely used by other projects, so changing m.Data from string to []string might break compatibility.
Since the new mount API, such as
open_tree,fsopen,fsmount, andmove_mount, has matured over the years, it's time to embrace it fully.