From: Stefan Metzmacher Date: Wed, 24 Feb 2021 16:44:12 +0000 (+0100) Subject: smb2_server: don't cancel pending request if at least one channel is still alive X-Git-Tag: tevent-0.11.0~1367 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f0e553783434dccf0637e6e9e3a87890ae56286c;p=thirdparty%2Fsamba.git smb2_server: don't cancel pending request if at least one channel is still alive In order to allow replays of requests on a channel failure, we should not cancel pending requests, the strategie that seems to make windows clients happy is to let the requests running and return NT_STATUS_FILE_NOT_AVAILABLE as long as the original request is still pending. Here we introduce xconn->transport.shutdown_wait_queue, this is used to keep the xconn alive for the lifetime of pending requests. Now we only cancel pending requests if the disconnected connection is the last channel for a session. In that case smbXsrv_session_remove_channel() and smb2srv_session_shutdown_send() will take care of it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14449 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- diff --git a/selftest/knownfail.d/smb2.replay b/selftest/knownfail.d/smb2.replay index 285081481c6..4cac8071ed8 100644 --- a/selftest/knownfail.d/smb2.replay +++ b/selftest/knownfail.d/smb2.replay @@ -1,12 +1,3 @@ -# These are temporary in order to demonstrate the current bugs -^samba3.smb2.replay.dhv2-pending2n-vs-oplock-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending2n-vs-lease-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending2l-vs-oplock-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending2l-vs-lease-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending2o-vs-oplock-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending2o-vs-lease-sane.nt4_dc -^samba3.smb2.replay.dhv2-pending2n-vs-oplock-sane.ad_dc -^samba3.smb2.replay.dhv2-pending2o-vs-oplock-sane.ad_dc # These tests demonstrate the broken Windows behavior # and check for ACCESS_DENIED instead of FILE_NOT_AVAILABLE # See https://bugzilla.samba.org/show_bug.cgi?id=14449 diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index e023670f869..f49096cba8a 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -368,6 +368,7 @@ struct smbXsrv_connection { struct { NTSTATUS status; bool terminating; + struct tevent_queue *shutdown_wait_queue; int sock; struct tevent_fd *fde; diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 930a4201359..d5b0df33dde 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -1498,7 +1498,6 @@ size_t smbXsrv_client_valid_connections(struct smbXsrv_client *client) } struct smbXsrv_connection_shutdown_state { - struct tevent_queue *wait_queue; struct smbXsrv_connection *xconn; }; @@ -1521,6 +1520,7 @@ static struct tevent_req *smbXsrv_connection_shutdown_send(TALLOC_CTX *mem_ctx, */ SMB_ASSERT(!NT_STATUS_IS_OK(xconn->transport.status)); SMB_ASSERT(xconn->transport.terminating); + SMB_ASSERT(xconn->transport.shutdown_wait_queue == NULL); req = tevent_req_create(mem_ctx, &state, struct smbXsrv_connection_shutdown_state); @@ -1531,45 +1531,48 @@ static struct tevent_req *smbXsrv_connection_shutdown_send(TALLOC_CTX *mem_ctx, state->xconn = xconn; tevent_req_defer_callback(req, ev); - status = smbXsrv_session_disconnect_xconn(xconn); - if (tevent_req_nterror(req, status)) { - return tevent_req_post(req, ev); - } - - state->wait_queue = tevent_queue_create(state, "smbXsrv_connection_shutdown_queue"); - if (tevent_req_nomem(state->wait_queue, req)) { + xconn->transport.shutdown_wait_queue = + tevent_queue_create(state, "smbXsrv_connection_shutdown_queue"); + if (tevent_req_nomem(xconn->transport.shutdown_wait_queue, req)) { return tevent_req_post(req, ev); } for (preq = xconn->smb2.requests; preq != NULL; preq = preq->next) { - /* - * The connection is gone so we - * don't need to take care of - * any crypto - */ - preq->session = NULL; - preq->do_signing = false; - preq->do_encryption = false; - preq->preauth = NULL; - - if (preq->subreq != NULL) { - tevent_req_cancel(preq->subreq); - } - /* * Now wait until the request is finished. * * We don't set a callback, as we just want to block the * wait queue and the talloc_free() of the request will * remove the item from the wait queue. + * + * Note that we don't cancel the requests here + * in order to keep the replay detection logic correct. + * + * However if we teardown the last channel of + * a connection, we'll call some logic via + * smbXsrv_session_disconnect_xconn() + * -> smbXsrv_session_disconnect_xconn_callback() + * -> smbXsrv_session_remove_channel() + * -> smb2srv_session_shutdown_send() + * will indeed cancel the request. */ - subreq = tevent_queue_wait_send(preq, ev, state->wait_queue); + subreq = tevent_queue_wait_send(preq, ev, + xconn->transport.shutdown_wait_queue); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } } - len = tevent_queue_length(state->wait_queue); + /* + * This may attach sessions with num_channels == 0 + * to xconn->transport.shutdown_wait_queue. + */ + status = smbXsrv_session_disconnect_xconn(xconn); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); + } + + len = tevent_queue_length(xconn->transport.shutdown_wait_queue); if (len == 0) { tevent_req_done(req); return tevent_req_post(req, ev); @@ -1580,7 +1583,7 @@ static struct tevent_req *smbXsrv_connection_shutdown_send(TALLOC_CTX *mem_ctx, * this way we get notified when all pending requests are finished * and send to the socket. */ - subreq = tevent_queue_wait_send(state, ev, state->wait_queue); + subreq = tevent_queue_wait_send(state, ev, xconn->transport.shutdown_wait_queue); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c index 45750565529..f41a817cf27 100644 --- a/source3/smbd/smbXsrv_session.c +++ b/source3/smbd/smbXsrv_session.c @@ -1596,8 +1596,32 @@ NTSTATUS smbXsrv_session_remove_channel(struct smbXsrv_session *session, global->num_channels--; if (global->num_channels == 0) { struct smbXsrv_client *client = session->client; + struct tevent_queue *xconn_wait_queue = + xconn->transport.shutdown_wait_queue; struct tevent_req *subreq = NULL; + /* + * Let the connection wait until the session is + * destroyed. + * + * We don't set a callback, as we just want to block the + * wait queue and the talloc_free() of the session will + * remove the item from the wait queue in order + * to remove allow the connection to disapear. + */ + if (xconn_wait_queue != NULL) { + subreq = tevent_queue_wait_send(session, + client->raw_ev_ctx, + xconn_wait_queue); + if (subreq == NULL) { + status = NT_STATUS_NO_MEMORY; + DBG_ERR("tevent_queue_wait_send() session(%llu) failed: %s\n", + (unsigned long long)session->global->session_wire_id, + nt_errstr(status)); + return status; + } + } + /* * This is garanteed to set * session->status = NT_STATUS_USER_SESSION_DELETED