From a9441aea2780da8c93da1c73da860219f98438de Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Wed, 3 Mar 2021 10:23:55 -0800 Subject: [op] Replace syscall package usage with golang.org/x/sys/unix in pkg/. The syscall package has been deprecated in favor of golang.org/x/sys. Note that syscall is still used in the following places: - pkg/sentry/socket/hostinet/stack.go: some netlink related functionalities are not yet available in golang.org/x/sys. - syscall.Stat_t is still used in some places because os.FileInfo.Sys() still returns it and not unix.Stat_t. Updates #214 PiperOrigin-RevId: 360701387 --- pkg/sentry/fs/fdpipe/BUILD | 2 + pkg/sentry/fs/fdpipe/pipe.go | 14 +++--- pkg/sentry/fs/fdpipe/pipe_opener.go | 8 ++-- pkg/sentry/fs/fdpipe/pipe_opener_test.go | 78 ++++++++++++++++---------------- pkg/sentry/fs/fdpipe/pipe_test.go | 56 +++++++++++------------ 5 files changed, 80 insertions(+), 78 deletions(-) (limited to 'pkg/sentry/fs/fdpipe') diff --git a/pkg/sentry/fs/fdpipe/BUILD b/pkg/sentry/fs/fdpipe/BUILD index 1d09e983c..c83baf464 100644 --- a/pkg/sentry/fs/fdpipe/BUILD +++ b/pkg/sentry/fs/fdpipe/BUILD @@ -24,6 +24,7 @@ go_library( "//pkg/syserror", "//pkg/usermem", "//pkg/waiter", + "@org_golang_x_sys//unix:go_default_library", ], ) @@ -44,5 +45,6 @@ go_test( "//pkg/syserror", "//pkg/usermem", "@com_github_google_uuid//:go_default_library", + "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/pkg/sentry/fs/fdpipe/pipe.go b/pkg/sentry/fs/fdpipe/pipe.go index b99199798..757b7d511 100644 --- a/pkg/sentry/fs/fdpipe/pipe.go +++ b/pkg/sentry/fs/fdpipe/pipe.go @@ -17,8 +17,8 @@ package fdpipe import ( "os" - "syscall" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/fd" "gvisor.dev/gvisor/pkg/fdnotifier" @@ -82,16 +82,16 @@ func newPipeOperations(ctx context.Context, opener NonBlockingOpener, flags fs.F // init initializes p.file. func (p *pipeOperations) init() error { - var s syscall.Stat_t - if err := syscall.Fstat(p.file.FD(), &s); err != nil { + var s unix.Stat_t + if err := unix.Fstat(p.file.FD(), &s); err != nil { log.Warningf("pipe: cannot stat fd %d: %v", p.file.FD(), err) - return syscall.EINVAL + return unix.EINVAL } - if (s.Mode & syscall.S_IFMT) != syscall.S_IFIFO { + if (s.Mode & unix.S_IFMT) != unix.S_IFIFO { log.Warningf("pipe: cannot load fd %d as pipe, file type: %o", p.file.FD(), s.Mode) - return syscall.EINVAL + return unix.EINVAL } - if err := syscall.SetNonblock(p.file.FD(), true); err != nil { + if err := unix.SetNonblock(p.file.FD(), true); err != nil { return err } return fdnotifier.AddFD(int32(p.file.FD()), &p.Queue) diff --git a/pkg/sentry/fs/fdpipe/pipe_opener.go b/pkg/sentry/fs/fdpipe/pipe_opener.go index 0c3595998..adda19168 100644 --- a/pkg/sentry/fs/fdpipe/pipe_opener.go +++ b/pkg/sentry/fs/fdpipe/pipe_opener.go @@ -17,9 +17,9 @@ package fdpipe import ( "io" "os" - "syscall" "time" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/fd" "gvisor.dev/gvisor/pkg/sentry/fs" @@ -96,7 +96,7 @@ func (p *pipeOpenState) TryOpen(ctx context.Context, opener NonBlockingOpener, f switch { // Reject invalid configurations so they don't accidentally succeed below. case !flags.Read && !flags.Write: - return nil, syscall.EINVAL + return nil, unix.EINVAL // Handle opening RDWR or with O_NONBLOCK: will never block, so try only once. case (flags.Read && flags.Write) || flags.NonBlocking: @@ -155,7 +155,7 @@ func (p *pipeOpenState) TryOpenReadOnly(ctx context.Context, opener NonBlockingO // Any error that is not EWOULDBLOCK also means we're not // ready yet, and probably never will be ready. In this // case we need to close the host pipe we opened. - if unwrapError(rerr) != syscall.EWOULDBLOCK { + if unwrapError(rerr) != unix.EWOULDBLOCK { p.hostFile.Close() return nil, rerr } @@ -183,7 +183,7 @@ func (p *pipeOpenState) TryOpenReadOnly(ctx context.Context, opener NonBlockingO // to an syserror.ErrWouldBlock, to tell callers to retry. func (*pipeOpenState) TryOpenWriteOnly(ctx context.Context, opener NonBlockingOpener) (*pipeOperations, error) { hostFile, err := opener.NonBlockingOpen(ctx, fs.PermMask{Write: true}) - if unwrapError(err) == syscall.ENXIO { + if unwrapError(err) == unix.ENXIO { return nil, syserror.ErrWouldBlock } if err != nil { diff --git a/pkg/sentry/fs/fdpipe/pipe_opener_test.go b/pkg/sentry/fs/fdpipe/pipe_opener_test.go index b9cec4b13..7b3ff191f 100644 --- a/pkg/sentry/fs/fdpipe/pipe_opener_test.go +++ b/pkg/sentry/fs/fdpipe/pipe_opener_test.go @@ -20,11 +20,11 @@ import ( "io" "os" "path" - "syscall" "testing" "time" "github.com/google/uuid" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/fd" @@ -42,15 +42,15 @@ func (h *hostOpener) NonBlockingOpen(_ context.Context, p fs.PermMask) (*fd.FD, var flags int switch { case p.Read && p.Write: - flags = syscall.O_RDWR + flags = unix.O_RDWR case p.Write: - flags = syscall.O_WRONLY + flags = unix.O_WRONLY case p.Read: - flags = syscall.O_RDONLY + flags = unix.O_RDONLY default: - return nil, syscall.EINVAL + return nil, unix.EINVAL } - f, err := syscall.Open(h.name, flags|syscall.O_NONBLOCK, 0666) + f, err := unix.Open(h.name, flags|unix.O_NONBLOCK, 0666) if err != nil { return nil, err } @@ -62,7 +62,7 @@ func pipename() string { } func mkpipe(name string) error { - return syscall.Mknod(name, syscall.S_IFIFO|0666, 0) + return unix.Mknod(name, unix.S_IFIFO|0666, 0) } func TestTryOpen(t *testing.T) { @@ -87,14 +87,14 @@ func TestTryOpen(t *testing.T) { makePipe: false, flags: fs.FileFlags{}, /* bogus */ expectFile: false, - err: syscall.EINVAL, + err: unix.EINVAL, }, { desc: "NonBlocking Read only error returns immediately", makePipe: false, /* causes the error */ flags: fs.FileFlags{Read: true, NonBlocking: true}, expectFile: false, - err: syscall.ENOENT, + err: unix.ENOENT, }, { desc: "NonBlocking Read only success returns immediately", @@ -108,21 +108,21 @@ func TestTryOpen(t *testing.T) { makePipe: false, /* causes the error */ flags: fs.FileFlags{Write: true, NonBlocking: true}, expectFile: false, - err: syscall.ENOENT, + err: unix.ENOENT, }, { desc: "NonBlocking Write only no reader error returns immediately", makePipe: true, flags: fs.FileFlags{Write: true, NonBlocking: true}, expectFile: false, - err: syscall.ENXIO, + err: unix.ENXIO, }, { desc: "ReadWrite error returns immediately", makePipe: false, /* causes the error */ flags: fs.FileFlags{Read: true, Write: true}, expectFile: false, - err: syscall.ENOENT, + err: unix.ENOENT, }, { desc: "ReadWrite returns immediately", @@ -136,14 +136,14 @@ func TestTryOpen(t *testing.T) { makePipe: false, /* causes the error */ flags: fs.FileFlags{Write: true}, expectFile: false, - err: syscall.ENOENT, /* from bogus perms */ + err: unix.ENOENT, /* from bogus perms */ }, { desc: "Blocking Read only returns open error", makePipe: false, /* causes the error */ flags: fs.FileFlags{Read: true}, expectFile: false, - err: syscall.ENOENT, + err: unix.ENOENT, }, { desc: "Blocking Write only returns with syserror.ErrWouldBlock", @@ -167,7 +167,7 @@ func TestTryOpen(t *testing.T) { t.Errorf("%s: failed to make host pipe: %v", test.desc, err) continue } - defer syscall.Unlink(name) + defer unix.Unlink(name) } // Use a host opener to keep things simple. @@ -238,7 +238,7 @@ func TestPipeOpenUnblocksEventually(t *testing.T) { t.Errorf("%s: failed to make host pipe: %v", test.desc, err) continue } - defer syscall.Unlink(name) + defer unix.Unlink(name) // Spawn the partner. type fderr struct { @@ -249,18 +249,18 @@ func TestPipeOpenUnblocksEventually(t *testing.T) { go func() { var flags int if test.partnerIsReader { - flags = syscall.O_RDONLY + flags = unix.O_RDONLY } else { - flags = syscall.O_WRONLY + flags = unix.O_WRONLY } if test.partnerIsBlocking { - fd, err := syscall.Open(name, flags, 0666) + fd, err := unix.Open(name, flags, 0666) errch <- fderr{fd: fd, err: err} } else { var fd int - err := error(syscall.ENXIO) - for err == syscall.ENXIO { - fd, err = syscall.Open(name, flags|syscall.O_NONBLOCK, 0666) + err := error(unix.ENXIO) + for err == unix.ENXIO { + fd, err = unix.Open(name, flags|unix.O_NONBLOCK, 0666) time.Sleep(1 * time.Second) } errch <- fderr{fd: fd, err: err} @@ -289,7 +289,7 @@ func TestPipeOpenUnblocksEventually(t *testing.T) { continue } // If so, then close the partner fd to avoid leaking an fd. - syscall.Close(e.fd) + unix.Close(e.fd) // Check that our blocking open was successful. if err != nil { @@ -309,7 +309,7 @@ func TestCopiedReadAheadBuffer(t *testing.T) { if err := mkpipe(name); err != nil { t.Fatalf("failed to make host pipe: %v", err) } - defer syscall.Unlink(name) + defer unix.Unlink(name) // We're taking advantage of the fact that pipes opened read only always return // success, but internally they are not deemed "opened" until we're sure that @@ -326,35 +326,35 @@ func TestCopiedReadAheadBuffer(t *testing.T) { pipeOps, err := pipeOpenState.TryOpen(ctx, opener, fs.FileFlags{Read: true}) if pipeOps != nil { pipeOps.Release(ctx) - t.Fatalf("open(%s, %o) got file, want nil", name, syscall.O_RDONLY) + t.Fatalf("open(%s, %o) got file, want nil", name, unix.O_RDONLY) } if err != syserror.ErrWouldBlock { - t.Fatalf("open(%s, %o) got error %v, want %v", name, syscall.O_RDONLY, err, syserror.ErrWouldBlock) + t.Fatalf("open(%s, %o) got error %v, want %v", name, unix.O_RDONLY, err, syserror.ErrWouldBlock) } // Then open the same pipe write only and write some bytes to it. The next // time we try to open the pipe read only again via the pipeOpenState, we should // succeed and buffer some of the bytes written. - fd, err := syscall.Open(name, syscall.O_WRONLY, 0666) + fd, err := unix.Open(name, unix.O_WRONLY, 0666) if err != nil { - t.Fatalf("open(%s, %o) got error %v, want nil", name, syscall.O_WRONLY, err) + t.Fatalf("open(%s, %o) got error %v, want nil", name, unix.O_WRONLY, err) } - defer syscall.Close(fd) + defer unix.Close(fd) data := []byte("hello") - if n, err := syscall.Write(fd, data); n != len(data) || err != nil { + if n, err := unix.Write(fd, data); n != len(data) || err != nil { t.Fatalf("write(%v) got (%d, %v), want (%d, nil)", data, n, err, len(data)) } // Try the read again, knowing that it should succeed this time. pipeOps, err = pipeOpenState.TryOpen(ctx, opener, fs.FileFlags{Read: true}) if pipeOps == nil { - t.Fatalf("open(%s, %o) got nil file, want not nil", name, syscall.O_RDONLY) + t.Fatalf("open(%s, %o) got nil file, want not nil", name, unix.O_RDONLY) } defer pipeOps.Release(ctx) if err != nil { - t.Fatalf("open(%s, %o) got error %v, want nil", name, syscall.O_RDONLY, err) + t.Fatalf("open(%s, %o) got error %v, want nil", name, unix.O_RDONLY, err) } inode := fs.NewMockInode(ctx, fs.NewMockMountSource(nil), fs.StableAttr{ @@ -432,7 +432,7 @@ func TestPipeHangup(t *testing.T) { t.Errorf("%s: failed to make host pipe: %v", test.desc, err) continue } - defer syscall.Unlink(name) + defer unix.Unlink(name) // Fire off a partner routine which tries to open the same pipe blocking, // which will synchronize with us. The channel allows us to get back the @@ -444,11 +444,11 @@ func TestPipeHangup(t *testing.T) { // misconfiguration. var flags int if test.flags.Read { - flags = syscall.O_WRONLY + flags = unix.O_WRONLY } else { - flags = syscall.O_RDONLY + flags = unix.O_RDONLY } - fd, err := syscall.Open(name, flags, 0666) + fd, err := unix.Open(name, flags, 0666) if err != nil { t.Logf("Open(%q, %o, 0666) partner failed: %v", name, flags, err) } @@ -489,7 +489,7 @@ func TestPipeHangup(t *testing.T) { } } else { // Hangup our partner and expect us to get the hangup error. - syscall.Close(f) + unix.Close(f) defer pipeOps.Release(ctx) if test.flags.Read { @@ -515,8 +515,8 @@ func assertReaderHungup(t *testing.T, desc string, reader io.Reader) bool { } func assertWriterHungup(t *testing.T, desc string, writer io.Writer) bool { - if _, err := writer.Write([]byte("hello")); unwrapError(err) != syscall.EPIPE { - t.Errorf("%s: write to self after hangup got error %v, want %v", desc, err, syscall.EPIPE) + if _, err := writer.Write([]byte("hello")); unwrapError(err) != unix.EPIPE { + t.Errorf("%s: write to self after hangup got error %v, want %v", desc, err, unix.EPIPE) return false } return true diff --git a/pkg/sentry/fs/fdpipe/pipe_test.go b/pkg/sentry/fs/fdpipe/pipe_test.go index 1c9e82562..faeb3908c 100644 --- a/pkg/sentry/fs/fdpipe/pipe_test.go +++ b/pkg/sentry/fs/fdpipe/pipe_test.go @@ -18,9 +18,9 @@ import ( "bytes" "io" "os" - "syscall" "testing" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/fd" "gvisor.dev/gvisor/pkg/fdnotifier" "gvisor.dev/gvisor/pkg/sentry/contexttest" @@ -31,15 +31,15 @@ import ( func singlePipeFD() (int, error) { fds := make([]int, 2) - if err := syscall.Pipe(fds); err != nil { + if err := unix.Pipe(fds); err != nil { return -1, err } - syscall.Close(fds[1]) + unix.Close(fds[1]) return fds[0], nil } func singleDirFD() (int, error) { - return syscall.Open(os.TempDir(), syscall.O_RDONLY, 0666) + return unix.Open(os.TempDir(), unix.O_RDONLY, 0666) } func mockPipeDirent(t *testing.T) *fs.Dirent { @@ -77,12 +77,12 @@ func TestNewPipe(t *testing.T) { { desc: "Cannot make new pipe from bad fd", getfd: func() (int, error) { return -1, nil }, - err: syscall.EINVAL, + err: unix.EINVAL, }, { desc: "Cannot make new pipe from non-pipe fd", getfd: singleDirFD, - err: syscall.EINVAL, + err: unix.EINVAL, }, { desc: "Can make new pipe from pipe fd", @@ -127,12 +127,12 @@ func TestNewPipe(t *testing.T) { t.Errorf("%s: got read ahead buffer length %d, want %d", test.desc, len(p.readAheadBuffer), len(test.readAheadBuffer)) continue } - fileFlags, _, errno := syscall.Syscall(syscall.SYS_FCNTL, uintptr(p.file.FD()), syscall.F_GETFL, 0) + fileFlags, _, errno := unix.Syscall(unix.SYS_FCNTL, uintptr(p.file.FD()), unix.F_GETFL, 0) if errno != 0 { t.Errorf("%s: failed to get file flags for fd %d, got %v, want 0", test.desc, p.file.FD(), errno) continue } - if fileFlags&syscall.O_NONBLOCK == 0 { + if fileFlags&unix.O_NONBLOCK == 0 { t.Errorf("%s: pipe is blocking, expected non-blocking", test.desc) continue } @@ -145,13 +145,13 @@ func TestNewPipe(t *testing.T) { func TestPipeDestruction(t *testing.T) { fds := make([]int, 2) - if err := syscall.Pipe(fds); err != nil { + if err := unix.Pipe(fds); err != nil { t.Fatalf("failed to create pipes: got %v, want nil", err) } f := fd.New(fds[0]) // We don't care about the other end, just use the read end. - syscall.Close(fds[1]) + unix.Close(fds[1]) // Test the read end, but it doesn't really matter which. ctx := contexttest.Context(t) @@ -207,17 +207,17 @@ func TestPipeRequest(t *testing.T) { { desc: "ReadDir on pipe returns ENOTDIR", context: &ReadDir{}, - err: syscall.ENOTDIR, + err: unix.ENOTDIR, }, { desc: "Fsync on pipe returns EINVAL", context: &Fsync{}, - err: syscall.EINVAL, + err: unix.EINVAL, }, { desc: "Seek on pipe returns ESPIPE", context: &Seek{}, - err: syscall.ESPIPE, + err: unix.ESPIPE, }, { desc: "Readv on pipe from empty buffer returns nil", @@ -246,7 +246,7 @@ func TestPipeRequest(t *testing.T) { desc: "Writev on pipe from non-empty buffer and closed partner returns EPIPE", context: &Writev{Src: usermem.BytesIOSequence([]byte("hello"))}, flags: fs.FileFlags{Write: true}, - err: syscall.EPIPE, + err: unix.EPIPE, }, { desc: "Writev on pipe from non-empty buffer and open partner succeeds", @@ -260,7 +260,7 @@ func TestPipeRequest(t *testing.T) { } fds := make([]int, 2) - if err := syscall.Pipe(fds); err != nil { + if err := unix.Pipe(fds); err != nil { t.Errorf("%s: failed to create pipes: got %v, want nil", test.desc, err) continue } @@ -273,9 +273,9 @@ func TestPipeRequest(t *testing.T) { // Configure closing the fds. if test.keepOpenPartner { - defer syscall.Close(partnerFd) + defer unix.Close(partnerFd) } else { - syscall.Close(partnerFd) + unix.Close(partnerFd) } // Create the pipe. @@ -313,17 +313,17 @@ func TestPipeRequest(t *testing.T) { func TestPipeReadAheadBuffer(t *testing.T) { fds := make([]int, 2) - if err := syscall.Pipe(fds); err != nil { + if err := unix.Pipe(fds); err != nil { t.Fatalf("failed to create pipes: got %v, want nil", err) } rfile := fd.New(fds[0]) // Eventually close the write end, which is not wrapped in a pipe object. - defer syscall.Close(fds[1]) + defer unix.Close(fds[1]) // Write some bytes to this end. data := []byte("world") - if n, err := syscall.Write(fds[1], data); n != len(data) || err != nil { + if n, err := unix.Write(fds[1], data); n != len(data) || err != nil { rfile.Close() t.Fatalf("write to pipe got (%d, %v), want (%d, nil)", n, err, len(data)) } @@ -365,13 +365,13 @@ func TestPipeReadAheadBuffer(t *testing.T) { // all of the data (and report it as such). func TestPipeReadsAccumulate(t *testing.T) { fds := make([]int, 2) - if err := syscall.Pipe(fds); err != nil { + if err := unix.Pipe(fds); err != nil { t.Fatalf("failed to create pipes: got %v, want nil", err) } rfile := fd.New(fds[0]) // Eventually close the write end, it doesn't depend on a pipe object. - defer syscall.Close(fds[1]) + defer unix.Close(fds[1]) // Get a new read only pipe reference. ctx := contexttest.Context(t) @@ -391,7 +391,7 @@ func TestPipeReadsAccumulate(t *testing.T) { // Write some some bytes to the pipe. data := []byte("some message") - if n, err := syscall.Write(fds[1], data); n != len(data) || err != nil { + if n, err := unix.Write(fds[1], data); n != len(data) || err != nil { t.Fatalf("write to pipe got (%d, %v), want (%d, nil)", n, err, len(data)) } @@ -409,7 +409,7 @@ func TestPipeReadsAccumulate(t *testing.T) { // Write a few more bytes to allow us to read more/accumulate. extra := []byte("extra") - if n, err := syscall.Write(fds[1], extra); n != len(extra) || err != nil { + if n, err := unix.Write(fds[1], extra); n != len(extra) || err != nil { t.Fatalf("write to pipe got (%d, %v), want (%d, nil)", n, err, len(extra)) } @@ -433,13 +433,13 @@ func TestPipeReadsAccumulate(t *testing.T) { // Same as TestReadsAccumulate. func TestPipeWritesAccumulate(t *testing.T) { fds := make([]int, 2) - if err := syscall.Pipe(fds); err != nil { + if err := unix.Pipe(fds); err != nil { t.Fatalf("failed to create pipes: got %v, want nil", err) } wfile := fd.New(fds[1]) // Eventually close the read end, it doesn't depend on a pipe object. - defer syscall.Close(fds[0]) + defer unix.Close(fds[0]) // Get a new write only pipe reference. ctx := contexttest.Context(t) @@ -457,7 +457,7 @@ func TestPipeWritesAccumulate(t *testing.T) { }) file := fs.NewFile(ctx, fs.NewDirent(ctx, inode, "pipe"), fs.FileFlags{Read: true}, p) - pipeSize, _, errno := syscall.Syscall(syscall.SYS_FCNTL, uintptr(wfile.FD()), syscall.F_GETPIPE_SZ, 0) + pipeSize, _, errno := unix.Syscall(unix.SYS_FCNTL, uintptr(wfile.FD()), unix.F_GETPIPE_SZ, 0) if errno != 0 { t.Fatalf("fcntl(F_GETPIPE_SZ) failed: %v", errno) } @@ -483,7 +483,7 @@ func TestPipeWritesAccumulate(t *testing.T) { // Read the entire pipe buf size to make space for the second half. readBuffer := make([]byte, n) - if n, err := syscall.Read(fds[0], readBuffer); n != len(readBuffer) || err != nil { + if n, err := unix.Read(fds[0], readBuffer); n != len(readBuffer) || err != nil { t.Fatalf("write to pipe got (%d, %v), want (%d, nil)", n, err, len(readBuffer)) } if !bytes.Equal(readBuffer, writeBuffer[:len(readBuffer)]) { -- cgit v1.2.3