shaba/systemd-stable
1
1
Fork 0
mirror of https://github.com/systemd/systemd-stable.git synced 2025-09-03 21:45:02 +03:00
Code Activity

copy: handle copy_file_range() weirdness on procfs/sysfs

Browse Source 
This addresses the issue described in https://lwn.net/Articles/846403/
and makes sure we will be able to stream bytes from procfs/sysfs via
copy_bytes() if people ask us to.
This commit is contained in:
Lennart Poettering
2021-02-26 10:25:24 +01:00
parent 97e535c724
commit ee1aa61c47
2 changed files with 65 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

70
src/basic/copy.c
View File

@@ -112,7 +112,7 @@ int copy_bytes_full(
copy_progress_bytes_t progress, copy_progress_bytes_t progress,
void *userdata) { void *userdata) {
bool try_cfr = true, try_sendfile = true, try_splice = true; bool try_cfr = true, try_sendfile = true, try_splice = true, copied_something = false;
int r, nonblock_pipe = -1; int r, nonblock_pipe = -1;
size_t m = SSIZE_MAX; /* that is the maximum that sendfile and c_f_r accept */ size_t m = SSIZE_MAX; /* that is the maximum that sendfile and c_f_r accept */
@@ -211,9 +211,20 @@ int copy_bytes_full(
try_cfr = false; try_cfr = false;
/* use fallback below */ /* use fallback below */
} else if (n == 0) /* EOF */ } else if (n == 0) { /* likely EOF */
if (copied_something)
break; break;
else
/* So, we hit EOF immediately, without having copied a single byte. This
* could indicate two things: the file is actually empty, or we are on some
* virtual file system such as procfs/sysfs where the syscall actually
* doesn't work but doesn't return an error. Try to handle that, by falling
* back to simple read()s in case we encounter empty files.
*
* See: https://lwn.net/Articles/846403/ */
try_cfr = try_sendfile = try_splice = false;
} else
/* Success! */ /* Success! */
goto next; goto next;
} }
@@ -227,9 +238,14 @@ int copy_bytes_full(
try_sendfile = false; try_sendfile = false;
/* use fallback below */ /* use fallback below */
} else if (n == 0) /* EOF */ } else if (n == 0) { /* likely EOF */
if (copied_something)
break; break;
else
try_sendfile = try_splice = false; /* same logic as above for copy_file_range() */
break;
} else
/* Success! */ /* Success! */
goto next; goto next;
} }
@@ -239,14 +255,14 @@ int copy_bytes_full(
/* splice()'s asynchronous I/O support is a bit weird. When it encounters a pipe file /* splice()'s asynchronous I/O support is a bit weird. When it encounters a pipe file
* descriptor, then it will ignore its O_NONBLOCK flag and instead only honour the * descriptor, then it will ignore its O_NONBLOCK flag and instead only honour the
* SPLICE_F_NONBLOCK flag specified in its flag parameter. Let's hide this behaviour here, and * SPLICE_F_NONBLOCK flag specified in its flag parameter. Let's hide this behaviour
* check if either of the specified fds are a pipe, and if so, let's pass the flag * here, and check if either of the specified fds are a pipe, and if so, let's pass
* automatically, depending on O_NONBLOCK being set. * the flag automatically, depending on O_NONBLOCK being set.
* *
* Here's a twist though: when we use it to move data between two pipes of which one has * Here's a twist though: when we use it to move data between two pipes of which one
* O_NONBLOCK set and the other has not, then we have no individual control over O_NONBLOCK * has O_NONBLOCK set and the other has not, then we have no individual control over
* behaviour. Hence in that case we can't use splice() and still guarantee systematic * O_NONBLOCK behaviour. Hence in that case we can't use splice() and still guarantee
* O_NONBLOCK behaviour, hence don't. */ * systematic O_NONBLOCK behaviour, hence don't. */
if (nonblock_pipe < 0) { if (nonblock_pipe < 0) {
int a, b; int a, b;
@@ -264,12 +280,13 @@ int copy_bytes_full(
(a == FD_IS_BLOCKING_PIPE && b == FD_IS_NONBLOCKING_PIPE) || (a == FD_IS_BLOCKING_PIPE && b == FD_IS_NONBLOCKING_PIPE) ||
(a == FD_IS_NONBLOCKING_PIPE && b == FD_IS_BLOCKING_PIPE)) (a == FD_IS_NONBLOCKING_PIPE && b == FD_IS_BLOCKING_PIPE))
/* splice() only works if one of the fds is a pipe. If neither is, let's skip /* splice() only works if one of the fds is a pipe. If neither is,
* this step right-away. As mentioned above, if one of the two fds refers to a * let's skip this step right-away. As mentioned above, if one of the
* blocking pipe and the other to a non-blocking pipe, we can't use splice() * two fds refers to a blocking pipe and the other to a non-blocking
* either, hence don't try either. This hence means we can only use splice() if * pipe, we can't use splice() either, hence don't try either. This
* either only one of the two fds is a pipe, or if both are pipes with the same * hence means we can only use splice() if either only one of the two
* nonblocking flag setting. */ * fds is a pipe, or if both are pipes with the same nonblocking flag
* setting. */
try_splice = false; try_splice = false;
else else
@@ -285,9 +302,13 @@ int copy_bytes_full(
try_splice = false; try_splice = false;
/* use fallback below */ /* use fallback below */
} else if (n == 0) /* EOF */ } else if (n == 0) { /* likely EOF */
if (copied_something)
break; break;
else
try_splice = false; /* same logic as above for copy_file_range() + sendfile() */
} else
/* Success! */ /* Success! */
goto next; goto next;
} }
@@ -345,11 +366,12 @@ int copy_bytes_full(
max_bytes -= n; max_bytes -= n;
} }
/* sendfile accepts at most SSIZE_MAX-offset bytes to copy, /* sendfile accepts at most SSIZE_MAX-offset bytes to copy, so reduce our maximum by the
* so reduce our maximum by the amount we already copied, * amount we already copied, but don't go below our copy buffer size, unless we are close the
* but don't go below our copy buffer size, unless we are * limit of bytes we are allowed to copy. */
* close the limit of bytes we are allowed to copy. */
m = MAX(MIN(COPY_BUFFER_SIZE, max_bytes), m - n); m = MAX(MIN(COPY_BUFFER_SIZE, max_bytes), m - n);
copied_something = true;
} }
return 0; /* return 0 if we hit EOF earlier than the size limit */ return 0; /* return 0 if we hit EOF earlier than the size limit */

