s3:smbd: make sure a SHUTDOWN_CLOSE applies to a stream fsp before its base fsp

Before we had open_pathref_fsp() we had the stream fsp before the base
fsp in the linked list we traverse for SHUTDOWN_CLOSE.

Now the order has changed. I could have used some DLIST_PROMOTE()
hacks, but that's still fragile.

Now we reference both fsp's via ->base_fsp and ->stream_fsp.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 04c8c3e4c761efd918ffc8460ad247f702f6173c..49726618c5ca14c6dde5aa765e0af5d129010876 100644 (file)
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -446,6 +446,7 @@ typedef struct files_struct {
        struct notify_change_buf *notify;
 
        struct files_struct *base_fsp; /* placeholder for delete on close */
+       struct files_struct *stream_fsp; /* backlink from base_fsp */
 
        /*
         * Cache of share_mode_data->flags

diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index 20f2ed8a17271b4be5ab08fbac4f73e268992ba1..83b75677d9296d56695f19d3363ae333955d108b 100644 (file)
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -1290,6 +1290,7 @@ NTSTATUS close_file(struct smb_request *req, files_struct *fsp,
 {
        NTSTATUS status;
        struct files_struct *base_fsp = fsp->base_fsp;
+       bool close_base_fsp = false;
 
        /*
         * This fsp can never be an internal dirfsp. They must
@@ -1297,6 +1298,45 @@ NTSTATUS close_file(struct smb_request *req, files_struct *fsp,
         */
        SMB_ASSERT(!fsp->fsp_flags.is_dirfsp);
 
+       if (fsp->stream_fsp != NULL) {
+               /*
+                * fsp is the base for a stream.
+                *
+                * We're called with SHUTDOWN_CLOSE from files.c which walks the
+                * complete list of files.
+                *
+                * We need to wait until the stream is closed.
+                */
+               SMB_ASSERT(close_type == SHUTDOWN_CLOSE);
+               return NT_STATUS_OK;
+       }
+
+       if (base_fsp != NULL) {
+               /*
+                * We need to remove the link in order to
+                * recurse for the base fsp below.
+                */
+               SMB_ASSERT(base_fsp->base_fsp == NULL);
+               SMB_ASSERT(base_fsp->stream_fsp == fsp);
+               base_fsp->stream_fsp = NULL;
+
+               if (close_type == SHUTDOWN_CLOSE) {
+                       /*
+                        * We're called with SHUTDOWN_CLOSE from files.c
+                        * which walks the complete list of files.
+                        *
+                        * We may need to defer the SHUTDOWN_CLOSE
+                        * if it's the next in the linked list.
+                        *
+                        * So we only close if the base is *not* the
+                        * next in the list.
+                        */
+                       close_base_fsp = (fsp->next != base_fsp);
+               } else {
+                       close_base_fsp = true;
+               }
+       }
+
        if (fsp->fsp_flags.is_directory) {
                status = close_directory(req, fsp, close_type);
        } else if (fsp->fake_file_handle != NULL) {
@@ -1310,18 +1350,18 @@ NTSTATUS close_file(struct smb_request *req, files_struct *fsp,
                status = close_normal_file(req, fsp, close_type);
        }
 
-       if ((base_fsp != NULL) && (close_type != SHUTDOWN_CLOSE)) {
+       if (close_base_fsp) {
 
                /*
                 * fsp was a stream, the base fsp can't be a stream as well
                 *
-                * For SHUTDOWN_CLOSE this is not possible here, because
+                * For SHUTDOWN_CLOSE this is not possible here
+                * (if the base_fsp was the next in the linked list), because
                 * SHUTDOWN_CLOSE only happens from files.c which walks the
                 * complete list of files. If we mess with more than one fsp
                 * those loops will become confused.
                 */
 
-               SMB_ASSERT(base_fsp->base_fsp == NULL);
                close_file(req, base_fsp, close_type);
        }
 

diff --git a/source3/smbd/files.c b/source3/smbd/files.c
index 39dc7b657d2afc1e7754fe7c73e45e10d93a410f..f60d5979f533ca45fc95c912ae3fbf931c25e14f 100644 (file)
--- a/source3/smbd/files.c
+++ b/source3/smbd/files.c
@@ -1331,9 +1331,19 @@ size_t fsp_fullbasepath(struct files_struct *fsp, char *buf, size_t buflen)
 
 void fsp_set_base_fsp(struct files_struct *fsp, struct files_struct *base_fsp)
 {
+       SMB_ASSERT(fsp->stream_fsp == NULL);
        if (base_fsp != NULL) {
                SMB_ASSERT(base_fsp->base_fsp == NULL);
+               SMB_ASSERT(base_fsp->stream_fsp == NULL);
+       }
+
+       if (fsp->base_fsp != NULL) {
+               SMB_ASSERT(fsp->base_fsp->stream_fsp == fsp);
+               fsp->base_fsp->stream_fsp = NULL;
        }
 
        fsp->base_fsp = base_fsp;
+       if (fsp->base_fsp != NULL) {
+               fsp->base_fsp->stream_fsp = fsp;
+       }
 }