From aa1a66353af5f2daaab5cbbcd263c26efb86ca5d Mon Sep 17 00:00:00 2001
From: Andrei Vagin <avagin@google.com>
Date: Thu, 15 Feb 2024 15:54:54 -0800
Subject: [PATCH] process_vm_{read,write}v returns EFAILT if iov-s describe
 inaccessible memory

PiperOrigin-RevId: 607482542
---
 pkg/sentry/syscalls/linux/sys_process_vm.go  | 70 ++++++++------------
 test/syscalls/linux/BUILD                    |  1 +
 test/syscalls/linux/process_vm_read_write.cc | 19 ++++++
 3 files changed, 49 insertions(+), 41 deletions(-)

diff --git a/pkg/sentry/syscalls/linux/sys_process_vm.go b/pkg/sentry/syscalls/linux/sys_process_vm.go
index 2653d80b4..d46db8291 100644
--- a/pkg/sentry/syscalls/linux/sys_process_vm.go
+++ b/pkg/sentry/syscalls/linux/sys_process_vm.go
@@ -127,7 +127,7 @@ func processVMOp(t *kernel.Task, args arch.SyscallArguments, op processVMOpType)
 			n, err = doProcessVMOpMaybeLocked(t, opArgs)
 		})
 	}
-	if err != nil {
+	if n == 0 && err != nil {
 		return 0, nil, err
 	}
 	return uintptr(n), nil, nil
@@ -172,52 +172,40 @@ func doProcessVMOpMaybeLocked(t *kernel.Task, args processVMOpArgs) (int, error)
 
 	// Number of bytes written.
 	var n int
-	for _, readIovec := range readIovecs {
-		if len(writeIovecs) == 0 {
-			break
+	for len(readIovecs) != 0 && len(writeIovecs) != 0 {
+		readIovec := readIovecs[0]
+		length := readIovec.Length()
+		if length == 0 {
+			readIovecs = readIovecs[1:]
+			continue
 		}
+		if length > maxScratchBufferSize {
+			length = maxScratchBufferSize
+		}
+		buf = buf[0:int(length)]
+		bytes, err := args.readCtx.CopyInBytes(readIovec.Start, buf)
+		if bytes == 0 {
+			return n, err
+		}
+		readIovecs[0].Start += hostarch.Addr(bytes)
 
-		for readIovec.Length() != 0 {
-			length := readIovec.Length()
-			if length > maxScratchBufferSize {
-				length = maxScratchBufferSize
+		start := 0
+		for bytes > start && len(writeIovecs) > 0 {
+			writeLength := int(writeIovecs[0].Length())
+			if writeLength == 0 {
+				writeIovecs = writeIovecs[1:]
+				continue
 			}
-			buf = buf[0:int(length)]
-			bytes, err := args.readCtx.CopyInBytes(readIovec.Start, buf)
-			if linuxerr.Equals(linuxerr.EFAULT, err) {
-				return n, nil
+			if writeLength > (bytes - start) {
+				writeLength = bytes - start
 			}
-			if bytes == 0 && err != nil {
+			out, err := args.writeCtx.CopyOutBytes(writeIovecs[0].Start, buf[start:writeLength+start])
+			n += out
+			start += out
+			if out != writeLength {
 				return n, err
 			}
-			readIovec.Start += hostarch.Addr(bytes)
-
-			start := 0
-			for bytes > start && 0 < len(writeIovecs) {
-				writeLength := int(writeIovecs[0].Length())
-				if writeLength > (bytes - start) {
-					writeLength = bytes - start
-				}
-				out, err := args.writeCtx.CopyOutBytes(writeIovecs[0].Start, buf[start:writeLength+start])
-				n += out
-				start += out
-				if linuxerr.Equals(linuxerr.EFAULT, err) {
-					return n, nil
-				}
-				if err != nil {
-					return n, err
-				}
-				if out != writeLength {
-					return n, nil
-				}
-				writeIovecs[0].Start += hostarch.Addr(out)
-				if !writeIovecs[0].WellFormed() {
-					return n, err
-				}
-				if writeIovecs[0].Length() == 0 {
-					writeIovecs = writeIovecs[1:]
-				}
-			}
+			writeIovecs[0].Start += hostarch.Addr(out)
 		}
 	}
 	return n, nil
diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD
index 2500f402e..e89552f1c 100644
--- a/test/syscalls/linux/BUILD
+++ b/test/syscalls/linux/BUILD
@@ -4658,6 +4658,7 @@ cc_binary(
         gtest,
         "//test/util:cleanup",
         "//test/util:logging",
+        "//test/util:memory_util",
         "//test/util:posix_error",
         "//test/util:test_main",
         "//test/util:test_util",
diff --git a/test/syscalls/linux/process_vm_read_write.cc b/test/syscalls/linux/process_vm_read_write.cc
index 5d4df10a9..3d482a6e2 100644
--- a/test/syscalls/linux/process_vm_read_write.cc
+++ b/test/syscalls/linux/process_vm_read_write.cc
@@ -43,6 +43,7 @@
 #include "absl/strings/str_join.h"
 #include "test/util/cleanup.h"
 #include "test/util/logging.h"
+#include "test/util/memory_util.h"
 #include "test/util/posix_error.h"
 #include "test/util/test_util.h"
 #include "test/util/thread_util.h"
@@ -406,6 +407,24 @@ TEST(ProcessVMInvalidTest, PartialReadWrite) {
       SyscallSucceedsWithValue(iov_content_1.size()));
 }
 
+TEST(ProcessVMInvalidTest, AccessInvalidMemoryFailsWithEINVAL) {
+  auto const mapping =
+      ASSERT_NO_ERRNO_AND_VALUE(MmapAnon(kPageSize, PROT_NONE, MAP_PRIVATE));
+  struct iovec iov_none, iov_valid;
+  char buf[128];
+  iov_valid.iov_base = buf;
+  iov_valid.iov_len = sizeof(buf);
+  iov_none.iov_base = mapping.ptr();
+  iov_none.iov_len = mapping.len();
+
+  EXPECT_THAT(
+      RetryEINTR(process_vm_writev)(getpid(), &iov_none, 1, &iov_valid, 1, 0),
+      SyscallFailsWithErrno(EFAULT));
+  EXPECT_THAT(
+      RetryEINTR(process_vm_writev)(getpid(), &iov_valid, 1, &iov_none, 1, 0),
+      SyscallFailsWithErrno(EFAULT));
+}
+
 TEST(ProcessVMTest, WriteToZombie) {
   char* data = {0};
   pid_t child;