b677e7
From 8df650c7c5adc2bb24a0077d8332f5ee342e7fd8 Mon Sep 17 00:00:00 2001
b677e7
From: Lennart Poettering <lennart@poettering.net>
b677e7
Date: Fri, 26 Feb 2021 10:25:24 +0100
b677e7
Subject: [PATCH] copy: handle copy_file_range() weirdness on procfs/sysfs
b677e7
b677e7
This addresses the issue described in https://lwn.net/Articles/846403/
b677e7
and makes sure we will be able to stream bytes from procfs/sysfs via
b677e7
copy_bytes() if people ask us to.
b677e7
b677e7
Based on: ee1aa61c4710ae567a2b844e0f0bb8cb0456ab8c
b677e7
Related: #1970860
b677e7
---
b677e7
 src/basic/copy.c     | 75 +++++++++++++++++++++++++++++---------------
b677e7
 src/test/test-copy.c | 17 ++++++++++
b677e7
 2 files changed, 66 insertions(+), 26 deletions(-)
b677e7
b677e7
diff --git a/src/basic/copy.c b/src/basic/copy.c
b677e7
index e06a503a29..a48c42c5c6 100644
b677e7
--- a/src/basic/copy.c
b677e7
+++ b/src/basic/copy.c
b677e7
@@ -92,7 +92,7 @@ int copy_bytes_full(
b677e7
                 void **ret_remains,
b677e7
                 size_t *ret_remains_size) {
b677e7
 
b677e7
-        bool try_cfr = true, try_sendfile = true, try_splice = true;
b677e7
+        bool try_cfr = true, try_sendfile = true, try_splice = true, copied_something = false;
b677e7
         int r, nonblock_pipe = -1;
b677e7
         size_t m = SSIZE_MAX; /* that is the maximum that sendfile and c_f_r accept */
b677e7
 
b677e7
@@ -185,9 +185,20 @@ int copy_bytes_full(
b677e7
 
b677e7
                                 try_cfr = false;
b677e7
                                 /* use fallback below */
b677e7
-                        } else if (n == 0) /* EOF */
b677e7
-                                break;
b677e7
-                        else
b677e7
+                        } else if (n == 0) { /* likely EOF */
b677e7
+
b677e7
+                                if (copied_something)
b677e7
+                                        break;
b677e7
+
b677e7
+                                /* So, we hit EOF immediately, without having copied a single byte. This
b677e7
+                                 * could indicate two things: the file is actually empty, or we are on some
b677e7
+                                 * virtual file system such as procfs/sysfs where the syscall actually
b677e7
+                                 * doesn't work but doesn't return an error. Try to handle that, by falling
b677e7
+                                 * back to simple read()s in case we encounter empty files.
b677e7
+                                 *
b677e7
+                                 * See: https://lwn.net/Articles/846403/ */
b677e7
+                                try_cfr = try_sendfile = try_splice = false;
b677e7
+                        } else
b677e7
                                 /* Success! */
b677e7
                                 goto next;
b677e7
                 }
b677e7
@@ -201,9 +212,14 @@ int copy_bytes_full(
b677e7
 
b677e7
                                 try_sendfile = false;
b677e7
                                 /* use fallback below */
b677e7
-                        } else if (n == 0) /* EOF */
b677e7
+                        } else if (n == 0) { /* likely EOF */
b677e7
+
b677e7
+                                if (copied_something)
b677e7
+                                        break;
b677e7
+
b677e7
+                                try_sendfile = try_splice = false; /* same logic as above for copy_file_range() */
b677e7
                                 break;
b677e7
-                        else
b677e7
+                        } else
b677e7
                                 /* Success! */
b677e7
                                 goto next;
b677e7
                 }
