From e6e40842d15850de35d00d5138fcaac858e37adc Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 1 May 2014 11:07:44 -0700 Subject: [PATCH] s3: smbd: change file_set_dosmode() to use get_file_handle_for_metadata() instead of open_file_fchmod(). get_file_handle_for_metadata() is a new function that finds an existing open handle (fsp->fh->fd != -1) for a given dev/ino if there is one available, and uses INTERNAL_OPEN_ONLY with WRITE_DATA access if not. Allows open_file_fchmod() to be removed next. Bug 10564 - Lock order violation and file lost https://bugzilla.samba.org/show_bug.cgi?id=10564 Signed-off-by: Jeremy Allison Signed-off-by: Volker Lendecke --- source3/smbd/dosmode.c | 104 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 92 insertions(+), 12 deletions(-) diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c index 831f6d0b6de..a99f6f2a845 100644 --- a/source3/smbd/dosmode.c +++ b/source3/smbd/dosmode.c @@ -26,6 +26,11 @@ #include "smbd/smbd.h" #include "lib/param/loadparm.h" +static NTSTATUS get_file_handle_for_metadata(connection_struct *conn, + struct smb_filename *smb_fname, + files_struct **ret_fsp, + bool *need_close); + static void dos_mode_debug_print(uint32_t mode) { DEBUG(8,("dos_mode returning ")); @@ -420,6 +425,7 @@ static bool set_ea_dos_attribute(connection_struct *conn, SAMBA_XATTR_DOS_ATTRIB, blob.data, blob.length, 0) == -1) { bool ret = false; + bool need_close = false; files_struct *fsp = NULL; if((errno != EPERM) && (errno != EACCES)) { @@ -451,14 +457,17 @@ static bool set_ea_dos_attribute(connection_struct *conn, } /* - * We need to open the file with write access whilst - * still in our current user context. This ensures we - * are not violating security in doing the setxattr. + * We need to get an open file handle to do the + * metadata operation under root. */ - if (!NT_STATUS_IS_OK(open_file_fchmod(conn, smb_fname, - &fsp))) + if (!NT_STATUS_IS_OK(get_file_handle_for_metadata(conn, + smb_fname, + &fsp, + &need_close))) { return false; + } + become_root(); if (SMB_VFS_FSETXATTR(fsp, SAMBA_XATTR_DOS_ATTRIB, blob.data, @@ -466,7 +475,9 @@ static bool set_ea_dos_attribute(connection_struct *conn, ret = true; } unbecome_root(); - close_file(NULL, fsp, NORMAL_CLOSE); + if (need_close) { + close_file(NULL, fsp, NORMAL_CLOSE); + } return ret; } DEBUG(10,("set_ea_dos_attribute: set EA 0x%x on file %s\n", @@ -777,6 +788,8 @@ int file_set_dosmode(connection_struct *conn, struct smb_filename *smb_fname, uint32_t old_mode; struct timespec new_create_timespec; files_struct *fsp = NULL; + bool need_close = false; + NTSTATUS status; if (!CAN_WRITE(conn)) { errno = EROFS; @@ -945,17 +958,25 @@ int file_set_dosmode(connection_struct *conn, struct smb_filename *smb_fname, } /* - * We need to open the file with write access whilst - * still in our current user context. This ensures we - * are not violating security in doing the fchmod. + * We need to get an open file handle to do the + * metadata operation under root. */ - if (!NT_STATUS_IS_OK(open_file_fchmod(conn, smb_fname, - &fsp))) + + status = get_file_handle_for_metadata(conn, + smb_fname, + &fsp, + &need_close); + if (!NT_STATUS_IS_OK(status)) { + errno = map_errno_from_nt_status(status); return -1; + } + become_root(); ret = SMB_VFS_FCHMOD(fsp, unixmode); unbecome_root(); - close_file(NULL, fsp, NORMAL_CLOSE); + if (need_close) { + close_file(NULL, fsp, NORMAL_CLOSE); + } if (!newfile) { notify_fname(conn, NOTIFY_ACTION_MODIFIED, FILE_NOTIFY_CHANGE_ATTRIBUTES, @@ -1191,3 +1212,62 @@ struct timespec get_change_timespec(connection_struct *conn, { return smb_fname->st.st_ex_mtime; } + +/**************************************************************************** + Get a real open file handle we can do meta-data operations on. As it's + going to be used under root access only on meta-data we should look for + any existing open file handle first, and use that in preference (also to + avoid kernel self-oplock breaks). If not use an INTERNAL_OPEN_ONLY handle. +****************************************************************************/ + +static NTSTATUS get_file_handle_for_metadata(connection_struct *conn, + struct smb_filename *smb_fname, + files_struct **ret_fsp, + bool *need_close) +{ + NTSTATUS status; + files_struct *fsp; + struct file_id file_id; + + *need_close = false; + + if (!VALID_STAT(smb_fname->st)) { + return NT_STATUS_INVALID_PARAMETER; + } + + file_id = vfs_file_id_from_sbuf(conn, &smb_fname->st); + + for(fsp = file_find_di_first(conn->sconn, file_id); + fsp; + fsp = file_find_di_next(fsp)) { + if (fsp->fh->fd != -1) { + *ret_fsp = fsp; + return NT_STATUS_OK; + } + } + + /* Opens an INTERNAL_OPEN_ONLY write handle. */ + status = SMB_VFS_CREATE_FILE( + conn, /* conn */ + NULL, /* req */ + 0, /* root_dir_fid */ + smb_fname, /* fname */ + FILE_WRITE_DATA, /* access_mask */ + (FILE_SHARE_READ | FILE_SHARE_WRITE | /* share_access */ + FILE_SHARE_DELETE), + FILE_OPEN, /* create_disposition*/ + 0, /* create_options */ + 0, /* file_attributes */ + INTERNAL_OPEN_ONLY, /* oplock_request */ + 0, /* allocation_size */ + 0, /* private_flags */ + NULL, /* sd */ + NULL, /* ea_list */ + ret_fsp, /* result */ + NULL); /* pinfo */ + + if (NT_STATUS_IS_OK(status)) { + *need_close = true; + } + return status; +} -- 2.11.4.GIT