Incorrect file size returned in the Respond of "FILE_SUPERSEDE Create"

Jeremy Allison jra at samba.org
Tue Apr 28 18:11:28 MDT 2015


On Tue, Apr 28, 2015 at 03:11:00PM -0700, Kenny Dinh wrote:
> The patch looks good to me.
> 
> 
> I have been trying to verify the test case I wrote in source4/torture/raw/
> open.c.  
> 
> I have been following direction from https://wiki.samba.org/index.php/
> Writing_Torture_Tests .
> 
> It wasn't clear if torture test in source3 or source4 should be updated. 
> Attached is my version of of smbtorture, I tried to run my test but I'm not
> hitting the error using the un-patched code.  The file size returned seems to
> be 0 on open supersede.  There are other parameters in the create operation
> that I trying to play around to trigger the error case.

OK, I've gotten the following test to run and expose the error on
an unpatched Samba, and run successfully against Win2K12r2 and a
patched Samba.

I had to make a change:

+       CHECK_VAL(io.ntcreatex.out.create_action, FILE_WAS_SUPERSEDED);

you were checking for NTCREATEX_ACTION_TRUNCATED, which is not
correct in the SUPERSEDED case. Plus I also needed to add
the old s4 ntfs server to the knownfail lists as it doesn't
pass this new test (helps 'make test' work correctly :-).

Let me know if this works for you and I'll get it submitted
to master !

Cheers,

	Jeremy.
-------------- next part --------------
From c3dfc79673c2d9b7bb42d0cb19157b1073e70e73 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 28 Apr 2015 14:22:42 -0700
Subject: [PATCH 1/2] s3: smbd: Incorrect file size returned in the response of
 "FILE_SUPERSEDE Create"

https://bugzilla.samba.org/show_bug.cgi?id=11240

Signed-off-by: Kenny Dinh <kdinh at peaxy.net>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_default.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index dbcd601..7d2a0e5 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -1940,8 +1940,6 @@ static int vfswrap_ftruncate(vfs_handle_struct *handle, files_struct *fsp, off_t
 	   ftruncate extend but ext2 can. */
 
 	result = ftruncate(fsp->fh->fd, len);
-	if (result == 0)
-		goto done;
 
 	/* According to W. R. Stevens advanced UNIX prog. Pure 4.3 BSD cannot
 	   extend a file with ftruncate. Provide alternate implementation
@@ -1955,6 +1953,12 @@ static int vfswrap_ftruncate(vfs_handle_struct *handle, files_struct *fsp, off_t
 	if (!NT_STATUS_IS_OK(status)) {
 		goto done;
 	}
+
+	/* We need to update the files_struct after successful ftruncate */
+	if (result == 0) {
+		goto done;
+	}
+
 	pst = &fsp->fsp_name->st;
 
 #ifdef S_ISFIFO
-- 
2.2.0.rc0.207.ga3a616c


From e2756230bba9e7e7a8d7bc55eb697f8ff21db680 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 28 Apr 2015 16:33:30 -0700
Subject: [PATCH 2/2] s4: torture: Test for incorrect file size returned in the
 response of "FILE_SUPERSEDE Create".

https://bugzilla.samba.org/show_bug.cgi?id=11240

Signed-off-by: Kenny Dinh <kdinh at peaxy.net>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 selftest/knownfail         |   1 +
 source4/torture/raw/open.c | 102 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/selftest/knownfail b/selftest/knownfail
index d4a6923..ab77e0f 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -129,6 +129,7 @@
 # some operations don't work over the CIFS NTVFS backend yet (eg. root_fid)
 ^samba4.ntvfs.cifs.*.base.createx_sharemodes_dir
 ^samba4.raw.lock.*.async # bug 6960
+^samba4.raw.open.ntcreatex_supersede
 ^samba4.smb2.lock.*.multiple-unlock # bug 6959
 ^samba4.raw.sfileinfo.*.end-of-file\(.*\)$ # bug 6962
 ^samba4.raw.oplock.*.batch22 # bug 6963
diff --git a/source4/torture/raw/open.c b/source4/torture/raw/open.c
index d1c09bc..dc76b0b 100644
--- a/source4/torture/raw/open.c
+++ b/source4/torture/raw/open.c
@@ -2118,6 +2118,107 @@ done:
 	return ret;
 }
 
