[PATCH] fix bug 10671
Volker Lendecke
Volker.Lendecke at SerNet.DE
Wed Jun 25 02:58:54 MDT 2014
Hi!
Attached find Jeremys patches for 10671 with my r-b together
with two minor patches on top.
Review & push would be appreciated.
Thanks,
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From b86b8fc5630cde6340f31c3309e99527a1799ee2 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 24 Jun 2014 14:19:30 -0700
Subject: [PATCH 1/4] s3: smbd - Prevent file truncation on an open that fails
with share mode violation.
Fix from Volker, really - just tidied up a little.
The S_ISFIFO check may not be strictly neccessary,
but doesn't hurt (might make the code a bit more complex
than it needs to be).
Fixes bug #10671 - Samba file corruption as a result of failed lock check.
https://bugzilla.samba.org/show_bug.cgi?id=10671
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/open.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index b913c9c..687caee 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -840,8 +840,11 @@ static NTSTATUS open_file(files_struct *fsp,
}
}
- /* Actually do the open */
- status = fd_open_atomic(conn, fsp, local_flags,
+ /*
+ * Actually do the open - if O_TRUNC is needed handle it
+ * below under the share mode lock.
+ */
+ status = fd_open_atomic(conn, fsp, local_flags & ~O_TRUNC,
unx_mode, p_file_created);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(3,("Error opening file %s (%s) (local_flags=%d) "
@@ -2676,6 +2679,21 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
return status;
}
+ /* Should we atomically (to the client at least) truncate ? */
+ if (!new_file_created) {
+ if (flags2 & O_TRUNC) {
+ if (!S_ISFIFO(fsp->fsp_name->st.st_ex_mode)) {
+ int ret = vfs_set_filelen(fsp, 0);
+ if (ret != 0) {
+ status = map_nt_error_from_unix(errno);
+ TALLOC_FREE(lck);
+ fd_close(fsp);
+ return status;
+ }
+ }
+ }
+ }
+
grant_fsp_oplock_type(fsp, lck, oplock_request);
/*
--
1.8.1.2
From 1cea87287639be5a0d2e39abe524343544e51756 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 24 Jun 2014 14:22:24 -0700
Subject: [PATCH 2/4] s4: torture: Add regression test case for #10671 - Samba
file corruption as a result of failed lock check.
Adds a new test to raw.open.
Opens a file with SHARE_NONE, writes 1 byte at offset 1023,
attempts a second open with r/w access+truncate disposition,
then checks that open fails with SHARING_VIOLATION, and
the file is not truncated (is still size 1024). Correctly
detects the bug and fixed smbd for me.
https://bugzilla.samba.org/show_bug.cgi?id=10671
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
source4/torture/raw/open.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/source4/torture/raw/open.c b/source4/torture/raw/open.c
index 0ec28c3..494f2a2 100644
--- a/source4/torture/raw/open.c
+++ b/source4/torture/raw/open.c
@@ -2034,6 +2034,78 @@ static bool test_ntcreatexdir(struct torture_context *tctx,
return true;
}
+/*
+ test opening with truncate on an already open file
+ returns share violation and doesn't truncate the file.
+ Regression test for bug #10671.
+*/
+static bool test_open_for_truncate(struct torture_context *tctx, struct smbcli_state *cli)
+{
+ union smb_open io;
+ union smb_fileinfo finfo;
+ const char *fname = BASEDIR "\\torture_open_for_truncate.txt";
+ NTSTATUS status;
+ int fnum = -1;
+ ssize_t val = 0;
+ char c = '\0';
+ bool ret = true;
+
+ torture_assert(tctx, torture_setup_dir(cli, BASEDIR),
+ "Failed to setup up test directory: " BASEDIR);
+
+ torture_comment(tctx, "Testing open truncate disposition.\n");
+
+ /* reasonable default parameters */
+ ZERO_STRUCT(io);
+ io.generic.level = RAW_OPEN_NTCREATEX;
+ io.ntcreatex.in.flags = NTCREATEX_FLAGS_EXTENDED;
+ io.ntcreatex.in.root_fid.fnum = 0;
+ io.ntcreatex.in.alloc_size = 0;
+ io.ntcreatex.in.access_mask = SEC_RIGHTS_FILE_ALL;
+ 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;
+
+ /* Write a byte at offset 1k-1. */
+ val =smbcli_write(cli->tree, fnum, 0, &c, 1023, 1);
+ torture_assert_int_equal(tctx, val, 1, "write failed\n");
+
+ /* Now try and open for read/write with truncate - should fail. */
+ io.ntcreatex.in.access_mask = SEC_RIGHTS_FILE_WRITE|SEC_RIGHTS_FILE_READ;
+ io.ntcreatex.in.file_attr = 0;
+ io.ntcreatex.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
+ NTCREATEX_SHARE_ACCESS_WRITE |
+ NTCREATEX_SHARE_ACCESS_DELETE;
+ io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OVERWRITE;
+ status = smb_raw_open(cli->tree, tctx, &io);
+ CHECK_STATUS(status, NT_STATUS_SHARING_VIOLATION);
+
+ /* Ensure file size is still 1k */
+ finfo.generic.level = RAW_FILEINFO_GETATTRE;
+ finfo.generic.in.file.fnum = fnum;
+ status = smb_raw_fileinfo(cli->tree, tctx, &finfo);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ CHECK_VAL(finfo.getattre.out.size, 1024);
+
+ smbcli_close(cli->tree, fnum);
+ smbcli_unlink(cli->tree, fname);
+
+done:
+ smbcli_close(cli->tree, fnum);
+ smbcli_deltree(cli->tree, BASEDIR);
+
+ return ret;
+}
+
+
/* basic testing of all RAW_OPEN_* calls
*/
struct torture_suite *torture_raw_open(TALLOC_CTX *mem_ctx)
@@ -2057,6 +2129,7 @@ struct torture_suite *torture_raw_open(TALLOC_CTX *mem_ctx)
torture_suite_add_1smb_test(suite, "open-for-delete", test_open_for_delete);
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);
return suite;
}
--
1.8.1.2
From 3274b82dcc4303cb743e8c15c832fb4dbb0f6c14 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Jun 2014 08:36:47 +0000
Subject: [PATCH 3/4] smbd: Remove 2 indentation levels
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/open.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 687caee..a8bd974 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2680,17 +2680,17 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
}
/* Should we atomically (to the client at least) truncate ? */
- if (!new_file_created) {
- if (flags2 & O_TRUNC) {
- if (!S_ISFIFO(fsp->fsp_name->st.st_ex_mode)) {
- int ret = vfs_set_filelen(fsp, 0);
- if (ret != 0) {
- status = map_nt_error_from_unix(errno);
- TALLOC_FREE(lck);
- fd_close(fsp);
- return status;
- }
- }
+ if ((!new_file_created) &&
+ (flags2 & O_TRUNC) &&
+ (!S_ISFIFO(fsp->fsp_name->st.st_ex_mode))) {
+ int ret;
+
+ ret = vfs_set_filelen(fsp, 0);
+ if (ret != 0) {
+ status = map_nt_error_from_unix(errno);
+ TALLOC_FREE(lck);
+ fd_close(fsp);
+ return status;
}
}
--
1.8.1.2
From e163e8af22bdd1d882e68867cbb889805a354a44 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 25 Jun 2014 08:49:45 +0000
Subject: [PATCH 4/4] torture4: Add a little test that truncate actually works
:-)
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/torture/raw/open.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/source4/torture/raw/open.c b/source4/torture/raw/open.c
index 494f2a2..763c718 100644
--- a/source4/torture/raw/open.c
+++ b/source4/torture/raw/open.c
@@ -2096,6 +2096,19 @@ static bool test_open_for_truncate(struct torture_context *tctx, struct smbcli_s
CHECK_VAL(finfo.getattre.out.size, 1024);
smbcli_close(cli->tree, fnum);
+
+ status = smb_raw_open(cli->tree, tctx, &io);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ fnum = io.ntcreatex.out.file.fnum;
+
+ /* Ensure truncate actually works */
+ finfo.generic.level = RAW_FILEINFO_GETATTRE;
+ finfo.generic.in.file.fnum = fnum;
+ status = smb_raw_fileinfo(cli->tree, tctx, &finfo);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ CHECK_VAL(finfo.getattre.out.size, 0);
+
+ smbcli_close(cli->tree, fnum);
smbcli_unlink(cli->tree, fname);
done:
--
1.8.1.2
More information about the samba-technical
mailing list