1
0
mirror of https://github.com/samba-team/samba.git synced 2025-01-05 09:18:06 +03:00

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 995a31c8d4)
This commit is contained in:
Ralph Boehme 2023-12-20 18:32:25 +01:00 committed by Jule Anger
parent 15536403f6
commit 77a71bc993
5 changed files with 31 additions and 22 deletions

View File

@ -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) )

View File

@ -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

View File

@ -3626,8 +3626,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;
@ -3857,8 +3858,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;
}
@ -3992,8 +3994,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;
}
@ -4271,8 +4274,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;
}
@ -4983,6 +4987,7 @@ void reply_writeclose(struct smb_request *req)
struct timespec mtime;
files_struct *fsp;
struct lock_struct lock;
NTSTATUS status;
START_PROFILE(SMBwriteclose);
@ -4998,8 +5003,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;
}
@ -5799,6 +5805,7 @@ void reply_printwrite(struct smb_request *req)
int numtowrite;
const char *data;
files_struct *fsp;
NTSTATUS status;
START_PROFILE(SMBsplwr);
@ -5821,8 +5828,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;
}

View File

@ -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;

View File

@ -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);
}