]> git.ipfire.org Git - thirdparty/samba.git/commitdiff
s3: VFS: vfs_aio_pthread: Make aio opens safe against connection teardown.
authorJeremy Allison <jra@samba.org>
Thu, 5 Mar 2020 18:22:00 +0000 (10:22 -0800)
committerJeremy Allison <jra@samba.org>
Sun, 8 Mar 2020 18:07:44 +0000 (18:07 +0000)
Allocate state off fsp->conn, not NULL, and add a destructor
that catches deallocation of conn which happens
on connection shutdown or force close.

Note - We don't allocate off fsp as the passed in
fsp will get freed once we return EINPROGRESS/NT_STATUS_MORE_PROCESSING_REQUIRED.
A new fsp pointer gets allocated on every re-run of the
open code path.

The destructor allows us to NULL out the saved conn struct pointer
when conn is deallocated so we know not to access deallocated memory.
This matches the async teardown code changes for bug #14301
in pread/pwrite/fsync vfs_default.c and vfs_glusterfs.c

state is still correctly deallocated in all code
paths so no memory leaks.

This allows us to safely complete when the openat()
returns and then return the error NT_STATUS_NETWORK_NAME_DELETED
to the client open request.

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

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
source3/modules/vfs_aio_pthread.c

index be84e90cfb253faf75424d1648bbe72b83112b40..1ccf89a6d8c86b0615850cf1c778ee96527e496d 100644 (file)
@@ -95,6 +95,37 @@ static void aio_open_handle_completion(struct tevent_req *subreq)
 
        ret = pthreadpool_tevent_job_recv(subreq);
        TALLOC_FREE(subreq);
+
+       /*
+        * We're no longer in flight. Remove the
+        * destructor used to preserve opd so
+        * a talloc_free actually removes it.
+        */
+       talloc_set_destructor(opd, NULL);
+
+       if (opd->conn == NULL) {
+               /*
+                * We were shutdown closed in flight. No one
+                * wants the result, and state has been reparented
+                * to the NULL context, so just free it so we
+                * don't leak memory.
+                */
+               DBG_NOTICE("aio open request for %s/%s abandoned in flight\n",
+                       opd->dname,
+                       opd->fname);
+               if (opd->ret_fd != -1) {
+                       close(opd->ret_fd);
+                       opd->ret_fd = -1;
+               }
+               /*
+                * Find outstanding event and reschedule so the client
+                * gets an error message return from the open.
+                */
+               schedule_deferred_open_message_smb(opd->xconn, opd->mid);
+               opd_free(opd);
+               return;
+       }
+
        if (ret != 0) {
                bool ok;
 
@@ -286,6 +317,22 @@ static struct aio_open_private_data *create_private_open_data(TALLOC_CTX *ctx,
        return opd;
 }
 
+static int opd_inflight_destructor(struct aio_open_private_data *opd)
+{
+       /*
+        * Setting conn to NULL allows us to
+        * discover the connection was torn
+        * down which kills the fsp that owns
+        * opd.
+        */
+       DBG_NOTICE("aio open request for %s/%s cancelled\n",
+               opd->dname,
+               opd->fname);
+       opd->conn = NULL;
+       /* Don't let opd go away. */
+       return -1;
+}
+
 /*****************************************************************
  Setup an async open.
 *****************************************************************/
@@ -297,7 +344,18 @@ static int open_async(const files_struct *fsp,
        struct aio_open_private_data *opd = NULL;
        struct tevent_req *subreq = NULL;
 
-       opd = create_private_open_data(NULL, fsp, flags, mode);
+       /*
+        * Allocate off fsp->conn, not NULL or fsp. As we're going
+        * async fsp will get talloc_free'd when we return
+        * EINPROGRESS/NT_STATUS_MORE_PROCESSING_REQUIRED. A new fsp
+        * pointer gets allocated on every re-run of the
+        * open code path. Allocating on fsp->conn instead
+        * of NULL allows use to get notified via destructor
+        * if the conn is force-closed or we shutdown.
+        * opd is always safely freed in all codepath so no
+        * memory leaks.
+        */
+       opd = create_private_open_data(fsp->conn, fsp, flags, mode);
        if (opd == NULL) {
                DEBUG(10, ("open_async: Could not create private data.\n"));
                return -1;
@@ -318,6 +376,12 @@ static int open_async(const files_struct *fsp,
                opd->dname,
                opd->fname));
 
+       /*
+        * Add a destructor to protect us from connection
+        * teardown whilst the open thread is in flight.
+        */
+       talloc_set_destructor(opd, opd_inflight_destructor);
+
        /* Cause the calling code to reschedule us. */
        errno = EINPROGRESS; /* Maps to NT_STATUS_MORE_PROCESSING_REQUIRED. */
        return -1;