On Fri, Sep 25, 2015 at 07:12:22PM -0700, slow at samba.org wrote:
> Hi,
>
> just came across this one. Looks like we don't handle ctime properly,
> patch with test attached.
>
> The test passes with Windows but fails with Samba without the fix.
Looks completely correct to me ! Mod times can be tricky though,
so I hope it passes a full make test :-).
Did you check if the ntvfs server passes this test or do you
have to mark it skip ?
If so, mark it 'Reviewed-by: Jeremy Allison <jra at samba.org>
Cheers,
Jeremy.
> From 506464ca114d9fdcd5f2c35f73e51b40a2a18e2d Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Sat, 26 Sep 2015 01:59:05 +0200
> Subject: [PATCH 1/2] smbd: dosmode: fix change time
>
> According to [MS-FSSA] 2.1.1.3 the last changed time for a file is
> defined as
>
> LastChangeTime: The time that identifies when the file metadata or
> contents were last changed ...
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> source3/smbd/dosmode.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c
> index 72acd4e..bf6fbd4 100644
> --- a/source3/smbd/dosmode.c
> +++ b/source3/smbd/dosmode.c
> @@ -1098,7 +1098,12 @@ struct timespec get_change_timespec(connection_struct *conn,
> struct files_struct *fsp,
> const struct smb_filename *smb_fname)
> {
> - return smb_fname->st.st_ex_mtime;
> + if (timespec_compare(&smb_fname->st.st_ex_mtime,
> + &smb_fname->st.st_ex_ctime) > 0)
> + {
> + return smb_fname->st.st_ex_mtime;
> + }
> + return smb_fname->st.st_ex_ctime;
> }
>
> /****************************************************************************
> --
> 2.1.0
>
>
> From 9bd1dcc6193eae70575d926bd1623bf2399c7eb6 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Sat, 26 Sep 2015 03:01:35 +0200
> Subject: [PATCH 2/2] s4:torture: smb2 rename ctime test
>
> According to [MS-FSSA] 2.1.1.3 the last changed time for a file is
> defined as
>
> LastChangeTime: The time that identifies when the file metadata or
> contents were last changed ...
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> source4/torture/smb2/rename.c | 130 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 130 insertions(+)
>
> diff --git a/source4/torture/smb2/rename.c b/source4/torture/smb2/rename.c
> index 23fe4f9..8599802 100755
> --- a/source4/torture/smb2/rename.c
> +++ b/source4/torture/smb2/rename.c
> @@ -20,6 +20,7 @@
> */
>
> #include "includes.h"
> +#include <unistd.h>
> #include "libcli/smb2/smb2.h"
> #include "libcli/smb2/smb2_calls.h"
> #include "lib/tevent/tevent.h"
> @@ -1392,6 +1393,131 @@ static bool torture_smb2_rename_dir_bench(struct torture_context *tctx,
> }
>
>
> +static bool torture_smb2_rename_ctime(struct torture_context *torture,
> + struct smb2_tree *tree1)
> +{
> + bool ret = true;
> + NTSTATUS status;
> + union smb_open io;
> + union smb_close cl;
> + union smb_setfileinfo sinfo;
> + union smb_fileinfo fi;
> + struct smb2_handle h1;
> + NTTIME ctime1, ctime2;
> +
> + smb2_deltree(tree1, BASEDIR);
> + smb2_util_rmdir(tree1, BASEDIR);
> +
> + torture_comment(torture, "Creating base directory\n");
> +
> + smb2_util_mkdir(tree1, BASEDIR);
> +
> + torture_comment(torture, "Creating test file\n");
> +
> + ZERO_STRUCT(io.smb2);
> + io.generic.level = RAW_OPEN_SMB2;
> + io.smb2.in.create_flags = 0;
> + io.smb2.in.desired_access = SEC_FILE_ALL|SEC_STD_DELETE;
> + io.smb2.in.create_options = NTCREATEX_OPTIONS_NON_DIRECTORY_FILE;
> + io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
> + io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
> + NTCREATEX_SHARE_ACCESS_WRITE | NTCREATEX_SHARE_ACCESS_DELETE;
> + io.smb2.in.alloc_size = 0;
> + io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE;
> + io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
> + io.smb2.in.security_flags = 0;
> + io.smb2.in.fname = BASEDIR "\\file.txt";
> +
> + status = smb2_create(tree1, torture, &(io.smb2));
> + torture_assert_ntstatus_ok_goto(torture, status, ret, done,
> + "smb2_create failed\n");
> + h1 = io.smb2.out.file.handle;
> +
> + ZERO_STRUCT(fi);
> + fi.generic.level = RAW_FILEINFO_NETWORK_OPEN_INFORMATION;
> + fi.generic.in.file.handle = h1;
> + status = smb2_getinfo_file(tree1, torture, &fi);
> + torture_assert_ntstatus_ok_goto(torture, status, ret, done,
> + "smb2_getinfo failed\n");
> +
> + ctime1 = fi.network_open_information.out.change_time;
> + torture_comment(torture, "ctime: %jd\n",
> + (uintmax_t)nt_time_to_unix(ctime1));
> +
> + sleep(1);
> +
> + torture_comment(torture, "Renaming test file\n");
> +
> + ZERO_STRUCT(sinfo);
> + sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION;
> + sinfo.rename_information.in.file.handle = io.smb2.out.file.handle;
> + sinfo.rename_information.in.overwrite = 0;
> + sinfo.rename_information.in.root_fid = 0;
> + sinfo.rename_information.in.new_name =
> + BASEDIR "\\newname.txt";
> + status = smb2_setinfo_file(tree1, &sinfo);
> + torture_assert_ntstatus_ok_goto(torture, status, ret, done,
> + "smb2_setinfo failed\n");
> +
> + ZERO_STRUCT(cl.smb2);
> + cl.smb2.level = RAW_CLOSE_SMB2;
> + cl.smb2.in.file.handle = h1;
> + status = smb2_close(tree1, &(cl.smb2));
> + torture_assert_ntstatus_ok_goto(torture, status, ret, done,
> + "smb2_close failed\n");
> +
> + torture_comment(torture, "Checking for new filename\n");
> +
> + sleep(2);
> +
> + ZERO_STRUCT(io.smb2);
> + io.generic.level = RAW_OPEN_SMB2;
> + io.smb2.in.create_flags = 0;
> + io.smb2.in.desired_access = SEC_FILE_ALL|SEC_STD_DELETE;
> + io.smb2.in.create_options = NTCREATEX_OPTIONS_NON_DIRECTORY_FILE;
> + io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
> + io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
> + NTCREATEX_SHARE_ACCESS_WRITE | NTCREATEX_SHARE_ACCESS_DELETE;
> + io.smb2.in.alloc_size = 0;
> + io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN;
> + io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
> + io.smb2.in.security_flags = 0;
> + io.smb2.in.fname = BASEDIR "\\newname.txt";
> +
> + status = smb2_create(tree1, torture, &(io.smb2));
> + torture_assert_ntstatus_ok_goto(torture, status, ret, done,
> + "smb2_create failed\n");
> + h1 = io.smb2.out.file.handle;
> +
> + ZERO_STRUCT(fi);
> + fi.generic.level = RAW_FILEINFO_NETWORK_OPEN_INFORMATION;
> + fi.generic.in.file.handle = h1;
> + status = smb2_getinfo_file(tree1, torture, &fi);
> + torture_assert_ntstatus_ok_goto(torture, status, ret, done,
> + "smb2_getinfo failed\n");
> +
> + ctime2 = fi.network_open_information.out.change_time;
> + torture_comment(torture, "ctime: %jd\n",
> + (uintmax_t)nt_time_to_unix(ctime2));
> + torture_assert_goto(torture, ctime2 > ctime1, ret, done,
> + "rename didn't update ctime");
> +
> + torture_comment(torture, "Closing test file\n");
> +
> + ZERO_STRUCT(cl.smb2);
> + cl.smb2.level = RAW_CLOSE_SMB2;
> + cl.smb2.in.file.handle = h1;
> + status = smb2_close(tree1, &(cl.smb2));
> + torture_assert_ntstatus_ok_goto(torture, status, ret, done,
> + "smb2_close failed\n");
> +
> + ZERO_STRUCT(h1);
> +
> +done:
> + smb2_deltree(tree1, BASEDIR);
> + return ret;
> +}
> +
> /*
> basic testing of SMB2 rename
> */
> @@ -1437,6 +1563,10 @@ struct torture_suite *torture_smb2_rename_init(void)
> "rename_dir_bench",
> torture_smb2_rename_dir_bench);
>
> + torture_suite_add_1smb2_test(suite,
> + "rename_ctime",
> + torture_smb2_rename_ctime);
> +
> suite->description = talloc_strdup(suite, "smb2.rename tests");
>
> return suite;
> --
> 2.1.0
>