b677e7
@@ -213,14 +229,14 @@ int copy_bytes_full(
b677e7
 
b677e7
                         /* splice()'s asynchronous I/O support is a bit weird. When it encounters a pipe file
b677e7
                          * descriptor, then it will ignore its O_NONBLOCK flag and instead only honour the
b677e7
-                         * SPLICE_F_NONBLOCK flag specified in its flag parameter. Let's hide this behaviour here, and
b677e7
-                         * check if either of the specified fds are a pipe, and if so, let's pass the flag
b677e7
-                         * automatically, depending on O_NONBLOCK being set.
b677e7
+                         * SPLICE_F_NONBLOCK flag specified in its flag parameter. Let's hide this behaviour
b677e7
+                         * here, and check if either of the specified fds are a pipe, and if so, let's pass
b677e7
+                         * the flag automatically, depending on O_NONBLOCK being set.
b677e7
                          *
b677e7
-                         * Here's a twist though: when we use it to move data between two pipes of which one has
b677e7
-                         * O_NONBLOCK set and the other has not, then we have no individual control over O_NONBLOCK
b677e7
-                         * behaviour. Hence in that case we can't use splice() and still guarantee systematic
b677e7
-                         * O_NONBLOCK behaviour, hence don't. */
b677e7
+                         * Here's a twist though: when we use it to move data between two pipes of which one
b677e7
+                         * has O_NONBLOCK set and the other has not, then we have no individual control over
b677e7
+                         * O_NONBLOCK behaviour. Hence in that case we can't use splice() and still guarantee
b677e7
+                         * systematic O_NONBLOCK behaviour, hence don't. */
b677e7
 
b677e7
                         if (nonblock_pipe < 0) {
b677e7
                                 int a, b;
b677e7
@@ -238,12 +254,13 @@ int copy_bytes_full(
b677e7
                                     (a == FD_IS_BLOCKING_PIPE && b == FD_IS_NONBLOCKING_PIPE) ||
b677e7
                                     (a == FD_IS_NONBLOCKING_PIPE && b == FD_IS_BLOCKING_PIPE))
b677e7
 
b677e7
-                                        /* splice() only works if one of the fds is a pipe. If neither is, let's skip
b677e7
-                                         * this step right-away. As mentioned above, if one of the two fds refers to a
b677e7
-                                         * blocking pipe and the other to a non-blocking pipe, we can't use splice()
b677e7
-                                         * either, hence don't try either. This hence means we can only use splice() if
b677e7
-                                         * either only one of the two fds is a pipe, or if both are pipes with the same
b677e7
-                                         * nonblocking flag setting. */
b677e7
+                                        /* splice() only works if one of the fds is a pipe. If neither is,
b677e7
+                                         * let's skip this step right-away. As mentioned above, if one of the
b677e7
+                                         * two fds refers to a blocking pipe and the other to a non-blocking
b677e7
+                                         * pipe, we can't use splice() either, hence don't try either. This
b677e7
+                                         * hence means we can only use splice() if either only one of the two
b677e7
+                                         * fds is a pipe, or if both are pipes with the same nonblocking flag
b677e7
+                                         * setting. */
b677e7
 
b677e7
                                         try_splice = false;
b677e7
                                 else
b677e7
@@ -259,9 +276,13 @@ int copy_bytes_full(
b677e7
 
b677e7
                                 try_splice = false;
b677e7
                                 /* use fallback below */
b677e7
-                        } else if (n == 0) /* EOF */
b677e7
-                                break;
b677e7
-                        else
b677e7
+                        } else if (n == 0) { /* likely EOF */
b677e7
+
b677e7
+                                if (copied_something)
b677e7
+                                        break;
b677e7
+
b677e7
+                                try_splice = false; /* same logic as above for copy_file_range() + sendfile() */
b677e7
+                        } else
b677e7
                                 /* Success! */
b677e7
                                 goto next;
b677e7
                 }
b677e7
@@ -312,11 +333,13 @@ int copy_bytes_full(
b677e7
                         assert(max_bytes >= (uint64_t) n);
b677e7
                         max_bytes -= n;
b677e7
                 }
b677e7
-                /* sendfile accepts at most SSIZE_MAX-offset bytes to copy,
b677e7
-                 * so reduce our maximum by the amount we already copied,
b677e7
-                 * but don't go below our copy buffer size, unless we are
b677e7
-                 * close the limit of bytes we are allowed to copy. */
b677e7
+
b677e7
+                /* sendfile accepts at most SSIZE_MAX-offset bytes to copy, so reduce our maximum by the
b677e7
+                 * amount we already copied, but don't go below our copy buffer size, unless we are close the
b677e7
+                 * limit of bytes we are allowed to copy. */
b677e7
                 m = MAX(MIN(COPY_BUFFER_SIZE, max_bytes), m - n);
b677e7
+
b677e7
+                copied_something = true;
b677e7
         }
b677e7
 
b677e7
         return 0; /* return 0 if we hit EOF earlier than the size limit */
b677e7
diff --git a/src/test/test-copy.c b/src/test/test-copy.c
b677e7
index 2e8d251ac1..29ac33e47a 100644
b677e7
--- a/src/test/test-copy.c
b677e7
+++ b/src/test/test-copy.c
b677e7
@@ -253,6 +253,22 @@ static void test_copy_atomic(void) {
b677e7
         assert_se(copy_file_atomic("/etc/fstab", q, 0644, 0, COPY_REPLACE) >= 0);
b677e7
 }
b677e7
 
b677e7
+static void test_copy_proc(void) {
b677e7
+        _cleanup_(rm_rf_physical_and_freep) char *p = NULL;
b677e7
+        _cleanup_free_ char *f = NULL, *a = NULL, *b = NULL;
b677e7
+
b677e7
+        /* Check if copying data from /proc/ works correctly, i.e. let's see if https://lwn.net/Articles/846403/ is a problem for us */
b677e7
+
b677e7
+        assert_se(mkdtemp_malloc(NULL, &p) >= 0);
b677e7
+        assert_se(f = path_join(NULL, p, "version"));
b677e7
+        assert_se(copy_file("/proc/version", f, 0, (mode_t) -1, 0, 0) >= 0);
b677e7
+
b677e7
+        assert_se(read_one_line_file("/proc/version", &a) >= 0);
b677e7
+        assert_se(read_one_line_file(f, &b) >= 0);
b677e7
+        assert_se(streq(a, b));
b677e7
+        assert_se(strlen(a) > 0);
b677e7
+}
b677e7
+
b677e7
 int main(int argc, char *argv[]) {
b677e7
         log_set_max_level(LOG_DEBUG);
b677e7
 
b677e7
@@ -267,6 +283,7 @@ int main(int argc, char *argv[]) {
b677e7
         test_copy_bytes_regular_file(argv[0], false, 32000); /* larger than copy buffer size */
b677e7
         test_copy_bytes_regular_file(argv[0], true, 32000);
b677e7
         test_copy_atomic();
b677e7
+        test_copy_proc();
b677e7
 
b677e7
         return 0;
b677e7
 }