The Samba-Bugzilla – Attachment 18510 Details for
Bug 15724
vfs crossrename seems not work correctly
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
4.21 patch
crossrename-4.21.patch (text/plain), 14.95 KB, created by
Pavel Filipenský
on 2024-12-17 16:14:30 UTC
(
hide
)
Description:
4.21 patch
Filename:
MIME Type:
Creator:
Pavel Filipenský
Created:
2024-12-17 16:14:30 UTC
Size:
14.95 KB
patch
obsolete
>From a0bfa89790897a0f7cc4de81835e04672ce02f3c Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org> >Date: Thu, 28 Nov 2024 18:39:53 +0100 >Subject: [PATCH 1/5] s3:vfs_crossrename: avoid locking panic in copy_reg() >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >Use low level backend functions that don't go through the FSA layer. >Done via calling transfer_file() as it was in version before 5c18f07 > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15724 > >Signed-off-by: Pavel Filipenský <pfilipensky@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 0a5da82f75a43838be3419cab10a50750fa500d7) >--- > source3/modules/vfs_crossrename.c | 125 ++++++++++++++++++++---------- > 1 file changed, 84 insertions(+), 41 deletions(-) > >diff --git a/source3/modules/vfs_crossrename.c b/source3/modules/vfs_crossrename.c >index 042144bfc4d..7c0ae94d257 100644 >--- a/source3/modules/vfs_crossrename.c >+++ b/source3/modules/vfs_crossrename.c >@@ -54,10 +54,12 @@ static NTSTATUS copy_reg(vfs_handle_struct *handle, > struct files_struct *dstfsp, > const struct smb_filename *dest) > { >- NTSTATUS status; >- struct smb_filename *full_fname_src = NULL; >- struct smb_filename *full_fname_dst = NULL; >+ NTSTATUS status = NT_STATUS_OK; > int ret; >+ off_t off; >+ int ifd = -1; >+ int ofd = -1; >+ struct timespec ts[2]; > > if (!VALID_STAT(source->st)) { > status = NT_STATUS_OBJECT_PATH_NOT_FOUND; >@@ -79,21 +81,6 @@ static NTSTATUS copy_reg(vfs_handle_struct *handle, > goto out; > } > >- full_fname_src = full_path_from_dirfsp_atname(talloc_tos(), >- srcfsp, >- source); >- if (full_fname_src == NULL) { >- status = NT_STATUS_NO_MEMORY; >- goto out; >- } >- full_fname_dst = full_path_from_dirfsp_atname(talloc_tos(), >- dstfsp, >- dest); >- if (full_fname_dst == NULL) { >- status = NT_STATUS_NO_MEMORY; >- goto out; >- } >- > ret = SMB_VFS_NEXT_UNLINKAT(handle, > dstfsp, > dest, >@@ -103,40 +90,96 @@ static NTSTATUS copy_reg(vfs_handle_struct *handle, > goto out; > } > >+ ifd = openat(fsp_get_pathref_fd(srcfsp), >+ source->base_name, >+ O_RDONLY, >+ 0); >+ if (ifd < 0) { >+ status = map_nt_error_from_unix(errno); >+ goto out; >+ } >+ >+ ofd = openat(fsp_get_pathref_fd(dstfsp), >+ dest->base_name, >+ O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW, >+ 0600); >+ if (ofd < 0) { >+ status = map_nt_error_from_unix(errno); >+ goto out; >+ } >+ >+ off = transfer_file(ifd, ofd, source->st.st_ex_size); >+ if (off == -1) { >+ status = map_nt_error_from_unix(errno); >+ goto out; >+ } >+ >+ ret = fchown(ofd, source->st.st_ex_uid, source->st.st_ex_gid); >+ if (ret == -1 && errno != EPERM) { >+ status = map_nt_error_from_unix(errno); >+ goto out; >+ } >+ > /* >- * copy_internals() takes attribute values from the NTrename call. >- * >- * From MS-CIFS: >- * >- * "If the attribute is 0x0000, then only normal files are renamed. >- * If the system file or hidden attributes are specified, then the >- * rename is inclusive of both special types." >+ * fchown turns off set[ug]id bits for non-root, >+ * so do the chmod last. > */ >- status = copy_internals(talloc_tos(), >- handle->conn, >- NULL, >- srcfsp, /* src_dirfsp */ >- full_fname_src, >- dstfsp, /* dst_dirfsp */ >- full_fname_dst, >- FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM); >- if (!NT_STATUS_IS_OK(status)) { >+ ret = fchmod(ofd, source->st.st_ex_mode & 07777); >+ if (ret == -1 && errno != EPERM) { >+ status = map_nt_error_from_unix(errno); > goto out; > } > >- ret = SMB_VFS_NEXT_UNLINKAT(handle, >- srcfsp, >- source, >- 0); >+ /* Try to copy the old file's modtime and access time. */ >+ ts[0] = source->st.st_ex_atime; >+ ts[1] = source->st.st_ex_mtime; >+ ret = futimens(ofd, ts); >+ if (ret == -1) { >+ DBG_DEBUG("Updating the time stamp on destinaton '%s' failed " >+ "with '%s'. Rename operation can continue.\n", >+ dest->base_name, >+ strerror(errno)); >+ } >+ >+ ret = close(ifd); >+ if (ret == -1) { >+ status = map_nt_error_from_unix(errno); >+ goto out; >+ } >+ ifd = -1; >+ >+ ret = close(ofd); > if (ret == -1) { > status = map_nt_error_from_unix(errno); > goto out; > } >+ ofd = -1; >+ >+ ret = SMB_VFS_NEXT_UNLINKAT(handle, srcfsp, source, 0); >+ if (ret == -1) { >+ status = map_nt_error_from_unix(errno); >+ } > >- out: >+out: >+ if (ifd != -1) { >+ ret = close(ifd); >+ if (ret == -1) { >+ DBG_DEBUG("Failed to close %s (%d): %s.\n", >+ source->base_name, >+ ifd, >+ strerror(errno)); >+ } >+ } >+ if (ofd != -1) { >+ ret = close(ofd); >+ if (ret == -1) { >+ DBG_DEBUG("Failed to close %s (%d): %s.\n", >+ dest->base_name, >+ ofd, >+ strerror(errno)); >+ } >+ } > >- TALLOC_FREE(full_fname_src); >- TALLOC_FREE(full_fname_dst); > return status; > } > >-- >2.47.0 > > >From cdfc4ba0c369af8f87a0fd5e08da4dddff0385c4 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org> >Date: Thu, 28 Nov 2024 18:32:25 +0100 >Subject: [PATCH 2/5] s3:vfs_crossrename: crossrename_renameat() needs to > return 0 if copy_reg() is successful >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15724 > >Signed-off-by: Pavel Filipenský <pfilipensky@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 0a9adc85e77bc557bb8be12237fa31c4142dd3d5) >--- > source3/modules/vfs_crossrename.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/source3/modules/vfs_crossrename.c b/source3/modules/vfs_crossrename.c >index 7c0ae94d257..a4f035d3141 100644 >--- a/source3/modules/vfs_crossrename.c >+++ b/source3/modules/vfs_crossrename.c >@@ -211,6 +211,7 @@ static int crossrename_renameat(vfs_handle_struct *handle, > smb_fname_src, > dstfsp, > smb_fname_dst); >+ result = 0; > if (!NT_STATUS_IS_OK(status)) { > errno = map_errno_from_nt_status(status); > result = -1; >-- >2.47.0 > > >From 8fadcd0c78fadd066e7a5da52c8a537fa58c49d7 Mon Sep 17 00:00:00 2001 >From: Jones Syue <jonessyue@qnap.com> >Date: Thu, 26 Sep 2024 17:17:14 +0800 >Subject: [PATCH 3/5] s3:vfs_crossrename: add back checking for errno ENOENT > >strace gives a clue: samba try to remove 'file.txt' in the dst folder but >actually it is not existed yet, and got an errno = ENOENT, > >renameat(32, "file.txt", 31, "file.txt") = -1 EXDEV (Invalid cross-device link) >unlinkat(31, "file.txt", 0) = -1 ENOENT (No such file or directory) > >Commit 5c18f074be92 ("s3: VFS: crossrename. Use real dirfsp for >SMB_VFS_RENAMEAT()") seems unintentionally removed errno ENOENT checking, >so add it back could address 1st issue. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15724 > >Signed-off-by: Jones Syue <jonessyue@qnap.com> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 1a089a16c40e0b3bc5d4fcde559157cf137056c2) >--- > source3/modules/vfs_crossrename.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/source3/modules/vfs_crossrename.c b/source3/modules/vfs_crossrename.c >index a4f035d3141..1da36706ecb 100644 >--- a/source3/modules/vfs_crossrename.c >+++ b/source3/modules/vfs_crossrename.c >@@ -85,7 +85,7 @@ static NTSTATUS copy_reg(vfs_handle_struct *handle, > dstfsp, > dest, > 0); >- if (ret == -1) { >+ if (ret == -1 && errno != ENOENT) { > status = map_nt_error_from_unix(errno); > goto out; > } >-- >2.47.0 > > >From 0401b36db54dd105d389830e74e54a415c4d0b4c Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org> >Date: Mon, 2 Dec 2024 22:27:39 +0100 >Subject: [PATCH 4/5] docs:manpage: vfs_crossrename is not fully stackable VFS > module >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15724 > >Signed-off-by: Pavel Filipenský <pfilipensky@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 94c9a99c56db438c391a966c927ec2f862c373e7) >--- > docs-xml/manpages/vfs_crossrename.8.xml | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > >diff --git a/docs-xml/manpages/vfs_crossrename.8.xml b/docs-xml/manpages/vfs_crossrename.8.xml >index 72d67d685b1..7ea0b50cc9b 100644 >--- a/docs-xml/manpages/vfs_crossrename.8.xml >+++ b/docs-xml/manpages/vfs_crossrename.8.xml >@@ -62,7 +62,10 @@ > </varlistentry> > </variablelist> > >- <para>This module is stackable.</para> >+ <para> This module is not fully stackable. It can be combined with other >+ modules, but should be the last module in the <command>vfs objects</command> >+ list. It directly access the files in the OS filesystem. >+ </para> > > </refsect1> > >-- >2.47.0 > > >From f6a239b5a836770344dab714eb03d92875dc076f Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Pavel=20Filipensk=C3=BD?= <pfilipensky@samba.org> >Date: Wed, 4 Dec 2024 11:02:18 +0100 >Subject: [PATCH 5/5] selftest: Add test for vfs crossrename module >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15724 > >Signed-off-by: Pavel Filipenský <pfilipensky@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 02d4f58a2f7ac2db60dd2e4d16a3cbf71b3f08a9) >--- > selftest/target/Samba3.pm | 12 +++++ > source3/script/tests/test_recycle.sh | 80 +++++++++++++++++++++++++++- > source3/selftest/tests.py | 2 +- > 3 files changed, 92 insertions(+), 2 deletions(-) > >diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm >index a7dd1b20e66..17343e63e52 100755 >--- a/selftest/target/Samba3.pm >+++ b/selftest/target/Samba3.pm >@@ -2780,6 +2780,9 @@ sub provision($$) > my $recycle_shrdir="$shrdir/recycle"; > push(@dirs,$recycle_shrdir); > >+ my $recycle_shrdir2="$shrdir/recycle2"; >+ push(@dirs,$recycle_shrdir2); >+ > my $fakedircreatetimes_shrdir="$shrdir/fakedircreatetimes"; > push(@dirs,$fakedircreatetimes_shrdir); > >@@ -3715,6 +3718,15 @@ sub provision($$) > recycle : exclude = *.tmp > recycle : directory_mode = 755 > >+[recycle2] >+ copy = tmp >+ path = $recycle_shrdir2 >+ vfs objects = recycle crossrename >+ recycle : repository = .trash >+ recycle : exclude = *.tmp >+ recycle : directory_mode = 755 >+ wide links = yes >+ > [fakedircreatetimes] > copy = tmp > path = $fakedircreatetimes_shrdir >diff --git a/source3/script/tests/test_recycle.sh b/source3/script/tests/test_recycle.sh >index ba1d0a598b1..779683f4822 100755 >--- a/source3/script/tests/test_recycle.sh >+++ b/source3/script/tests/test_recycle.sh >@@ -29,7 +29,8 @@ export SAMBA_DEPRECATED_SUPPRESS > > # Define the test environment/filenames. > # >-share_test_dir="$LOCAL_PATH" >+share_test_dir="$LOCAL_PATH/recycle" >+share_test_dir2="$LOCAL_PATH/recycle2" > > # > # Cleanup function. >@@ -43,6 +44,13 @@ do_cleanup() > rm -f testfile2.tmp > rm -rf .trash > ) >+ ( >+ #subshell. >+ cd "$share_test_dir2" || return >+ rm -f testfile3 >+ rm -f testfile4.tmp >+ rm -rf .trash >+ ) > } > > # >@@ -50,6 +58,25 @@ do_cleanup() > # > do_cleanup > >+# Setup .trash on a different filesystem to test crossrename >+# /tmp or /dev/shm should provide tmpfs >+# >+for T in /tmp /dev/shm >+do >+ if df --portability --print-type $T 2>/dev/null | grep -q tmpfs; then >+ TRASHDIR=$T >+ break >+ fi >+done >+ >+if [ -z $TRASHDIR ]; then >+ echo "No tmpfs filesystem found." >+ exit 1 >+fi >+ >+TRASHDIR=$(mktemp -d /$TRASHDIR/.trash_XXXXXX) >+chmod 0755 $TRASHDIR >+ln -s $TRASHDIR $share_test_dir2/.trash > > test_recycle() > { >@@ -90,12 +117,61 @@ quit > return 0 > } > >+test_recycle_crossrename() >+{ >+ tmpfile=$PREFIX/smbclient_interactive_prompt_commands >+ echo " >+put $tmpfile testfile3 >+put $tmpfile testfile4.tmp >+del testfile3 >+del testfile4.tmp >+quit >+" > $tmpfile >+ cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT -U$USERNAME%$PASSWORD //$SERVER/recycle2 -I$SERVER_IP $ADDARGS < $tmpfile 2>&1' >+ eval echo "$cmd" >+ out=$(eval "$cmd") >+ ret=$? >+ rm -f "$tmpfile" >+ >+ if [ $ret != 0 ]; then >+ printf "%s\n" "$out" >+ printf "failed recycle smbclient run with error %s\n" "$ret" >+ return 1 >+ fi >+ >+ test -e "$share_test_dir2/.trash/testfile3" || { >+ printf ".trash/testfile3 expected to exist but does NOT exist\n" >+ return 1 >+ } >+ test -e "$share_test_dir2/.trash/testfile4.tmp" && { >+ printf ".trash/testfile4.tmp not expected to exist but DOES exist\n" >+ return 1 >+ } >+ deviceid1=`stat -c '%d' "$share_test_dir2/"` >+ deviceid2=`stat -c '%d' "$share_test_dir2/.trash/"` >+ test "$deviceid1=" != "$deviceid2" || { >+ printf ".trash/ should be on a different filesystem!\n" >+ return 1 >+ } >+ perm_want=755 >+ perm_is=`stat -c '%a' "$share_test_dir2/.trash/"` >+ test "$perm_is" = "$perm_want" || { >+ printf ".trash/ permission should be $perm_want but is $perm_is\n" >+ return 1 >+ } >+ return 0 >+} >+ > panic_count_0=$(grep -c PANIC $SMBD_TEST_LOG) > > testit "recycle" \ > test_recycle || > failed=$((failed + 1)) > >+testit "recycle_crossrename" \ >+ test_recycle_crossrename || >+ failed=$((failed + 1)) >+ > panic_count_1=$(grep -c PANIC $SMBD_TEST_LOG) > > testit "check_panic" test $panic_count_0 -eq $panic_count_1 || failed=$(expr $failed + 1) >@@ -103,5 +179,7 @@ testit "check_panic" test $panic_count_0 -eq $panic_count_1 || failed=$(expr $fa > # > # Cleanup. > do_cleanup >+# Cleanup above only deletes a symlink, delete also /tmp/.trash_XXXXXX dir >+rm -rf "$TRASHDIR" > > testok "$0" "$failed" >diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py >index 88151caea11..fe67a4df896 100755 >--- a/source3/selftest/tests.py >+++ b/source3/selftest/tests.py >@@ -784,7 +784,7 @@ for env in ["fileserver"]: > plantestsuite("samba3.blackbox.force_create_mode", env, [os.path.join(samba3srcdir, "script/tests/test_force_create_mode.sh"), '$SERVER', '$DOMAIN', '$USERNAME', '$PASSWORD', '$PREFIX', env, smbclient3]) > plantestsuite("samba3.blackbox.dropbox", env, [os.path.join(samba3srcdir, "script/tests/test_dropbox.sh"), '$SERVER', '$DOMAIN', 'gooduser', '$PASSWORD', '$PREFIX', env, smbclient3]) > plantestsuite("samba3.blackbox.offline", env, [os.path.join(samba3srcdir, "script/tests/test_offline.sh"), '$SERVER', '$SERVER_IP', '$DOMAIN', '$USERNAME', '$PASSWORD', '$LOCAL_PATH/offline', smbclient3]) >- plantestsuite("samba3.blackbox.recycle", env, [os.path.join(samba3srcdir, "script/tests/test_recycle.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$LOCAL_PATH/recycle', '$PREFIX', smbclient3]) >+ plantestsuite("samba3.blackbox.recycle", env, [os.path.join(samba3srcdir, "script/tests/test_recycle.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$LOCAL_PATH', '$PREFIX', smbclient3]) > plantestsuite("samba3.blackbox.fakedircreatetimes", env, [os.path.join(samba3srcdir, "script/tests/test_fakedircreatetimes.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$LOCAL_PATH/fakedircreatetimes', '$PREFIX', smbclient3]) > plantestsuite("samba3.blackbox.shadow_copy2.NT1", env + "_smb1_done", [os.path.join(samba3srcdir, "script/tests/test_shadow_copy.sh"), '$SERVER', '$SERVER_IP', '$DOMAIN', '$USERNAME', '$PASSWORD', '$LOCAL_PATH/shadow', smbclient3, '-m', 'NT1']) > plantestsuite("samba3.blackbox.shadow_copy2.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_shadow_copy.sh"), '$SERVER', '$SERVER_IP', '$DOMAIN', '$USERNAME', '$PASSWORD', '$LOCAL_PATH/shadow', smbclient3, '-m', 'SMB3']) >-- >2.47.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
slow
:
review+
Actions:
View
Attachments on
bug 15724
: 18510 |
18513