+/**
+ * Test for file size to be 0 after create with FILE_SUPERSEDE
+ */
+static bool test_ntcreatex_supersede(struct torture_context *tctx, struct smbcli_state *cli)
+{
+	union smb_open io;
+	union smb_setfileinfo sfi;
+	union smb_fileinfo finfo;
+	const char *fname = BASEDIR "\\torture_ntcreatex_supersede.txt";
+	NTSTATUS status;
+	int fnum = -1;
+	bool ret = true;
+
+	torture_assert(tctx, torture_setup_dir(cli, BASEDIR), "Failed to setup up test directory: " BASEDIR);
+
+	/* reasonable default parameters */
+	io.generic.level = RAW_OPEN_NTCREATEX;
+	io.ntcreatex.in.flags = 0;
+	io.ntcreatex.in.root_fid.fnum = 0;
+	io.ntcreatex.in.access_mask = SEC_RIGHTS_FILE_ALL;
+	io.ntcreatex.in.alloc_size = 0;
+	io.ntcreatex.in.file_attr = FILE_ATTRIBUTE_NORMAL;
+	io.ntcreatex.in.share_access = NTCREATEX_SHARE_ACCESS_NONE;
+	io.ntcreatex.in.open_disposition = NTCREATEX_DISP_CREATE;
+	io.ntcreatex.in.create_options = 0;
+	io.ntcreatex.in.impersonation = NTCREATEX_IMPERSONATION_ANONYMOUS;
+	io.ntcreatex.in.security_flags = 0;
+	io.ntcreatex.in.fname = fname;
+
+	status = smb_raw_open(cli->tree, tctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	fnum = io.ntcreatex.out.file.fnum;
+
+	CHECK_VAL(io.ntcreatex.out.oplock_level, 0);
+	CHECK_VAL(io.ntcreatex.out.create_action, NTCREATEX_ACTION_CREATED);
+	CHECK_NTTIME(io.ntcreatex.out.create_time, create_time);
+	CHECK_NTTIME(io.ntcreatex.out.access_time, access_time);
+	CHECK_NTTIME(io.ntcreatex.out.write_time, write_time);
+	CHECK_NTTIME(io.ntcreatex.out.change_time, change_time);
+	CHECK_ALL_INFO(io.ntcreatex.out.attrib, attrib);
+	CHECK_ALL_INFO(io.ntcreatex.out.alloc_size, alloc_size);
+	CHECK_ALL_INFO(io.ntcreatex.out.size, size);
+	CHECK_ALL_INFO(io.ntcreatex.out.is_directory, directory);
+	CHECK_VAL(io.ntcreatex.out.file_type, FILE_TYPE_DISK);
+
+	/* extend the file size */
+	ZERO_STRUCT(sfi);
+	sfi.generic.level = RAW_SFILEINFO_END_OF_FILE_INFO;
+	sfi.generic.in.file.fnum = fnum;
+	sfi.end_of_file_info.in.size = 512;
+	status = smb_raw_setfileinfo(cli->tree, &sfi);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/* close the file and re-open with to verify new size */
+	smbcli_close(cli->tree, fnum);
+	io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OPEN;
+	io.ntcreatex.in.access_mask = SEC_RIGHTS_FILE_READ;
+	status = smb_raw_open(cli->tree, tctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	fnum = io.ntcreatex.out.file.fnum;
+
+	CHECK_VAL(io.ntcreatex.out.oplock_level, 0);
+	CHECK_VAL(io.ntcreatex.out.create_action, NTCREATEX_ACTION_EXISTED);
+	CHECK_NTTIME(io.ntcreatex.out.create_time, create_time);
+	CHECK_NTTIME(io.ntcreatex.out.access_time, access_time);
+	CHECK_NTTIME(io.ntcreatex.out.write_time, write_time);
+	CHECK_NTTIME(io.ntcreatex.out.change_time, change_time);
+	CHECK_ALL_INFO(io.ntcreatex.out.attrib, attrib);
+	CHECK_ALL_INFO(io.ntcreatex.out.alloc_size, alloc_size);
+	CHECK_VAL(io.ntcreatex.out.size, 512);
+	CHECK_ALL_INFO(io.ntcreatex.out.is_directory, directory);
+	CHECK_VAL(io.ntcreatex.out.file_type, FILE_TYPE_DISK);
+
+	/* close and re-open the file with SUPERSEDE flag */
+	smbcli_close(cli->tree, fnum);
+	io.ntcreatex.in.open_disposition = NTCREATEX_DISP_SUPERSEDE;
+	io.ntcreatex.in.access_mask = SEC_RIGHTS_FILE_READ;
+	io.ntcreatex.in.file_attr = FILE_ATTRIBUTE_NORMAL;
+	io.ntcreatex.in.share_access = NTCREATEX_SHARE_ACCESS_NONE;
+	io.ntcreatex.in.create_options = 0;
+
+	status = smb_raw_open(cli->tree, tctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	fnum = io.ntcreatex.out.file.fnum;
+
+	/* The file size in the superseded create response should be 0 */
+	CHECK_VAL(io.ntcreatex.out.size, 0);
+	CHECK_VAL(io.ntcreatex.out.oplock_level, 0);
+	CHECK_VAL(io.ntcreatex.out.create_action, FILE_WAS_SUPERSEDED);
+	CHECK_NTTIME(io.ntcreatex.out.create_time, create_time);
+	CHECK_NTTIME(io.ntcreatex.out.access_time, access_time);
+	CHECK_ALL_INFO(io.ntcreatex.out.attrib, attrib);
+	CHECK_ALL_INFO(io.ntcreatex.out.alloc_size, alloc_size);
+	CHECK_ALL_INFO(io.ntcreatex.out.is_directory, directory);
+	CHECK_VAL(io.ntcreatex.out.file_type, FILE_TYPE_DISK);
+done:
+	smbcli_close(cli->tree, fnum);
+	smbcli_deltree(cli->tree, BASEDIR);
+
+	return ret;
+}
 
 /* basic testing of all RAW_OPEN_* calls
 */
@@ -2143,6 +2244,7 @@ struct torture_suite *torture_raw_open(TALLOC_CTX *mem_ctx)
 	torture_suite_add_1smb_test(suite, "opendisp-dir", test_ntcreatex_opendisp_dir);
 	torture_suite_add_1smb_test(suite, "ntcreatedir", test_ntcreatexdir);
 	torture_suite_add_1smb_test(suite, "open-for-truncate", test_open_for_truncate);
+	torture_suite_add_1smb_test(suite, "ntcreatex_supersede", test_ntcreatex_supersede);
 
 	return suite;
 }
-- 
2.2.0.rc0.207.ga3a616c



More information about the samba-technical mailing list