From: Jeremy Allison Date: Thu, 1 Jul 2021 01:40:59 +0000 (-0700) Subject: s3: smbd: Fix fsp->base_fsp->fsp_name->fsp == fsp->base_fsp invarient in non_widelink... X-Git-Tag: tevent-0.11.0~50 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6a366012aaedf400a438e97ffd5e875f0ee649d3;p=thirdparty%2Fsamba.git 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 --- 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);