From a6ffaa5714c963c3e6f8af47482651c2ca241597 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 24 Jun 2021 14:13:50 +0200 Subject: [PATCH 1/6] replace: copy_file_range() BUG: https://bugzilla.samba.org/show_bug.cgi?id=12033 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 4dcc04228df233be15efe9c31bcbaba822b140c4) --- lib/replace/replace.c | 25 +++++++++++++++++++++++++ lib/replace/replace.h | 10 ++++++++++ lib/replace/wscript | 12 +++++++++++- 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/lib/replace/replace.c b/lib/replace/replace.c index 99b18e82590..0652cb4e6d6 100644 --- a/lib/replace/replace.c +++ b/lib/replace/replace.c @@ -1056,3 +1056,28 @@ const char *rep_getprogname(void) #endif /* HAVE_PROGRAM_INVOCATION_SHORT_NAME */ } #endif /* HAVE_GETPROGNAME */ + +#ifndef HAVE_COPY_FILE_RANGE +# ifdef HAVE_SYSCALL_COPY_FILE_RANGE +# include +# endif +ssize_t rep_copy_file_range(int fd_in, + loff_t *off_in, + int fd_out, + loff_t *off_out, + size_t len, + unsigned int flags) +{ +# ifdef HAVE_SYSCALL_COPY_FILE_RANGE + return syscall(__NR_copy_file_range, + fd_in, + off_in, + fd_out, + off_out, + len, + flags); +# endif /* HAVE_SYSCALL_COPY_FILE_RANGE */ + errno = ENOSYS; + return -1; +} +#endif /* HAVE_COPY_FILE_RANGE */ diff --git a/lib/replace/replace.h b/lib/replace/replace.h index e08bf7c2e58..a546185d47a 100644 --- a/lib/replace/replace.h +++ b/lib/replace/replace.h @@ -964,6 +964,16 @@ int rep_memset_s(void *dest, size_t destsz, int ch, size_t count); const char *rep_getprogname(void); #endif +#ifndef HAVE_COPY_FILE_RANGE +#define copy_file_range rep_copy_file_range +ssize_t rep_copy_file_range(int fd_in, + loff_t *off_in, + int fd_out, + loff_t *off_out, + size_t len, + unsigned int flags); +#endif /* HAVE_COPY_FILE_RANGE */ + #ifndef FALL_THROUGH # ifdef HAVE_FALLTHROUGH_ATTRIBUTE # define FALL_THROUGH __attribute__ ((fallthrough)) diff --git a/lib/replace/wscript b/lib/replace/wscript index 2c856b61a0f..3811084db3c 100644 --- a/lib/replace/wscript +++ b/lib/replace/wscript @@ -465,6 +465,16 @@ samba_dist.DIST_DIRS('lib/replace buildtools:buildtools third_party/waf:third_pa conf.CHECK_FUNCS('getpwent_r getpwnam_r getpwuid_r epoll_create') conf.CHECK_FUNCS('port_create') conf.CHECK_FUNCS('getprogname') + if not conf.CHECK_FUNCS('copy_file_range'): + conf.CHECK_CODE(''' +#include +#include +syscall(SYS_copy_file_range,0,NULL,0,NULL,0,0); + ''', + 'HAVE_SYSCALL_COPY_FILE_RANGE', + msg='Checking whether we have copy_file_range system call') + if conf.CONFIG_SET('HAVE_COPY_FILE_RANGE') or conf.CONFIG_SET('HAVE_SYSCALL_COPY_FILE_RANGE'): + conf.DEFINE('USE_COPY_FILE_RANGE', 1) conf.SET_TARGET_TYPE('attr', 'EMPTY') @@ -846,7 +856,7 @@ REPLACEMENT_FUNCTIONS = { 'strsep', 'strtok_r', 'strtoll', 'strtoull', 'setenv', 'unsetenv', 'utime', 'utimes', 'dup2', 'chown', 'link', 'readlink', 'symlink', 'lchown', 'realpath', 'memmem', 'vdprintf', - 'dprintf', 'get_current_dir_name', + 'dprintf', 'get_current_dir_name', 'copy_file_range', 'strerror_r', 'clock_gettime', 'memset_s'], 'timegm.c': ['timegm'], # Note: C99_VSNPRINTF is not a function, but a special condition -- 2.31.1 From f51239a3d63b9d973e0fd8077612c114e0cb8040 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 25 Jun 2021 15:47:38 +0200 Subject: [PATCH 2/6] vfs_default: properly track written bytes for copy-chunk No change in behavour, this just makes the logic slightly more understandable. In theory it would also allow the logic to be adjusted for allowing short reads which is not quite clear from MS-SMB2 if we should allow it. The file could be truncated while we're reading it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12033 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit e2d524d4baee193b0d03df3e7edda48b3cb4bddf) --- source3/modules/vfs_default.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 8d592bbad64..2d116ec1a2e 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1968,6 +1968,7 @@ struct vfswrap_offload_write_state { off_t dst_off; off_t to_copy; off_t remaining; + off_t copied; size_t next_io_size; }; @@ -2278,6 +2279,7 @@ static void vfswrap_offload_write_write_done(struct tevent_req *subreq) tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); return; } + state->copied += nwritten; state->remaining -= nwritten; if (state->remaining == 0) { tevent_req_done(req); @@ -2314,7 +2316,7 @@ static NTSTATUS vfswrap_offload_write_recv(struct vfs_handle_struct *handle, return status; } - *copied = state->to_copy; + *copied = state->copied; DBG_DEBUG("copy chunk copied %lu\n", (unsigned long)*copied); tevent_req_received(req); -- 2.31.1 From 653f61a37d0b771733470a81a4eaa37bf3d7b99c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 26 Jun 2021 12:21:19 +0200 Subject: [PATCH 3/6] lib: add sys_io_ranges_overlap() BUG: https://bugzilla.samba.org/show_bug.cgi?id=12033 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 4f1a02909b8694dcc30fd5c7c6772fcfa1092ed9) --- lib/util/sys_rw.c | 25 ++++++++ lib/util/sys_rw.h | 2 + lib/util/tests/test_sys_rw.c | 110 +++++++++++++++++++++++++++++++++++ lib/util/wscript_build | 6 ++ selftest/tests.py | 2 + 5 files changed, 145 insertions(+) create mode 100644 lib/util/tests/test_sys_rw.c diff --git a/lib/util/sys_rw.c b/lib/util/sys_rw.c index d74395fc409..02aaa871a39 100644 --- a/lib/util/sys_rw.c +++ b/lib/util/sys_rw.c @@ -48,6 +48,31 @@ bool sys_valid_io_range(off_t offset, size_t length) return true; } +bool sys_io_ranges_overlap(size_t c1, off_t o1, + size_t c2, off_t o2) +{ + if (c1 == 0 || c2 == 0) { + return false; + } + if (o2 < o1) { + /* + * o1 + * |····c1····| + * o2 + * |····c2···| ? + */ + return (o2 + c2 > o1); + } else { + /* + * o1 + * |····c1···| + * o2 + * |····c2····| ? + */ + return (o1 + c1 > o2); + } +} + /******************************************************************* A read wrapper that will deal with EINTR/EWOULDBLOCK ********************************************************************/ diff --git a/lib/util/sys_rw.h b/lib/util/sys_rw.h index b224ecb30ac..6693d34de57 100644 --- a/lib/util/sys_rw.h +++ b/lib/util/sys_rw.h @@ -28,6 +28,8 @@ struct iovec; bool sys_valid_io_range(off_t offset, size_t length); +bool sys_io_ranges_overlap(size_t c1, off_t o1, + size_t c2, off_t o2); ssize_t sys_read(int fd, void *buf, size_t count); void sys_read_v(int fd, void *buf, size_t count); ssize_t sys_write(int fd, const void *buf, size_t count); diff --git a/lib/util/tests/test_sys_rw.c b/lib/util/tests/test_sys_rw.c new file mode 100644 index 00000000000..551a6a09bda --- /dev/null +++ b/lib/util/tests/test_sys_rw.c @@ -0,0 +1,110 @@ +/* + * Unix SMB/CIFS implementation. + * + * Unit test for sys_rw.c + * + * Copyright (C) Ralph Böhme 2021 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see . + */ + +#include +#include +#include +#include +#include + +#include "lib/replace/replace.h" +#include "system/dir.h" + +#include "lib/util/sys_rw.c" + +static void test_sys_io_ranges_overlap(void **state) +{ + bool overlap; + + /* + * sys_io_ranges_overlap() args are: + * + * src size, src offset, dst size, dst offset + */ + + /* src and dst size 0 => no overlap */ + overlap = sys_io_ranges_overlap(0, 0, 0, 0); + assert_false(overlap); + + /* dst size 0 => no overlap */ + overlap = sys_io_ranges_overlap(1, 0, 0, 0); + assert_false(overlap); + + /* src size 0 => no overlap */ + overlap = sys_io_ranges_overlap(0, 0, 1, 0); + assert_false(overlap); + + /* same range => overlap */ + overlap = sys_io_ranges_overlap(1, 0, 1, 0); + assert_true(overlap); + + /* + * |.| + * |.| + * src before dst => no overlap + */ + overlap = sys_io_ranges_overlap(1, 0, 1, 1); + assert_false(overlap); + + /* + * |..| + * |..| + * src into dst => overlap + */ + overlap = sys_io_ranges_overlap(2, 0, 2, 1); + assert_true(overlap); + + /* + * |....| + * |..| + * src encompasses dst => overlap + */ + overlap = sys_io_ranges_overlap(4, 0, 1, 2); + assert_true(overlap); + + + /* + * |..| + * |..| + * dst into src => overlap + */ + overlap = sys_io_ranges_overlap(2, 1, 2, 0); + assert_true(overlap); + + /* + * |..| + * |....| + * dst encompasses src => overlap + */ + overlap = sys_io_ranges_overlap(2, 1, 4, 0); + assert_true(overlap); +} + +int main(int argc, char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_sys_io_ranges_overlap), + }; + + cmocka_set_message_output(CM_OUTPUT_SUBUNIT); + + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/lib/util/wscript_build b/lib/util/wscript_build index 460661e9a63..839d3c2e2e9 100644 --- a/lib/util/wscript_build +++ b/lib/util/wscript_build @@ -356,3 +356,9 @@ bld.SAMBA_LIBRARY('genrand', deps='cmocka replace talloc samba-util', local_include=False, for_selftest=True) + + bld.SAMBA_BINARY('test_sys_rw', + source='tests/test_sys_rw.c', + deps='cmocka replace samba-util', + local_include=False, + for_selftest=True) diff --git a/selftest/tests.py b/selftest/tests.py index 6bf46ae5621..3645904801a 100644 --- a/selftest/tests.py +++ b/selftest/tests.py @@ -407,6 +407,8 @@ plantestsuite("samba.unittests.util", "none", [os.path.join(bindir(), "default/lib/util/test_util")]) plantestsuite("samba.unittests.memcache", "none", [os.path.join(bindir(), "default/lib/util/test_memcache")]) +plantestsuite("samba.unittests.sys_rw", "none", + [os.path.join(bindir(), "default/lib/util/test_sys_rw")]) plantestsuite("samba.unittests.ntlm_check", "none", [os.path.join(bindir(), "default/libcli/auth/test_ntlm_check")]) plantestsuite("samba.unittests.gnutls", "none", -- 2.31.1 From caebb4ad4b5af7c5836bbde3c9ae11f3d910657c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 28 Jun 2021 15:50:32 +0200 Subject: [PATCH 4/6] smbd: use sys_io_ranges_overlap() in fsctl_dup_extents_check_overlap() BUG: https://bugzilla.samba.org/show_bug.cgi?id=12033 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit e72be5213335ab1ea0f9f396ab071669231c151b) --- source3/smbd/smb2_ioctl_filesys.c | 38 +++++-------------------------- 1 file changed, 6 insertions(+), 32 deletions(-) diff --git a/source3/smbd/smb2_ioctl_filesys.c b/source3/smbd/smb2_ioctl_filesys.c index e7bd11f90ab..f5c472c2cd1 100644 --- a/source3/smbd/smb2_ioctl_filesys.c +++ b/source3/smbd/smb2_ioctl_filesys.c @@ -30,6 +30,7 @@ #include "../librpc/ndr/libndr.h" #include "librpc/gen_ndr/ndr_ioctl.h" #include "smb2_ioctl_private.h" +#include "lib/util/sys_rw.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_SMB2 @@ -96,43 +97,16 @@ static NTSTATUS fsctl_dup_extents_check_overlap(struct files_struct *src_fsp, struct files_struct *dst_fsp, struct fsctl_dup_extents_to_file *dup_extents) { - uint64_t src_off_last; - uint64_t tgt_off_last; - if (!file_id_equal(&src_fsp->file_id, &dst_fsp->file_id)) { /* src and dest refer to different files */ return NT_STATUS_OK; } - if (dup_extents->byte_count == 0) { - /* no range to overlap */ - return NT_STATUS_OK; - } - - /* - * [MS-FSCC] 2.3.8 FSCTL_DUPLICATE_EXTENTS_TO_FILE Reply - * STATUS_NOT_SUPPORTED: - * The source and target destination ranges overlap on the same file. - */ - - src_off_last = dup_extents->source_off + dup_extents->byte_count - 1; - if ((dup_extents->target_off >= dup_extents->source_off) - && (dup_extents->target_off <= src_off_last)) { - /* - * src: |-----------| - * tgt: |-----------| - */ - return NT_STATUS_NOT_SUPPORTED; - } - - - tgt_off_last = dup_extents->target_off + dup_extents->byte_count - 1; - if ((tgt_off_last >= dup_extents->source_off) - && (tgt_off_last <= src_off_last)) { - /* - * src: |-----------| - * tgt: |-----------| - */ + if (sys_io_ranges_overlap(dup_extents->byte_count, + dup_extents->source_off, + dup_extents->byte_count, + dup_extents->target_off)) + { return NT_STATUS_NOT_SUPPORTED; } -- 2.31.1 From 8c923341429c4d75b471770fa30ce5c65fc0be96 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 24 Jun 2021 16:21:42 +0200 Subject: [PATCH 5/6] vfs_default: use copy_file_range() Original file on an XFS filesystem: $ ls -l /mnt/test/1048578-file -rw-rw-r--. 1 slow slow 1048578 Jun 25 11:40 /mnt/test/1048578-file $ xfs_bmap /mnt/test/1048578-file /mnt/test/1048578-file: 0: [0..2055]: 192..2247 Copy created with cp --reflink=never: $ xfs_bmap /mnt/test/1048578-file-reflink-never /mnt/test/1048578-file-reflink-never: 0: [0..2055]: 2248..4303 Copy created with cp --reflink=always $ xfs_bmap /mnt/test/1048578-file-reflink-always /mnt/test/1048578-file-reflink-always: 0: [0..2055]: 192..2247 Copy done from a Windows client: $ xfs_bmap /mnt/test/1048578-file\ -\ Copy /mnt/test/1048578-file - Copy: 0: [0..2055]: 192..2247 BUG: https://bugzilla.samba.org/show_bug.cgi?id=12033 RN: smbd should support copy_file_range() for FSCTL_SRV_COPYCHUNK Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Wed Jun 30 17:40:23 UTC 2021 on sn-devel-184 (cherry picked from commit accaa2f1f67a7f064a4ce03a120d7b2f8e847ccf) --- source3/modules/vfs_default.c | 134 ++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 2d116ec1a2e..eafe1068d3b 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1988,6 +1988,7 @@ static void vfswrap_offload_write_cleanup(struct tevent_req *req, state->dst_fsp = NULL; } +static NTSTATUS vfswrap_offload_copy_file_range(struct tevent_req *req); static NTSTATUS vfswrap_offload_write_loop(struct tevent_req *req); static struct tevent_req *vfswrap_offload_write_send( @@ -2127,6 +2128,16 @@ static struct tevent_req *vfswrap_offload_write_send( return tevent_req_post(req, ev); } + status = vfswrap_offload_copy_file_range(req); + if (NT_STATUS_IS_OK(status)) { + tevent_req_done(req); + return tevent_req_post(req, ev); + } + if (!NT_STATUS_EQUAL(status, NT_STATUS_MORE_PROCESSING_REQUIRED)) { + tevent_req_nterror(req, status); + return tevent_req_post(req, ev); + } + state->buf = talloc_array(state, uint8_t, num); if (tevent_req_nomem(state->buf, req)) { return tevent_req_post(req, ev); @@ -2141,6 +2152,129 @@ static struct tevent_req *vfswrap_offload_write_send( return req; } +static NTSTATUS vfswrap_offload_copy_file_range(struct tevent_req *req) +{ + struct vfswrap_offload_write_state *state = tevent_req_data( + req, struct vfswrap_offload_write_state); + struct lock_struct lck; + ssize_t nwritten; + NTSTATUS status; + bool same_file; + bool ok; + +#ifndef USE_COPY_FILE_RANGE + return NT_STATUS_MORE_PROCESSING_REQUIRED; +#endif + + same_file = file_id_equal(&state->src_fsp->file_id, + &state->dst_fsp->file_id); + if (same_file && + sys_io_ranges_overlap(state->remaining, + state->src_off, + state->remaining, + state->dst_off)) + { + return NT_STATUS_MORE_PROCESSING_REQUIRED; + } + + if (is_named_stream(state->src_fsp->fsp_name) || + is_named_stream(state->dst_fsp->fsp_name)) + { + return NT_STATUS_MORE_PROCESSING_REQUIRED; + } + + init_strict_lock_struct(state->src_fsp, + state->src_fsp->op->global->open_persistent_id, + state->src_off, + state->remaining, + READ_LOCK, + &lck); + + ok = SMB_VFS_STRICT_LOCK_CHECK(state->src_fsp->conn, + state->src_fsp, + &lck); + if (!ok) { + return NT_STATUS_FILE_LOCK_CONFLICT; + } + + ok = change_to_user_and_service_by_fsp(state->dst_fsp); + if (!ok) { + return NT_STATUS_INTERNAL_ERROR; + } + + init_strict_lock_struct(state->dst_fsp, + state->dst_fsp->op->global->open_persistent_id, + state->dst_off, + state->remaining, + WRITE_LOCK, + &lck); + + ok = SMB_VFS_STRICT_LOCK_CHECK(state->dst_fsp->conn, + state->dst_fsp, + &lck); + if (!ok) { + return NT_STATUS_FILE_LOCK_CONFLICT; + } + + while (state->remaining > 0) { + nwritten = copy_file_range(fsp_get_pathref_fd(state->src_fsp), + &state->src_off, + fsp_get_pathref_fd(state->dst_fsp), + &state->dst_off, + state->remaining, + 0); + if (nwritten == -1) { + DBG_DEBUG("copy_file_range src [%s]:[%jd] dst [%s]:[%jd] " + "n [%jd] failed: %s\n", + fsp_str_dbg(state->src_fsp), + (intmax_t)state->src_off, + fsp_str_dbg(state->dst_fsp), + (intmax_t)state->dst_off, + (intmax_t)state->remaining, + strerror(errno)); + switch (errno) { + case EXDEV: + status = NT_STATUS_MORE_PROCESSING_REQUIRED; + break; + default: + status = map_nt_error_from_unix(errno); + if (NT_STATUS_EQUAL( + status, + NT_STATUS_MORE_PROCESSING_REQUIRED)) + { + /* Avoid triggering the fallback */ + status = NT_STATUS_INTERNAL_ERROR; + } + break; + } + return status; + } + + if (state->remaining < nwritten) { + DBG_DEBUG("copy_file_range src [%s] dst [%s] " + "n [%jd] remaining [%jd]\n", + fsp_str_dbg(state->src_fsp), + fsp_str_dbg(state->dst_fsp), + (intmax_t)nwritten, + (intmax_t)state->remaining); + return NT_STATUS_INTERNAL_ERROR; + } + + if (nwritten == 0) { + break; + } + state->copied += nwritten; + state->remaining -= nwritten; + } + + /* + * Tell the req cleanup function there's no need to call + * change_to_user_and_service_by_fsp() on the dst handle. + */ + state->dst_fsp = NULL; + return NT_STATUS_OK; +} + static void vfswrap_offload_write_read_done(struct tevent_req *subreq); static NTSTATUS vfswrap_offload_write_loop(struct tevent_req *req) -- 2.31.1 From d5849d35798240ced7928cbc655efd50b44cf2bc Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 1 Jul 2021 15:19:56 +0200 Subject: [PATCH 6/6] vfs_default: use fsp_get_io_fd() for copy_file_range() Unintentionally used fsp_get_pathref_fd() in the initial patchset. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12033 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Thu Jul 1 17:45:49 UTC 2021 on sn-devel-184 (cherry picked from commit 0e3ddc27ed6d603a21cb2b187f3295506d560604) --- source3/modules/vfs_default.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index eafe1068d3b..56d4ea28d33 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -2217,9 +2217,9 @@ static NTSTATUS vfswrap_offload_copy_file_range(struct tevent_req *req) } while (state->remaining > 0) { - nwritten = copy_file_range(fsp_get_pathref_fd(state->src_fsp), + nwritten = copy_file_range(fsp_get_io_fd(state->src_fsp), &state->src_off, - fsp_get_pathref_fd(state->dst_fsp), + fsp_get_io_fd(state->dst_fsp), &state->dst_off, state->remaining, 0); -- 2.31.1