17
src/test/test-copy.c
View File

@@ -304,6 +304,22 @@ static void test_copy_atomic(void) {
assert_se(copy_file_atomic("/etc/fstab", q, 0644, 0, 0, COPY_REPLACE) >= 0); assert_se(copy_file_atomic("/etc/fstab", q, 0644, 0, 0, COPY_REPLACE) >= 0);
} }
static void test_copy_proc(void) {
_cleanup_(rm_rf_physical_and_freep) char *p = NULL;
_cleanup_free_ char *f = NULL, *a = NULL, *b = NULL;
/* Check if copying data from /proc/ works correctly, i.e. let's see if https://lwn.net/Articles/846403/ is a problem for us */
assert_se(mkdtemp_malloc(NULL, &p) >= 0);
assert_se(f = path_join(p, "version"));
assert_se(copy_file("/proc/version", f, 0, (mode_t) -1, 0, 0, 0) >= 0);
assert_se(read_one_line_file("/proc/version", &a) >= 0);
assert_se(read_one_line_file(f, &b) >= 0);
assert_se(streq(a, b));
assert_se(strlen(a) > 0);
}
int main(int argc, char *argv[]) { int main(int argc, char *argv[]) {
test_setup_logging(LOG_DEBUG); test_setup_logging(LOG_DEBUG);
@@ -318,6 +334,7 @@ int main(int argc, char *argv[]) {
test_copy_bytes_regular_file(argv[0], false, 32000); /* larger than copy buffer size */ test_copy_bytes_regular_file(argv[0], false, 32000); /* larger than copy buffer size */
test_copy_bytes_regular_file(argv[0], true, 32000); test_copy_bytes_regular_file(argv[0], true, 32000);
test_copy_atomic(); test_copy_atomic();
test_copy_proc();
return 0; return 0;
} }