From e12f6576244cea30d9999437503de1cad36c7833 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 20 Mar 2018 15:27:44 +0100 Subject: [PATCH 1/3] s3: libsmb: use smb2cli_conn_max_trans_size() in cli_smb2_list() BUG: https://bugzilla.samba.org/show_bug.cgi?id=13736 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher (cherry picked from commit 580ff206431969dc2924d520053b956b7169ca07) --- source3/libsmb/cli_smb2_fnum.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c index 7192521e38a..3cf16b325dd 100644 --- a/source3/libsmb/cli_smb2_fnum.c +++ b/source3/libsmb/cli_smb2_fnum.c @@ -911,6 +911,7 @@ NTSTATUS cli_smb2_list(struct cli_state *cli, TALLOC_CTX *frame = talloc_stackframe(); TALLOC_CTX *subframe = NULL; bool mask_has_wild; + uint32_t max_trans = smb2cli_conn_max_trans_size(cli->conn); if (smbXcli_conn_has_async_calls(cli->conn)) { /* @@ -974,7 +975,7 @@ NTSTATUS cli_smb2_list(struct cli_state *cli, ph->fid_persistent, ph->fid_volatile, mask, - 0xffff, + max_trans, subframe, &dir_data, &dir_data_length); -- 2.20.1.321.g9e740568ce-goog From 04e712f285b1b0490a0f501375ba3aab79a07f31 Mon Sep 17 00:00:00 2001 From: Tim Beale Date: Mon, 7 Jan 2019 12:06:15 +1300 Subject: [PATCH 2/3] libcli: Add error log if insufficient SMB2 credits Although it's unusual to hit this case, I was seeing it happen while working on the SMB python bindings. Even with debug level 10, there was nothing coming out to help pin down the source of the NT_STATUS_INTERNAL_ERROR. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13736 Signed-off-by: Tim Beale Reviewed-by: Stefan Metzmacher (cherry picked from commit bf229de7926f12e329cdb3201f68f20ae776fe32) --- libcli/smb/smbXcli_base.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c index 40480c83aa0..a237bf17d0a 100644 --- a/libcli/smb/smbXcli_base.c +++ b/libcli/smb/smbXcli_base.c @@ -3231,6 +3231,9 @@ NTSTATUS smb2cli_req_compound_submit(struct tevent_req **reqs, avail = MIN(avail, state->conn->smb2.cur_credits); if (avail < charge) { + DBG_ERR("Insufficient credits. " + "%"PRIu64" available, %"PRIu16" needed\n", + avail, charge); return NT_STATUS_INTERNAL_ERROR; } -- 2.20.1.321.g9e740568ce-goog From 94b1222b8561c85d91806f1b882f5206e73a4321 Mon Sep 17 00:00:00 2001 From: Tim Beale Date: Mon, 7 Jan 2019 15:28:12 +1300 Subject: [PATCH 3/3] s3:libsmb: cli_smb2_list() can sometimes fail initially on a connection cli_smb2_list() appears to be a slightly unique SMB operation in that it specifies the max transaction size for the response buffer size. The Python bindings highlighted a problem where if cli_smb2_list() were one of the first operations performed on the SMBv2 connection, it would fail due to insufficient credits. Because the response buffer size is (potentially) so much larger, it requires more credits (128) compared with other SMB operations. When talking to a samba DC, the connection credits seem to start off at 1, then increase by 32 for every SMB reply we receive back from the server. After cli_full_connection(), the connection has 65 credits. The cli_smb2_create_fnum() in cli_smb2_list() adds another 32 credits, but this is still less than the 128 that smb2cli_query_directory() requires. This problem doesn't happen for smbclient because the cli_cm_open() API it uses ends up sending more messages, and so the connection has more credits. This patch changes cli_smb2_list(), so it requests a smaller response buffer size if it doesn't have enough credits available for the max transaction size. smb2cli_query_directory() is already in a loop, so it can span multiple SMB messages if for some reason the transaction size isn't big enough for the listings. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13736 Signed-off-by: Tim Beale Reviewed-by: Stefan Metzmacher Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Thu Jan 10 02:40:16 CET 2019 on sn-devel-144 (cherry picked from commit fd355dff906f5f4832901bce76544f1a4e50c33d) --- source3/libsmb/cli_smb2_fnum.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c index 3cf16b325dd..1cfa50ffbac 100644 --- a/source3/libsmb/cli_smb2_fnum.c +++ b/source3/libsmb/cli_smb2_fnum.c @@ -911,7 +911,9 @@ NTSTATUS cli_smb2_list(struct cli_state *cli, TALLOC_CTX *frame = talloc_stackframe(); TALLOC_CTX *subframe = NULL; bool mask_has_wild; - uint32_t max_trans = smb2cli_conn_max_trans_size(cli->conn); + uint32_t max_trans; + uint32_t max_avail_len; + bool ok; if (smbXcli_conn_has_async_calls(cli->conn)) { /* @@ -959,6 +961,16 @@ NTSTATUS cli_smb2_list(struct cli_state *cli, goto fail; } + /* + * ideally, use the max transaction size, but don't send a request + * bigger than we have credits available for + */ + max_trans = smb2cli_conn_max_trans_size(cli->conn); + ok = smb2cli_conn_req_possible(cli->conn, &max_avail_len); + if (ok) { + max_trans = MIN(max_trans, max_avail_len); + } + do { uint8_t *dir_data = NULL; uint32_t dir_data_length = 0; -- 2.20.1.321.g9e740568ce-goog