From 22fae651656eb4e151d75b0464ef549e4af235f7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 26 Jul 2022 14:34:27 -0700 Subject: [PATCH] s3: smbd: Inside filename_convert_dirfsp_nosymlink() ensure the returned smb_fname is always allocated off mem_ctx. Without this, if we just return smb_fname_rel->fsp->fsp_name as the smb_fname then we return something allocated off fsp (which itself is allocated off the conn struct), not the passed in talloc_ctx. Do this for both non-stream and stream returns. This matters for two reasons. 1). If we error out after calling filename_convert_dirfsp() but before getting to the code inside create_file_unixpath() that takes ownership of the passed in smb_fname->fsp we will leak the fsp as the destructor for smb_fname that closes the fsp will never fire on return to the client, as smb_fname is owned by smb_fname->fsp, not the talloc_tos() context. 2). Some uses of filename_convert() expect to be able to TALLOC_FREE the returned smb_fname once they've successfully called SMB_VFS_CREATE_FILE() as they consider the passed in smb_fname no longer used. It would be nice to be able to just change filename_convert() -> filename_convert_dirfsp() without having to change the lifetime handling of smb_fname. Signed-off-by: Jeremy Allison Reviewed-by: Volker Lendecke --- source3/smbd/filename.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 7600fc2c25d..0f9921b62fa 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -2916,7 +2916,16 @@ static NTSTATUS filename_convert_dirfsp_nosymlink( } if (saved_streamname == NULL) { - smb_fname = smb_fname_rel->fsp->fsp_name; + /* smb_fname must be allocated off mem_ctx. */ + smb_fname = cp_smb_filename(mem_ctx, + smb_fname_rel->fsp->fsp_name); + if (smb_fname == NULL) { + goto fail; + } + status = move_smb_fname_fsp_link(smb_fname, smb_fname_rel); + if (!NT_STATUS_IS_OK(status)) { + goto fail; + } goto done; } @@ -2947,7 +2956,16 @@ static NTSTATUS filename_convert_dirfsp_nosymlink( } if (NT_STATUS_IS_OK(status)) { - smb_fname = smb_fname_rel->fsp->fsp_name; + /* smb_fname must be allocated off mem_ctx. */ + smb_fname = cp_smb_filename(mem_ctx, + smb_fname_rel->fsp->fsp_name); + if (smb_fname == NULL) { + goto fail; + } + status = move_smb_fname_fsp_link(smb_fname, smb_fname_rel); + if (!NT_STATUS_IS_OK(status)) { + goto fail; + } goto done; } -- 2.47.3