From 5fc5b0574ba3e81228e50e8d31a550b822872931 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Oct 2024 12:35:47 +0100 Subject: [PATCH 1/3] internal/safepath: kubernetesSafeOpen: explicitly suppress unhandled err Signed-off-by: Sebastiaan van Stijn --- internal/safepath/k8s_safeopen_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/safepath/k8s_safeopen_linux.go b/internal/safepath/k8s_safeopen_linux.go index ebbe7e17a5..02db43a66b 100644 --- a/internal/safepath/k8s_safeopen_linux.go +++ b/internal/safepath/k8s_safeopen_linux.go @@ -78,7 +78,7 @@ func kubernetesSafeOpen(base, subpath string) (int, error) { // Trigger auto mount if it's an auto-mounted directory, ignore error if not a directory. // Notice the trailing slash is mandatory, see "automount" in openat(2) and open_by_handle_at(2). - unix_noeintr.Fstatat(parentFD, seg+"/", &deviceStat, unix.AT_SYMLINK_NOFOLLOW) + _ = unix_noeintr.Fstatat(parentFD, seg+"/", &deviceStat, unix.AT_SYMLINK_NOFOLLOW) log.G(context.TODO()).Debugf("Opening path %s", currentPath) childFD, err = unix_noeintr.Openat(parentFD, seg, openFDFlags|unix.O_CLOEXEC, 0) From 4b60c68803594b516f9c79f6292f66216bd8e351 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Oct 2024 12:40:44 +0100 Subject: [PATCH 2/3] internal/safepath: Join(): log some unhandled errors Similar to the kubernetesSafeOpen function. Signed-off-by: Sebastiaan van Stijn --- internal/safepath/join_linux.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/safepath/join_linux.go b/internal/safepath/join_linux.go index 68cb0d7abe..ef98957e6a 100644 --- a/internal/safepath/join_linux.go +++ b/internal/safepath/join_linux.go @@ -20,7 +20,7 @@ import ( // After use, it is the caller's responsibility to call Close on the returned // SafePath object, which will unmount the temporary file/directory // and remove it. -func Join(_ context.Context, path, subpath string) (*SafePath, error) { +func Join(ctx context.Context, path, subpath string) (*SafePath, error) { base, subpart, err := evaluatePath(path, subpath) if err != nil { return nil, err @@ -33,7 +33,11 @@ func Join(_ context.Context, path, subpath string) (*SafePath, error) { return nil, err } - defer unix_noeintr.Close(fd) + defer func() { + if err := unix_noeintr.Close(fd); err != nil { + log.G(ctx).WithError(err).Errorf("Closing FD %d failed for safeOpenFd(%s, %s)", fd, base, subpart) + } + }() tmpMount, err := tempMountPoint(fd) if err != nil { @@ -48,7 +52,9 @@ func Join(_ context.Context, path, subpath string) (*SafePath, error) { mountSource := "/proc/" + pid + "/fd/" + strconv.Itoa(fd) if err := unix_noeintr.Mount(mountSource, tmpMount, "none", unix.MS_BIND, ""); err != nil { - os.Remove(tmpMount) + if err := os.Remove(tmpMount); err != nil { + log.G(ctx).WithError(err).Warn("failed to remove tmpMount after failed mount") + } return nil, errors.Wrap(err, "failed to mount resolved path") } From 75e8f57579aac881d7a8e6f14a5792aca9c67e5c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Oct 2024 13:09:14 +0100 Subject: [PATCH 3/3] internal/safepath: Join(): remove workaround for ECI / Sysbox This workaround was added in 9a0cde66ba6ea04bd7d27fbfdbaefdbb01ad5787 to work around an issue on Docker Desktop with ECI (Enhanced Container Isolation) enabled, which uses the Sysbox runtime under the hood. A comment was added during review of the PR that added it (see [1]), and the internal discussion on Slack tracked down the issue to code in [nestybox/sysfs]. That issue was resolved Sysbox EE, and upstreamed to Sysbox CE through [nestybox/sysbox-fs@9cf74e4], which is part of Sysbox CE v0.6.3, so we can remove this workaround. [1]: https://github.com/moby/moby/pull/45687#discussion_r1280867905 [nestybox/sysfs]: https://github.com/nestybox/sysbox-fs/blob/30fd49edbd51048fed8b2ad0af327598d30b29eb/process/process.go#L644-L684 [nestybox/sysbox-fs@9cf74e4]: https://github.com/nestybox/sysbox-fs/commit/9cf74e4cbf3a61b1881bf5775460463900b55a87 Signed-off-by: Sebastiaan van Stijn --- internal/safepath/join_linux.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/internal/safepath/join_linux.go b/internal/safepath/join_linux.go index ef98957e6a..3832b0f30f 100644 --- a/internal/safepath/join_linux.go +++ b/internal/safepath/join_linux.go @@ -44,14 +44,7 @@ func Join(ctx context.Context, path, subpath string) (*SafePath, error) { return nil, errors.Wrap(err, "failed to create temporary file for safe mount") } - pid := strconv.Itoa(unix.Gettid()) - // Using explicit pid path, because /proc/self/fd/ fails with EACCES - // when running under "Enhanced Container Isolation" in Docker Desktop - // which uses sysbox runtime under the hood. - // TODO(vvoland): Investigate. - mountSource := "/proc/" + pid + "/fd/" + strconv.Itoa(fd) - - if err := unix_noeintr.Mount(mountSource, tmpMount, "none", unix.MS_BIND, ""); err != nil { + if err := unix_noeintr.Mount("/proc/self/fd/"+strconv.Itoa(fd), tmpMount, "none", unix.MS_BIND, ""); err != nil { if err := os.Remove(tmpMount); err != nil { log.G(ctx).WithError(err).Warn("failed to remove tmpMount after failed mount") }