From bda624268098e99a7c4ce92ea7b6484eb3324d78 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 9 Mar 2016 09:43:09 -0800 Subject: [PATCH] s3: smbd: Fix error in mkdir race condition detection. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11780 The smb2.create.mkdir-dup test creates a race, and against an AD DC this can cause a flapping test if the lstat() and stat() calls are made either side of the chown() due to creation of a file by administrator. We know when we've hit the race, so relax the check_same_stat() check in that specific case. Fix based on an original patch by Andrew Bartlett and Douglas Bagnall . Signed-off-by: Andrew Bartlett Signed-off-by: Douglas Bagnall Signed-off-by: Jeremy Allison --- source3/smbd/open.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index baebd7c..8f41fb0 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -3397,6 +3397,7 @@ static NTSTATUS open_directory(connection_struct *conn, struct timespec mtimespec; int info = 0; bool ok; + bool hit_create_race = false; if (is_ntfs_stream_smb_fname(smb_dname)) { DEBUG(2, ("open_directory: %s is a stream name!\n", @@ -3521,6 +3522,7 @@ static NTSTATUS open_directory(connection_struct *conn, } info = FILE_WAS_OPENED; + hit_create_race = true; } } @@ -3635,14 +3637,50 @@ static NTSTATUS open_directory(connection_struct *conn, return status; } - /* Ensure there was no race condition. */ - if (!check_same_stat(&smb_dname->st, &fsp->fsp_name->st)) { - DEBUG(5,("open_directory: stat struct differs for " - "directory %s.\n", - smb_fname_str_dbg(smb_dname))); + /* + * Now we should have a fd handle, ensure it + * really was a directory handle. + */ + + if(!S_ISDIR(fsp->fsp_name->st.st_ex_mode)) { + DEBUG(5,("open_directory: %s is not a directory !\n", + smb_fname_str_dbg(smb_dname))); fd_close(fsp); file_free(req, fsp); - return NT_STATUS_ACCESS_DENIED; + return NT_STATUS_NOT_A_DIRECTORY; + } + + if (hit_create_race) { + + /* + * https://bugzilla.samba.org/show_bug.cgi?id=11780 + * + * If we were not the actual creator, the real directory + * creator can be in the middle of changing the ownership + * so check_same_stat() can fail. + * + * In this specific case just check the dev,ino pair + * hasn't changed. + */ + + if (!check_same_dev_ino(&smb_dname->st, &fsp->fsp_name->st)) { + DEBUG(5,("open_directory: dev,ino pair differ for " + "directory %s.\n", + smb_fname_str_dbg(smb_dname))); + fd_close(fsp); + file_free(req, fsp); + return NT_STATUS_ACCESS_DENIED; + } + } else { + /* Ensure there was no race condition. */ + if (!check_same_stat(&smb_dname->st, &fsp->fsp_name->st)) { + DEBUG(5,("open_directory: stat struct differs for " + "directory %s.\n", + smb_fname_str_dbg(smb_dname))); + fd_close(fsp); + file_free(req, fsp); + return NT_STATUS_ACCESS_DENIED; + } } lck = get_share_mode_lock(talloc_tos(), fsp->file_id, -- 2.7.0.rc3.207.g0ac5344