]> git.ipfire.org Git - thirdparty/samba.git/commitdiff
s3: smbd: Fix fsp->base_fsp->fsp_name->fsp == fsp->base_fsp invarient in non_widelink...
authorJeremy Allison <jra@samba.org>
Thu, 1 Jul 2021 01:40:59 +0000 (18:40 -0700)
committerJeremy Allison <jra@samba.org>
Sun, 4 Jul 2021 17:15:34 +0000 (17:15 +0000)
Currently in master when we call into openat() in the VFS
we violate the invarient:

fsp->base_fsp->fsp_name->fsp == fsp->base_fsp.

The reason for this is subtle. Inside open.c:non_widelink_open()
we change the fsp->base_fsp to be relative to the new $cwd.

We do this by the following code in open.c:non_widelink_open():

        /* Also setup base_fsp to be relative to the new cwd */
        if (fsp->base_fsp != NULL) {
                base_smb_fname_rel = (struct smb_filename) {
                        .base_name = smb_fname_rel->base_name,
                };
                orig_base_fsp_name = fsp->base_fsp->fsp_name;
                fsp->base_fsp->fsp_name = &base_smb_fname_rel;
        }

Note that fsp->base_fsp->fsp_name now points at a
stack variable struct smb_filename, with smb_fname->fsp == NULL.

This fixes that problem by removing the horrid
stack based smb_filename and changing to use a
talloc'ed fsp->base_fsp->fsp_name, with
correctly linked fsp->base_fsp->fsp_name-> pointer.

Remove the selftest/knownfail.d/fruit_vfs_invariant
file as all vfs_fruit tests now pass again.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
selftest/knownfail.d/fruit_vfs_invariant [deleted file]
source3/smbd/open.c

diff --git a/selftest/knownfail.d/fruit_vfs_invariant b/selftest/knownfail.d/fruit_vfs_invariant
deleted file mode 100644 (file)
index e8a435e..0000000
+++ /dev/null
@@ -1,18 +0,0 @@
-^samba3.vfs.fruit metadata_netatalk.read metadata\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.write metadata\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.SMB2/CREATE context AAPL\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.create delete-on-close AFP_AfpInfo\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.setinfo delete-on-close AFP_AfpInfo\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.setinfo eof AFP_AfpInfo\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.delete AFP_AfpInfo by writing all 0\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.null afpinfo\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.copy-chunk streams\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.OS X AppleDouble file conversion\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.readdir_attr with names with illegal ntfs characters\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.empty_stream\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.writing_afpinfo\(nt4_dc\)
-^samba3.vfs.unfruit metadata_netatalk.unconvert\(nt4_dc:local\)
-^samba3.vfs.fruit_netatalk.read netatalk metadata\(nt4_dc\)
-^samba3.vfs.fruit_netatalk.stream names with locally created xattr\(nt4_dc\)
-^samba3.vfs.fruit_netatalk.locking conflict\(nt4_dc\)
index 9527b939f880e3e4ef9b175ca7a07ae2770e1550..49f02fb5be9841da10950e539bb3bbc9395dfe6c 100644 (file)
@@ -664,7 +664,6 @@ static NTSTATUS non_widelink_open(const struct files_struct *dirfsp,
        struct smb_filename *orig_fsp_name = fsp->fsp_name;
        struct smb_filename *orig_base_fsp_name = NULL;
        struct smb_filename *smb_fname_rel = NULL;
-       struct smb_filename base_smb_fname_rel;
        struct smb_filename *oldwd_fname = NULL;
        struct smb_filename *parent_dir_fname = NULL;
        bool have_opath = false;
@@ -728,11 +727,35 @@ static NTSTATUS non_widelink_open(const struct files_struct *dirfsp,
 
                /* Also setup base_fsp to be relative to the new cwd */
                if (fsp->base_fsp != NULL) {
-                       base_smb_fname_rel = (struct smb_filename) {
-                               .base_name = smb_fname_rel->base_name,
-                       };
+                       struct smb_filename *base_smb_fname_rel = NULL;
+
+                       /* Check the invarient is true. */
+                       SMB_ASSERT(fsp->base_fsp->fsp_name->fsp ==
+                                  fsp->base_fsp);
+
+                       base_smb_fname_rel = synthetic_smb_fname(
+                                               talloc_tos(),
+                                               smb_fname_rel->base_name,
+                                               NULL,
+                                               &smb_fname_rel->st,
+                                               smb_fname_rel->twrp,
+                                               smb_fname_rel->flags);
+                       if (base_smb_fname_rel == NULL) {
+                               status = NT_STATUS_NO_MEMORY;
+                               goto out;
+                       }
+
+                       base_smb_fname_rel->fsp = fsp->base_fsp;
+
                        orig_base_fsp_name = fsp->base_fsp->fsp_name;
-                       fsp->base_fsp->fsp_name = &base_smb_fname_rel;
+                       fsp->base_fsp->fsp_name = base_smb_fname_rel;
+
+                       /*
+                        * We should have preserved the invarient
+                        * fsp->base_fsp->fsp_name->fsp == fsp->base_fsp.
+                        */
+                       SMB_ASSERT(fsp->base_fsp->fsp_name->fsp ==
+                                  fsp->base_fsp);
                }
        } else {
                /*
@@ -835,7 +858,20 @@ static NTSTATUS non_widelink_open(const struct files_struct *dirfsp,
   out:
        fsp->fsp_name = orig_fsp_name;
        if (fsp->base_fsp != NULL) {
+               /* Save off the temporary name. */
+               struct smb_filename *base_smb_fname_rel =
+                       fsp->base_fsp->fsp_name;
+               /* It no longer has an associated fsp. */
+               base_smb_fname_rel->fsp = NULL;
+
+               /* Replace the original name. */
                fsp->base_fsp->fsp_name = orig_base_fsp_name;
+               /*
+                * We should have preserved the invarient
+                * fsp->base_fsp->fsp_name->fsp == fsp->base_fsp.
+                */
+               SMB_ASSERT(fsp->base_fsp->fsp_name->fsp == fsp->base_fsp);
+               TALLOC_FREE(base_smb_fname_rel);
        }
        TALLOC_FREE(parent_dir_fname);