From e29a086830834cb4f21406d7fad083eb488216c2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Jul 2018 12:15:12 -0700 Subject: [PATCH 1/2] s3: torture: Test SMB1 cli_splice() fallback path when doing a non-full file splice. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13527 Signed-off-by: Jeremy Allison Reviewed-by: David Disseldorp (cherry picked from commit 1c8d1cceff852acaca4a0ec0da37b053ed03fe4a) (cherry picked from commit 49d6c3f061284aac31c3ef21f88f9d69bdd86bd8) Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Sat Jul 14 00:14:13 CEST 2018 on sn-devel-144 --- selftest/knownfail | 2 + source3/selftest/tests.py | 5 +- source3/torture/torture.c | 153 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail b/selftest/knownfail index 01b6b3aff5b..85ed2f40abc 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -16,6 +16,8 @@ ^samba3.*rap.sam.*.useradd # Not provided by Samba 3 ^samba3.*rap.sam.*.userdelete # Not provided by Samba 3 ^samba3.libsmbclient.opendir # This requires a workgroup called 'WORKGROUP' and for netbios browse lists to have been registered +^samba3.smbtorture_s3.plain\(fileserver\).CLI_SPLICE +^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).CLI_SPLICE # see bug 8412 ^samba3.smb2.rename.*.simple_nodelete ^samba3.smb2.rename.*.no_share_delete_no_delete_access diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index c89c27cc9db..f7effad517e 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -74,7 +74,7 @@ tests = ["FDPASS", "LOCK1", "LOCK2", "LOCK3", "LOCK4", "LOCK5", "LOCK6", "LOCK7" "DIR", "DIR1", "DIR-CREATETIME", "TCON", "TCONDEV", "RW1", "RW2", "RW3", "LARGE_READX", "RW-SIGNING", "OPEN", "XCOPY", "RENAME", "DELETE", "DELETE-LN", "WILDDELETE", "PROPERTIES", "W2K", "TCON2", "IOCTL", "CHKPATH", "FDSESS", "CHAIN1", "CHAIN2", "OWNER-RIGHTS", - "CHAIN3", "PIDHIGH", + "CHAIN3", "PIDHIGH", "CLI_SPLICE", "GETADDRINFO", "UID-REGRESSION-TEST", "SHORTNAME-TEST", "CASE-INSENSITIVE-CREATE", "SMB2-BASIC", "NTTRANS-FSCTL", "SMB2-NEGPROT", "SMB2-SESSION-REAUTH", "SMB2-SESSION-RECONNECT", "SMB2-FTRUNCATE", @@ -91,6 +91,9 @@ for t in tests: # this is a negative test to verify that the server rejects # access without encryption plantestsuite("samba3.smbtorture_s3.crypt_server(nt4_dc).%s" % t, "nt4_dc", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/tmpenc', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"]) + if t == "CLI_SPLICE": + # We must test this against the SMB1 fallback. + plantestsuite("samba3.smbtorture_s3.plain(fileserver).%s" % t, "fileserver", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/tmp', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH", "-mNT1"]) plantestsuite("samba3.smbtorture_s3.plain(ad_dc_ntvfs).%s" % t, "ad_dc_ntvfs", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/tmp', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"]) # diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 245a72300c4..f0eb059427f 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -43,6 +43,7 @@ #include "../libcli/smb/smbXcli_base.h" #include "lib/util/sys_rw_data.h" #include "lib/util/base64.h" +#include "lib/crypto/md5.h" extern char *optarg; extern int optind; @@ -9166,6 +9167,157 @@ static bool run_cli_echo(int dummy) return NT_STATUS_IS_OK(status); } +static int splice_status(off_t written, void *priv) +{ + return true; +} + +static bool run_cli_splice(int dummy) +{ + uint8_t *buf = NULL; + struct cli_state *cli1 = NULL; + bool correct = false; + const char *fname_src = "\\splice_src.dat"; + const char *fname_dst = "\\splice_dst.dat"; + NTSTATUS status; + uint16_t fnum1 = UINT16_MAX; + uint16_t fnum2 = UINT16_MAX; + size_t file_size = 2*1024*1024; + size_t splice_size = 1*1024*1024 + 713; + MD5_CTX md5_ctx; + uint8_t digest1[16], digest2[16]; + off_t written = 0; + size_t nread = 0; + TALLOC_CTX *frame = talloc_stackframe(); + + printf("starting cli_splice test\n"); + + if (!torture_open_connection(&cli1, 0)) { + goto out; + } + + cli_unlink(cli1, fname_src, + FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); + cli_unlink(cli1, fname_dst, + FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); + + /* Create a file */ + status = cli_ntcreate(cli1, fname_src, 0, GENERIC_ALL_ACCESS, + FILE_ATTRIBUTE_NORMAL, 0, FILE_OVERWRITE_IF, + 0, 0, &fnum1, NULL); + + if (!NT_STATUS_IS_OK(status)) { + d_printf("open %s failed: %s\n", fname_src, nt_errstr(status)); + goto out; + } + + /* Write file_size bytes - must be bigger than splice_size. */ + buf = talloc_zero_array(frame, uint8_t, file_size); + if (buf == NULL) { + d_printf("talloc_fail\n"); + goto out; + } + + /* Fill it with random numbers. */ + generate_random_buffer(buf, file_size); + + /* MD5 the first 1MB + 713 bytes. */ + MD5Init(&md5_ctx); + MD5Update(&md5_ctx, buf, splice_size); + MD5Final(digest1, &md5_ctx); + + status = cli_writeall(cli1, + fnum1, + 0, + buf, + 0, + file_size, + NULL); + if (!NT_STATUS_IS_OK(status)) { + d_printf("cli_writeall failed: %s\n", nt_errstr(status)); + goto out; + } + + status = cli_ntcreate(cli1, fname_dst, 0, GENERIC_ALL_ACCESS, + FILE_ATTRIBUTE_NORMAL, 0, FILE_OVERWRITE_IF, + 0, 0, &fnum2, NULL); + + if (!NT_STATUS_IS_OK(status)) { + d_printf("open %s failed: %s\n", fname_dst, nt_errstr(status)); + goto out; + } + + /* Now splice 1MB + 713 bytes. */ + status = cli_splice(cli1, + cli1, + fnum1, + fnum2, + splice_size, + 0, + 0, + &written, + splice_status, + NULL); + + if (!NT_STATUS_IS_OK(status)) { + d_printf("cli_splice failed: %s\n", nt_errstr(status)); + goto out; + } + + /* Clear the old buffer. */ + memset(buf, '\0', file_size); + + /* Read the new file. */ + status = cli_read(cli1, fnum2, (char *)buf, 0, splice_size, &nread); + if (!NT_STATUS_IS_OK(status)) { + d_printf("cli_read failed: %s\n", nt_errstr(status)); + goto out; + } + if (nread != splice_size) { + d_printf("bad read of 0x%x, should be 0x%x\n", + (unsigned int)nread, + (unsigned int)splice_size); + goto out; + } + + /* MD5 the first 1MB + 713 bytes. */ + MD5Init(&md5_ctx); + MD5Update(&md5_ctx, buf, splice_size); + MD5Final(digest2, &md5_ctx); + + /* Must be the same. */ + if (memcmp(digest1, digest2, 16) != 0) { + d_printf("bad MD5 compare\n"); + goto out; + } + + correct = true; + printf("Success on cli_splice test\n"); + + out: + + if (cli1) { + if (fnum1 != UINT16_MAX) { + cli_close(cli1, fnum1); + } + if (fnum2 != UINT16_MAX) { + cli_close(cli1, fnum2); + } + + cli_unlink(cli1, fname_src, + FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); + cli_unlink(cli1, fname_dst, + FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); + + if (!torture_close_connection(cli1)) { + correct = false; + } + } + + TALLOC_FREE(frame); + return correct; +} + static bool run_uid_regression_test(int dummy) { static struct cli_state *cli; @@ -11632,6 +11784,7 @@ static struct { { "NTTRANS-CREATE", run_nttrans_create, 0}, { "NTTRANS-FSCTL", run_nttrans_fsctl, 0}, { "CLI_ECHO", run_cli_echo, 0}, + { "CLI_SPLICE", run_cli_splice, 0}, { "GETADDRINFO", run_getaddrinfo_send, 0}, { "TLDAP", run_tldap }, { "STREAMERROR", run_streamerror }, -- 2.18.0.203.gfac676dfb9-goog From f5a6233478bc29bbd74f928952ef5c3b6250db32 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Jul 2018 12:18:50 -0700 Subject: [PATCH 2/2] s3: libsmbclient: Fix cli_splice() fallback when reading less than a complete file. We were always asking for SPLICE_BLOCK_SIZE even when the remaining bytes we wanted were smaller than that. This works when using cli_splice() on a complete file, as the cli_read() terminated the read at the right place. We always have the space to read SPLICE_BLOCK_SIZE bytes so this isn't an overflow. Found by Bailey Berro BUG: https://bugzilla.samba.org/show_bug.cgi?id=13527 Signed-off-by: Bailey Berro Reviewed-by: Jeremy Allison Reviewed-by: David Disseldorp Autobuild-User(master): David Disseldorp Autobuild-Date(master): Fri Jul 13 14:57:14 CEST 2018 on sn-devel-144 (cherry picked from commit c9656fd2977557ab20ec4e3d87c385a9b2f1bf43) --- selftest/knownfail | 2 -- source3/libsmb/clireadwrite.c | 4 +++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail b/selftest/knownfail index 85ed2f40abc..01b6b3aff5b 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -16,8 +16,6 @@ ^samba3.*rap.sam.*.useradd # Not provided by Samba 3 ^samba3.*rap.sam.*.userdelete # Not provided by Samba 3 ^samba3.libsmbclient.opendir # This requires a workgroup called 'WORKGROUP' and for netbios browse lists to have been registered -^samba3.smbtorture_s3.plain\(fileserver\).CLI_SPLICE -^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).CLI_SPLICE # see bug 8412 ^samba3.smb2.rename.*.simple_nodelete ^samba3.smb2.rename.*.no_share_delete_no_delete_access diff --git a/source3/libsmb/clireadwrite.c b/source3/libsmb/clireadwrite.c index 00ee09ece89..67870d8c40b 100644 --- a/source3/libsmb/clireadwrite.c +++ b/source3/libsmb/clireadwrite.c @@ -1462,8 +1462,10 @@ static NTSTATUS cli_splice_fallback(TALLOC_CTX *frame, *written = 0; while (remaining) { + size_t to_read = MIN(remaining, SPLICE_BLOCK_SIZE); + status = cli_read(srccli, src_fnum, - (char *)buf, src_offset, SPLICE_BLOCK_SIZE, + (char *)buf, src_offset, to_read, &nread); if (!NT_STATUS_IS_OK(status)) { return status; -- 2.18.0.203.gfac676dfb9-goog