smbd: fix pathref unlinking in create_file_unixpath()

This is really subtle. If someone passes in an smb_fname where smb_fname
actually is taken from fsp->fsp_name, then the lifetime of these objects is
meant to be the same.

This is commonly the case from an SMB1 path-based call
(eg call_trans2qfilepathinfo()) where we use the pathref fsp
(smb_fname->fsp) as the handle. In this case we must not unlink smb_fname->fsp
from it's owner.

The asserts below:

  SMB_ASSERT(fsp->fsp_name->fsp != NULL);
  SMB_ASSERT(fsp->fsp_name->fsp == fsp);

ensure the required invarients are met.

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

Pair-Programmed-With: Ralph Boehme <slow@samba.org>
Signed-off-by: Jeremy Allison <jra@samba.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Tue Jun  8 20:44:41 UTC 2021 on sn-devel-184

(cherry picked from commit 8a427783e5e780d3ffbe4f9710ac4a17c483ca33)

Autobuild-User(v4-14-test): Karolin Seeger <kseeger@samba.org>
Autobuild-Date(v4-14-test): Thu Jun 10 10:31:11 UTC 2021 on sn-devel-184

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index fbaa951d4304b4b3c7aaaa07249946cc4d848576..9a18088c929716861033d1cd04de596d182b3759 100644 (file)
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -5814,13 +5814,39 @@ static NTSTATUS create_file_unixpath(connection_struct *conn,
         * request to create a file that doesn't exist.
         */
        if (smb_fname->fsp != NULL) {
-               fsp = smb_fname->fsp;
+               bool need_fsp_unlink = true;
 
                /*
-                * Unlink the fsp from the smb_fname so the fsp is not
-                * autoclosed by the smb_fname pathref fsp talloc destructor.
+                * This is really subtle. If someone passes in an smb_fname
+                * where smb_fname actually is taken from fsp->fsp_name, then
+                * the lifetime of these objects is meant to be the same.
+                *
+                * This is commonly the case from an SMB1 path-based call,
+                * (call_trans2qfilepathinfo) where we use the pathref fsp
+                * (smb_fname->fsp) as the handle. In this case we must not
+                * unlink smb_fname->fsp from it's owner.
+                *
+                * The asserts below:
+                *
+                * SMB_ASSERT(fsp->fsp_name->fsp != NULL);
+                * SMB_ASSERT(fsp->fsp_name->fsp == fsp);
+                *
+                * ensure the required invarients are met.
                 */
-               smb_fname_fsp_unlink(smb_fname);
+               if (smb_fname->fsp->fsp_name == smb_fname) {
+                       need_fsp_unlink = false;
+               }
+
+               fsp = smb_fname->fsp;
+
+               if (need_fsp_unlink) {
+                       /*
+                        * Unlink the fsp from the smb_fname so the fsp is not
+                        * autoclosed by the smb_fname pathref fsp talloc
+                        * destructor.
+                        */
+                       smb_fname_fsp_unlink(smb_fname);
+               }
 
                status = fsp_bind_smb(fsp, req);
                if (!NT_STATUS_IS_OK(status)) {
@@ -5850,6 +5876,9 @@ static NTSTATUS create_file_unixpath(connection_struct *conn,
                }
        }
 
+       SMB_ASSERT(fsp->fsp_name->fsp != NULL);
+       SMB_ASSERT(fsp->fsp_name->fsp == fsp);
+
        if (base_fsp) {
                /*
                 * We're opening the stream element of a