From 20b95f23ca33cb1751ee0b5318e1d5f3676032c6 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Tue, 8 Aug 2023 11:42:24 +1000 Subject: [PATCH] libcontainer: seccomp: pass around *os.File for notifyfd *os.File is correctly tracked by the garbage collector, and there's no need to use raw file descriptors for this code. Signed-off-by: Aleksa Sarai --- libcontainer/init_linux.go | 8 ++--- libcontainer/seccomp/patchbpf/enosys_linux.go | 13 ++++--- libcontainer/seccomp/seccomp_linux.go | 35 +++++++++---------- libcontainer/seccomp/seccomp_unsupported.go | 7 ++-- 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index cef1142a2..7b6c8113b 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -404,11 +404,11 @@ func syncParentHooks(pipe *os.File) error { // syncParentSeccomp sends the fd associated with the seccomp file descriptor // to the parent, and wait for the parent to do pidfd_getfd() to grab a copy. -func syncParentSeccomp(pipe *os.File, seccompFd int) error { - if seccompFd == -1 { +func syncParentSeccomp(pipe *os.File, seccompFd *os.File) error { + if seccompFd == nil { return nil } - defer unix.Close(seccompFd) + defer seccompFd.Close() // Tell parent to grab our fd. // @@ -419,7 +419,7 @@ func syncParentSeccomp(pipe *os.File, seccompFd int) error { // before the parent gets the file descriptor would deadlock "runc init" if // we allowed it for SCMP_ACT_NOTIFY). See seccomp.InitSeccomp() for more // details. - if err := writeSyncArg(pipe, procSeccomp, seccompFd); err != nil { + if err := writeSyncArg(pipe, procSeccomp, seccompFd.Fd()); err != nil { return err } // Wait for parent to tell us they've grabbed the seccompfd. diff --git a/libcontainer/seccomp/patchbpf/enosys_linux.go b/libcontainer/seccomp/patchbpf/enosys_linux.go index 7fc9fd662..40e515334 100644 --- a/libcontainer/seccomp/patchbpf/enosys_linux.go +++ b/libcontainer/seccomp/patchbpf/enosys_linux.go @@ -690,17 +690,17 @@ func sysSeccompSetFilter(flags uint, filter []unix.SockFilter) (fd int, err erro // patches said filter to handle -ENOSYS in a much nicer manner than the // default libseccomp default action behaviour, and loads the patched filter // into the kernel for the current process. -func PatchAndLoad(config *configs.Seccomp, filter *libseccomp.ScmpFilter) (int, error) { +func PatchAndLoad(config *configs.Seccomp, filter *libseccomp.ScmpFilter) (*os.File, error) { // Generate a patched filter. fprog, err := enosysPatchFilter(config, filter) if err != nil { - return -1, fmt.Errorf("error patching filter: %w", err) + return nil, fmt.Errorf("error patching filter: %w", err) } // Get the set of libseccomp flags set. seccompFlags, noNewPrivs, err := filterFlags(config, filter) if err != nil { - return -1, fmt.Errorf("unable to fetch seccomp filter flags: %w", err) + return nil, fmt.Errorf("unable to fetch seccomp filter flags: %w", err) } // Set no_new_privs if it was requested, though in runc we handle @@ -708,15 +708,14 @@ func PatchAndLoad(config *configs.Seccomp, filter *libseccomp.ScmpFilter) (int, if noNewPrivs { logrus.Warnf("potentially misconfigured filter -- setting no_new_privs in seccomp path") if err := unix.Prctl(unix.PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); err != nil { - return -1, fmt.Errorf("error enabling no_new_privs bit: %w", err) + return nil, fmt.Errorf("error enabling no_new_privs bit: %w", err) } } // Finally, load the filter. fd, err := sysSeccompSetFilter(seccompFlags, fprog) if err != nil { - return -1, fmt.Errorf("error loading seccomp filter: %w", err) + return nil, fmt.Errorf("error loading seccomp filter: %w", err) } - - return fd, nil + return os.NewFile(uintptr(fd), "[seccomp filter]"), nil } diff --git a/libcontainer/seccomp/seccomp_linux.go b/libcontainer/seccomp/seccomp_linux.go index fed02bced..2c64ebbba 100644 --- a/libcontainer/seccomp/seccomp_linux.go +++ b/libcontainer/seccomp/seccomp_linux.go @@ -6,6 +6,7 @@ package seccomp import ( "errors" "fmt" + "os" libseccomp "github.com/seccomp/libseccomp-golang" "github.com/sirupsen/logrus" @@ -27,17 +28,16 @@ const ( ) // InitSeccomp installs the seccomp filters to be used in the container as -// specified in config. -// Returns the seccomp file descriptor if any of the filters include a -// SCMP_ACT_NOTIFY action, otherwise returns -1. -func InitSeccomp(config *configs.Seccomp) (int, error) { +// specified in config. Returns the seccomp file descriptor if any of the +// filters include a SCMP_ACT_NOTIFY action. +func InitSeccomp(config *configs.Seccomp) (*os.File, error) { if config == nil { - return -1, errors.New("cannot initialize Seccomp - nil config passed") + return nil, errors.New("cannot initialize Seccomp - nil config passed") } defaultAction, err := getAction(config.DefaultAction, config.DefaultErrnoRet) if err != nil { - return -1, errors.New("error initializing seccomp - invalid default action") + return nil, errors.New("error initializing seccomp - invalid default action") } // Ignore the error since pre-2.4 libseccomp is treated as API level 0. @@ -45,7 +45,7 @@ func InitSeccomp(config *configs.Seccomp) (int, error) { for _, call := range config.Syscalls { if call.Action == configs.Notify { if apiLevel < 6 { - return -1, fmt.Errorf("seccomp notify unsupported: API level: got %d, want at least 6. Please try with libseccomp >= 2.5.0 and Linux >= 5.7", apiLevel) + return nil, fmt.Errorf("seccomp notify unsupported: API level: got %d, want at least 6. Please try with libseccomp >= 2.5.0 and Linux >= 5.7", apiLevel) } // We can't allow the write syscall to notify to the seccomp agent. @@ -61,36 +61,36 @@ func InitSeccomp(config *configs.Seccomp) (int, error) { // agent allows those syscalls to proceed, initialization works just fine and the agent can // handle future read()/close() syscalls as it wanted. if call.Name == "write" { - return -1, errors.New("SCMP_ACT_NOTIFY cannot be used for the write syscall") + return nil, errors.New("SCMP_ACT_NOTIFY cannot be used for the write syscall") } } } // See comment on why write is not allowed. The same reason applies, as this can mean handling write too. if defaultAction == libseccomp.ActNotify { - return -1, errors.New("SCMP_ACT_NOTIFY cannot be used as default action") + return nil, errors.New("SCMP_ACT_NOTIFY cannot be used as default action") } filter, err := libseccomp.NewFilter(defaultAction) if err != nil { - return -1, fmt.Errorf("error creating filter: %w", err) + return nil, fmt.Errorf("error creating filter: %w", err) } // Add extra architectures for _, arch := range config.Architectures { scmpArch, err := libseccomp.GetArchFromString(arch) if err != nil { - return -1, fmt.Errorf("error validating Seccomp architecture: %w", err) + return nil, fmt.Errorf("error validating Seccomp architecture: %w", err) } if err := filter.AddArch(scmpArch); err != nil { - return -1, fmt.Errorf("error adding architecture to seccomp filter: %w", err) + return nil, fmt.Errorf("error adding architecture to seccomp filter: %w", err) } } // Add extra flags. for _, flag := range config.Flags { if err := setFlag(filter, flag); err != nil { - return -1, err + return nil, err } } @@ -110,25 +110,24 @@ func InitSeccomp(config *configs.Seccomp) (int, error) { // Unset no new privs bit if err := filter.SetNoNewPrivsBit(false); err != nil { - return -1, fmt.Errorf("error setting no new privileges: %w", err) + return nil, fmt.Errorf("error setting no new privileges: %w", err) } // Add a rule for each syscall for _, call := range config.Syscalls { if call == nil { - return -1, errors.New("encountered nil syscall while initializing Seccomp") + return nil, errors.New("encountered nil syscall while initializing Seccomp") } if err := matchCall(filter, call, defaultAction); err != nil { - return -1, err + return nil, err } } seccompFd, err := patchbpf.PatchAndLoad(config, filter) if err != nil { - return -1, fmt.Errorf("error loading seccomp filter into kernel: %w", err) + return nil, fmt.Errorf("error loading seccomp filter into kernel: %w", err) } - return seccompFd, nil } diff --git a/libcontainer/seccomp/seccomp_unsupported.go b/libcontainer/seccomp/seccomp_unsupported.go index 885529dc7..b08a34986 100644 --- a/libcontainer/seccomp/seccomp_unsupported.go +++ b/libcontainer/seccomp/seccomp_unsupported.go @@ -5,6 +5,7 @@ package seccomp import ( "errors" + "os" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runtime-spec/specs-go" @@ -13,11 +14,11 @@ import ( var ErrSeccompNotEnabled = errors.New("seccomp: config provided but seccomp not supported") // InitSeccomp does nothing because seccomp is not supported. -func InitSeccomp(config *configs.Seccomp) (int, error) { +func InitSeccomp(config *configs.Seccomp) (*os.File, error) { if config != nil { - return -1, ErrSeccompNotEnabled + return nil, ErrSeccompNotEnabled } - return -1, nil + return nil, nil } // FlagSupported tells if a provided seccomp flag is supported.