r23457: After Jeremy's ack:

The attached patch removes a little race condition for
people with real kernel oplock support, and reduces some
code paths. It changes reply_unlink to open_file_ntcreate,
set_delete_on_close and close_file.

The race condition happens if we break the oplock in
can_delete via open_file_ntcreate, we close the file,
someone else gets a batch oplock and we try to unlink.

It reduces code paths by calling SMB_VFS_UNLINK in 2 fewer
places.
(This used to be commit 0342ce7057045a362134281bcc7030111276dea0)

diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 6490856f04ffa115379eff55920f96d870b57db4..be350994701af403730f5287a1226baafd89d558 100644 (file)
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -1830,11 +1830,11 @@ static NTSTATUS can_rename(connection_struct *conn, char *fname, uint16 dirtype,
 }
 
 /*******************************************************************
- Check if a user is allowed to delete a file.
-********************************************************************/
+ * unlink a file with all relevant access checks
+ *******************************************************************/
 
-static NTSTATUS can_delete(connection_struct *conn, char *fname,
-                          uint32 dirtype, BOOL can_defer)
+static NTSTATUS do_unlink(connection_struct *conn, char *fname,
+                         uint32 dirtype, BOOL can_defer)
 {
        SMB_STRUCT_STAT sbuf;
        uint32 fattr;
@@ -1935,10 +1935,19 @@ static NTSTATUS can_delete(connection_struct *conn, char *fname,
                                    can_defer ? 0 : INTERNAL_OPEN_ONLY,
                                    NULL, &fsp);
 
-       if (NT_STATUS_IS_OK(status)) {
-               close_file(fsp,NORMAL_CLOSE);
+       if (!NT_STATUS_IS_OK(status)) {
+               DEBUG(10, ("open_file_ntcreate failed: %s\n",
+                          nt_errstr(status)));
+               return status;
        }
-       return status;
+
+       /* The set is across all open files on this dev/inode pair. */
+       if (!set_delete_on_close(fsp, True, &current_user.ut)) {
+               close_file(fsp, NORMAL_CLOSE);
+               return NT_STATUS_ACCESS_DENIED;
+       }
+
+       return close_file(fsp,NORMAL_CLOSE);
 }
 
 /****************************************************************************
@@ -1997,17 +2006,15 @@ NTSTATUS unlink_internals(connection_struct *conn, uint32 dirtype,
                        return status;
                }
 
-               status = can_delete(conn,directory,dirtype,can_defer);
+               status = do_unlink(conn,directory,dirtype,can_defer);
                if (!NT_STATUS_IS_OK(status)) {
                        return status;
                }
 
-               if (SMB_VFS_UNLINK(conn,directory) == 0) {
-                       count++;
-                       notify_fname(conn, NOTIFY_ACTION_REMOVED,
-                                    FILE_NOTIFY_CHANGE_FILE_NAME,
-                                    directory);
-               }
+               count++;
+               notify_fname(conn, NOTIFY_ACTION_REMOVED,
+                            FILE_NOTIFY_CHANGE_FILE_NAME,
+                            directory);
        } else {
                struct smb_Dir *dir_hnd = NULL;
                long offset = 0;
@@ -2066,19 +2073,17 @@ NTSTATUS unlink_internals(connection_struct *conn, uint32 dirtype,
                                return status;
                        }
 
-                       status = can_delete(conn, fname, dirtype, can_defer);
+                       status = do_unlink(conn, fname, dirtype, can_defer);
                        if (!NT_STATUS_IS_OK(status)) {
                                continue;
                        }
-                       if (SMB_VFS_UNLINK(conn,fname) == 0) {
-                               count++;
-                               DEBUG(3,("unlink_internals: succesful unlink "
-                                        "[%s]\n",fname));
-                               notify_fname(conn, NOTIFY_ACTION_REMOVED,
-                                            FILE_NOTIFY_CHANGE_FILE_NAME,
-                                            fname);
-                       }
-                               
+
+                       count++;
+                       DEBUG(3,("unlink_internals: succesful unlink [%s]\n",
+                                fname));
+                       notify_fname(conn, NOTIFY_ACTION_REMOVED,
+                                    FILE_NOTIFY_CHANGE_FILE_NAME,
+                                    fname);
                }
                CloseDir(dir_hnd);
        }