s3:smb2_create: avoid reusing the 'tevent_req' within smbd_smb2_create_send()
authorStefan Metzmacher <metze@samba.org>
Fri, 9 Jun 2017 10:30:33 +0000 (12:30 +0200)
committerStefan Metzmacher <metze@samba.org>
Tue, 27 Jun 2017 14:57:47 +0000 (16:57 +0200)
As the caller ("smbd_smb2_request_process_create()") already sets the callback,
the first time, it's not safe to reuse the tevent_req structure.

The typical 'tevent_req_nterror(); return tevent_req_post()' will
crash as the tevent_req_nterror() already triggered the former callback,
which calls smbd_smb2_create_recv(), were tevent_req_received() invalidates
the tevent_req structure, so that tevent_req_post() will crash.

We just remember the required values from the old state
and move them to the new state.

We tried to write reproducers for this, but sadly weren't able to trigger
the backtrace we had from a create a customer (using recent code)
with commit 6beba782f1bf951236813e0b46115b8102212c03
included. And this patch fixed the situation for the
customer.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12832

Pair-Programmed-With: Volker Lendecke <vl@samba.org>

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
source3/smbd/smb2_create.c

index 81e0818..346e0a6 100644 (file)
@@ -483,35 +483,38 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx,
                requested_oplock_level = in_oplock_level;
        }
 
-
-       if (smb2req->subreq == NULL) {
-               /* New create call. */
-               req = tevent_req_create(mem_ctx, &state,
+       req = tevent_req_create(mem_ctx, &state,
                                struct smbd_smb2_create_state);
-               if (req == NULL) {
-                       return NULL;
-               }
-               state->smb2req = smb2req;
+       if (req == NULL) {
+               return NULL;
+       }
+       state->smb2req = smb2req;
 
-               smb1req = smbd_smb2_fake_smb_request(smb2req);
-               if (tevent_req_nomem(smb1req, req)) {
-                       return tevent_req_post(req, ev);
-               }
-               state->smb1req = smb1req;
-               smb2req->subreq = req;
+       smb1req = smbd_smb2_fake_smb_request(smb2req);
+       if (tevent_req_nomem(smb1req, req)) {
+               return tevent_req_post(req, ev);
+       }
+       state->smb1req = smb1req;
+
+       if (smb2req->subreq == NULL) {
                DEBUG(10,("smbd_smb2_create: name[%s]\n",
                        in_name));
        } else {
-               /* Re-entrant create call. */
-               req = smb2req->subreq;
-               state = tevent_req_data(req,
-                               struct smbd_smb2_create_state);
-               smb1req = state->smb1req;
-               TALLOC_FREE(state->out_context_blobs);
+               struct smbd_smb2_create_state *old_state = tevent_req_data(
+                       smb2req->subreq, struct smbd_smb2_create_state);
+
                DEBUG(10,("smbd_smb2_create_send: reentrant for file %s\n",
                        in_name ));
+
+               state->id = old_state->id;
+               state->request_time = old_state->request_time;
+               state->open_rec = talloc_move(state, &old_state->open_rec);
+               state->open_was_deferred = old_state->open_was_deferred;
        }
 
+       TALLOC_FREE(smb2req->subreq);
+       smb2req->subreq = req;
+
        state->out_context_blobs = talloc_zero(state, struct smb2_create_blobs);
        if (tevent_req_nomem(state->out_context_blobs, req)) {
                return tevent_req_post(req, ev);