From 5e04692bc4efc873009eb9c4b8489767f95c7311 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 8 Jun 2021 18:53:18 +0200 Subject: [PATCH] smbd: fix pathref unlinking in create_file_unixpath() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is really subtle. If someone passes in an smb_fname where smb_fname actually is taken from fsp->fsp_name, then the lifetime of these objects is meant to be the same. This is commonly the case from an SMB1 path-based call (eg call_trans2qfilepathinfo()) where we use the pathref fsp (smb_fname->fsp) as the handle. In this case we must not unlink smb_fname->fsp from it's owner. The asserts below: SMB_ASSERT(fsp->fsp_name->fsp != NULL); SMB_ASSERT(fsp->fsp_name->fsp == fsp); ensure the required invarients are met. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14732 Pair-Programmed-With: Ralph Boehme Signed-off-by: Jeremy Allison Signed-off-by: Ralph Boehme Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Tue Jun 8 20:44:41 UTC 2021 on sn-devel-184 (cherry picked from commit 8a427783e5e780d3ffbe4f9710ac4a17c483ca33) --- source3/smbd/open.c | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index acb248047bf..0ace5b5a0da 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -5824,13 +5824,39 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, * request to create a file that doesn't exist. */ if (smb_fname->fsp != NULL) { - fsp = smb_fname->fsp; + bool need_fsp_unlink = true; /* - * Unlink the fsp from the smb_fname so the fsp is not - * autoclosed by the smb_fname pathref fsp talloc destructor. + * This is really subtle. If someone passes in an smb_fname + * where smb_fname actually is taken from fsp->fsp_name, then + * the lifetime of these objects is meant to be the same. + * + * This is commonly the case from an SMB1 path-based call, + * (call_trans2qfilepathinfo) where we use the pathref fsp + * (smb_fname->fsp) as the handle. In this case we must not + * unlink smb_fname->fsp from it's owner. + * + * The asserts below: + * + * SMB_ASSERT(fsp->fsp_name->fsp != NULL); + * SMB_ASSERT(fsp->fsp_name->fsp == fsp); + * + * ensure the required invarients are met. */ - smb_fname_fsp_unlink(smb_fname); + if (smb_fname->fsp->fsp_name == smb_fname) { + need_fsp_unlink = false; + } + + fsp = smb_fname->fsp; + + if (need_fsp_unlink) { + /* + * Unlink the fsp from the smb_fname so the fsp is not + * autoclosed by the smb_fname pathref fsp talloc + * destructor. + */ + smb_fname_fsp_unlink(smb_fname); + } status = fsp_bind_smb(fsp, req); if (!NT_STATUS_IS_OK(status)) { @@ -5860,6 +5886,9 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, } } + SMB_ASSERT(fsp->fsp_name->fsp != NULL); + SMB_ASSERT(fsp->fsp_name->fsp == fsp); + if (base_fsp) { /* * We're opening the stream element of a -- 2.27.0