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