s3 onefs: Fix a race condition exists in onefs_open.c between multiple opens to the...
authorSteven Danneman <steven.danneman@isilon.com>
Thu, 14 May 2009 23:12:23 +0000 (23:12 +0000)
committerTim Prouty <tprouty@samba.org>
Thu, 24 Sep 2009 18:31:21 +0000 (11:31 -0700)
Two openers can stat a file at the same time, see that it doesn't exist,
and then both race to open it first.  The loser will enter
onefs_open_file_ntcreate believing that the file doesnt exist, and thus
skip any current state lookups for that file.  This includes setting
the file_id, and having a valid stat buffer.

Normally on first create the file_id will be set during the open, but
the second opener in this scenario may fail the open (oplock/share mode)
and file_id will not be set, nor will the stat buffer be valid.

In the error paths of this patch, we now double check that the file_id
and the stat buffer are valid before doing other operations.

source3/modules/onefs_open.c

index 4ae81356b56f3b08a6735e5efe22bcd2e6c0ff18..fa1f883c591fe7b7562ac343514e7157e663b8f8 100644 (file)
@@ -824,7 +824,6 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
 
        DEBUG(10, ("fsp = %p\n", fsp));
 
-       fsp->file_id = vfs_file_id_from_sbuf(conn, &smb_fname->st);
        fsp->share_access = share_access;
        fsp->fh->private_options = create_options;
        fsp->access_mask = open_access_mask; /* We change this to the
@@ -929,11 +928,16 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
                        SMB_ASSERT(req);
 
                        if (lck == NULL) {
+                               /*
+                                * We hit the race that when we did the stat
+                                * on the file it did not exist, and someone
+                                * has created it in between the stat and the
+                                * open_file() call. Defer our open waiting,
+                                * to break the oplock of the first opener.
+                                */
 
                                struct timespec old_write_time;
 
-                               old_write_time = smb_fname->st.st_ex_mtime;
-
                                DEBUG(3, ("Someone created file %s with an "
                                          "oplock after we looked: Retrying\n",
                                          smb_fname_str_dbg(smb_fname)));
@@ -981,6 +985,16 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
                        uint32 can_access_mask;
                        bool can_access = True;
 
+                       /* If we raced on open we may not have a valid file_id
+                        * or stat buf.  Get them again. */
+                       if (SMB_VFS_STAT(conn, fname, psbuf) == -1) {
+                               DEBUG(0,("Error doing stat on file %s "
+                                       "(%s)\n", fname, strerror(errno)));
+                               status = NT_STATUS_SHARING_VIOLATION;
+                               goto cleanup_destroy;
+                       }
+                       id = vfs_file_id_from_sbuf(conn, psbuf);
+
                        /* Check if this can be done with the deny_dos and fcb
                         * calls. */
 
@@ -1135,8 +1149,8 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
 
                /*
                 * Now the file exists and fsp is successfully opened,
-                * fsp->dev and fsp->inode are valid and should replace the
-                * dev=0,inode=0 from a non existent file. Spotted by
+                * fsp->file_id is valid and should replace the
+                * dev=0, inode=0 from a non existent file. Spotted by
                 * Nadav Danieli <nadavd@exanet.com>. JRA.
                 */