From 99340bb0bd6cf2f3560ab5fb69eecf0b55b12a39 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Tue, 1 Aug 2023 16:30:05 +0200 Subject: [PATCH 1/5] contrib/fs-idmap: Reap childs This is what we should do, although in practice this probably won't be a big issue as the parent also exits. While we are there, instead of waiting for the child to finish, kill it if we did everything we wanted to do. Signed-off-by: Rodrigo Campos --- contrib/cmd/fs-idmap/fs-idmap.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contrib/cmd/fs-idmap/fs-idmap.go b/contrib/cmd/fs-idmap/fs-idmap.go index b48114dfe..c060bc5a2 100644 --- a/contrib/cmd/fs-idmap/fs-idmap.go +++ b/contrib/cmd/fs-idmap/fs-idmap.go @@ -31,6 +31,10 @@ func main() { if err := cmd.Start(); err != nil { log.Fatalf("failed to run the helper binary: %v", err) } + defer func() { + _ = cmd.Process.Kill() + _ = cmd.Wait() + }() path := fmt.Sprintf("/proc/%d/ns/user", cmd.Process.Pid) var userNsFile *os.File From 821d0018f55523ec139338236792327866ed9910 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Tue, 1 Aug 2023 16:34:51 +0200 Subject: [PATCH 2/5] contrib/fs-idmap: Remove not needed flags We don't really need to check if AT_RECURSIVE is possible here. We just want to check if we can idmap the src, it doesn't matter other nested mounts. While we are there, allow relative paths too. Signed-off-by: Rodrigo Campos --- contrib/cmd/fs-idmap/fs-idmap.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/cmd/fs-idmap/fs-idmap.go b/contrib/cmd/fs-idmap/fs-idmap.go index c060bc5a2..45f171c69 100644 --- a/contrib/cmd/fs-idmap/fs-idmap.go +++ b/contrib/cmd/fs-idmap/fs-idmap.go @@ -16,7 +16,7 @@ func main() { } src := os.Args[1] - treeFD, err := unix.OpenTree(-1, src, uint(unix.OPEN_TREE_CLONE|unix.OPEN_TREE_CLOEXEC|unix.AT_EMPTY_PATH|unix.AT_RECURSIVE)) + treeFD, err := unix.OpenTree(unix.AT_FDCWD, src, uint(unix.OPEN_TREE_CLONE|unix.OPEN_TREE_CLOEXEC|unix.AT_EMPTY_PATH)) if err != nil { log.Fatalf("error calling open_tree %q: %v", src, err) } @@ -48,7 +48,7 @@ func main() { Attr_set: unix.MOUNT_ATTR_IDMAP, Userns_fd: uint64(userNsFile.Fd()), } - if err := unix.MountSetattr(treeFD, "", unix.AT_EMPTY_PATH|unix.AT_RECURSIVE, &attr); err != nil { + if err := unix.MountSetattr(treeFD, "", unix.AT_EMPTY_PATH, &attr); err != nil { log.Fatalf("error calling mount_setattr: %v", err) } } From 882e5fe3bab0a27005f83d954c7dd43fbc00d8b3 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Tue, 1 Aug 2023 16:36:18 +0200 Subject: [PATCH 3/5] contrib/fs-idmap: Check exactly 2 args are received If more args are passed, let's just throw an error. Signed-off-by: Rodrigo Campos --- contrib/cmd/fs-idmap/fs-idmap.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/cmd/fs-idmap/fs-idmap.go b/contrib/cmd/fs-idmap/fs-idmap.go index 45f171c69..5879c74e7 100644 --- a/contrib/cmd/fs-idmap/fs-idmap.go +++ b/contrib/cmd/fs-idmap/fs-idmap.go @@ -11,7 +11,7 @@ import ( ) func main() { - if len(os.Args) < 2 { + if len(os.Args) != 2 { log.Fatalf("usage: %s path_to_mount_set_attr", os.Args[0]) } From 855c5a0e8b51b156effa486ee1bc70fdf59a0258 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Tue, 1 Aug 2023 16:37:27 +0200 Subject: [PATCH 4/5] contrib/fs-idmap: Don't hardcode sleep path Let's just rely on the lookup performed to find the sleep binary. This didn't cause any issues as far as I know, I just saw this while doing other cleanups. Signed-off-by: Rodrigo Campos --- contrib/cmd/fs-idmap/fs-idmap.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/cmd/fs-idmap/fs-idmap.go b/contrib/cmd/fs-idmap/fs-idmap.go index 5879c74e7..71971056d 100644 --- a/contrib/cmd/fs-idmap/fs-idmap.go +++ b/contrib/cmd/fs-idmap/fs-idmap.go @@ -22,7 +22,7 @@ func main() { } defer unix.Close(treeFD) - cmd := exec.Command("/usr/bin/sleep", "5") + cmd := exec.Command("sleep", "5") cmd.SysProcAttr = &syscall.SysProcAttr{ Cloneflags: syscall.CLONE_NEWUSER, UidMappings: []syscall.SysProcIDMap{{ContainerID: 0, HostID: 65536, Size: 65536}}, From 0688288876deb45cba0a9bb3f87798aaf5a6d676 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Tue, 1 Aug 2023 16:33:23 +0200 Subject: [PATCH 5/5] contrib/fs-idmap: Move logic to a new function We can't call log.Fatalf() and defer functions, as the former doesn't call any defers. Let's just move the code to a new function and call os.Exit() only in main, when all defer executed. Now that all the code is one function, we only print twice to stderr. It is simpler to just print to stderr instead of logging and having also the timestamp we don't really want. Signed-off-by: Rodrigo Campos --- contrib/cmd/fs-idmap/fs-idmap.go | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/contrib/cmd/fs-idmap/fs-idmap.go b/contrib/cmd/fs-idmap/fs-idmap.go index 71971056d..4593490f3 100644 --- a/contrib/cmd/fs-idmap/fs-idmap.go +++ b/contrib/cmd/fs-idmap/fs-idmap.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "log" "os" "os/exec" "syscall" @@ -12,13 +11,20 @@ import ( func main() { if len(os.Args) != 2 { - log.Fatalf("usage: %s path_to_mount_set_attr", os.Args[0]) + fmt.Fprintln(os.Stderr, "usage:", os.Args[0], "path_to_mount_set_attr") + os.Exit(1) } - src := os.Args[1] + if err := supportsIDMap(src); err != nil { + fmt.Fprintln(os.Stderr, "fatal error:", err) + os.Exit(1) + } +} + +func supportsIDMap(src string) error { treeFD, err := unix.OpenTree(unix.AT_FDCWD, src, uint(unix.OPEN_TREE_CLONE|unix.OPEN_TREE_CLOEXEC|unix.AT_EMPTY_PATH)) if err != nil { - log.Fatalf("error calling open_tree %q: %v", src, err) + return fmt.Errorf("error calling open_tree %q: %w", src, err) } defer unix.Close(treeFD) @@ -29,7 +35,7 @@ func main() { GidMappings: []syscall.SysProcIDMap{{ContainerID: 0, HostID: 65536, Size: 65536}}, } if err := cmd.Start(); err != nil { - log.Fatalf("failed to run the helper binary: %v", err) + return fmt.Errorf("failed to run the helper binary: %w", err) } defer func() { _ = cmd.Process.Kill() @@ -39,8 +45,7 @@ func main() { path := fmt.Sprintf("/proc/%d/ns/user", cmd.Process.Pid) var userNsFile *os.File if userNsFile, err = os.Open(path); err != nil { - log.Fatalf("unable to get user ns file descriptor: %v", err) - return + return fmt.Errorf("unable to get user ns file descriptor: %w", err) } defer userNsFile.Close() @@ -49,6 +54,8 @@ func main() { Userns_fd: uint64(userNsFile.Fd()), } if err := unix.MountSetattr(treeFD, "", unix.AT_EMPTY_PATH, &attr); err != nil { - log.Fatalf("error calling mount_setattr: %v", err) + return fmt.Errorf("error calling mount_setattr: %w", err) } + + return nil }