I found lots of places where we assume error will be set when calling

one of our virtualised functions, such as db_open(), but error is only
set when a system call fails, and it is not uncommon for us to fail a
function internally without ever making a system call. That led to us
passing back success when a function had in fact failed.

I found two places where we relied on map_nt_error_from_unix()
returning success when errno==0, but lots and lots of places where we
relied on the reverse, so I fixed those two places.

map_nt_error_from_unix() will now always return an error, returning
NT_STATUS_UNSUCCESSFUL if errno is 0
(cherry picked from commit 69d40ca4c1af925d4b0e59ddc69ef8c26e6501d1)

diff --git a/source/lib/errmap_unix.c b/source/lib/errmap_unix.c
index 8194cf80cce48ae9760a1bb7d3b277e2d6060e2a..2cd2386c5c011ebef8f0851f5de07306f5e1fa64 100644 (file)
--- a/source/lib/errmap_unix.c
+++ b/source/lib/errmap_unix.c
@@ -107,8 +107,16 @@ NTSTATUS map_nt_error_from_unix(int unix_error)
 {
        int i = 0;
 
-       if (unix_error == 0)
-               return NT_STATUS_OK;
+       if (unix_error == 0) {
+               /* we map this to an error, not success, as this
+                  function is only called in an error path. Lots of
+                  our virtualised functions may fail without making a
+                  unix system call that fails (such as when they are
+                  checking for some handle existing), so unix_error
+                  may be unset
+               */
+               return NT_STATUS_UNSUCCESSFUL;
+       }
 
        /* Look through list */
        while(unix_dos_nt_errmap[i].unix_error != 0) {

diff --git a/source/smbd/reply.c b/source/smbd/reply.c
index 80afd582f6e57bf286c39516a344d49fb4158c3d..d32d998da83821f213850713fc450ab8b803ed4b 100644 (file)
--- a/source/smbd/reply.c
+++ b/source/smbd/reply.c
@@ -2527,7 +2527,7 @@ NTSTATUS unlink_internals(connection_struct *conn, struct smb_request *req,
                TALLOC_FREE(dir_hnd);
        }
 
-       if (count == 0 && NT_STATUS_IS_OK(status)) {
+       if (count == 0 && NT_STATUS_IS_OK(status) && errno != 0) {
                status = map_nt_error_from_unix(errno);
        }
 
@@ -5910,7 +5910,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx,
        }
        TALLOC_FREE(dir_hnd);
 
-       if (count == 0 && NT_STATUS_IS_OK(status)) {
+       if (count == 0 && NT_STATUS_IS_OK(status) && errno != 0) {
                status = map_nt_error_from_unix(errno);
        }