From 6502f59a98e6f46cfa40e6f5b76b8098b9083da7 Mon Sep 17 00:00:00 2001
From: Zim <zezeozue@google.com>
Date: Mon, 12 Apr 2021 13:06:20 +0100
Subject: [PATCH] Fix FileUtils#copy to fallback on userspace copy if sendfile
 fails

As per the manpage, sendfile(2) might fail for the following reasons:
1. input fd doesn't support mmap
2. output was opened with O_APPEND

If sendfile(2) fails with EINVAL or ENOSYS, we fallback to userspace
copy

Fixed failing FileUtilsTest#convertToModernFd test since
I7d6ecf4549e8ed5134d20977c2089240be804827 that stoped matching valid
files to convert to modernfd with regex

Test: atest FileUtilsTest
Bug: 184236915
Change-Id: I0955fe2a7dfb40ac1f78bde4a325eb624a34f28b
---
 core/java/android/os/FileUtils.java           | 16 ++++++++++-
 .../src/android/os/FileUtilsTest.java         | 27 ++++++++++++++-----
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/core/java/android/os/FileUtils.java b/core/java/android/os/FileUtils.java
index 40c658f01e288..a06a8577a56ac 100644
--- a/core/java/android/os/FileUtils.java
+++ b/core/java/android/os/FileUtils.java
@@ -22,6 +22,8 @@ import static android.os.ParcelFileDescriptor.MODE_READ_ONLY;
 import static android.os.ParcelFileDescriptor.MODE_READ_WRITE;
 import static android.os.ParcelFileDescriptor.MODE_TRUNCATE;
 import static android.os.ParcelFileDescriptor.MODE_WRITE_ONLY;
+import static android.system.OsConstants.EINVAL;
+import static android.system.OsConstants.ENOSYS;
 import static android.system.OsConstants.F_OK;
 import static android.system.OsConstants.O_ACCMODE;
 import static android.system.OsConstants.O_APPEND;
@@ -441,7 +443,19 @@ public final class FileUtils {
                 final StructStat st_in = Os.fstat(in);
                 final StructStat st_out = Os.fstat(out);
                 if (S_ISREG(st_in.st_mode) && S_ISREG(st_out.st_mode)) {
-                    return copyInternalSendfile(in, out, count, signal, executor, listener);
+                    try {
+                        return copyInternalSendfile(in, out, count, signal, executor, listener);
+                    } catch (ErrnoException e) {
+                        if (e.errno == EINVAL || e.errno == ENOSYS) {
+                            // sendfile(2) will fail in at least any of the following conditions:
+                            // 1. |in| doesn't support mmap(2)
+                            // 2. |out| was opened with O_APPEND
+                            // We fallback to userspace copy if that fails
+                            return copyInternalUserspace(in, out, count, signal, executor,
+                                    listener);
+                        }
+                        throw e;
+                    }
                 } else if (S_ISFIFO(st_in.st_mode) || S_ISFIFO(st_out.st_mode)) {
                     return copyInternalSplice(in, out, count, signal, executor, listener);
                 }
diff --git a/core/tests/coretests/src/android/os/FileUtilsTest.java b/core/tests/coretests/src/android/os/FileUtilsTest.java
index 1a86678a30b4c..c1e72fe756667 100644
--- a/core/tests/coretests/src/android/os/FileUtilsTest.java
+++ b/core/tests/coretests/src/android/os/FileUtilsTest.java
@@ -227,6 +227,27 @@ public class FileUtilsTest {
         }
     }
 
+    @Test
+    public void testCopyFileWithAppend() throws Exception {
+        final File src = new File(mTarget, "src");
+        final File dest = new File(mTarget, "dest");
+
+        byte[] expected = new byte[10];
+        byte[] actual = new byte[10];
+        new Random().nextBytes(expected);
+        writeFile(src, expected);
+
+        try (FileInputStream in = new FileInputStream(src);
+                FileOutputStream out = new FileOutputStream(dest, true /* append */)) {
+            // sendfile(2) fails if output fd is opened with O_APPEND, but FileUtils#copy should
+            // fallback to userspace copy
+            FileUtils.copy(in, out);
+        }
+
+        actual = readFile(dest);
+        assertArrayEquals(expected, actual);
+    }
+
     @Test
     public void testIsFilenameSafe() throws Exception {
         assertTrue(FileUtils.isFilenameSafe(new File("foobar")));
@@ -577,7 +598,6 @@ public class FileUtilsTest {
 
         final File validVideoCameraDir = new File(cameraDir, "validVideo-" + nonce + ".mp4");
         final File validImageCameraDir = new File(cameraDir, "validImage-" + nonce + ".jpg");
-        final File invalidVideoCameraDir = new File(cameraDir, ".invalidVideo-" + nonce + ".mp4");
 
         final File validVideoNonCameraDir = new File(nonCameraDir, "validVideo-" + nonce + ".mp4");
         final File validImageNonCameraDir = new File(nonCameraDir, "validImage-" + nonce + ".jpg");
@@ -589,9 +609,6 @@ public class FileUtilsTest {
             FileDescriptor pfdValidImageCameraDir =
                     ParcelFileDescriptor.open(validImageCameraDir,
                             MODE_CREATE | MODE_READ_WRITE).getFileDescriptor();
-            FileDescriptor pfdInvalidVideoCameraDir =
-                    ParcelFileDescriptor.open(invalidVideoCameraDir,
-                            MODE_CREATE | MODE_READ_WRITE).getFileDescriptor();
 
             FileDescriptor pfdValidVideoNonCameraDir =
                     ParcelFileDescriptor.open(validVideoNonCameraDir,
@@ -603,13 +620,11 @@ public class FileUtilsTest {
             assertNotNull(convertToModernFd(pfdValidVideoCameraDir));
 
             assertNull(convertToModernFd(pfdValidImageCameraDir));
-            assertNull(convertToModernFd(pfdInvalidVideoCameraDir));
             assertNull(convertToModernFd(pfdValidVideoNonCameraDir));
             assertNull(convertToModernFd(pfdValidImageNonCameraDir));
         } finally {
             validVideoCameraDir.delete();
             validImageCameraDir.delete();
-            invalidVideoCameraDir.delete();
             validVideoNonCameraDir.delete();
             validImageNonCameraDir.delete();
         }