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)
committerKarolin Seeger <kseeger@samba.org>
Fri, 30 Jun 2017 12:57:14 +0000 (14: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>
(cherry picked from commit 02146ea5ee729de0e49ecf617e6983f4e61fbe59)

Autobuild-User(v4-5-test): Karolin Seeger <kseeger@samba.org>
Autobuild-Date(v4-5-test): Fri Jun 30 14:57:14 CEST 2017 on sn-devel-144

source3/smbd/smb2_create.c

index 821199108166a355ad68ec1cb7d0e5628e6b3166..0158924f45a1c1187db7f00de8d5a63b4fcda4e1 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);