s3: smbd: Inside filename_convert_dirfsp_nosymlink() ensure the returned smb_fname...

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 <jra@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 7600fc2c25d470c31b6ddc13fc9199a9131253cd..0f9921b62fac098960e05c2a35ba9a06d4202232 100644 (file)
--- 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;
        }