From 94716131cc1367a0014e368a27d88fae733e4055 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 24 Jun 2014 14:19:30 -0700 Subject: [PATCH 1/2] 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 --- 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 5f7bff9..72b8b59 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -839,8 +839,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) " @@ -2646,6 +2649,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, oplock_request, got_level2_oplock, -- 2.0.0.526.g5318336 From 3bb9703107b3184164a200456fedc9b381aa95b3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 24 Jun 2014 14:22:24 -0700 Subject: [PATCH 2/2] 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 --- 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 0968cfe..ff77d16 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; } -- 2.0.0.526.g5318336