[Samba] Recvfile patch issue with windows.
Jones
jones.kstw at gmail.com
Fri May 30 00:41:10 MDT 2014
Hello Bglr,
2014-05-27 19:04 GMT+08:00 sp bglr <spbglr666 at gmail.com>:
> I have a recvfile patch for samba4.1. This patch is working with linux
> client, But if am trying this with windows client am getting following
> error in my server.
>
> " Unexpected RPC Fragment size! (0)"
>
> In client side am getting "Remote procedure call failed and did not
> execute"
>
I saw this symptom once on my linux box,
after applying the attached patch this symptom is resolved,
this attached is came from Metze's more recvfile optimizations.
Client OS is windows 7,
can reproduce this symptom with following sort on my linux box,
1) samba-4.1.7 and custom kernel patch like
https://lkml.org/lkml/2013/7/23/37
2) max protocol = SMB2
3) min receivefile size = 1
Earlier days wo/ Metze's patch,
found 2 methods to way around this symptom:
A) max protocol = NT1
Looks like been suffered if with SMB >= 2.0,
hence use NT1 instead.
B) min receivefile size = 8192
8192 is going to cover 4280 to avoid recvfile() involved,
see comment on samba-4.1.7/source3/libsmb/cli_np_tstream.c
''Windows uses 4280 (the max xmit/recv size negotiated on DCERPC)."
Hmm perhaps could try this attached patch, thanks.
--
Regards,
Jones
-------------- next part --------------
From f71780202a3e736432020cf894b138a06e4bb869 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 5 Dec 2013 11:20:49 +0100
Subject: [PATCH 1/9] s3:lib: use stack buffers in drain_socket() and
default_sys_recvfile()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/lib/recvfile.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/source3/lib/recvfile.c b/source3/lib/recvfile.c
index 72f4257..500a7e4 100644
--- a/source3/lib/recvfile.c
+++ b/source3/lib/recvfile.c
@@ -51,7 +51,7 @@ static ssize_t default_sys_recvfile(int fromfd,
size_t total = 0;
size_t bufsize = MIN(TRANSFER_BUF_SIZE,count);
size_t total_written = 0;
- char *buffer = NULL;
+ char buffer[bufsize];
DEBUG(10,("default_sys_recvfile: from = %d, to = %d, "
"offset=%.0f, count = %lu\n",
@@ -70,11 +70,6 @@ static ssize_t default_sys_recvfile(int fromfd,
}
}
- buffer = SMB_MALLOC_ARRAY(char, bufsize);
- if (buffer == NULL) {
- return -1;
- }
-
while (total < count) {
size_t num_written = 0;
ssize_t read_ret;
@@ -84,7 +79,6 @@ static ssize_t default_sys_recvfile(int fromfd,
read_ret = sys_read(fromfd, buffer, toread);
if (read_ret <= 0) {
/* EOF or socket error. */
- free(buffer);
return -1;
}
@@ -119,7 +113,6 @@ static ssize_t default_sys_recvfile(int fromfd,
total += read_ret;
}
- free(buffer);
if (saved_errno) {
/* Return the correct write error. */
errno = saved_errno;
@@ -247,21 +240,15 @@ ssize_t drain_socket(int sockfd, size_t count)
{
size_t total = 0;
size_t bufsize = MIN(TRANSFER_BUF_SIZE,count);
- char *buffer = NULL;
+ char buffer[bufsize];
int old_flags = 0;
if (count == 0) {
return 0;
}
- buffer = SMB_MALLOC_ARRAY(char, bufsize);
- if (buffer == NULL) {
- return -1;
- }
-
old_flags = fcntl(sockfd, F_GETFL, 0);
if (set_blocking(sockfd, true) == -1) {
- free(buffer);
return -1;
}
@@ -281,7 +268,6 @@ ssize_t drain_socket(int sockfd, size_t count)
out:
- free(buffer);
if (fcntl(sockfd, F_SETFL, old_flags) == -1) {
return -1;
}
--
1.8.3.4
From 8d45b75df31d5c382345186791a375e2b800066e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 11 Apr 2014 00:29:48 +0200
Subject: [PATCH 2/9] s3:smbXsrv_open: allow now==0 to skip the idle_time
update.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smbXsrv_open.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/smbXsrv_open.c b/source3/smbd/smbXsrv_open.c
index b15be28..c3ff3bb 100644
--- a/source3/smbd/smbXsrv_open.c
+++ b/source3/smbd/smbXsrv_open.c
@@ -444,7 +444,9 @@ static NTSTATUS smbXsrv_open_local_lookup(struct smbXsrv_open_table *table,
return NT_STATUS_FILE_CLOSED;
}
- state.op->idle_time = now;
+ if (now != 0) {
+ state.op->idle_time = now;
+ }
*_open = state.op;
return state.op->status;
--
1.8.3.4
From 77b6860668437bbb3bb66f57f75ea4fc378de49b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 11 Apr 2014 00:51:32 +0200
Subject: [PATCH 3/9] s3:smbd: use smb1srv_open_lookup() in
is_valid_writeX_buffer()
It's more logical to check the fnum instead of tid here.
This will make it easier to reuse the logic for SMB2 and
allows per fsp recvfile detection.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/reply.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index e58735e..da59ca7 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -4598,12 +4598,12 @@ bool is_valid_writeX_buffer(struct smbd_server_connection *sconn,
const uint8_t *inbuf)
{
size_t numtowrite;
- connection_struct *conn = NULL;
unsigned int doff = 0;
size_t len = smb_len_large(inbuf);
- struct smbXsrv_tcon *tcon;
+ uint16_t fnum;
+ struct smbXsrv_open *op = NULL;
+ struct files_struct *fsp = NULL;
NTSTATUS status;
- NTTIME now = 0;
if (is_encrypted_packet(sconn, inbuf)) {
/* Can't do this on encrypted
@@ -4622,19 +4622,30 @@ bool is_valid_writeX_buffer(struct smbd_server_connection *sconn,
return false;
}
- status = smb1srv_tcon_lookup(sconn->conn, SVAL(inbuf, smb_tid),
- now, &tcon);
+ fnum = SVAL(inbuf, smb_vwv2);
+ status = smb1srv_open_lookup(sconn->conn,
+ fnum,
+ 0, /* now */
+ &op);
if (!NT_STATUS_IS_OK(status)) {
- DEBUG(10,("is_valid_writeX_buffer: bad tid\n"));
+ DEBUG(10,("is_valid_writeX_buffer: bad fnum\n"));
+ return false;
+ }
+ fsp = op->compat;
+ if (fsp == NULL) {
+ DEBUG(10,("is_valid_writeX_buffer: bad fsp\n"));
+ return false;
+ }
+ if (fsp->conn == NULL) {
+ DEBUG(10,("is_valid_writeX_buffer: bad fsp->conn\n"));
return false;
}
- conn = tcon->compat;
- if (IS_IPC(conn)) {
+ if (IS_IPC(fsp->conn)) {
DEBUG(10,("is_valid_writeX_buffer: IPC$ tid\n"));
return false;
}
- if (IS_PRINT(conn)) {
+ if (IS_PRINT(fsp->conn)) {
DEBUG(10,("is_valid_writeX_buffer: printing tid\n"));
return false;
}
--
1.8.3.4
From 796874912d9935e202b8c767ca33d557dce9c43e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 11 Apr 2014 00:43:46 +0200
Subject: [PATCH 4/9] s3:smb2_server: use the same logic to avoid recvfile()
for IPC/PRINT shares
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smb2_server.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 3c46efd..13626f3 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -2841,7 +2841,19 @@ static size_t get_min_receive_file_size(struct smbd_smb2_request *smb2_req)
static bool is_smb2_recvfile_write(struct smbd_smb2_request_read_state *state)
{
+ NTSTATUS status;
uint32_t flags;
+ uint64_t file_id_persistent;
+ uint64_t file_id_volatile;
+ struct smbXsrv_open *op = NULL;
+ struct files_struct *fsp = NULL;
+ const uint8_t *body = NULL;
+
+ /*
+ * This is only called with a pktbuf
+ * of at least SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN
+ * bytes
+ */
if (IVAL(state->pktbuf, 0) == SMB2_TF_MAGIC) {
/* Transform header. Cannot recvfile. */
@@ -2873,6 +2885,35 @@ static bool is_smb2_recvfile_write(struct smbd_smb2_request_read_state *state)
return false;
}
+ body = &state->pktbuf[SMB2_HDR_BODY];
+
+ file_id_persistent = BVAL(body, 0x10);
+ file_id_volatile = BVAL(body, 0x18);
+
+ status = smb2srv_open_lookup(state->req->sconn->conn,
+ file_id_persistent,
+ file_id_volatile,
+ 0, /* now */
+ &op);
+ if (!NT_STATUS_IS_OK(status)) {
+ return false;
+ }
+
+ fsp = op->compat;
+ if (fsp == NULL) {
+ return false;
+ }
+ if (fsp->conn == NULL) {
+ return false;
+ }
+
+ if (IS_IPC(fsp->conn)) {
+ return false;
+ }
+ if (IS_PRINT(fsp->conn)) {
+ return false;
+ }
+
DEBUG(10,("Doing recvfile write len = %u\n",
(unsigned int)(state->pktlen -
SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN)));
--
1.8.3.4
From 95df39b99f17810cb230c4a6a0d0952919cd0b81 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 11 Apr 2014 01:05:21 +0200
Subject: [PATCH 5/9] s3:smb2_server: make sure we don't try recvfile for
special NBT messages
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smb2_server.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 13626f3..4949fd2 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -3271,6 +3271,9 @@ again:
/*
* Now we analyze the NBT header
*/
+ if (state->hdr.nbt[0] != 0x00) {
+ state->min_recv_size = 0;
+ }
state->pktlen = smb2_len(state->hdr.nbt);
if (state->pktlen == 0) {
goto got_full;
--
1.8.3.4
From 1f767b29a89e15c4d7187cd0bc9b0c2e3152edd7 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 18 Nov 2013 13:45:37 +0100
Subject: [PATCH 6/9] s3:smb2_write: allow SMBD_SMB2_IN_DYN_LEN() to be 0 for
the recvfile case.
For recvfile we haven't read and may not allocated the dyn buffer.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smb2_write.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/source3/smbd/smb2_write.c b/source3/smbd/smb2_write.c
index 4e138fe..c61254f 100644
--- a/source3/smbd/smb2_write.c
+++ b/source3/smbd/smb2_write.c
@@ -48,6 +48,8 @@ NTSTATUS smbd_smb2_request_process_write(struct smbd_smb2_request *req)
uint64_t in_file_id_volatile;
struct files_struct *in_fsp;
uint32_t in_flags;
+ size_t in_dyn_len = 0;
+ uint8_t *in_dyn_ptr = NULL;
struct tevent_req *subreq;
status = smbd_smb2_request_verify_sizes(req, 0x31);
@@ -67,7 +69,15 @@ NTSTATUS smbd_smb2_request_process_write(struct smbd_smb2_request *req)
return smbd_smb2_request_error(req, NT_STATUS_INVALID_PARAMETER);
}
- if (in_data_length > SMBD_SMB2_IN_DYN_LEN(req)) {
+ if (req->smb1req != NULL && req->smb1req->unread_bytes > 0) {
+ in_dyn_ptr = NULL;
+ in_dyn_len = req->smb1req->unread_bytes;
+ } else {
+ in_dyn_ptr = SMBD_SMB2_IN_DYN_PTR(req);
+ in_dyn_len = SMBD_SMB2_IN_DYN_LEN(req);
+ }
+
+ if (in_data_length > in_dyn_len) {
return smbd_smb2_request_error(req, NT_STATUS_INVALID_PARAMETER);
}
@@ -79,7 +89,10 @@ NTSTATUS smbd_smb2_request_process_write(struct smbd_smb2_request *req)
return smbd_smb2_request_error(req, NT_STATUS_INVALID_PARAMETER);
}
- in_data_buffer.data = SMBD_SMB2_IN_DYN_PTR(req);
+ /*
+ * Note: that in_dyn_ptr is NULL for the recvfile case.
+ */
+ in_data_buffer.data = in_dyn_ptr;
in_data_buffer.length = in_data_length;
status = smbd_smb2_request_verify_creditcharge(req, in_data_length);
@@ -340,6 +353,9 @@ static struct tevent_req *smbd_smb2_write_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
+ /*
+ * Note: in_data.data is NULL for the recvfile case.
+ */
nwritten = write_file(smbreq, fsp,
(const char *)in_data.data,
in_offset,
--
1.8.3.4
From 80de72bb57520a1a14c3d3db3f31dc588d0afd64 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 18 Nov 2013 13:46:10 +0100
Subject: [PATCH 7/9] s3:smb2_server: prepare smbd_smb2_request_verify_sizes()
for the optimized recvfile() case
For recvfile we haven't read and may not allocated the dyn buffer.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smb2_server.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 4949fd2..0e42b7f 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -1870,6 +1870,15 @@ NTSTATUS smbd_smb2_request_verify_sizes(struct smbd_smb2_request *req,
case SMB2_OP_GETINFO:
min_dyn_size = 0;
break;
+ case SMB2_OP_WRITE:
+ if (req->smb1req != NULL && req->smb1req->unread_bytes > 0) {
+ if (req->smb1req->unread_bytes < min_dyn_size) {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ min_dyn_size = 0;
+ }
+ break;
}
/*
--
1.8.3.4
From 535103e7a38e175fec082a658c2b70dbdd9af33e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 15 Nov 2013 09:12:40 +0100
Subject: [PATCH 8/9] s3:smb2_server: only allocate the required buffer in the
smb2 recvfile() code path
This way the buffer will likely be allocated within the existing talloc_pool,
which avoids one malloc() per request.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/globals.h | 1 +
source3/smbd/smb2_server.c | 48 ++++++++++++++++++++++++++++------------------
2 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index 3baa048..cd99fe7 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -768,6 +768,7 @@ struct smbd_server_connection {
struct iovec vector;
bool doing_receivefile;
size_t min_recv_size;
+ size_t pktfull;
size_t pktlen;
uint8_t *pktbuf;
} request_read_state;
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 0e42b7f..4f69679 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -2924,8 +2924,7 @@ static bool is_smb2_recvfile_write(struct smbd_smb2_request_read_state *state)
}
DEBUG(10,("Doing recvfile write len = %u\n",
- (unsigned int)(state->pktlen -
- SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN)));
+ (unsigned int)(state->pktfull - state->pktlen)));
return true;
}
@@ -3263,10 +3262,21 @@ again:
* Read the rest of the data.
*/
state->doing_receivefile = false;
+
+ state->pktbuf = talloc_realloc(state->req,
+ state->pktbuf,
+ uint8_t,
+ state->pktfull);
+ if (state->pktbuf == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
state->vector.iov_base = (void *)(state->pktbuf +
- SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN);
- state->vector.iov_len = (state->pktlen -
- SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN);
+ state->pktlen);
+ state->vector.iov_len = (state->pktfull -
+ state->pktlen);
+
+ state->pktlen = state->pktfull;
goto again;
}
@@ -3283,24 +3293,17 @@ again:
if (state->hdr.nbt[0] != 0x00) {
state->min_recv_size = 0;
}
- state->pktlen = smb2_len(state->hdr.nbt);
- if (state->pktlen == 0) {
+ state->pktfull = smb2_len(state->hdr.nbt);
+ if (state->pktfull == 0) {
goto got_full;
}
- state->pktbuf = talloc_array(state->req, uint8_t, state->pktlen);
- if (state->pktbuf == NULL) {
- return NT_STATUS_NO_MEMORY;
- }
-
- state->vector.iov_base = (void *)state->pktbuf;
-
if (state->min_recv_size != 0) {
min_recvfile_size = SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN;
min_recvfile_size += state->min_recv_size;
}
- if (state->pktlen > min_recvfile_size) {
+ if (state->pktfull > min_recvfile_size) {
/*
* Might be a receivefile write. Read the SMB2 HEADER +
* SMB2_WRITE header first. Set 'doing_receivefile'
@@ -3309,12 +3312,20 @@ again:
* not suitable then we'll just read the rest of the data
* the next time this function is called.
*/
- state->vector.iov_len = SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN;
+ state->pktlen = SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN;
state->doing_receivefile = true;
} else {
- state->vector.iov_len = state->pktlen;
+ state->pktlen = state->pktfull;
}
+ state->pktbuf = talloc_array(state->req, uint8_t, state->pktlen);
+ if (state->pktbuf == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ state->vector.iov_base = (void *)state->pktbuf;
+ state->vector.iov_len = state->pktlen;
+
goto again;
got_full:
@@ -3353,8 +3364,7 @@ got_full:
if (req->smb1req == NULL) {
return NT_STATUS_NO_MEMORY;
}
- req->smb1req->unread_bytes =
- state->pktlen - SMBD_SMB2_SHORT_RECEIVEFILE_WRITE_LEN;
+ req->smb1req->unread_bytes = state->pktfull - state->pktlen;
}
ZERO_STRUCTP(state);
--
1.8.3.4
From e49bae7ac9fb0be4cbb4530d75a4431cbbb495c8 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 11 Apr 2014 01:37:42 +0200
Subject: [PATCH 9/9] s3:smb2_server: remove unused get_min_receive_file_size()
wrapper function
smb2req always comes from talloc_zero().
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Autobuild-User(master): Jeremy Allison <jra at samba.org>
Autobuild-Date(master): Fri Apr 11 23:55:17 CEST 2014 on sn-devel-104
---
source3/smbd/smb2_server.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 4f69679..4a7abcb 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -2837,17 +2837,6 @@ NTSTATUS smbd_smb2_send_oplock_break(struct smbd_server_connection *sconn,
return NT_STATUS_OK;
}
-static size_t get_min_receive_file_size(struct smbd_smb2_request *smb2_req)
-{
- if (smb2_req->do_signing) {
- return 0;
- }
- if (smb2_req->do_encryption) {
- return 0;
- }
- return (size_t)lp_min_receive_file_size();
-}
-
static bool is_smb2_recvfile_write(struct smbd_smb2_request_read_state *state)
{
NTSTATUS status;
@@ -2969,7 +2958,7 @@ static NTSTATUS smbd_smb2_request_next_incoming(struct smbd_server_connection *s
return NT_STATUS_NO_MEMORY;
}
state->req->sconn = sconn;
- state->min_recv_size = get_min_receive_file_size(state->req);
+ state->min_recv_size = lp_min_receive_file_size();
TEVENT_FD_READABLE(sconn->smb2.fde);
@@ -3337,7 +3326,7 @@ got_full:
req = state->req;
ZERO_STRUCTP(state);
state->req = req;
- state->min_recv_size = get_min_receive_file_size(state->req);
+ state->min_recv_size = lp_min_receive_file_size();
req = NULL;
goto again;
}
--
1.8.3.4
More information about the samba
mailing list