smbd: replace CHECK_WRITE() macro with calls to check_any_access_fsp()

The additional check if fd underlying fd is valid and not -1 should not be done
at this place. I actually would prefer an write to fail with EBADF if this
happens, as it's likely easier to debug why this happened. These days we should
always have a valid fd.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit 995a31c8d4c1789c16bae6b8196f2565d4b1dfdb)

diff --git a/source3/include/smb_macros.h b/source3/include/smb_macros.h
index 42ff9ffb0d45f9d05029fc16630b34b86bafc2a9..1851709774a4ea84fd57c8d6c9aa6750614f7754 100644 (file)
--- a/source3/include/smb_macros.h
+++ b/source3/include/smb_macros.h
@@ -74,11 +74,6 @@
         (fsp_get_io_fd(fsp) != -1) && \
         (((fsp)->fsp_flags.can_read)))
 
-#define CHECK_WRITE(fsp) \
-       ((fsp)->fsp_flags.can_write && \
-       (!(fsp)->fsp_flags.is_pathref) && \
-        (fsp_get_io_fd(fsp) != -1))
-
 #define ERROR_WAS_LOCK_DENIED(status) (NT_STATUS_EQUAL((status), NT_STATUS_LOCK_NOT_GRANTED) || \
                                NT_STATUS_EQUAL((status), NT_STATUS_FILE_LOCK_CONFLICT) )
 

