diff --git a/pkg/sentry/kernel/task_usermem.go b/pkg/sentry/kernel/task_usermem.go index 7309196cd..cc36aa9dc 100644 --- a/pkg/sentry/kernel/task_usermem.go +++ b/pkg/sentry/kernel/task_usermem.go @@ -131,24 +131,18 @@ func (t *Task) CopyInVector(addr hostarch.Addr, maxElemSize, maxTotalSize int) ( // - The caller must be running on the task goroutine. // - t's AddressSpace must be active. func (t *Task) CopyOutIovecs(addr hostarch.Addr, src hostarch.AddrRangeSeq) error { - return copyOutIovecs(t, t, addr, src) -} - -// copyOutIovecs converts src to an array of struct iovecs and copies it to the -// memory mapped at addr. -func copyOutIovecs(ctx marshal.CopyContext, t *Task, addr hostarch.Addr, src hostarch.AddrRangeSeq) error { switch t.Arch().Width() { case 8: if _, ok := addr.AddLength(uint64(src.NumRanges()) * iovecLength); !ok { return linuxerr.EFAULT } - b := ctx.CopyScratchBuffer(iovecLength) + b := t.CopyScratchBuffer(iovecLength) for ; !src.IsEmpty(); src = src.Tail() { ar := src.Head() hostarch.ByteOrder.PutUint64(b[0:8], uint64(ar.Start)) hostarch.ByteOrder.PutUint64(b[8:16], uint64(ar.Length())) - if _, err := ctx.CopyOutBytes(addr, b); err != nil { + if _, err := t.CopyOutBytes(addr, b); err != nil { return err } addr += iovecLength @@ -178,6 +172,15 @@ func (t *Task) CopyInIovecs(addr hostarch.Addr, numIovecs int) (hostarch.AddrRan return hostarch.AddrRangeSeqFromSlice(iovecs), nil } +// CopyInIovecsAsSlice copies in IoVecs and returns them in a slice. +// +// Preconditions: Same as usermem.IO.CopyIn, plus: +// - The caller must be running on the task goroutine or hold t.mu. +// - t's AddressSpace must be active. +func (t *Task) CopyInIovecsAsSlice(addr hostarch.Addr, numIovecs int) ([]hostarch.AddrRange, error) { + return copyInIovecs(t, t, addr, numIovecs) +} + func copyInIovec(ctx marshal.CopyContext, t *Task, addr hostarch.Addr) (hostarch.AddrRangeSeq, error) { if err := checkArch(t); err != nil { return hostarch.AddrRangeSeq{}, err @@ -322,10 +325,9 @@ func (t *Task) IovecsIOSequence(addr hostarch.Addr, iovcnt int, opts usermem.IOO } type taskCopyContext struct { - ctx context.Context - t *Task - opts usermem.IOOpts - allocateNewBuffers bool + ctx context.Context + t *Task + opts usermem.IOOpts } // CopyContext returns a marshal.CopyContext that copies to/from t's address @@ -340,7 +342,7 @@ func (t *Task) CopyContext(ctx context.Context, opts usermem.IOOpts) *taskCopyCo // CopyScratchBuffer implements marshal.CopyContext.CopyScratchBuffer. func (cc *taskCopyContext) CopyScratchBuffer(size int) []byte { - if ctxTask, ok := cc.ctx.(*Task); ok && !cc.allocateNewBuffers { + if ctxTask, ok := cc.ctx.(*Task); ok { return ctxTask.CopyScratchBuffer(size) } return make([]byte, size) @@ -357,13 +359,6 @@ func (cc *taskCopyContext) getMemoryManager() (*mm.MemoryManager, error) { return tmm, nil } -// WithTaskMutexLocked runs the given function with the task's mutex locked. -func (cc *taskCopyContext) WithTaskMutexLocked(fn func() error) error { - cc.t.mu.Lock() - defer cc.t.mu.Unlock() - return fn() -} - // CopyInBytes implements marshal.CopyContext.CopyInBytes. // // Preconditions: Same as usermem.IO.CopyIn, plus: @@ -392,25 +387,6 @@ func (cc *taskCopyContext) CopyOutBytes(addr hostarch.Addr, src []byte) (int, er return tmm.CopyOut(cc.ctx, addr, src, cc.opts) } -// CopyOutIovecs converts src to an array of struct iovecs and copies it to the -// memory mapped at addr for Task. -// -// Preconditions: Same as usermem.IO.CopyOut, plus: -// - The caller must be running on the task goroutine or hold the cc.t.mu -// - t's AddressSpace must be active. -func (cc *taskCopyContext) CopyOutIovecs(addr hostarch.Addr, src hostarch.AddrRangeSeq) error { - return copyOutIovecs(cc, cc.t, addr, src) -} - -// CopyInIovecs copies in IoVecs for taskCopyContext. -// -// Preconditions: Same as usermem.IO.CopyIn, plus: -// - The caller must be running on the task goroutine or hold the cc.t.mu -// - t's AddressSpace must be active. -func (cc *taskCopyContext) CopyInIovecs(addr hostarch.Addr, numIovecs int) ([]hostarch.AddrRange, error) { - return copyInIovecs(cc, cc.t, addr, numIovecs) -} - type ownTaskCopyContext struct { t *Task opts usermem.IOOpts diff --git a/pkg/sentry/strace/strace.go b/pkg/sentry/strace/strace.go index e43287a26..c6b4d13a1 100644 --- a/pkg/sentry/strace/strace.go +++ b/pkg/sentry/strace/strace.go @@ -392,7 +392,7 @@ func (i *SyscallInfo) pre(t *kernel.Task, args arch.SyscallArguments, maximumBlo output = append(output, dump(t, args[arg].Pointer(), args[arg+1].SizeT(), maximumBlobSize, LogAppDataAllowed /* content */)) case WriteIOVec: output = append(output, iovecs(t, args[arg].Pointer(), int(args[arg+1].Int()), LogAppDataAllowed /* content */, uint64(maximumBlobSize))) - case IOVec: + case ReadIOVec, IOVec: output = append(output, iovecs(t, args[arg].Pointer(), int(args[arg+1].Int()), false /* content */, uint64(maximumBlobSize))) case SendMsgHdr: output = append(output, msghdr(t, args[arg].Pointer(), LogAppDataAllowed /* content */, uint64(maximumBlobSize))) diff --git a/pkg/sentry/syscalls/linux/linux64.go b/pkg/sentry/syscalls/linux/linux64.go index 327a4d5e3..e95411f56 100644 --- a/pkg/sentry/syscalls/linux/linux64.go +++ b/pkg/sentry/syscalls/linux/linux64.go @@ -362,8 +362,8 @@ var AMD64 = &kernel.SyscallTable{ 307: syscalls.Supported("sendmmsg", SendMMsg), 308: syscalls.Supported("setns", Setns), 309: syscalls.Supported("getcpu", Getcpu), - 310: syscalls.ErrorWithEvent("process_vm_readv", linuxerr.ENOSYS, "", []string{"gvisor.dev/issue/158"}), // TODO(b/260724654) - 311: syscalls.ErrorWithEvent("process_vm_writev", linuxerr.ENOSYS, "", []string{"gvisor.dev/issue/158"}), // TODO(b/260724654) + 310: syscalls.Supported("process_vm_readv", ProcessVMReadv), + 311: syscalls.Supported("process_vm_writev", ProcessVMWritev), 312: syscalls.CapError("kcmp", linux.CAP_SYS_PTRACE, "", nil), 313: syscalls.CapError("finit_module", linux.CAP_SYS_MODULE, "", nil), 314: syscalls.ErrorWithEvent("sched_setattr", linuxerr.ENOSYS, "gVisor does not implement a scheduler.", []string{"gvisor.dev/issue/264"}), // TODO(b/118902272) @@ -685,8 +685,8 @@ var ARM64 = &kernel.SyscallTable{ 267: syscalls.Supported("syncfs", Syncfs), 268: syscalls.Supported("setns", Setns), 269: syscalls.Supported("sendmmsg", SendMMsg), - 270: syscalls.ErrorWithEvent("process_vm_readv", linuxerr.ENOSYS, "", []string{"gvisor.dev/issue/158"}), // TODO(b/260724654) - 271: syscalls.ErrorWithEvent("process_vm_writev", linuxerr.ENOSYS, "", []string{"gvisor.dev/issue/158"}), // TODO(b/260724654) + 270: syscalls.Supported("process_vm_readv", ProcessVMReadv), + 271: syscalls.Supported("process_vm_writev", ProcessVMWritev), 272: syscalls.CapError("kcmp", linux.CAP_SYS_PTRACE, "", nil), 273: syscalls.CapError("finit_module", linux.CAP_SYS_MODULE, "", nil), 274: syscalls.ErrorWithEvent("sched_setattr", linuxerr.ENOSYS, "gVisor does not implement a scheduler.", []string{"gvisor.dev/issue/264"}), // TODO(b/118902272) diff --git a/pkg/sentry/syscalls/linux/sys_process_vm.go b/pkg/sentry/syscalls/linux/sys_process_vm.go index ff4ce0e71..afff8532d 100644 --- a/pkg/sentry/syscalls/linux/sys_process_vm.go +++ b/pkg/sentry/syscalls/linux/sys_process_vm.go @@ -15,33 +15,35 @@ package linux import ( + "fmt" + "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" + "gvisor.dev/gvisor/pkg/marshal" "gvisor.dev/gvisor/pkg/sentry/arch" "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/usermem" ) -type vmReadWriteOp int +type processVMOpType int const ( - localReadLocalWrite vmReadWriteOp = iota - remoteReadLocalWrite - localReadRemoteWrite + processVMOpRead = iota + processVMOpWrite ) // ProcessVMReadv implements process_vm_readv(2). func ProcessVMReadv(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr, *kernel.SyscallControl, error) { - return processVMRW(t, args, false /*isWrite*/) + return processVMOp(t, args, processVMOpRead) } // ProcessVMWritev implements process_vm_writev(2). func ProcessVMWritev(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr, *kernel.SyscallControl, error) { - return processVMRW(t, args, true /*isWrite*/) + return processVMOp(t, args, processVMOpWrite) } -func processVMRW(t *kernel.Task, args arch.SyscallArguments, isWrite bool) (uintptr, *kernel.SyscallControl, error) { +func processVMOp(t *kernel.Task, args arch.SyscallArguments, op processVMOpType) (uintptr, *kernel.SyscallControl, error) { pid := kernel.ThreadID(args[0].Int()) lvec := hostarch.Addr(args[1].Pointer()) liovcnt := int(args[2].Int64()) @@ -49,6 +51,7 @@ func processVMRW(t *kernel.Task, args arch.SyscallArguments, isWrite bool) (uint riovcnt := int(args[4].Int64()) flags := args[5].Int() + // Parse the flags. switch { case flags != 0 || liovcnt < 0 || @@ -56,132 +59,159 @@ func processVMRW(t *kernel.Task, args arch.SyscallArguments, isWrite bool) (uint liovcnt > linux.UIO_MAXIOV || riovcnt > linux.UIO_MAXIOV: return 0, nil, linuxerr.EINVAL - case lvec == 0 || rvec == 0: - return 0, nil, linuxerr.EFAULT case liovcnt == 0 || riovcnt == 0: return 0, nil, nil + case lvec == 0 || rvec == 0: + return 0, nil, linuxerr.EFAULT } - localProcess := t.ThreadGroup().Leader() - if localProcess == nil { + // Determine local and remote processes. + // Local process is always the caller. + localTask := t.ThreadGroup().Leader() + if localTask == nil { return 0, nil, linuxerr.ESRCH } - remoteThreadGroup := localProcess.PIDNamespace().ThreadGroupWithID(pid) + // Remote process is the pid specified in the syscall arguments. It is + // allowed to be the same as the caller process. + remoteThreadGroup := localTask.PIDNamespace().ThreadGroupWithID(pid) if remoteThreadGroup == nil { return 0, nil, linuxerr.ESRCH } - remoteProcess := remoteThreadGroup.Leader() + remoteTask := remoteThreadGroup.Leader() + if remoteTask.ExitState() >= kernel.TaskExitInitiated { + return 0, nil, linuxerr.ESRCH + } - isRemote := localProcess == remoteProcess + // man 2 process_vm_read: "Permission to read from or write to another + // process is governed by a ptrace access mode + // PTRACE_MODE_ATTACH_REALCREDS check; see ptrace(2)." + if !localTask.CanTrace(remoteTask, true /* attach */) { + return 0, nil, linuxerr.EPERM + } - // For the write case, we read from the local process and write to the remote process. - op := localReadLocalWrite - if isWrite { - if isRemote { - op = remoteReadLocalWrite + // Figure out which processes and arguments (local or remote) are for + // writing and which are for reading, based on the operation. + var opArgs processVMOpArgs + switch op { + case processVMOpRead: + // Read from remote process and write into local. + opArgs = processVMOpArgs{ + readCtx: remoteTask.CopyContext(t, usermem.IOOpts{}), + readAddr: rvec, + readIovecCount: riovcnt, + writeCtx: localTask.CopyContext(t, usermem.IOOpts{AddressSpaceActive: true}), + writeAddr: lvec, + writeIovecCount: liovcnt, } - return doProcessVMReadWrite(localProcess, remoteProcess, lvec, rvec, liovcnt, riovcnt, op) + case processVMOpWrite: + // Read from local process and write into remote. + opArgs = processVMOpArgs{ + readCtx: localTask.CopyContext(t, usermem.IOOpts{AddressSpaceActive: true}), + readAddr: lvec, + readIovecCount: liovcnt, + writeCtx: remoteTask.CopyContext(t, usermem.IOOpts{}), + writeAddr: rvec, + writeIovecCount: riovcnt, + } + default: + panic(fmt.Sprintf("unknown process vm op type: %v", op)) } - // For the read case, we read from the remote process and write to the local process. - if isRemote { - op = localReadRemoteWrite + + var ( + n int + err error + ) + if localTask == remoteTask { + // No need to lock remote process's task mutex since it is the + // same as this process. + n, err = doProcessVMOpMaybeLocked(t, opArgs) + } else { + // Need to take remote process's task mutex. + remoteTask.WithMuLocked(func(*kernel.Task) { + n, err = doProcessVMOpMaybeLocked(t, opArgs) + }) } - return doProcessVMReadWrite(remoteProcess, localProcess, rvec, lvec, riovcnt, liovcnt, op) + if err != nil { + return 0, nil, err + } + return uintptr(n), nil, nil } -func doProcessVMReadWrite(rProcess, wProcess *kernel.Task, rAddr, wAddr hostarch.Addr, rIovecCount, wIovecCount int, op vmReadWriteOp) (uintptr, *kernel.SyscallControl, error) { - rCtx := rProcess.CopyContext(rProcess, usermem.IOOpts{}) - wCtx := wProcess.CopyContext(wProcess, usermem.IOOpts{}) +type processVMOpArgs struct { + readCtx marshal.CopyContext + readAddr hostarch.Addr + readIovecCount int + writeCtx marshal.CopyContext + writeAddr hostarch.Addr + writeIovecCount int +} - var wCount int - doProcessVMReadWriteMaybeLocked := func() error { - rIovecs, err := rCtx.CopyInIovecs(rAddr, rIovecCount) +func doProcessVMOpMaybeLocked(t *kernel.Task, args processVMOpArgs) (int, error) { + // Copy IOVecs in to kernel. + readIovecs, err := t.CopyInIovecsAsSlice(args.readAddr, args.readIovecCount) + if err != nil { + return 0, err + } + writeIovecs, err := t.CopyInIovecsAsSlice(args.writeAddr, args.writeIovecCount) + if err != nil { + return 0, err + } + + // Get scratch buffer from the calling task. + // Size should be max be size of largest read iovec. + var bufSize int + for _, readIovec := range readIovecs { + if int(readIovec.Length()) > bufSize { + bufSize = int(readIovec.Length()) + } + } + buf := t.CopyScratchBuffer(bufSize) + + // Number of bytes written. + var n int + for _, readIovec := range readIovecs { + if len(writeIovecs) == 0 { + break + } + + buf = buf[0:int(readIovec.Length())] + bytes, err := args.readCtx.CopyInBytes(readIovec.Start, buf) + if linuxerr.Equals(linuxerr.EFAULT, err) { + return n, nil + } if err != nil { - return err + return n, err } - wIovecs, err := wCtx.CopyInIovecs(wAddr, wIovecCount) - if err != nil { - return err + if bytes != int(readIovec.Length()) { + return n, nil } - bufSize := 0 - for _, rIovec := range rIovecs { - if int(rIovec.Length()) > bufSize { - bufSize = int(rIovec.Length()) + start := 0 + for bytes > start && 0 < len(writeIovecs) { + writeLength := int(writeIovecs[0].Length()) + if writeLength > (bytes - start) { + writeLength = bytes - start } - } - - var buf []byte - // We need to copy the called task's scratch buffer so we don't get a data race. If we are - // reading a remote process's memory, then we are on the writer's task goroutine, so use - // the write context's scratch buffer. - if op == remoteReadLocalWrite { - buf = wCtx.CopyScratchBuffer(bufSize) - } else { - buf = rCtx.CopyScratchBuffer(bufSize) - } - - for _, rIovec := range rIovecs { - if len(wIovecs) <= 0 { - break - } - - buf = buf[0:int(rIovec.Length())] - bytes, err := rCtx.CopyInBytes(rIovec.Start, buf) + out, err := args.writeCtx.CopyOutBytes(writeIovecs[0].Start, buf[start:writeLength+start]) + n += out + start += out if linuxerr.Equals(linuxerr.EFAULT, err) { - return nil + return n, nil } if err != nil { - return err + return n, err } - if bytes != int(rIovec.Length()) { - return nil + if out != writeLength { + return n, nil } - start := 0 - for bytes > start && 0 < len(wIovecs) { - writeLength := int(wIovecs[0].Length()) - if writeLength > (bytes - start) { - writeLength = bytes - start - } - out, err := wCtx.CopyOutBytes(wIovecs[0].Start, buf[start:writeLength+start]) - wCount += out - start += out - if linuxerr.Equals(linuxerr.EFAULT, err) { - return nil - } - if err != nil { - return err - } - if out != writeLength { - return nil - } - wIovecs[0].Start += hostarch.Addr(out) - if !wIovecs[0].WellFormed() { - return err - } - if wIovecs[0].Length() == 0 { - wIovecs = wIovecs[1:] - } + writeIovecs[0].Start += hostarch.Addr(out) + if !writeIovecs[0].WellFormed() { + return n, err + } + if writeIovecs[0].Length() == 0 { + writeIovecs = writeIovecs[1:] } } - return nil } - - var err error - - switch op { - case remoteReadLocalWrite: - err = rCtx.WithTaskMutexLocked(doProcessVMReadWriteMaybeLocked) - case localReadRemoteWrite: - err = wCtx.WithTaskMutexLocked(doProcessVMReadWriteMaybeLocked) - - case localReadLocalWrite: - // in the case of local reads/writes, we don't have to lock the task mutex, because we are - // running on the top of the task's goroutine already. - err = doProcessVMReadWriteMaybeLocked() - default: - panic("unsupported operation passed") - } - - return uintptr(wCount), nil, err + return n, nil } diff --git a/test/syscalls/BUILD b/test/syscalls/BUILD index 64fcb2450..e95a1e594 100644 --- a/test/syscalls/BUILD +++ b/test/syscalls/BUILD @@ -1,5 +1,5 @@ -load("//tools:defs.bzl", "more_shards", "most_shards") load("//test/runner:defs.bzl", "syscall_test") +load("//tools:defs.bzl", "more_shards", "most_shards") package( default_applicable_licenses = ["//:license"], @@ -1189,6 +1189,10 @@ syscall_test( test = "//test/syscalls/linux:processes_test", ) +syscall_test( + test = "//test/syscalls/linux:process_vm_read_write_test", +) + syscall_test( test = "//test/syscalls/linux:deleted_test", ) diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 7ad457d20..67caaac8b 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -4632,18 +4632,17 @@ cc_binary( ) cc_binary( - name = "process_vm_read_write", + name = "process_vm_read_write_test", testonly = 1, srcs = ["process_vm_read_write.cc"], linkstatic = 1, deps = [ gtest, - "//test/util:capability_util", "//test/util:logging", - "//test/util:multiprocess_util", "//test/util:posix_error", "//test/util:test_main", "//test/util:test_util", + "@com_google_absl//absl/cleanup", "@com_google_absl//absl/strings", ], ) diff --git a/test/syscalls/linux/process_vm_read_write.cc b/test/syscalls/linux/process_vm_read_write.cc index c0b4d10c2..d3e9b6346 100644 --- a/test/syscalls/linux/process_vm_read_write.cc +++ b/test/syscalls/linux/process_vm_read_write.cc @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -34,11 +35,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "absl/cleanup/cleanup.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_join.h" -#include "test/util/linux_capability_util.h" #include "test/util/logging.h" -#include "test/util/multiprocess_util.h" #include "test/util/posix_error.h" #include "test/util/test_util.h" @@ -49,59 +49,62 @@ namespace { class TestIovecs { public: - TestIovecs(std::vector& data) { - data_ = std::vector(data.size()); - initial_ = std::vector(data.size()); + TestIovecs(std::vector data) { + data_.resize(data.size()); + iovecs_.resize(data.size()); for (size_t i = 0; i < data.size(); ++i) { data_[i] = data[i]; - initial_[i] = data[i]; - struct iovec iov; - iov.iov_len = data_[i].size(); - iov.iov_base = data_[i].data(); - iovecs_.push_back(iov); bytes_ += data[i].size(); + iovecs_[i].iov_len = data_[i].size(); + iovecs_[i].iov_base = data_[i].data(); } } - bool compare(std::vector other) { - auto want = absl::StrJoin(other, ""); - auto got = absl::StrJoin(data_, ""); - // If the other buffer is smaller than this, make sure the remaining bytes - // haven't been overwritten. - if (want.size() < got.size()) { - auto initial = absl::StrJoin(initial_, ""); - want = absl::StrCat(want, initial.substr(want.size())); + void erase() { + for (size_t i = 0; i < data_.size(); i++) { + data_[i].clear(); } - // If the other buffer is smaller, truncate it so we can compare the two. - if (want.size() > got.size()) { - want = want.substr(0, got.size()); - } - if (want != got) { - std::cerr << "Mismatch buffers:\n want: " << want << "\n got: " << got - << std::endl; - return false; - } - - return true; } - std::vector marshal() { - std::vector ret(iovecs_.size()); - for (size_t i = 0; i < iovecs_.size(); ++i) { - ret[i] = &iovecs_[i]; - } - return ret; - } - - ssize_t total_bytes() { return bytes_; } - - private: - ssize_t bytes_ = 0; + // Backing data that will be read/written. std::vector data_; - std::vector initial_; - std::vector iovecs_; + + // Total size of data_. + ssize_t bytes_ = 0; + + // Iovec structs that point into data_ + std::vector iovecs_; }; +// bytes_match checks that the two TestIovecs are at least min_bytes in length, +// and that they agree in the first min_bytes. +bool bytes_match(TestIovecs first_iov, TestIovecs second_iov, + size_t min_bytes) { + auto first = absl::StrJoin(first_iov.data_, ""); + if (first.size() < min_bytes) { + std::cout << "First buffer smaller than min_bytes: " << min_bytes + << " buffer: " << first << std::endl; + return false; + } + first = first.substr(0, min_bytes); + + auto second = absl::StrJoin(second_iov.data_, ""); + if (second.size() < min_bytes) { + std::cout << "First buffer smaller than min_bytes: " << min_bytes + << " buffer: " << second << std::endl; + return false; + } + second = second.substr(0, min_bytes); + + if (first != second) { + std::cout << "Mismatch buffers:\n first: " << first + << "\n second: " << second << std::endl; + return false; + } + + return true; +} + struct ProcessVMTestCase { std::string test_name; std::vector local_data; @@ -110,115 +113,6 @@ struct ProcessVMTestCase { using ProcessVMTest = ::testing::TestWithParam; -bool ProcessVMCallsNotSupported() { - struct iovec iov = {}; - // Flags should be 0. - ssize_t ret = process_vm_readv(0, &iov, 1, &iov, 1, 10); - if (ret != 0 && errno == ENOSYS) return true; - - ret = process_vm_writev(0, &iov, 1, &iov, 1, 10); - return ret != 0 && errno == ENOSYS; -} - -// TestReadvSameProcess calls process_vm_readv in the same process with -// various local/remote buffers. -TEST_P(ProcessVMTest, TestReadvSameProcess) { - SKIP_IF(ProcessVMCallsNotSupported()); - auto local_data = GetParam().local_data; - auto remote_data = GetParam().remote_data; - TestIovecs local_iovecs(local_data); - TestIovecs remote_iovecs(remote_data); - - auto local = local_iovecs.marshal(); - auto remote = remote_iovecs.marshal(); - auto expected_bytes = - std::min(remote_iovecs.total_bytes(), local_iovecs.total_bytes()); - EXPECT_THAT(process_vm_readv(getpid(), *(local.data()), local.size(), - *(remote.data()), remote.size(), 0), - SyscallSucceedsWithValue(expected_bytes)); - EXPECT_TRUE(local_iovecs.compare(remote_data)); -} - -// TestReadvSubProcess repeats the previous test in a forked process. -TEST_P(ProcessVMTest, TestReadvSubProcess) { - SKIP_IF(ProcessVMCallsNotSupported()); - SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE((HaveCapability(CAP_SYS_PTRACE)))); - auto local_data = GetParam().local_data; - auto remote_data = GetParam().remote_data; - - TestIovecs remote_iovecs(remote_data); - auto remote = remote_iovecs.marshal(); - auto remote_ptr = remote[0]; - auto remote_size = remote.size(); - auto remote_total_bytes = remote_iovecs.total_bytes(); - - const std::function fn = [local_data, remote_data, remote_ptr, - remote_size, remote_total_bytes] { - std::vector local_fn_data = local_data; - TestIovecs local_iovecs(local_fn_data); - auto local = local_iovecs.marshal(); - int ret = process_vm_readv(getppid(), local[0], local.size(), remote_ptr, - remote_size, 0); - auto expected_bytes = - std::min(remote_total_bytes, local_iovecs.total_bytes()); - TEST_CHECK_MSG( - ret == expected_bytes, - absl::StrCat("want: ", expected_bytes, " got: ", ret).c_str()); - TEST_CHECK(local_iovecs.compare(remote_data)); - }; - EXPECT_THAT(InForkedProcess(fn), IsPosixErrorOkAndHolds(0)); -} - -// TestWritevSameProcess calls process_vm_writev in the same process with -// various local/remote buffers. -TEST_P(ProcessVMTest, TestWritevSameProcess) { - SKIP_IF(ProcessVMCallsNotSupported()); - auto local_data = GetParam().local_data; - auto remote_data = GetParam().remote_data; - - TestIovecs local_iovecs(local_data); - TestIovecs remote_iovecs(remote_data); - - auto local = local_iovecs.marshal(); - auto remote = remote_iovecs.marshal(); - auto expected_bytes = - std::min(remote_iovecs.total_bytes(), local_iovecs.total_bytes()); - EXPECT_THAT(process_vm_writev(getpid(), remote[0], remote.size(), local[0], - local.size(), 0), - SyscallSucceedsWithValue(expected_bytes)); - EXPECT_TRUE(local_iovecs.compare(remote_data)); -} - -// TestWritevSubProcess repeats the previous test in a forked process. -TEST_P(ProcessVMTest, TestWritevSubProcess) { - SKIP_IF(ProcessVMCallsNotSupported()); - SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE((HaveCapability(CAP_SYS_PTRACE)))); - auto local_data = GetParam().local_data; - auto remote_data = GetParam().remote_data; - TestIovecs remote_iovecs(remote_data); - auto remote = remote_iovecs.marshal(); - auto remote_ptr = remote[0]; - auto remote_size = remote.size(); - auto remote_total_bytes = remote_iovecs.total_bytes(); - - const std::function fn = [local_data, remote_ptr, remote_size, - remote_total_bytes] { - std::vector local_fn_data = local_data; - TestIovecs local_iovecs(local_fn_data); - auto local = local_iovecs.marshal(); - int ret = process_vm_writev(getppid(), local[0], local.size(), remote_ptr, - remote_size, 0); - auto expected_bytes = - std::min(remote_total_bytes, local_iovecs.total_bytes()); - TEST_CHECK_MSG( - ret == expected_bytes, - absl::StrCat("want: ", expected_bytes, " got: ", ret).c_str()); - }; - - EXPECT_THAT(InForkedProcess(fn), IsPosixErrorOkAndHolds(0)); - EXPECT_TRUE(remote_iovecs.compare(local_data)); -} - INSTANTIATE_TEST_SUITE_P( ProcessVMTests, ProcessVMTest, ::testing::ValuesIn( @@ -243,8 +137,110 @@ INSTANTIATE_TEST_SUITE_P( return info.param.test_name; }); +// TestReadvSameProcess calls process_vm_readv in the same process with various +// local/remote buffers. +TEST_P(ProcessVMTest, TestReadvSameProcess) { + TestIovecs local(GetParam().local_data); + TestIovecs remote(GetParam().remote_data); + + auto want_size = std::min(remote.bytes_, local.bytes_); + EXPECT_THAT( + process_vm_readv(getpid(), local.iovecs_.data(), local.iovecs_.size(), + remote.iovecs_.data(), remote.iovecs_.size(), 0), + SyscallSucceedsWithValue(want_size)); + EXPECT_TRUE(bytes_match(local, remote, want_size)); +} + +// TestReadvSubProcess reads data from a forked child process. +TEST_P(ProcessVMTest, TestReadvSubProcess) { + TestIovecs local = TestIovecs(GetParam().local_data); + TestIovecs remote = TestIovecs(GetParam().remote_data); + + pid_t pid = fork(); + TEST_CHECK_SUCCESS(pid); + if (pid == 0) { + // Child. This is the "remote" process. + // Wait for parent to read data. + sleep(10); // NOLINT - SleepFor is not async-signal-safe. + _exit(0); + } + + auto cleanup = absl::MakeCleanup([&] { kill(pid, SIGKILL); }); + + // Erase the string data in parent's copy of remote, to make sure we are + // reading data from the child. + remote.erase(); + + // Compare against actual remote (not what we sent to the child, since we + // emptied it). + TestIovecs want_remote = TestIovecs(GetParam().remote_data); + auto want_size = std::min(local.bytes_, want_remote.bytes_); + ASSERT_THAT(process_vm_readv(pid, local.iovecs_.data(), local.iovecs_.size(), + remote.iovecs_.data(), remote.iovecs_.size(), 0), + SyscallSucceedsWithValue(want_size)); + EXPECT_TRUE(bytes_match(local, want_remote, want_size)); +} + +// TestWritevSameProcess calls process_vm_readv in the same process with various +// local/remote buffers. +TEST_P(ProcessVMTest, TestWritevSameProcess) { + TestIovecs local(GetParam().local_data); + TestIovecs remote(GetParam().remote_data); + + auto want_size = std::min(remote.bytes_, local.bytes_); + EXPECT_THAT( + process_vm_writev(getpid(), local.iovecs_.data(), local.iovecs_.size(), + remote.iovecs_.data(), remote.iovecs_.size(), 0), + SyscallSucceedsWithValue(want_size)); + EXPECT_TRUE(bytes_match(local, remote, want_size)); +} + +// TestWritevSubProcess writes data to a forked child process. +TEST_P(ProcessVMTest, TestWritevSubProcess) { + TestIovecs local(GetParam().local_data); + TestIovecs remote(GetParam().remote_data); + + // A pipe is used to wait on the write call from the parent, so we can block + // asserting until the write is complete. + int pipefd[2]; + ASSERT_THAT(pipe(pipefd), SyscallSucceeds()); + + pid_t pid = fork(); + TEST_CHECK_SUCCESS(pid); + if (pid == 0) { + // Child. This is the "remote" process. + close(pipefd[1]); + + // Wait on pipefd. It will be closed after the parent has written. + char buf; + TEST_CHECK_SUCCESS(read(pipefd[0], &buf, sizeof(buf))); + close(pipefd[0]); + + // Check the data. This will exit non-0 in the case of a mismatch. + auto want_size = std::min(local.bytes_, remote.bytes_); + TEST_CHECK(bytes_match(local, remote, want_size)); + + _exit(0); + } + + auto cleanup = absl::MakeCleanup([&] { + int status = 0; + EXPECT_THAT(waitpid(pid, &status, 0), SyscallSucceeds()); + EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0); + }); + + // Write to the remote. + auto want_size = std::min(local.bytes_, remote.bytes_); + ASSERT_THAT( + process_vm_writev(pid, local.iovecs_.data(), local.iovecs_.size(), + remote.iovecs_.data(), remote.iovecs_.size(), 0), + SyscallSucceedsWithValue(want_size)); + + // Now that we've written, close pipefd to signal the child can continue. + close(pipefd[1]); +} + TEST(ProcessVMInvalidTest, NonZeroFlags) { - SKIP_IF(ProcessVMCallsNotSupported()); struct iovec iov = {}; // Flags should be 0. EXPECT_THAT(process_vm_readv(0, &iov, 1, &iov, 1, 10), @@ -254,13 +250,11 @@ TEST(ProcessVMInvalidTest, NonZeroFlags) { } TEST(ProcessVMInvalidTest, NullLocalIovec) { - SKIP_IF(ProcessVMCallsNotSupported()); struct iovec iov = {}; pid_t child = fork(); if (child == 0) { - while (true) { - sleep(1); - } + sleep(10); // NOLINT - SleepFor is not async-signal-safe. + _exit(0); } EXPECT_THAT(process_vm_readv(child, nullptr, 1, &iov, 1, 0), @@ -273,29 +267,26 @@ TEST(ProcessVMInvalidTest, NullLocalIovec) { } TEST(ProcessVMInvalidTest, NULLRemoteIovec) { - SKIP_IF(ProcessVMCallsNotSupported()); - const std::function fn = [] { - std::string contents = "3263827"; - struct iovec child_iov; - child_iov.iov_base = contents.data(); - child_iov.iov_len = contents.size(); + std::string contents = "3263827"; + struct iovec iov; + iov.iov_base = contents.data(); + iov.iov_len = contents.size(); - pid_t parent = getppid(); - int ret = - process_vm_readv(parent, &child_iov, contents.length(), nullptr, 1, 0); - TEST_CHECK(ret == -1); - TEST_CHECK(errno == EFAULT || errno == EINVAL); + pid_t pid = fork(); + TEST_CHECK_SUCCESS(pid); + if (pid == 0) { + sleep(10); // NOLINT - SleepFor is not async-signal-safe. + _exit(0); + } + auto cleanup = absl::MakeCleanup([&] { kill(pid, SIGKILL); }); - ret = - process_vm_writev(parent, &child_iov, contents.length(), nullptr, 1, 0); - TEST_CHECK(ret == -1); - TEST_CHECK(errno == EFAULT || errno == EINVAL); - }; - ASSERT_THAT(InForkedProcess(fn), IsPosixErrorOkAndHolds(0)); + EXPECT_THAT(process_vm_readv(pid, &iov, contents.length(), nullptr, 1, 0), + SyscallFailsWithErrno(::testing::AnyOf(EFAULT, EINVAL))); + EXPECT_THAT(process_vm_writev(pid, &iov, contents.length(), nullptr, 1, 0), + SyscallFailsWithErrno(::testing::AnyOf(EFAULT, EINVAL))); } TEST(ProcessVMInvalidTest, ProcessNoExist) { - SKIP_IF(ProcessVMCallsNotSupported()); struct iovec iov; EXPECT_THAT(process_vm_readv(-1, &iov, 1, &iov, 1, 0), SyscallFailsWithErrno(::testing::AnyOf(ESRCH, EFAULT))); @@ -304,43 +295,26 @@ TEST(ProcessVMInvalidTest, ProcessNoExist) { } TEST(ProcessVMInvalidTest, GreaterThanIOV_MAX) { - SKIP_IF(ProcessVMCallsNotSupported()); std::string contents = "3263827"; struct iovec iov; iov.iov_base = contents.data(); - auto iov_addr = &iov; - const std::function fn = [=] { - struct iovec child_iov; - std::string contents = "3263827"; - child_iov.iov_base = contents.data(); - child_iov.iov_len = contents.size(); + iov.iov_len = contents.size(); - pid_t parent = getppid(); - TEST_CHECK_MSG(-1 == process_vm_readv(parent, &child_iov, 1, iov_addr, - IOV_MAX + 1, 0) && - errno == EINVAL, - "read remote_process_over_IOV_MAX"); + pid_t pid = fork(); + TEST_CHECK_SUCCESS(pid); + if (pid == 0) { + sleep(10); // NOLINT - SleepFor is not async-signal-safe. + _exit(0); + } + auto cleanup = absl::MakeCleanup([&] { kill(pid, SIGKILL); }); - TEST_CHECK_MSG(-1 == process_vm_writev(parent, &child_iov, 1, iov_addr, - IOV_MAX + 3, 0) && - errno == EINVAL, - "write remote process over IOV_MAX"); - - TEST_CHECK_MSG(-1 == process_vm_readv(parent, &child_iov, IOV_MAX + 2, - iov_addr, 1, 0) && - errno == EINVAL, - "read local process over IOV_MAX"); - - TEST_CHECK_MSG(-1 == process_vm_writev(parent, &child_iov, IOV_MAX + 8, - iov_addr, 1, 0) && - errno == EINVAL, - "write local process over IOV_MAX"); - }; - EXPECT_THAT(InForkedProcess(fn), IsPosixErrorOkAndHolds(0)); + EXPECT_THAT(process_vm_readv(pid, &iov, 1, &iov, IOV_MAX + 1, 0), + SyscallFailsWithErrno(EINVAL)); + EXPECT_THAT(process_vm_writev(pid, &iov, 1, &iov, IOV_MAX + 1, 0), + SyscallFailsWithErrno(EINVAL)); } TEST(ProcessVMInvalidTest, PartialReadWrite) { - SKIP_IF(ProcessVMCallsNotSupported()); std::string iov_content_1 = "1138"; std::string iov_content_2 = "3720"; struct iovec iov[2]; @@ -371,8 +345,6 @@ TEST(ProcessVMInvalidTest, PartialReadWrite) { } TEST(ProcessVMTest, WriteToZombie) { - SKIP_IF(ProcessVMCallsNotSupported()); - SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE((HaveCapability(CAP_SYS_PTRACE)))); char* data = {0}; pid_t child; ASSERT_THAT(child = fork(), SyscallSucceeds()); @@ -388,6 +360,41 @@ TEST(ProcessVMTest, WriteToZombie) { ASSERT_THAT(process_vm_writev(child, &iov, 1, &iov, 1, 0), SyscallFailsWithErrno(ESRCH)); } + +// TestReadvNull calls process_vm_readv with null iovecs and checks that they +// succeed but return 0; +TEST(ProcessVMTest, TestReadvNull) { + TestIovecs local(std::vector{"foo"}); + TestIovecs remote(std::vector{"bar"}); + + // Pass 0 for local. + EXPECT_THAT(process_vm_readv(getpid(), 0, 0, remote.iovecs_.data(), + remote.iovecs_.size(), 0), + SyscallSucceedsWithValue(0)); + + // Pass 0 for remote. + EXPECT_THAT(process_vm_readv(getpid(), local.iovecs_.data(), + local.iovecs_.size(), 0, 0, 0), + SyscallSucceedsWithValue(0)); +} + +// TestWritevNull calls process_vm_writev with null iovecs and checks that they +// succeed but return 0; +TEST(ProcessVMTest, TestWritevNull) { + TestIovecs local(std::vector{"foo"}); + TestIovecs remote(std::vector{"bar"}); + + // Pass 0 for local. + EXPECT_THAT(process_vm_writev(getpid(), 0, 0, remote.iovecs_.data(), + remote.iovecs_.size(), 0), + SyscallSucceedsWithValue(0)); + + // Pass 0 for remote. + EXPECT_THAT(process_vm_writev(getpid(), local.iovecs_.data(), + local.iovecs_.size(), 0, 0, 0), + SyscallSucceedsWithValue(0)); +} + } // namespace } // namespace testing } // namespace gvisor