[PATCH] Fix bug 12913 - SMBC_setatr() makes initially uses an SMB1 call before falling back
Jeremy Allison
jra at samba.org
Fri Jul 21 22:23:42 UTC 2017
This patchset fills in a hole we have in moving completely
from SMB1 -> SMB2+ in libsmbclient (will need back-porting
to 4.7.0).
Adds in the SMB2 backend to the previously SMB1-only
cli_setpathinfo_basic() call which is used by
libsmbclient (and thus the Gnome VFS). Also adds a proper
regression test to make sure the ABI won't change
in future.
Please review and push if happy !
Cheers,
Jeremy.
-------------- next part --------------
From 329aab444e230a7a88d365a3bf427980fec792ef Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 21 Jul 2017 09:56:45 -0700
Subject: [PATCH 1/4] s3: libsmbclient: Fix cli_setpathinfo_basic() to treat
mode == -1 as no change.
This is only called from SMBC_setatr(), so bring it into line with
the specification for that function.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12913
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/libsmb/clirap.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/source3/libsmb/clirap.c b/source3/libsmb/clirap.c
index dd2c30e2258..f897df63053 100644
--- a/source3/libsmb/clirap.c
+++ b/source3/libsmb/clirap.c
@@ -732,8 +732,17 @@ NTSTATUS cli_setpathinfo_basic(struct cli_state *cli, const char *fname,
put_long_date(p, change_time);
p += 8;
- /* Add attributes */
- SIVAL(p, 0, mode);
+ if (mode == (uint16_t)-1 || mode == FILE_ATTRIBUTE_NORMAL) {
+ /* No change. */
+ mode = 0;
+ } else if (mode == 0) {
+ /* Clear all existing attributes. */
+ mode = FILE_ATTRIBUTE_NORMAL;
+ }
+
+ /* Add attributes */
+ SIVAL(p, 0, mode);
+
p += 4;
/* Add padding */
--
2.14.0.rc0.284.gd933b75aa4-goog
From 31f4c169efde38cf6672a3b72e2d4e4652702d44 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 21 Jul 2017 12:41:11 -0700
Subject: [PATCH 2/4] s3: libsmb: Add cli_smb2_setpathinfo(), to be called by
cli_setpathinfo_basic().
Fix to prevent libsmbclient from accidently making SMB1 calls inside an SMB2
connection.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12913
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/libsmb/cli_smb2_fnum.c | 70 ++++++++++++++++++++++++++++++++++++++++++
source3/libsmb/cli_smb2_fnum.h | 5 +++
source3/libsmb/clirap.c | 14 +++++++++
3 files changed, 89 insertions(+)
diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
index 6967555797a..32c7c21bd6d 100644
--- a/source3/libsmb/cli_smb2_fnum.c
+++ b/source3/libsmb/cli_smb2_fnum.c
@@ -1645,6 +1645,75 @@ NTSTATUS cli_smb2_qpathinfo_streams(struct cli_state *cli,
return status;
}
+/***************************************************************
+ Wrapper that allows SMB2 to set SMB_FILE_BASIC_INFORMATION on
+ a pathname.
+ Synchronous only.
+***************************************************************/
+
+NTSTATUS cli_smb2_setpathinfo(struct cli_state *cli,
+ const char *name,
+ uint8_t in_info_type,
+ uint8_t in_file_info_class,
+ const DATA_BLOB *p_in_data)
+{
+ NTSTATUS status;
+ uint16_t fnum = 0xffff;
+ struct smb2_hnd *ph = NULL;
+ TALLOC_CTX *frame = talloc_stackframe();
+
+ if (smbXcli_conn_has_async_calls(cli->conn)) {
+ /*
+ * Can't use sync call while an async call is in flight
+ */
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto fail;
+ }
+
+ if (smbXcli_conn_protocol(cli->conn) < PROTOCOL_SMB2_02) {
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto fail;
+ }
+
+ status = get_fnum_from_path(cli,
+ name,
+ FILE_WRITE_ATTRIBUTES,
+ &fnum);
+
+ if (!NT_STATUS_IS_OK(status)) {
+ goto fail;
+ }
+
+ status = map_fnum_to_smb2_handle(cli,
+ fnum,
+ &ph);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto fail;
+ }
+
+ status = smb2cli_set_info(cli->conn,
+ cli->timeout,
+ cli->smb2.session,
+ cli->smb2.tcon,
+ in_info_type,
+ in_file_info_class,
+ p_in_data, /* in_input_buffer */
+ 0, /* in_additional_info */
+ ph->fid_persistent,
+ ph->fid_volatile);
+ fail:
+
+ if (fnum != 0xffff) {
+ cli_smb2_close_fnum(cli, fnum);
+ }
+
+ cli->raw_status = status;
+
+ TALLOC_FREE(frame);
+ return status;
+}
+
+
/***************************************************************
Wrapper that allows SMB2 to set pathname attributes.
Synchronous only.
@@ -1752,6 +1821,7 @@ NTSTATUS cli_smb2_setatr(struct cli_state *cli,
return status;
}
+
/***************************************************************
Wrapper that allows SMB2 to set file handle times.
Synchronous only.
diff --git a/source3/libsmb/cli_smb2_fnum.h b/source3/libsmb/cli_smb2_fnum.h
index 190ec59971b..8e240c6bc5b 100644
--- a/source3/libsmb/cli_smb2_fnum.h
+++ b/source3/libsmb/cli_smb2_fnum.h
@@ -114,6 +114,11 @@ NTSTATUS cli_smb2_qpathinfo_streams(struct cli_state *cli,
TALLOC_CTX *mem_ctx,
unsigned int *pnum_streams,
struct stream_struct **pstreams);
+NTSTATUS cli_smb2_setpathinfo(struct cli_state *cli,
+ const char *name,
+ uint8_t in_info_type,
+ uint8_t in_file_info_class,
+ const DATA_BLOB *p_in_data);
NTSTATUS cli_smb2_setatr(struct cli_state *cli,
const char *fname,
uint16_t attr,
diff --git a/source3/libsmb/clirap.c b/source3/libsmb/clirap.c
index f897df63053..e80dfc92a77 100644
--- a/source3/libsmb/clirap.c
+++ b/source3/libsmb/clirap.c
@@ -29,6 +29,7 @@
#include "libsmb/clirap.h"
#include "trans2.h"
#include "../libcli/smb/smbXcli_base.h"
+#include "cli_smb2_fnum.h"
#define PIPE_LANMAN "\\PIPE\\LANMAN"
@@ -751,6 +752,19 @@ NTSTATUS cli_setpathinfo_basic(struct cli_state *cli, const char *fname,
data_len = PTR_DIFF(p, data);
+ if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) {
+ DATA_BLOB in_data = data_blob_const(data, data_len);
+ /*
+ * Split out SMB2 here as we need to select
+ * the correct info type and level.
+ */
+ return cli_smb2_setpathinfo(cli,
+ fname,
+ 1, /* SMB2_SETINFO_FILE */
+ SMB_FILE_BASIC_INFORMATION - 1000,
+ &in_data);
+ }
+
return cli_setpathinfo(cli, SMB_FILE_BASIC_INFORMATION, fname,
(uint8_t *)data, data_len);
}
--
2.14.0.rc0.284.gd933b75aa4-goog
From 0fb1cb6020ba93e6672c95ba06ecd1ecd32de1e0 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 21 Jul 2017 12:46:23 -0700
Subject: [PATCH 3/4] s3: libsmb: Implement cli_smb2_setatr() by calling
cli_smb2_setpathinfo().
This removes duplicate code paths and ensures we have only one
function calling the underlying smb2cli_set_info() for setting
info levels by path.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12913
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/libsmb/cli_smb2_fnum.c | 57 ++++--------------------------------------
1 file changed, 5 insertions(+), 52 deletions(-)
diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
index 32c7c21bd6d..8ee97cad276 100644
--- a/source3/libsmb/cli_smb2_fnum.c
+++ b/source3/libsmb/cli_smb2_fnum.c
@@ -1724,41 +1724,8 @@ NTSTATUS cli_smb2_setatr(struct cli_state *cli,
uint16_t attr,
time_t mtime)
{
- NTSTATUS status;
- uint16_t fnum = 0xffff;
- struct smb2_hnd *ph = NULL;
uint8_t inbuf_store[40];
DATA_BLOB inbuf = data_blob_null;
- TALLOC_CTX *frame = talloc_stackframe();
-
- if (smbXcli_conn_has_async_calls(cli->conn)) {
- /*
- * Can't use sync call while an async call is in flight
- */
- status = NT_STATUS_INVALID_PARAMETER;
- goto fail;
- }
-
- if (smbXcli_conn_protocol(cli->conn) < PROTOCOL_SMB2_02) {
- status = NT_STATUS_INVALID_PARAMETER;
- goto fail;
- }
-
- status = get_fnum_from_path(cli,
- name,
- FILE_WRITE_ATTRIBUTES,
- &fnum);
-
- if (!NT_STATUS_IS_OK(status)) {
- goto fail;
- }
-
- status = map_fnum_to_smb2_handle(cli,
- fnum,
- &ph);
- if (!NT_STATUS_IS_OK(status)) {
- goto fail;
- }
/* setinfo on the handle with info_type SMB2_SETINFO_FILE (1),
level 4 (SMB_FILE_BASIC_INFORMATION - 1000). */
@@ -1799,26 +1766,12 @@ NTSTATUS cli_smb2_setatr(struct cli_state *cli,
SBVAL(inbuf.data, 8, 0xFFFFFFFFFFFFFFFFLL);
SBVAL(inbuf.data, 24, 0xFFFFFFFFFFFFFFFFLL);
- status = smb2cli_set_info(cli->conn,
- cli->timeout,
- cli->smb2.session,
- cli->smb2.tcon,
+ return cli_smb2_setpathinfo(cli,
+ name,
1, /* in_info_type */
- SMB_FILE_BASIC_INFORMATION - 1000, /* in_file_info_class */
- &inbuf, /* in_input_buffer */
- 0, /* in_additional_info */
- ph->fid_persistent,
- ph->fid_volatile);
- fail:
-
- if (fnum != 0xffff) {
- cli_smb2_close_fnum(cli, fnum);
- }
-
- cli->raw_status = status;
-
- TALLOC_FREE(frame);
- return status;
+ /* in_file_info_class */
+ SMB_FILE_BASIC_INFORMATION - 1000,
+ &inbuf);
}
--
2.14.0.rc0.284.gd933b75aa4-goog
From 5d801c61351960c108286e57ba3c74462e1a44bd Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 21 Jul 2017 15:11:08 -0700
Subject: [PATCH 4/4] s3: torture: Add a test for cli_setpathinfo_basic() to
smbtorture3.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12913
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/torture/torture.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 137 insertions(+)
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 6959b715b00..31e2bcc3497 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -3162,6 +3162,29 @@ static bool run_browsetest(int dummy)
}
+static bool check_attributes(struct cli_state *cli,
+ const char *fname,
+ uint16_t expected_attrs)
+{
+ uint16_t attrs = 0;
+ NTSTATUS status = cli_getatr(cli,
+ fname,
+ &attrs,
+ NULL,
+ NULL);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_getatr failed with %s\n",
+ nt_errstr(status));
+ return false;
+ }
+ if (attrs != expected_attrs) {
+ printf("Attributes incorrect 0x%x, should be 0x%x\n",
+ (unsigned int)attrs,
+ (unsigned int)expected_attrs);
+ return false;
+ }
+ return true;
+}
/*
This checks how the getatr calls works
@@ -3222,6 +3245,120 @@ static bool run_attrtest(int dummy)
cli_unlink(cli, fname, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+ /* Check cli_setpathinfo_basic() */
+ /* Re-create the file. */
+ status = cli_openx(cli, fname,
+ O_RDWR | O_CREAT | O_TRUNC, DENY_NONE, &fnum);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("Failed to recreate %s (%s)\n",
+ fname, nt_errstr(status));
+ correct = false;
+ }
+ cli_close(cli, fnum);
+
+ status = cli_setpathinfo_basic(cli,
+ fname,
+ 0, /* create */
+ 0, /* access */
+ 0, /* write */
+ 0, /* change */
+ FILE_ATTRIBUTE_SYSTEM |
+ FILE_ATTRIBUTE_HIDDEN |
+ FILE_ATTRIBUTE_READONLY);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_setpathinfo_basic failed with %s\n",
+ nt_errstr(status));
+ correct = false;
+ }
+
+ /* Check attributes are correct. */
+ correct = check_attributes(cli,
+ fname,
+ FILE_ATTRIBUTE_SYSTEM |
+ FILE_ATTRIBUTE_HIDDEN |
+ FILE_ATTRIBUTE_READONLY);
+ if (correct == false) {
+ goto out;
+ }
+
+ /* Setting to FILE_ATTRIBUTE_NORMAL should be ignored. */
+ status = cli_setpathinfo_basic(cli,
+ fname,
+ 0, /* create */
+ 0, /* access */
+ 0, /* write */
+ 0, /* change */
+ FILE_ATTRIBUTE_NORMAL);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_setpathinfo_basic failed with %s\n",
+ nt_errstr(status));
+ correct = false;
+ }
+
+ /* Check attributes are correct. */
+ correct = check_attributes(cli,
+ fname,
+ FILE_ATTRIBUTE_SYSTEM |
+ FILE_ATTRIBUTE_HIDDEN |
+ FILE_ATTRIBUTE_READONLY);
+ if (correct == false) {
+ goto out;
+ }
+
+ /* Setting to (uint16_t)-1 should also be ignored. */
+ status = cli_setpathinfo_basic(cli,
+ fname,
+ 0, /* create */
+ 0, /* access */
+ 0, /* write */
+ 0, /* change */
+ (uint16_t)-1);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_setpathinfo_basic failed with %s\n",
+ nt_errstr(status));
+ correct = false;
+ }
+
+ /* Check attributes are correct. */
+ correct = check_attributes(cli,
+ fname,
+ FILE_ATTRIBUTE_SYSTEM |
+ FILE_ATTRIBUTE_HIDDEN |
+ FILE_ATTRIBUTE_READONLY);
+ if (correct == false) {
+ goto out;
+ }
+
+ /* Setting to 0 should clear them all. */
+ status = cli_setpathinfo_basic(cli,
+ fname,
+ 0, /* create */
+ 0, /* access */
+ 0, /* write */
+ 0, /* change */
+ 0);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_setpathinfo_basic failed with %s\n",
+ nt_errstr(status));
+ correct = false;
+ }
+
+ /* Check attributes are correct. */
+ correct = check_attributes(cli,
+ fname,
+ FILE_ATTRIBUTE_NORMAL);
+ if (correct == false) {
+ goto out;
+ }
+
+ out:
+
+ cli_unlink(cli,
+ fname,
+ FILE_ATTRIBUTE_SYSTEM |
+ FILE_ATTRIBUTE_HIDDEN|
+ FILE_ATTRIBUTE_READONLY);
+
if (!torture_close_connection(cli)) {
correct = False;
}
--
2.14.0.rc0.284.gd933b75aa4-goog
More information about the samba-technical
mailing list