From 38d2b048fd78fb09fc30f5a01c22950e89dc4b2c Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Tue, 25 Oct 2022 15:13:21 -0700 Subject: [PATCH] Allow fsgofer to open pipes that connect with host It requires setting the flag `--host-fifo=open`, otherwise the caller will not be allowed to open host FIFOs (or named pipes) to protect the host by default. Closes #8037 PiperOrigin-RevId: 483787839 --- runsc/cmd/gofer.go | 13 ++- runsc/config/config.go | 53 ++++++++++ runsc/config/config_test.go | 4 + runsc/config/flags.go | 3 +- runsc/fsgofer/fsgofer.go | 59 ++++++++--- runsc/fsgofer/lisafs.go | 27 +++-- test/e2e/BUILD | 1 - test/e2e/integration_test.go | 26 ----- test/runner/defs.bzl | 21 ++-- test/runner/main.go | 21 +++- test/syscalls/BUILD | 13 ++- test/syscalls/linux/BUILD | 14 +++ test/syscalls/linux/pipe_external.cc | 107 +++++++++++++++++++ test/uds/BUILD | 1 + test/uds/uds.go | 152 +++++++++++++++++++++++---- 15 files changed, 423 insertions(+), 92 deletions(-) create mode 100644 test/syscalls/linux/pipe_external.cc diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go index dbf9905f6..f3509c392 100644 --- a/runsc/cmd/gofer.go +++ b/runsc/cmd/gofer.go @@ -259,7 +259,8 @@ func (g *Gofer) serveLisafs(spec *specs.Spec, conf *config.Config, root string) server := fsgofer.NewLisafsServer(fsgofer.Config{ // These are global options. Ignore readonly configuration, that is set on // a per connection basis. - HostUDS: conf.GetHostUDS(), + HostUDS: conf.GetHostUDS(), + HostFifo: conf.HostFifo, }) // Start with root mount, then add any other additional mount as needed. @@ -318,8 +319,9 @@ func (g *Gofer) serve9P(spec *specs.Spec, conf *config.Config, root string) subc // Start with root mount, then add any other additional mount as needed. ats := make([]p9.Attacher, 0, len(spec.Mounts)+1) ap, err := fsgofer.NewAttachPoint("/", fsgofer.Config{ - ROMount: spec.Root.Readonly || conf.Overlay, - HostUDS: conf.GetHostUDS(), + ROMount: spec.Root.Readonly || conf.Overlay, + HostUDS: conf.GetHostUDS(), + HostFifo: conf.HostFifo, }) if err != nil { util.Fatalf("creating attach point: %v", err) @@ -331,8 +333,9 @@ func (g *Gofer) serve9P(spec *specs.Spec, conf *config.Config, root string) subc for _, m := range spec.Mounts { if specutils.IsGoferMount(m) { cfg := fsgofer.Config{ - ROMount: isReadonlyMount(m.Options) || conf.Overlay, - HostUDS: conf.GetHostUDS(), + ROMount: isReadonlyMount(m.Options) || conf.Overlay, + HostUDS: conf.GetHostUDS(), + HostFifo: conf.HostFifo, } ap, err := fsgofer.NewAttachPoint(m.Destination, cfg) if err != nil { diff --git a/runsc/config/config.go b/runsc/config/config.go index cbbc98c84..245bdc16b 100644 --- a/runsc/config/config.go +++ b/runsc/config/config.go @@ -83,6 +83,9 @@ type Config struct { // DO NOT call it directly, use GetHostComm() instead. HostUDS HostUDS `flag:"host-uds"` + // HostFifo controls permission to access host FIFO (or named pipes). + HostFifo HostFifo `flag:"host-fifo"` + // Network indicates what type of network to use. Network NetworkType `flag:"network"` @@ -533,3 +536,53 @@ func (g HostUDS) AllowOpen() bool { func (g HostUDS) AllowCreate() bool { return g&HostUDSCreate != 0 } + +// HostFifo tells how much of the host FIFO (or named pipes) the file system has +// access to. +type HostFifo int + +const ( + // HostFifoNone doesn't allow FIFO from the host to be manipulated. + HostFifoNone HostFifo = 0x0 + + // HostFifoOpen allows FIFOs from the host to be opened. + HostFifoOpen HostFifo = 0x1 +) + +func hostFifoPtr(v HostFifo) *HostFifo { + return &v +} + +// Set implements flag.Value. +func (g *HostFifo) Set(v string) error { + switch v { + case "", "none": + *g = HostFifoNone + case "open": + *g = HostFifoOpen + default: + return fmt.Errorf("invalid host fifo type %q", v) + } + return nil +} + +// Get implements flag.Value. +func (g *HostFifo) Get() interface{} { + return *g +} + +// String implements flag.Value. +func (g HostFifo) String() string { + if g == HostFifoNone { + return "none" + } + if g == HostFifoOpen { + return "open" + } + panic(fmt.Sprintf("Invalid host fifo type %d", g)) +} + +// AllowOpen returns true if it can consume FIFOs from the host. +func (g HostFifo) AllowOpen() bool { + return g&HostFifoOpen != 0 +} diff --git a/runsc/config/config_test.go b/runsc/config/config_test.go index f67bb7592..b5b0889f3 100644 --- a/runsc/config/config_test.go +++ b/runsc/config/config_test.go @@ -141,6 +141,10 @@ func TestInvalidFlags(t *testing.T) { name: "host-uds", error: "invalid host UDS", }, + { + name: "host-fifo", + error: "invalid host fifo", + }, } { t.Run(tc.name, func(t *testing.T) { testFlags := flag.NewFlagSet("test", flag.ContinueOnError) diff --git a/runsc/config/flags.go b/runsc/config/flags.go index f90f2b0f5..4b94d7e70 100644 --- a/runsc/config/flags.go +++ b/runsc/config/flags.go @@ -80,7 +80,8 @@ func RegisterFlags(flagSet *flag.FlagSet) { flagSet.Var(fileAccessTypePtr(FileAccessShared), "file-access-mounts", "specifies which filesystem validation to use for volumes other than the root mount: shared (default), exclusive.") flagSet.Bool("overlay", false, "wrap filesystem mounts with writable overlay. All modifications are stored in memory inside the sandbox.") flagSet.Bool("fsgofer-host-uds", false, "DEPRECATED: use host-uds=all") - flagSet.Var(hostUDSPtr(0), "host-uds", "controls permission to access host Unix-domain sockets. Values: none|open|create|all, default: none") + flagSet.Var(hostUDSPtr(HostUDSNone), "host-uds", "controls permission to access host Unix-domain sockets. Values: none|open|create|all, default: none") + flagSet.Var(hostFifoPtr(HostFifoNone), "host-fifo", "controls permission to access host FIFOs (or named pipes). Values: none|open, default: none") flagSet.Bool("vfs2", true, "DEPRECATED: this flag has no effect.") flagSet.Bool("fuse", false, "TEST ONLY; use while FUSE in VFSv2 is landing. This allows the use of the new experimental FUSE filesystem.") diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go index ac06deb06..d8ea60b0f 100644 --- a/runsc/fsgofer/fsgofer.go +++ b/runsc/fsgofer/fsgofer.go @@ -65,6 +65,9 @@ type Config struct { // HostUDS signals whether the gofer can connect to host unix domain sockets. HostUDS config.HostUDS + + // HostFifo signals whether the gofer can connect to host FIFOs. + HostFifo config.HostFifo } type attachPoint struct { @@ -315,13 +318,19 @@ func openAnyFile(pathDebug string, fn func(mode int) (*fd.FD, error)) (*fd.FD, b return nil, false, extractErrno(err) } -func checkSupportedFileType(mode uint32, hostComm config.HostUDS) error { +func checkSupportedFileType(mode uint32, config *Config) error { switch mode & unix.S_IFMT { case unix.S_IFREG, unix.S_IFDIR, unix.S_IFLNK: return nil case unix.S_IFSOCK: - if !hostComm.AllowOpen() { + if !config.HostUDS.AllowOpen() { + return unix.EPERM + } + return nil + + case unix.S_IFIFO: + if !config.HostFifo.AllowOpen() { return unix.EPERM } return nil @@ -332,7 +341,7 @@ func checkSupportedFileType(mode uint32, hostComm config.HostUDS) error { } func newLocalFile(a *attachPoint, file *fd.FD, path string, readable bool, stat *unix.Stat_t) (*localFile, error) { - if err := checkSupportedFileType(stat.Mode, a.conf.HostUDS); err != nil { + if err := checkSupportedFileType(stat.Mode, &a.conf); err != nil { return nil, err } @@ -347,25 +356,33 @@ func newLocalFile(a *attachPoint, file *fd.FD, path string, readable bool, stat }, nil } -// newFDMaybe creates a fd.FD from a file, dup'ing the FD and setting it as -// non-blocking. If anything fails, returns nil. It's better to have a file -// without host FD, than to fail the operation. +// newFDMaybe is the same as newFD, but returns nil if anything fails. It's +// better to have a file without host FD, than to fail the operation. func newFDMaybe(file *fd.FD) *fd.FD { + fd, err := newFD(file) + if err != nil { + return nil + } + return fd +} + +// newFD creates a fd.FD from a file, dup'ing the FD and setting it as +// non-blocking. +func newFD(file *fd.FD) (*fd.FD, error) { dupFD, err := unix.Dup(file.FD()) // Technically, the runtime may call the finalizer on file as soon as // FD() returns. runtime.KeepAlive(file) if err != nil { - return nil + return nil, err } - dup := fd.New(dupFD) // fd is blocking; non-blocking is required. - if err := unix.SetNonblock(dup.FD(), true); err != nil { - _ = dup.Close() - return nil + if err := unix.SetNonblock(dupFD, true); err != nil { + _ = unix.Close(dupFD) + return nil, err } - return dup + return fd.New(dupFD), nil } func fstat(fd int) (unix.Stat_t, error) { @@ -386,7 +403,7 @@ func setOwnerIfNeeded(fd int, uid p9.UID, gid p9.GID) (unix.Stat_t, error) { return unix.Stat_t{}, err } - // Change ownership if not set accordinly. + // Change ownership if not set accordingly. if uint32(uid) != stat.Uid || uint32(gid) != stat.Gid { if err := fchown(fd, uid, gid); err != nil { return unix.Stat_t{}, err @@ -428,9 +445,21 @@ func (l *localFile) Open(flags p9.OpenFlags) (*fd.FD, p9.QID, uint32, error) { } var fd *fd.FD - if l.fileType == unix.S_IFREG { - // Donate FD for regular files only. + switch l.fileType { + case unix.S_IFREG: + // Best effort to donate file to the Sentry (for performance only). fd = newFDMaybe(newFile) + + case unix.S_IFIFO: + // Character devices and pipes can block indefinitely during reads/writes, + // which is not allowed for gofer operations. Ensure that it donates an FD + // back to the caller, so it can wait on the FD when reads/writes return + // EWOULDBLOCK. + var err error + fd, err = newFD(newFile) + if err != nil { + return nil, p9.QID{}, 0, extractErrno(err) + } } // Close old file in case a new one was created. diff --git a/runsc/fsgofer/lisafs.go b/runsc/fsgofer/lisafs.go index d16e5c4ac..23739b7ec 100644 --- a/runsc/fsgofer/lisafs.go +++ b/runsc/fsgofer/lisafs.go @@ -70,7 +70,7 @@ func (s *LisafsServer) Mount(c *lisafs.Connection, mountNode *lisafs.Node) (*lis return nil, linux.Statx{}, err } - if err := checkSupportedFileType(uint32(stat.Mode), s.config.HostUDS); err != nil { + if err := checkSupportedFileType(uint32(stat.Mode), &s.config); err != nil { log.Warningf("Mount: checkSupportedFileType() failed for file %q with mode %o: %v", mountPath, stat.Mode, err) return nil, linux.Statx{}, err } @@ -341,7 +341,7 @@ func (fd *controlFDLisa) Walk(name string) (*lisafs.ControlFD, linux.Statx, erro return nil, linux.Statx{}, err } - if err := checkSupportedFileType(uint32(stat.Mode), fd.Conn().ServerImpl().(*LisafsServer).config.HostUDS); err != nil { + if err := checkSupportedFileType(uint32(stat.Mode), &fd.Conn().ServerImpl().(*LisafsServer).config); err != nil { _ = unix.Close(childHostFD) log.Warningf("Walk: checkSupportedFileType() failed for %q with mode %o: %v", name, stat.Mode, err) return nil, linux.Statx{}, err @@ -396,7 +396,7 @@ func (fd *controlFDLisa) WalkStat(path lisafs.StringArray, recordStat func(linux if err != nil { return err } - if err := checkSupportedFileType(uint32(stat.Mode), server.config.HostUDS); err != nil { + if err := checkSupportedFileType(uint32(stat.Mode), &server.config); err != nil { log.Warningf("WalkStat: checkSupportedFileType() failed for file %q with mode %o while walking path %+v: %v", name, stat.Mode, path, err) return err } @@ -422,13 +422,20 @@ func (fd *controlFDLisa) Open(flags uint32) (*lisafs.OpenFD, int, error) { openFD := fd.newOpenFDLisa(newHostFD, flags) hostOpenFD := -1 - if fd.IsRegular() { - // Donate FD for regular files only. Since FD donation is a destructive - // operation, we should duplicate the to-be-donated FD. Eat the error if - // one occurs, it is better to have an FD without a host FD, than failing - // the Open attempt. - if dupFD, err := unix.Dup(openFD.hostFD); err == nil { - hostOpenFD = dupFD + switch fd.FileType() { + case unix.S_IFREG: + // Best effort to donate file to the Sentry (for performance only). + hostOpenFD, _ = unix.Dup(openFD.hostFD) + + case unix.S_IFIFO: + // Character devices and pipes can block indefinitely during reads/writes, + // which is not allowed for gofer operations. Ensure that it donates an FD + // back to the caller, so it can wait on the FD when reads/writes return + // EWOULDBLOCK. + var err error + hostOpenFD, err = unix.Dup(openFD.hostFD) + if err != nil { + return nil, 0, err } } diff --git a/test/e2e/BUILD b/test/e2e/BUILD index 225569276..d9f71f838 100644 --- a/test/e2e/BUILD +++ b/test/e2e/BUILD @@ -23,7 +23,6 @@ go_test( "//pkg/test/testutil", "//runsc/specutils", "@com_github_docker_docker//api/types/mount:go_default_library", - "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/test/e2e/integration_test.go b/test/e2e/integration_test.go index 10e531003..8caf32eb8 100644 --- a/test/e2e/integration_test.go +++ b/test/e2e/integration_test.go @@ -30,7 +30,6 @@ import ( "net/http" "os" "os/exec" - "path" "path/filepath" "regexp" "strconv" @@ -39,7 +38,6 @@ import ( "time" "github.com/docker/docker/api/types/mount" - "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/test/dockerutil" "gvisor.dev/gvisor/pkg/test/testutil" ) @@ -999,27 +997,3 @@ func TestNonSearchableWorkingDirectory(t *testing.T) { t.Errorf("ls error message not found, want: %q, got: %q", wantErrorMsg, got) } } - -func TestPipeMountFails(t *testing.T) { - ctx := context.Background() - d := dockerutil.MakeContainer(ctx, t) - defer d.CleanUp(ctx) - - fifoPath := path.Join(testutil.TmpDir(), "fifo") - if err := unix.Mkfifo(fifoPath, 0666); err != nil { - t.Fatalf("Mkfifo(%q) failed: %v", fifoPath, err) - } - opts := dockerutil.RunOpts{ - Image: "basic/alpine", - Mounts: []mount.Mount{ - { - Type: mount.TypeBind, - Source: fifoPath, - Target: "/foo", - }, - }, - } - if _, err := d.Run(ctx, opts, "ls"); err == nil { - t.Errorf("docker run succeded, but mounting a named pipe should not work") - } -} diff --git a/test/runner/defs.bzl b/test/runner/defs.bzl index 3e2070c5a..6e7212c71 100644 --- a/test/runner/defs.bzl +++ b/test/runner/defs.bzl @@ -68,7 +68,7 @@ def _syscall_test( network = "none", file_access = "exclusive", overlay = False, - add_uds_tree = False, + add_host_communication = False, lisafs = True, fuse = False, container = None, @@ -138,7 +138,7 @@ def _syscall_test( "--use-tmpfs=" + str(use_tmpfs), "--file-access=" + file_access, "--overlay=" + str(overlay), - "--add-uds-tree=" + str(add_uds_tree), + "--add-host-communication=" + str(add_host_communication), "--lisafs=" + str(lisafs), "--fuse=" + str(fuse), "--strace=" + str(debug), @@ -170,7 +170,7 @@ def syscall_test( test, use_tmpfs = False, add_overlay = False, - add_uds_tree = False, + add_host_communication = False, add_hostinet = False, one_sandbox = True, fuse = False, @@ -185,8 +185,9 @@ def syscall_test( test: the test target. use_tmpfs: use tmpfs in the defined tests. add_overlay: add an overlay test. - add_uds_tree: add a UDS test. + add_host_communication: setup UDS and pipe external communication for tests. add_hostinet: add a hostinet test. + one_sandbox: runs each unit test in a new sandbox instance. fuse: enable FUSE support. allow_native: generate a native test variant. debug: enable debug output. @@ -203,7 +204,7 @@ def syscall_test( test = test, platform = "native", use_tmpfs = False, - add_uds_tree = add_uds_tree, + add_host_communication = add_host_communication, tags = tags, debug = debug, container = container, @@ -216,7 +217,7 @@ def syscall_test( test = test, platform = platform, use_tmpfs = use_tmpfs, - add_uds_tree = add_uds_tree, + add_host_communication = add_host_communication, tags = platform_tags + tags, fuse = fuse, debug = debug, @@ -230,7 +231,7 @@ def syscall_test( test = test, platform = default_platform, use_tmpfs = use_tmpfs, - add_uds_tree = add_uds_tree, + add_host_communication = add_host_communication, tags = platforms[default_platform] + tags, debug = debug, fuse = fuse, @@ -244,7 +245,7 @@ def syscall_test( test = test, platform = default_platform, use_tmpfs = use_tmpfs, - add_uds_tree = add_uds_tree, + add_host_communication = add_host_communication, tags = platforms.get(default_platform, []) + tags, debug = debug, fuse = fuse, @@ -259,7 +260,7 @@ def syscall_test( platform = default_platform, use_tmpfs = use_tmpfs, network = "host", - add_uds_tree = add_uds_tree, + add_host_communication = add_host_communication, tags = platforms.get(default_platform, []) + tags, debug = debug, fuse = fuse, @@ -273,7 +274,7 @@ def syscall_test( test = test, platform = default_platform, use_tmpfs = use_tmpfs, - add_uds_tree = add_uds_tree, + add_host_communication = add_host_communication, tags = platforms.get(default_platform, []) + tags, debug = debug, container = container, diff --git a/test/runner/main.go b/test/runner/main.go index 45d0158cf..7c15c2f94 100644 --- a/test/runner/main.go +++ b/test/runner/main.go @@ -57,7 +57,7 @@ var ( setupContainerPath = flag.String("setup-container", "", "path to setup_container binary (for use with --container)") trace = flag.Bool("trace", false, "enables all trace points") - addUDSTree = flag.Bool("add-uds-tree", false, "expose a tree of UDS utilities for use in tests") + addUDSTree = flag.Bool("add-host-communication", false, "expose a tree of UDS and pipe utilities to test communication with the host") // TODO(gvisor.dev/issue/4572): properly support leak checking for runsc, and // set to true as the default for the test runner. leakCheck = flag.Bool("leak-check", false, "check for reference leaks") @@ -218,7 +218,7 @@ func runRunsc(tc *gtest.TestCase, spec *specs.Spec) error { args = append(args, "-strace") } if *addUDSTree { - args = append(args, "-host-uds=all") + args = append(args, "-host-uds=all", "-host-fifo=open") } if *leakCheck { args = append(args, "-ref-leak-mode=log-names") @@ -328,8 +328,9 @@ func runRunsc(tc *gtest.TestCase, spec *specs.Spec) error { return err } -// setupUDSTree updates the spec to expose a UDS tree for gofer socket testing. -func setupUDSTree(spec *specs.Spec) (cleanup func(), err error) { +// setupHostCommTree updates the spec to expose a UDS and pipe files tree for +// testing communication with the host. +func setupHostCommTree(spec *specs.Spec) (cleanup func(), err error) { socketDir, cleanup, err := uds.CreateSocketTree("/tmp") if err != nil { return nil, fmt.Errorf("failed to create socket tree: %v", err) @@ -369,6 +370,16 @@ func setupUDSTree(spec *specs.Spec) (cleanup func(), err error) { Source: filepath.Join(socketDir, "dgram/null"), Type: "bind", }) + spec.Mounts = append(spec.Mounts, specs.Mount{ + Destination: "/tmp/sockets-attach/pipe/in", + Source: filepath.Join(socketDir, "pipe/in"), + Type: "bind", + }) + spec.Mounts = append(spec.Mounts, specs.Mount{ + Destination: "/tmp/sockets-attach/pipe/out", + Source: filepath.Join(socketDir, "pipe/out"), + Type: "bind", + }) spec.Process.Env = append(spec.Process.Env, "TEST_UDS_TREE=/tmp/sockets") spec.Process.Env = append(spec.Process.Env, "TEST_UDS_ATTACH_TREE=/tmp/sockets-attach") @@ -467,7 +478,7 @@ func runTestCaseRunsc(testBin string, tc *gtest.TestCase, args []string, t *test spec.Process.Env = env if *addUDSTree { - cleanup, err := setupUDSTree(spec) + cleanup, err := setupHostCommTree(spec) if err != nil { t.Fatalf("error creating UDS tree: %v", err) } diff --git a/test/syscalls/BUILD b/test/syscalls/BUILD index 352fff6bb..efc0c982b 100644 --- a/test/syscalls/BUILD +++ b/test/syscalls/BUILD @@ -108,9 +108,20 @@ syscall_test( ) syscall_test( - add_uds_tree = True, + add_host_communication = True, one_sandbox = False, test = "//test/syscalls/linux:connect_external_test", + # Shared mode tests replace /tmp which hides the files created for + # add_host_communication. use_tmpfs makes shared mode be skipped. + use_tmpfs = True, +) + +syscall_test( + add_host_communication = True, + one_sandbox = False, + test = "//test/syscalls/linux:pipe_external_test", + # Shared mode tests replace /tmp which hides the files created for + # add_host_communication. use_tmpfs makes shared mode be skipped. use_tmpfs = True, ) diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 23755930c..14cd74885 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -554,6 +554,20 @@ cc_binary( ], ) +cc_binary( + name = "pipe_external_test", + testonly = 1, + srcs = ["pipe_external.cc"], + linkstatic = 1, + deps = [ + "//test/util:file_descriptor", + "//test/util:fs_util", + gtest, + "//test/util:test_main", + "//test/util:test_util", + ], +) + cc_binary( name = "creat_test", testonly = 1, diff --git a/test/syscalls/linux/pipe_external.cc b/test/syscalls/linux/pipe_external.cc new file mode 100644 index 000000000..ef77f9bb8 --- /dev/null +++ b/test/syscalls/linux/pipe_external.cc @@ -0,0 +1,107 @@ +// Copyright 2022 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "gtest/gtest.h" +#include "test/util/file_descriptor.h" +#include "test/util/fs_util.h" +#include "test/util/test_util.h" + +// This file contains tests specific to connecting to host UDS managed outside +// the sandbox / test. +// +// A set of ultity sockets will be created externally in $TEST_UDS_TREE and +// $TEST_UDS_ATTACH_TREE for these tests to interact with. + +namespace gvisor { +namespace testing { + +namespace { + +struct ProtocolSocket { + int protocol; + std::string name; +}; + +// Parameter is pipe/UDS root dir. +using HostPipeTest = ::testing::TestWithParam; + +TEST_P(HostPipeTest, Read) { + const std::string env = GetParam(); + + const char* val = getenv(env.c_str()); + ASSERT_NE(val, nullptr); + const std::string root(val); + + const std::string path = JoinPath(root, "pipe", "in"); + FileDescriptor reader = ASSERT_NO_ERRNO_AND_VALUE(Open(path, O_RDONLY)); + + char lastValue = 0; + ssize_t length = 0; + while (length < 1024 * 1024) { + char buf[1024]; + + ssize_t read = ReadFd(reader.get(), buf, sizeof(buf)); + ASSERT_THAT(read, SyscallSucceeds()); + for (uint i = 0; i < read; ++i) { + ASSERT_EQ(static_cast(lastValue + i), buf[i]); + } + lastValue += read; + length += read; + } +} + +TEST_P(HostPipeTest, Write) { + const std::string env = GetParam(); + + const char* val = getenv(env.c_str()); + ASSERT_NE(val, nullptr); + const std::string root(val); + + const std::string path = JoinPath(root, "pipe", "out"); + FileDescriptor writer = ASSERT_NO_ERRNO_AND_VALUE(Open(path, O_WRONLY)); + + char lastValue = 0; + ssize_t length = 0; + while (length < 1024 * 1024) { + char buf[1024]; + for (int i = 0; i < sizeof(buf); ++i) { + buf[i] = i + lastValue; + } + + ASSERT_THAT(WriteFd(writer.get(), buf, sizeof(buf)), + SyscallSucceedsWithValue(sizeof(buf))); + lastValue += sizeof(buf); + length += sizeof(buf); + } +} + +INSTANTIATE_TEST_SUITE_P(Paths, HostPipeTest, + // Test access via standard path and attach point. + ::testing::Values("TEST_UDS_TREE", + "TEST_UDS_ATTACH_TREE")); + +} // namespace + +} // namespace testing +} // namespace gvisor diff --git a/test/uds/BUILD b/test/uds/BUILD index a8f49b50c..ab3aa27fc 100644 --- a/test/uds/BUILD +++ b/test/uds/BUILD @@ -10,6 +10,7 @@ go_library( testonly = 1, srcs = ["uds.go"], deps = [ + "//pkg/cleanup", "//pkg/log", "//pkg/unet", "@org_golang_x_sys//unix:go_default_library", diff --git a/test/uds/uds.go b/test/uds/uds.go index 41b742533..d860c199a 100644 --- a/test/uds/uds.go +++ b/test/uds/uds.go @@ -25,6 +25,7 @@ import ( "time" "golang.org/x/sys/unix" + "gvisor.dev/gvisor/pkg/cleanup" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/unet" ) @@ -185,27 +186,132 @@ func createNullSocket(path string, protocol int) (cleanup func(), err error) { return cleanup, nil } -type socketCreator func(path string, proto int) (cleanup func(), err error) +// createPipeWriter creates a pipe that writes a sequence of bytes starting from +// 0 to 256, wrapping it back to 0. +func createPipeWriter(path string) (func(), error) { + if err := unix.Mkfifo(path, 0644); err != nil { + return nil, err + } -// CreateSocketTree creates a local tree of unix domain sockets for use in -// testing: + // Open in another goroutine because open blocks until there is reader. Use a + // channel to send the file over to the cleanup routine, because closing the + // file triggers the goroutine to exit. + writerCh := make(chan *os.File, 1) + go func() { + writer, err := os.OpenFile(path, os.O_WRONLY, 0) + writerCh <- writer + if err != nil { + log.Warningf("Failed to open pipe: %v", err) + return + } + + for i := 0; ; i++ { + if _, err := writer.Write([]byte{byte(i)}); err != nil { + log.Warningf("Failed to write to pipe: %v", err) + return + } + } + }() + + cleanup := func() { + // Kick the goroutine in case it's blocked waiting for a reader. + if kicker, err := os.OpenFile(path, os.O_RDONLY|unix.O_NONBLOCK, 0); err != nil { + log.Warningf("Failed to kick pipe writer: %v", err) + return + } else { + _ = kicker.Close() + } + + writer := <-writerCh + if writer != nil { + if err := writer.Close(); err != nil { + log.Warningf("Failed to close pipe writer: %v", err) + } + } + } + return cleanup, nil +} + +// createPipeReader creates a pipe that reads from the pipe and expects a +// sequence of bytes starting from 0 to 256, wrapping it back to 0. +func createPipeReader(path string) (func(), error) { + if err := unix.Mkfifo(path, 0644); err != nil { + return nil, err + } + + // Open in another goroutine because open blocks until there is writer. Use a + // channel to send the file over to the cleanup routine, because closing the + // file triggers the goroutine to exit. + readerCh := make(chan *os.File, 1) + go func() { + reader, err := os.OpenFile(path, os.O_RDONLY, 0) + readerCh <- reader + if err != nil { + log.Warningf("Failed to open pipe: %v", err) + return + } + + var buf [1]byte + prev := byte(0xff) + for { + if _, err := reader.Read(buf[:]); err != nil { + log.Warningf("Failed to read to pipe: %v", err) + return + } + if want, got := prev+1, buf[0]; want != got { + panic(fmt.Sprintf("Wrong byte read from pipe, want: %v, got: %v", want, got)) + } + prev = buf[0] + } + }() + + cleanup := func() { + // Kick the goroutine in case it's blocked waiting for a reader. + if kicker, err := os.OpenFile(path, os.O_WRONLY|unix.O_NONBLOCK, 0); err != nil { + log.Warningf("Failed to kick pipe reader: %v", err) + return + } else { + _ = kicker.Close() + } + + reader := <-readerCh + if reader != nil { + if err := reader.Close(); err != nil { + log.Warningf("Failed to close pipe reader: %v", err) + } + } + } + return cleanup, nil +} + +type socketCreator func(path string, proto int) (cleanup func(), err error) +type pipeCreator func(path string) (cleanup func(), err error) + +// CreateSocketTree creates a local tree of unix domain sockets and pipes for +// use in testing: // - /stream/echo // - /stream/nonlistening // - /seqpacket/echo // - /seqpacket/nonlistening // - /dgram/null +// - /pipe/in +// - /pipe/out // // Additionally, it will attempt to connect to sockets at the following // locations, and turn into an echo server once connected: // - /stream/created-in-sandbox // - /seqpacket/created-in-sandbox -func CreateSocketTree(baseDir string) (dir string, cleanup func(), err error) { - dir, err = ioutil.TempDir(baseDir, "sockets") +func CreateSocketTree(baseDir string) (string, func(), error) { + dir, err := ioutil.TempDir(baseDir, "sockets") if err != nil { return "", nil, fmt.Errorf("error creating temp dir: %v", err) } + cu := cleanup.Make(func() { + _ = os.RemoveAll(dir) + }) + defer cu.Clean() - var protocols = []struct { + for _, proto := range []struct { protocol int name string sockets map[string]socketCreator @@ -235,10 +341,7 @@ func CreateSocketTree(baseDir string) (dir string, cleanup func(), err error) { "null": createNullSocket, }, }, - } - - var cleanups []func() - for _, proto := range protocols { + } { protoDir := filepath.Join(dir, proto.name) if err := os.Mkdir(protoDir, 0755); err != nil { return "", nil, fmt.Errorf("error creating %s dir: %v", proto.name, err) @@ -250,18 +353,31 @@ func CreateSocketTree(baseDir string) (dir string, cleanup func(), err error) { if err != nil { return "", nil, fmt.Errorf("error creating %s %s socket: %v", proto.name, name, err) } - - cleanups = append(cleanups, cleanup) + cu.Add(cleanup) } } - cleanup = func() { - for _, c := range cleanups { - c() + pipeDir := filepath.Join(dir, "pipe") + if err := os.Mkdir(pipeDir, 0755); err != nil { + return "", nil, err + } + for _, pipe := range []struct { + name string + ctor pipeCreator + }{ + { + name: "in", ctor: createPipeWriter, + }, + { + name: "out", ctor: createPipeReader, + }, + } { + cleanup, err := pipe.ctor(filepath.Join(pipeDir, pipe.name)) + if err != nil { + return "", nil, fmt.Errorf("error creating %q pipe: %w", pipe.name, err) } - - os.RemoveAll(dir) + cu.Add(cleanup) } - return dir, cleanup, nil + return dir, cu.Release(), nil }