The Samba-Bugzilla – Attachment 11057 Details for
Bug 11277
Compound read responses must be padded to 8 bytes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch and torture test
compound-response.patch (text/plain), 10.25 KB, created by
Ralph Böhme
on 2015-05-17 16:45:01 UTC
(
hide
)
Description:
Patch and torture test
Filename:
MIME Type:
Creator:
Ralph Böhme
Created:
2015-05-17 16:45:01 UTC
Size:
10.25 KB
patch
obsolete
>From 26b16091f99a5ae17ff53b9fcdc7972a43cbabc1 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Wed, 13 May 2015 21:01:08 +0200 >Subject: [PATCH 1/2] s3:smb2: add padding to last command in compound requests > >The last command in a compound request must be padded to an 8 byte >boundary. According to [MS-SMB2] 3.3.4.1.3, "Sending Compounded >Responses", this shouldn't be necessary, but OS X clients crash hard if >we don't and this is also what Windows 2012 server does. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=11277 > >Signed-off-by: Ralph Boehme <slow@samba.org> >--- > source3/smbd/smb2_server.c | 57 ++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 47 insertions(+), 10 deletions(-) > >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index 9e5eff7..371294c 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -2619,7 +2619,9 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req, > uint8_t *outhdr; > struct iovec *outbody_v; > struct iovec *outdyn_v; >- uint32_t next_command_ofs; >+ uint32_t next_command_body_ofs, tmp_offset, next_command_ofs = 0; >+ uint16_t flags; >+ size_t pad_size = 0; > > DEBUG(10,("smbd_smb2_request_done_ex: " > "idx[%d] status[%s] body[%u] dyn[%s:%u] at %s\n", >@@ -2640,7 +2642,8 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req, > outbody_v = SMBD_SMB2_OUT_BODY_IOV(req); > outdyn_v = SMBD_SMB2_OUT_DYN_IOV(req); > >- next_command_ofs = IVAL(outhdr, SMB2_HDR_NEXT_COMMAND); >+ flags = SVAL(outhdr, SMB2_HDR_FLAGS); >+ next_command_body_ofs = IVAL(outhdr, SMB2_HDR_NEXT_COMMAND); > SIVAL(outhdr, SMB2_HDR_STATUS, NT_STATUS_V(status)); > > outbody_v->iov_base = (void *)body.data; >@@ -2654,15 +2657,41 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req, > outdyn_v->iov_len = 0; > } > >- /* see if we need to recalculate the offset to the next response */ >- if (next_command_ofs > 0) { >- next_command_ofs = SMB2_HDR_BODY; >- next_command_ofs += SMBD_SMB2_OUT_BODY_LEN(req); >- next_command_ofs += SMBD_SMB2_OUT_DYN_LEN(req); >+ /* >+ * See if we need to recalculate the offset to the next >+ * response, or add padding for the last request in a compound >+ * chain. >+ */ >+ >+ if ((next_command_body_ofs > 0) || >+ ((next_command_body_ofs == 0) && (flags & SMB2_HDR_FLAG_CHAINED))) { >+ tmp_offset = SMB2_HDR_BODY; >+ tmp_offset += SMBD_SMB2_OUT_BODY_LEN(req); >+ tmp_offset += SMBD_SMB2_OUT_DYN_LEN(req); >+ >+ if (next_command_body_ofs > 0) { >+ /* >+ * We have a next compound, use calculated >+ * offset and add padding. >+ */ >+ next_command_ofs = tmp_offset; >+ pad_size = 8 - (tmp_offset % 8); >+ } else if (flags & SMB2_HDR_FLAG_CHAINED) { >+ /* >+ * Last command (next_command_body_ofs == 0) >+ * in a compound request, add padding to 8 >+ * byte boundary if necessary. According to >+ * [MS-SMB2] 3.3.4.1.3, Sending Compounded >+ * Responses this, this shouldn't be >+ * necessary, but OS X client crash hard if we >+ * don't and this is also what Windows 2012 >+ * server does. >+ */ >+ pad_size = 8 - (tmp_offset % 8); >+ } > } > >- if ((next_command_ofs % 8) != 0) { >- size_t pad_size = 8 - (next_command_ofs % 8); >+ if (pad_size > 0) { > if (SMBD_SMB2_OUT_DYN_LEN(req) == 0) { > /* > * if the dyn buffer is empty >@@ -2706,7 +2735,15 @@ NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req, > outdyn_v->iov_base = (void *)new_dyn; > outdyn_v->iov_len = new_size; > } >- next_command_ofs += pad_size; >+ if (next_command_body_ofs > 0) { >+ /* >+ * The last command in a compound chain will >+ * have a next_command_body_ofs of 0, we don't >+ * want to add the padding in that case, we >+ * want to return 0 as next command offset. >+ */ >+ next_command_ofs += pad_size; >+ } > } > > SIVAL(outhdr, SMB2_HDR_NEXT_COMMAND, next_command_ofs); >-- >2.1.0 > > >From bce1de9f2915d8043727e4082ff033396cec139c Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Thu, 14 May 2015 04:27:54 +0200 >Subject: [PATCH 2/2] s4:torture:smb2:compound: compound read and padding > >Add test to check that compound read responses are padded to an 8 byte >boundary. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=11277 > >Signed-off-by: Ralph Boehme <slow@samba.org> >--- > source4/torture/smb2/compound.c | 170 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 170 insertions(+) > >diff --git a/source4/torture/smb2/compound.c b/source4/torture/smb2/compound.c >index 9b3cacc..c93b8dc 100644 >--- a/source4/torture/smb2/compound.c >+++ b/source4/torture/smb2/compound.c >@@ -34,6 +34,14 @@ > goto done; \ > }} while (0) > >+#define CHECK_VALUE(v, correct) do { \ >+ if ((v) != (correct)) { \ >+ torture_result(tctx, TORTURE_FAIL, \ >+ "(%s) Incorrect value %s=%d - should be %d\n", \ >+ __location__, #v, (int)v, (int)correct); \ >+ ret = false; \ >+ }} while (0) >+ > static struct { > struct smb2_handle handle; > uint8_t level; >@@ -433,6 +441,165 @@ done: > return ret; > } > >+static bool test_compound_read(struct torture_context *tctx, >+ struct smb2_tree *tree) >+{ >+ struct smb2_handle h; >+ struct smb2_create cr; >+ struct smb2_read r; >+ const char *fname = "compound_read.dat"; >+ const char *sname = "compound_read.dat:foo"; >+ struct smb2_request *req[3]; >+ NTSTATUS status; >+ bool ret = false; >+ >+ smb2_util_unlink(tree, fname); >+ >+ /* Write file */ >+ ZERO_STRUCT(cr); >+ cr.in.desired_access = SEC_FILE_WRITE_DATA; >+ cr.in.file_attributes = FILE_ATTRIBUTE_NORMAL; >+ cr.in.create_disposition = NTCREATEX_DISP_CREATE; >+ cr.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; >+ cr.in.fname = fname; >+ cr.in.share_access = NTCREATEX_SHARE_ACCESS_READ| >+ NTCREATEX_SHARE_ACCESS_WRITE| >+ NTCREATEX_SHARE_ACCESS_DELETE; >+ status = smb2_create(tree, tctx, &cr); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ h = cr.out.file.handle; >+ >+ status = smb2_util_write(tree, h, "123", 0, 3); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ smb2_util_close(tree, h); >+ >+ /* Write stream */ >+ ZERO_STRUCT(cr); >+ cr.in.desired_access = SEC_FILE_WRITE_DATA; >+ cr.in.file_attributes = FILE_ATTRIBUTE_NORMAL; >+ cr.in.create_disposition = NTCREATEX_DISP_CREATE; >+ cr.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; >+ cr.in.fname = sname; >+ cr.in.share_access = NTCREATEX_SHARE_ACCESS_READ| >+ NTCREATEX_SHARE_ACCESS_WRITE| >+ NTCREATEX_SHARE_ACCESS_DELETE; >+ status = smb2_create(tree, tctx, &cr); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ h = cr.out.file.handle; >+ >+ status = smb2_util_write(tree, h, "456", 0, 3); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ smb2_util_close(tree, h); >+ >+ /* Check compound read from basefile */ >+ smb2_transport_compound_start(tree->session->transport, 2); >+ >+ ZERO_STRUCT(cr); >+ cr.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; >+ cr.in.desired_access = SEC_FILE_READ_DATA; >+ cr.in.file_attributes = FILE_ATTRIBUTE_NORMAL; >+ cr.in.create_disposition = NTCREATEX_DISP_OPEN; >+ cr.in.fname = fname; >+ cr.in.share_access = NTCREATEX_SHARE_ACCESS_READ| >+ NTCREATEX_SHARE_ACCESS_WRITE| >+ NTCREATEX_SHARE_ACCESS_DELETE; >+ req[0] = smb2_create_send(tree, &cr); >+ >+ smb2_transport_compound_set_related(tree->session->transport, true); >+ >+ ZERO_STRUCT(r); >+ h.data[0] = UINT64_MAX; >+ h.data[1] = UINT64_MAX; >+ r.in.file.handle = h; >+ r.in.length = 3; >+ r.in.offset = 0; >+ r.in.min_count = 1; >+ req[1] = smb2_read_send(tree, &r); >+ >+ status = smb2_create_recv(req[0], tree, &cr); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* >+ * We must do a manual smb2_request_receive() in order to be >+ * able to check the transport layer info, as smb2_read_recv() >+ * will destroy the req. smb2_read_recv() will call >+ * smb2_request_receive() again, but that's ok. >+ */ >+ if (!smb2_request_receive(req[1]) || >+ !smb2_request_is_ok(req[1])) { >+ torture_fail(tctx, "failed to received read request"); >+ } >+ >+ /* >+ * size must be 24: 16 byte read response header plus 3 >+ * requested bytes padded to an 8 byte boundary. >+ */ >+ CHECK_VALUE(req[1]->in.body_size, 24); >+ >+ status = smb2_read_recv(req[1], tree, &r); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ smb2_util_close(tree, cr.out.file.handle); >+ >+ /* Check compound read from stream */ >+ smb2_transport_compound_start(tree->session->transport, 2); >+ >+ ZERO_STRUCT(cr); >+ cr.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; >+ cr.in.desired_access = SEC_FILE_READ_DATA; >+ cr.in.file_attributes = FILE_ATTRIBUTE_NORMAL; >+ cr.in.create_disposition = NTCREATEX_DISP_OPEN; >+ cr.in.fname = sname; >+ cr.in.share_access = NTCREATEX_SHARE_ACCESS_READ| >+ NTCREATEX_SHARE_ACCESS_WRITE| >+ NTCREATEX_SHARE_ACCESS_DELETE; >+ req[0] = smb2_create_send(tree, &cr); >+ >+ smb2_transport_compound_set_related(tree->session->transport, true); >+ >+ ZERO_STRUCT(r); >+ h.data[0] = UINT64_MAX; >+ h.data[1] = UINT64_MAX; >+ r.in.file.handle = h; >+ r.in.length = 3; >+ r.in.offset = 0; >+ r.in.min_count = 1; >+ req[1] = smb2_read_send(tree, &r); >+ >+ status = smb2_create_recv(req[0], tree, &cr); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* >+ * We must do a manual smb2_request_receive() in order to be >+ * able to check the transport layer info, as smb2_read_recv() >+ * will destroy the req. smb2_read_recv() will call >+ * smb2_request_receive() again, but that's ok. >+ */ >+ if (!smb2_request_receive(req[1]) || >+ !smb2_request_is_ok(req[1])) { >+ torture_fail(tctx, "failed to received read request"); >+ } >+ >+ /* >+ * size must be 24: 16 byte read response header plus 3 >+ * requested bytes padded to an 8 byte boundary. >+ */ >+ CHECK_VALUE(req[1]->in.body_size, 24); >+ >+ status = smb2_read_recv(req[1], tree, &r); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ smb2_util_close(tree, cr.out.file.handle); >+ status = smb2_util_unlink(tree, fname); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ ret = true; >+done: >+ return ret; >+} >+ > static bool test_compound_unrelated1(struct torture_context *tctx, > struct smb2_tree *tree) > { >@@ -869,6 +1036,9 @@ struct torture_suite *torture_smb2_compound_init(void) > { > struct torture_suite *suite = torture_suite_create(talloc_autofree_context(), "compound"); > >+ torture_suite_add_1smb2_test(suite, "compound-read-padding", test_compound_read); >+ suite->description = talloc_strdup(suite, "SMB2-COMPOUND tests"); >+ return suite; > torture_suite_add_1smb2_test(suite, "related1", test_compound_related1); > torture_suite_add_1smb2_test(suite, "related2", test_compound_related2); > torture_suite_add_1smb2_test(suite, "related3", >-- >2.1.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 11277
:
11057
|
11058
|
11063
|
11099
|
11103
|
11104