s3: smbd: change file_set_dosmode() to use get_file_handle_for_metadata() instead...

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

diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c
index 2d07dd9aa74527f7d5589efb51e7555781571861..b99e18033cc9e6ec339007623d8dbb8fb0268676 100644 (file)
--- a/source3/smbd/dosmode.c
+++ b/source3/smbd/dosmode.c
@@ -25,6 +25,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 uint32_t filter_mode_by_protocol(uint32_t mode)
 {
        if (get_Protocol() <= PROTOCOL_LANMAN2) {
@@ -387,6 +392,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)) {
@@ -418,14 +424,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,
@@ -433,7 +442,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",
@@ -711,6 +722,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;
@@ -879,17 +892,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,
@@ -1125,3 +1146,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;
+}