The branch, v4-6-test has been updated via c90accf torture: Test compound request request counters via fb602bd s3:smb2_server: correctly maintain request counters for compound requests from e1c58ec s3: smbd: Unix extensions attempts to change wrong field in fchown call.
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-6-test - Log ----------------------------------------------------------------- commit c90accf0275d17fb237ea01e7477d741ed8123bd Author: Volker Lendecke <v...@samba.org> Date: Wed Apr 11 15:11:10 2018 +0200 torture: Test compound request request counters This will send an unfixed smbd into the SMB_ASSERT(op->request_count > 0); in smbd_smb2_request_reply_update_counts BUG: https://bugzilla.samba.org/show_bug.cgi?id=13215 Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Stefan Metzmacher <me...@samba.org> Autobuild-User(master): Volker Lendecke <v...@samba.org> Autobuild-Date(master): Thu Apr 12 14:38:39 CEST 2018 on sn-devel-144 (cherry picked from commit 40edd1bc273f664d5567ef5be169033899acee1f) Autobuild-User(v4-6-test): Stefan Metzmacher <me...@samba.org> Autobuild-Date(v4-6-test): Thu Apr 12 21:56:31 CEST 2018 on sn-devel-144 commit fb602bddc4f968310b958f5aaaafd06eb8857a39 Author: Stefan Metzmacher <me...@samba.org> Date: Wed Apr 11 12:14:59 2018 +0200 s3:smb2_server: correctly maintain request counters for compound requests If a session expires during a compound request chain, we exit smbd_smb2_request_dispatch() with 'return smbd_smb2_request_error(req, ...)' before calling smbd_smb2_request_dispatch_update_counts(). As req->request_counters_updated was only reset within smbd_smb2_request_dispatch_update_counts(), smbd_smb2_request_reply_update_counts() was called twice on the same request, which triggers SMB_ASSERT(op->request_count > 0); BUG: https://bugzilla.samba.org/show_bug.cgi?id=13215 Signed-off-by: Stefan Metzmacher <me...@samba.org> Reviewed-by: Volker Lendecke <v...@samba.org> (cherry picked from commit 87e25cd1e45bfe57292b62ffc44ddafc01c61ca0) ----------------------------------------------------------------------- Summary of changes: source3/smbd/smb2_server.c | 6 +++- source4/torture/smb2/compound.c | 77 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) Changeset truncated at 500 lines: diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 573f5f6..23eb4b6 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -2148,7 +2148,7 @@ static NTSTATUS smbd_smb2_request_dispatch_update_counts( bool update_open = false; NTSTATUS status = NT_STATUS_OK; - req->request_counters_updated = false; + SMB_ASSERT(!req->request_counters_updated); if (xconn->protocol < PROTOCOL_SMB2_22) { return NT_STATUS_OK; @@ -2283,6 +2283,8 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) DO_PROFILE_INC(request); + SMB_ASSERT(!req->request_counters_updated); + /* TODO: verify more things */ flags = IVAL(inhdr, SMB2_HDR_FLAGS); @@ -2722,6 +2724,8 @@ static void smbd_smb2_request_reply_update_counts(struct smbd_smb2_request *req) return; } + req->request_counters_updated = false; + if (xconn->protocol < PROTOCOL_SMB2_22) { return; } diff --git a/source4/torture/smb2/compound.c b/source4/torture/smb2/compound.c index 1856054..da95479 100644 --- a/source4/torture/smb2/compound.c +++ b/source4/torture/smb2/compound.c @@ -1030,6 +1030,81 @@ done: return ret; } +static bool test_compound_invalid4(struct torture_context *tctx, + struct smb2_tree *tree) +{ + struct smb2_create cr; + struct smb2_read rd; + NTSTATUS status; + const char *fname = "compound_invalid4.dat"; + struct smb2_close cl; + bool ret = true; + bool ok; + struct smb2_request *req[2]; + + smb2_transport_credits_ask_num(tree->session->transport, 2); + + smb2_util_unlink(tree, fname); + + ZERO_STRUCT(cr); + cr.in.security_flags = 0x00; + cr.in.oplock_level = 0; + cr.in.impersonation_level = NTCREATEX_IMPERSONATION_IMPERSONATION; + cr.in.create_flags = 0x00000000; + cr.in.reserved = 0x00000000; + cr.in.desired_access = SEC_RIGHTS_FILE_ALL; + cr.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + cr.in.share_access = NTCREATEX_SHARE_ACCESS_READ | + NTCREATEX_SHARE_ACCESS_WRITE | + NTCREATEX_SHARE_ACCESS_DELETE; + cr.in.create_disposition = NTCREATEX_DISP_OPEN_IF; + cr.in.create_options = NTCREATEX_OPTIONS_SEQUENTIAL_ONLY | + NTCREATEX_OPTIONS_ASYNC_ALERT | + NTCREATEX_OPTIONS_NON_DIRECTORY_FILE | + 0x00200000; + cr.in.fname = fname; + + status = smb2_create(tree, tctx, &cr); + CHECK_STATUS(status, NT_STATUS_OK); + + smb2_transport_compound_start(tree->session->transport, 2); + + ZERO_STRUCT(rd); + rd.in.file.handle = cr.out.file.handle; + rd.in.length = 1; + rd.in.offset = 0; + req[0] = smb2_read_send(tree, &rd); + + smb2_transport_compound_set_related(tree->session->transport, true); + + /* + * Send a completely bogus request as second compound + * element. This triggers smbd_smb2_request_error() in in + * smbd_smb2_request_dispatch() before calling + * smbd_smb2_request_dispatch_update_counts(). + */ + + req[1] = smb2_request_init_tree(tree, 0xff, 0x04, false, 0); + smb2_transport_send(req[1]); + + status = smb2_read_recv(req[0], tctx, &rd); + CHECK_STATUS(status, NT_STATUS_END_OF_FILE); + + ok = smb2_request_receive(req[1]); + torture_assert(tctx, ok, "Invalid request failed\n"); + CHECK_STATUS(req[1]->status, NT_STATUS_INVALID_PARAMETER); + + ZERO_STRUCT(cl); + cl.in.file.handle = cr.out.file.handle; + + status = smb2_close(tree, &cl); + CHECK_STATUS(status, NT_STATUS_OK); + + smb2_util_unlink(tree, fname); +done: + return ret; +} + /* Send a compound request where we expect the last request (Create, Notify) * to go asynchronous. This works against a Win7 server and the reply is * sent in two different packets. */ @@ -1186,6 +1261,8 @@ struct torture_suite *torture_smb2_compound_init(void) torture_suite_add_1smb2_test(suite, "invalid1", test_compound_invalid1); torture_suite_add_1smb2_test(suite, "invalid2", test_compound_invalid2); torture_suite_add_1smb2_test(suite, "invalid3", test_compound_invalid3); + torture_suite_add_1smb2_test( + suite, "invalid4", test_compound_invalid4); torture_suite_add_1smb2_test(suite, "interim1", test_compound_interim1); torture_suite_add_1smb2_test(suite, "interim2", test_compound_interim2); torture_suite_add_1smb2_test(suite, "compound-break", test_compound_break); -- Samba Shared Repository