The branch, master has been updated
       via  e37e4d16e9c s3:open.c: Fix a typo
       via  02d4f58a2f7 selftest: Add test for vfs crossrename module
       via  94c9a99c56d docs:manpage: vfs_crossrename is not fully stackable 
VFS module
       via  1a089a16c40 s3:vfs_crossrename: add back checking for errno ENOENT
       via  0a9adc85e77 s3:vfs_crossrename: crossrename_renameat() needs to 
return 0 if copy_reg() is successful
       via  0a5da82f75a s3:vfs_crossrename: avoid locking panic in copy_reg()
      from  7b73c574d93 docs:manpages:  Update 'net ads keytab create'

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit e37e4d16e9c783ce264dbfd58bf6b8568be6f003
Author: Pavel Filipenský <[email protected]>
Date:   Wed Dec 11 22:33:17 2024 +0100

    s3:open.c: Fix a typo
    
    Signed-off-by: Pavel Filipenský <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>
    
    Autobuild-User(master): Pavel Filipensky <[email protected]>
    Autobuild-Date(master): Tue Dec 17 11:23:50 UTC 2024 on atb-devel-224

commit 02d4f58a2f7ac2db60dd2e4d16a3cbf71b3f08a9
Author: Pavel Filipenský <[email protected]>
Date:   Wed Dec 4 11:02:18 2024 +0100

    selftest: Add test for vfs crossrename module
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15724
    
    Signed-off-by: Pavel Filipenský <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>

commit 94c9a99c56db438c391a966c927ec2f862c373e7
Author: Pavel Filipenský <[email protected]>
Date:   Mon Dec 2 22:27:39 2024 +0100

    docs:manpage: vfs_crossrename is not fully stackable VFS module
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15724
    
    Signed-off-by: Pavel Filipenský <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>

commit 1a089a16c40e0b3bc5d4fcde559157cf137056c2
Author: Jones Syue <[email protected]>
Date:   Thu Sep 26 17:17:14 2024 +0800

    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 <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>

commit 0a9adc85e77bc557bb8be12237fa31c4142dd3d5
Author: Pavel Filipenský <[email protected]>
Date:   Thu Nov 28 18:32:25 2024 +0100

    s3:vfs_crossrename: crossrename_renameat() needs to return 0 if copy_reg() 
is successful
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15724
    
    Signed-off-by: Pavel Filipenský <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>

commit 0a5da82f75a43838be3419cab10a50750fa500d7
Author: Pavel Filipenský <[email protected]>
Date:   Thu Nov 28 18:39:53 2024 +0100

    s3:vfs_crossrename: avoid locking panic in copy_reg()
    
    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ý <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>

-----------------------------------------------------------------------

Summary of changes:
 docs-xml/manpages/vfs_crossrename.8.xml |   5 +-
 selftest/target/Samba3.pm               |  12 +++
 source3/modules/vfs_crossrename.c       | 126 +++++++++++++++++++++-----------
 source3/script/tests/test_recycle.sh    |  80 +++++++++++++++++++-
 source3/selftest/tests.py               |   2 +-
 source3/smbd/open.c                     |   2 +-
 6 files changed, 182 insertions(+), 45 deletions(-)


Changeset truncated at 500 lines:

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>
 
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 05027b26f46..d7abda75354 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2783,6 +2783,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);
 
@@ -3718,6 +3721,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/modules/vfs_crossrename.c 
b/source3/modules/vfs_crossrename.c
index 79f13c28da5..0ce4b44898d 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,64 +81,105 @@ 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;
+       ret = SMB_VFS_NEXT_UNLINKAT(handle,
+                                   dstfsp,
+                                   dest,
+                                   0);
+       if (ret == -1 && errno != ENOENT) {
+               status = map_nt_error_from_unix(errno);
                goto out;
        }
-       full_fname_dst = full_path_from_dirfsp_atname(talloc_tos(),
-                                                     dstfsp,
-                                                     dest);
-       if (full_fname_dst == NULL) {
-               status = NT_STATUS_NO_MEMORY;
+
+       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;
        }
 
-       ret = SMB_VFS_NEXT_UNLINKAT(handle,
-                                   dstfsp,
-                                   dest,
-                                   0);
-       if (ret == -1) {
+       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;
 
-  out:
+       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:
+       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;
 }
 
@@ -170,6 +213,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;
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 6b690e2fc1f..4540aef24b5 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'])
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index b87b74988ae..8aac03677a2 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -4749,7 +4749,7 @@ mkdir_first:
                               &rhow);
        if (ret == -1 && errno == EINVAL) {
                /*
-                * This is the strategie we use without having
+                * This is the strategy we use without having
                 * renameat2(RENAME_NOREPLACE):
                 *
                 * renameat() is able to replace a directory if the source is


-- 
Samba Shared Repository

Reply via email to