From 8fd89fd7a2b4a69c76b126fa52f47757b6076d36 Mon Sep 17 00:00:00 2001
From: Fabricio Voznika <fvoznika@google.com>
Date: Tue, 27 Aug 2019 10:51:23 -0700
Subject: [PATCH] Fix sendfile(2) error code

When output file is in append mode, sendfile(2) should fail
with EINVAL and not EBADF.

Closes #721

PiperOrigin-RevId: 265718958
---
 pkg/sentry/syscalls/linux/sys_splice.go | 25 ++++++++++++++++---------
 test/syscalls/linux/sendfile.cc         | 22 +++++++++++++++++++++-
 2 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/pkg/sentry/syscalls/linux/sys_splice.go b/pkg/sentry/syscalls/linux/sys_splice.go
index 17e3dde1f..8a98fedcb 100644
--- a/pkg/sentry/syscalls/linux/sys_splice.go
+++ b/pkg/sentry/syscalls/linux/sys_splice.go
@@ -91,24 +91,31 @@ func Sendfile(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Sysc
 	}
 
 	// Get files.
-	outFile := t.GetFile(outFD)
-	if outFile == nil {
-		return 0, nil, syserror.EBADF
-	}
-	defer outFile.DecRef()
-
 	inFile := t.GetFile(inFD)
 	if inFile == nil {
 		return 0, nil, syserror.EBADF
 	}
 	defer inFile.DecRef()
 
-	// Verify that the outfile Append flag is not set. Note that fs.Splice
-	// itself validates that the output file is writable.
-	if outFile.Flags().Append {
+	if !inFile.Flags().Read {
 		return 0, nil, syserror.EBADF
 	}
 
+	outFile := t.GetFile(outFD)
+	if outFile == nil {
+		return 0, nil, syserror.EBADF
+	}
+	defer outFile.DecRef()
+
+	if !outFile.Flags().Write {
+		return 0, nil, syserror.EBADF
+	}
+
+	// Verify that the outfile Append flag is not set.
+	if outFile.Flags().Append {
+		return 0, nil, syserror.EINVAL
+	}
+
 	// Verify that we have a regular infile. This is a requirement; the
 	// same check appears in Linux (fs/splice.c:splice_direct_to_actor).
 	if !fs.IsRegular(inFile.Dirent.Inode.StableAttr) {
diff --git a/test/syscalls/linux/sendfile.cc b/test/syscalls/linux/sendfile.cc
index e5d72e28a..9167ab066 100644
--- a/test/syscalls/linux/sendfile.cc
+++ b/test/syscalls/linux/sendfile.cc
@@ -299,10 +299,30 @@ TEST(SendFileTest, DoNotSendfileIfOutfileIsAppendOnly) {
 
   // Open the output file as append only.
   const FileDescriptor outf =
-      ASSERT_NO_ERRNO_AND_VALUE(Open(out_file.path(), O_APPEND));
+      ASSERT_NO_ERRNO_AND_VALUE(Open(out_file.path(), O_WRONLY | O_APPEND));
 
   // Send data and verify that sendfile returns the correct errno.
   EXPECT_THAT(sendfile(outf.get(), inf.get(), nullptr, kDataSize),
+              SyscallFailsWithErrno(EINVAL));
+}
+
+TEST(SendFileTest, AppendCheckOrdering) {
+  constexpr char kData[] = "And by opposing end them: to die, to sleep";
+  constexpr int kDataSize = sizeof(kData) - 1;
+  const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(
+      GetAbsoluteTestTmpdir(), kData, TempPath::kDefaultFileMode));
+
+  const FileDescriptor read =
+      ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDONLY));
+  const FileDescriptor write =
+      ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_WRONLY));
+  const FileDescriptor append =
+      ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_APPEND));
+
+  // Check that read/write file mode is verified before append.
+  EXPECT_THAT(sendfile(append.get(), read.get(), nullptr, kDataSize),
+              SyscallFailsWithErrno(EBADF));
+  EXPECT_THAT(sendfile(write.get(), write.get(), nullptr, kDataSize),
               SyscallFailsWithErrno(EBADF));
 }