diff --git a/source3/modules/offload_token.c b/source3/modules/offload_token.c
index 901991daf28d483130ee8191c783af83594abfec..3b71a0028fb77f8ecb3045ed3ef6f1116b488a7b 100644 (file)
--- a/source3/modules/offload_token.c
+++ b/source3/modules/offload_token.c
@@ -259,6 +259,8 @@ NTSTATUS vfs_offload_token_check_handles(uint32_t fsctl,
                                         files_struct *src_fsp,
                                         files_struct *dst_fsp)
 {
+       NTSTATUS status;
+
        if (src_fsp->vuid != dst_fsp->vuid) {
                DBG_INFO("copy chunk handles not in the same session.\n");
                return NT_STATUS_ACCESS_DENIED;
@@ -317,10 +319,11 @@ NTSTATUS vfs_offload_token_check_handles(uint32_t fsctl,
         *
         * A non writable dst handle also doesn't make sense for other fsctls.
         */
-       if (!CHECK_WRITE(dst_fsp)) {
+       status = check_any_access_fsp(dst_fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+       if (!NT_STATUS_IS_OK(status)) {
                DBG_INFO("dest handle not writable (%s).\n",
                        smb_fname_str_dbg(dst_fsp->fsp_name));
-               return NT_STATUS_ACCESS_DENIED;
+               return status;
        }
        /*
         * - The Open.GrantedAccess of the destination file does not include

diff --git a/source3/smbd/smb1_reply.c b/source3/smbd/smb1_reply.c
index 63ff98fb22b05b2cb7816b40ed904e900f66b705..92cc7bf7dd118f264fac1e6200b2e320ba5b588a 100644 (file)
--- a/source3/smbd/smb1_reply.c
+++ b/source3/smbd/smb1_reply.c
@@ -3926,8 +3926,9 @@ void reply_writebraw(struct smb_request *req)
                return;
        }
 
-       if (!CHECK_WRITE(fsp)) {
-               reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+       status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+       if (!NT_STATUS_IS_OK(status)) {
+               reply_nterror(req, status);
                error_to_writebrawerr(req);
                END_PROFILE(SMBwritebraw);
                return;
@@ -4157,8 +4158,9 @@ void reply_writeunlock(struct smb_request *req)
                return;
        }
 
-       if (!CHECK_WRITE(fsp)) {
-               reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+       status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+       if (!NT_STATUS_IS_OK(status)) {
+               reply_nterror(req, status);
                END_PROFILE(SMBwriteunlock);
                return;
        }
@@ -4292,8 +4294,9 @@ void reply_write(struct smb_request *req)
                return;
        }
 
-       if (!CHECK_WRITE(fsp)) {
-               reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+       status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+       if (!NT_STATUS_IS_OK(status)) {
+               reply_nterror(req, status);
                END_PROFILE(SMBwrite);
                return;
        }
@@ -4571,8 +4574,9 @@ void reply_write_and_X(struct smb_request *req)
                goto out;
        }
 
-       if (!CHECK_WRITE(fsp)) {
-               reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+       status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+       if (!NT_STATUS_IS_OK(status)) {
+               reply_nterror(req, status);
                goto out;
        }
 
@@ -5283,6 +5287,7 @@ void reply_writeclose(struct smb_request *req)
        struct timespec mtime;
        files_struct *fsp;
        struct lock_struct lock;
+       NTSTATUS status;
 
        START_PROFILE(SMBwriteclose);
 
@@ -5298,8 +5303,9 @@ void reply_writeclose(struct smb_request *req)
                END_PROFILE(SMBwriteclose);
                return;
        }
-       if (!CHECK_WRITE(fsp)) {
-               reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+       status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+       if (!NT_STATUS_IS_OK(status)) {
+               reply_nterror(req, status);
                END_PROFILE(SMBwriteclose);
                return;
        }
@@ -6086,6 +6092,7 @@ void reply_printwrite(struct smb_request *req)
        int numtowrite;
        const char *data;
        files_struct *fsp;
+       NTSTATUS status;
 
        START_PROFILE(SMBsplwr);
 
@@ -6108,8 +6115,9 @@ void reply_printwrite(struct smb_request *req)
                return;
        }
 
-       if (!CHECK_WRITE(fsp)) {
-               reply_nterror(req, NT_STATUS_ACCESS_DENIED);
+       status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+       if (!NT_STATUS_IS_OK(status)) {
+               reply_nterror(req, status);
                END_PROFILE(SMBsplwr);
                return;
        }

diff --git a/source3/smbd/smb2_flush.c b/source3/smbd/smb2_flush.c
index 2d3e6319697616a2d7fd3a0631ee3ce7f31653cc..3fec7908a46284e3f6c6452a016b0703e647ed2e 100644 (file)
--- a/source3/smbd/smb2_flush.c
+++ b/source3/smbd/smb2_flush.c
@@ -150,7 +150,8 @@ static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
                return tevent_req_post(req, ev);
        }
 
-       if (!CHECK_WRITE(fsp)) {
+       status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+       if (!NT_STATUS_IS_OK(status)) {
                bool allow_dir_flush = false;
                uint32_t flush_access = FILE_ADD_FILE | FILE_ADD_SUBDIRECTORY;
 

diff --git a/source3/smbd/smb2_write.c b/source3/smbd/smb2_write.c
index a7af20173a178bff159ecfc01f4faeccd2e7e500..2675997b0fdcccf7de8f3f786ac9de8e4b37f66d 100644 (file)
--- a/source3/smbd/smb2_write.c
+++ b/source3/smbd/smb2_write.c
@@ -24,6 +24,7 @@
 #include "../libcli/smb/smb_common.h"
 #include "../lib/util/tevent_ntstatus.h"
 #include "rpc_server/srv_pipe_hnd.h"
+#include "libcli/security/security.h"
 
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_SMB2
@@ -339,8 +340,9 @@ static struct tevent_req *smbd_smb2_write_send(TALLOC_CTX *mem_ctx,
                return req;
        }
 
-       if (!CHECK_WRITE(fsp)) {
-               tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
+       status = check_any_access_fsp(fsp, FILE_WRITE_DATA|FILE_APPEND_DATA);
+       if (!NT_STATUS_IS_OK(status)) {
+               tevent_req_nterror(req, status);
                return tevent_req_post(req, ev);
        }