From 6a366012aaedf400a438e97ffd5e875f0ee649d3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 30 Jun 2021 18:40:59 -0700 Subject: [PATCH] s3: smbd: Fix fsp->base_fsp->fsp_name->fsp == fsp->base_fsp invarient in non_widelink_open(). 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 Reviewed-by: Ralph Boehme --- selftest/knownfail.d/fruit_vfs_invariant | 18 ---------- source3/smbd/open.c | 46 +++++++++++++++++++++--- 2 files changed, 41 insertions(+), 23 deletions(-) delete mode 100644 selftest/knownfail.d/fruit_vfs_invariant diff --git a/selftest/knownfail.d/fruit_vfs_invariant b/selftest/knownfail.d/fruit_vfs_invariant deleted file mode 100644 index e8a435e7dc8..00000000000 --- a/selftest/knownfail.d/fruit_vfs_invariant +++ /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\) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 9527b939f88..49f02fb5be9 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -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); -- 2.47.3