r20338: Restructure open_directory a bit. This gets rid of a race condition regarding

error messages: We relied upon a stat that a directory did not exist to later
on then do the mkdir or not. This does the mkdir directly and copes with a
potential error.

The second one is more important: It's possible with Samba 3 to do a
ntcreate&x with NTCREATEX_OPTIONS_DIRECTORY and we happily do a NT_STATUS_OK.

Also move up the use_nt_status() logic a bit. I think this does not belong
into the core routines, the smb server as such should take care of it.

Jeremy, do you think this should go to 3.0.24?

I'll update samba4torture when the build farm has picked up this checkin.

Volker

diff --git a/source/smbd/nttrans.c b/source/smbd/nttrans.c
index 9497c5875c24afbc62f0aa8ed8cac00439e6fe2f..49a99a020f6db5490c517d64ceb3baef09114b5c 100644 (file)
--- a/source/smbd/nttrans.c
+++ b/source/smbd/nttrans.c
@@ -688,6 +688,10 @@ int reply_ntcreate_and_X(connection_struct *conn,
                restore_case_semantics(conn, file_attributes);
 
                if(!NT_STATUS_IS_OK(status)) {
+                       if (!use_nt_status() && NT_STATUS_EQUAL(
+                                   status, NT_STATUS_OBJECT_NAME_COLLISION)) {
+                               status = NT_STATUS_DOS(ERRDOS, ERRfilexists);
+                       }
                        END_PROFILE(SMBntcreateX);
                        return ERROR_NT(status);
                }
@@ -760,6 +764,10 @@ int reply_ntcreate_and_X(connection_struct *conn,
 
                                if(!NT_STATUS_IS_OK(status)) {
                                        restore_case_semantics(conn, file_attributes);
+                                       if (!use_nt_status() && NT_STATUS_EQUAL(
+                                                   status, NT_STATUS_OBJECT_NAME_COLLISION)) {
+                                               status = NT_STATUS_DOS(ERRDOS, ERRfilexists);
+                                       }
                                        END_PROFILE(SMBntcreateX);
                                        return ERROR_NT(status);
                                }

diff --git a/source/smbd/open.c b/source/smbd/open.c
index 3111f68ef59543c6d9ac84c9b9eece547d336af9..dcd1a7a36f8a75140ddccb039d68248a44303449 100644 (file)
--- a/source/smbd/open.c
+++ b/source/smbd/open.c
@@ -1867,7 +1867,6 @@ NTSTATUS open_directory(connection_struct *conn,
 {
        files_struct *fsp = NULL;
        BOOL dir_existed = VALID_STAT(*psbuf) ? True : False;
-       BOOL create_dir = False;
        struct share_mode_lock *lck = NULL;
        NTSTATUS status;
        int info = 0;
@@ -1888,44 +1887,47 @@ NTSTATUS open_directory(connection_struct *conn,
 
        switch( create_disposition ) {
                case FILE_OPEN:
-                       /* If directory exists open. If directory doesn't
-                        * exist error. */
-                       if (!dir_existed) {
-                               DEBUG(5,("open_directory: FILE_OPEN requested "
-                                        "for directory %s and it doesn't "
-                                        "exist.\n", fname ));
-                               return NT_STATUS_OBJECT_NAME_NOT_FOUND;
-                       }
+                       /*
+                        * Don't do anything. The check for existence is done
+                        * futher down.
+                        */
                        info = FILE_WAS_OPENED;
                        break;
 
                case FILE_CREATE:
+
                        /* If directory exists error. If directory doesn't
                         * exist create. */
-                       if (dir_existed) {
-                               DEBUG(5,("open_directory: FILE_CREATE "
-                                        "requested for directory %s and it "
-                                        "already exists.\n", fname ));
-                               if (use_nt_status()) {
-                                       return NT_STATUS_OBJECT_NAME_COLLISION;
-                               } else {
-                                       return NT_STATUS_DOS(ERRDOS,
-                                                            ERRfilexists);
-                               }
+
+                       status = mkdir_internal(conn, fname, False);
+                       if (!NT_STATUS_IS_OK(status)) {
+                               DEBUG(2, ("open_directory: unable to create "
+                                         "%s. Error was %s\n", fname,
+                                         nt_errstr(status)));
+                               return status;
                        }
-                       create_dir = True;
+
                        info = FILE_WAS_CREATED;
                        break;
 
                case FILE_OPEN_IF:
-                       /* If directory exists open. If directory doesn't
-                        * exist create. */
-                       if (!dir_existed) {
-                               create_dir = True;
+                       /*
+                        * If directory exists open. If directory doesn't
+                        * exist create.
+                        */
+
+                       status = mkdir_internal(conn, fname, False);
+
+                       if (NT_STATUS_IS_OK(status)) {
                                info = FILE_WAS_CREATED;
-                       } else {
+                       }
+
+                       if (NT_STATUS_EQUAL(status,
+                                           NT_STATUS_OBJECT_NAME_COLLISION)) {
                                info = FILE_WAS_OPENED;
+                               status = NT_STATUS_OK;
                        }
+                               
                        break;
 
                case FILE_SUPERSEDE:
@@ -1938,35 +1940,17 @@ NTSTATUS open_directory(connection_struct *conn,
                        return NT_STATUS_INVALID_PARAMETER;
        }
 
-       if (create_dir) {
-               /*
-                * Try and create the directory.
-                */
-
-               /* We know bad_path is false as it's caught earlier. */
+       /* Ensure we're checking for a symlink here.... */
+       /* We don't want to get caught by a symlink racer. */
 
-               status = mkdir_internal(conn, fname, False);
-
-               if (!NT_STATUS_IS_OK(status)) {
-                       DEBUG(2,("open_directory: unable to create %s. "
-                                "Error was %s\n", fname, strerror(errno) ));
-                       /* Ensure we return the correct NT status to the
-                        * client. */
-                       return status;
-               }
-
-               /* Ensure we're checking for a symlink here.... */
-               /* We don't want to get caught by a symlink racer. */
-
-               if(SMB_VFS_LSTAT(conn,fname, psbuf) != 0) {
-                       return map_nt_error_from_unix(errno);
-               }
+       if(SMB_VFS_LSTAT(conn,fname, psbuf) != 0) {
+               return map_nt_error_from_unix(errno);
+       }
 
-               if(!S_ISDIR(psbuf->st_mode)) {
-                       DEBUG(0,("open_directory: %s is not a directory !\n",
-                                fname ));
-                       return NT_STATUS_NOT_A_DIRECTORY;
-               }
+       if(!S_ISDIR(psbuf->st_mode)) {
+               DEBUG(0,("open_directory: %s is not a directory !\n",
+                        fname ));
+               return NT_STATUS_NOT_A_DIRECTORY;
        }
 
        status = file_new(conn, &fsp);

diff --git a/source/smbd/reply.c b/source/smbd/reply.c
index b8ee13043be35c7687c096aaae671f7798de2dd0..282779fcd3560c5c61de8ea0b87b5c4a4005f192 100644 (file)
--- a/source/smbd/reply.c
+++ b/source/smbd/reply.c
@@ -3818,8 +3818,9 @@ int reply_mkdir(connection_struct *conn, char *inbuf,char *outbuf, int dum_size,
 
        if (!NT_STATUS_IS_OK(status)) {
 
-               if (NT_STATUS_EQUAL(
-                           status, NT_STATUS_DOS(ERRDOS, ERRfilexists))) {
+               if (!use_nt_status()
+                   && NT_STATUS_EQUAL(status,
+                                      NT_STATUS_OBJECT_NAME_COLLISION)) {
                        /*
                         * Yes, in the DOS error code case we get a
                         * ERRDOS:ERRnoaccess here. See BASE-SAMBA3ERROR