[PATCH] Fix for bug 11522: smbd can't create or open a stream name on the root directory of a share.

Jeremy Allison jra at samba.org
Thu Sep 17 23:32:42 UTC 2015


On Thu, Sep 17, 2015 at 11:23:08AM -0700, Jeremy Allison wrote:
> 
> OK. Might be that the smbclient test which is run
> before samba3.base.dir2 leaves the :foobar stream
> on the base directory of the [tmp] share and doesn't
> get deleted (there's no stream delete in smbclient).
> Still shouldn't be visible to a old SMB1 directory
> list though. I might try replacing the smbclient
> test with a smbtorture test that explicitly deletes
> the stream on the toplevel share directory.

Nailed it ! When the smbclient test that adds
the :foobar stream on the base of the [tmp]
share runs, it adds the 'A' (ARCHIVE) attribute
to the directory as the directory has been
changed and thus would need backup.

Then when samba3.base.dir2 runs later it finds
the ARCHIVE attribute on the '..' directory and
so adds in that extra file to the list of files
it sees in the 'old' SMB1search request.

Phew, that took a lot of finding :-).

So the test needs to remove the 'A' bit
from the root of the share after creating
:foobar - using the 'setmode \ -a' command,
which can only be done on the root of the
share over SMB2.

Whilst making *that* work I discovered
the 'setmode' command in smbclient had
been moved from client/client.c into
client/clitar.c - which means it only
gets linked in if libarchive-dev has
been installed on the system. That's
not right - it should be (and always
used to be) part of the core smbclient
code :-(.

So here is the complete patchset for
your re-review.

1). Move the 'setmode' command from
clitar.c to client.c

2). Code tidyup (unchanged, already reviewed).

3). Actual bugfix ! (unchanged, already reviewed).

4). Regression test that adds the setmode -a
command after creating :foobar.

Sorry for the trouble Ralph. Please
re-review the attached (passes the two
tests run concurrently here :-).

Cheers,

	Jeremy.
-------------- next part --------------
From f230a9a54848f7b1602b7d0f38207a55303b2584 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 17 Sep 2015 15:54:40 -0700
Subject: [PATCH 1/4] s3: smbclient: Move cmd_setmode out of clitar.c and back
 into client.c

setmode <file> attribute is a valid smbclient command even if libarchive
isn't on the system and tarmode isn't compiled in.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/client/client.c       | 118 ++++++++++++++++++++++++++++++++++++++
 source3/client/client_proto.h |   6 ++
 source3/client/clitar.c       | 129 ------------------------------------------
 3 files changed, 124 insertions(+), 129 deletions(-)

diff --git a/source3/client/client.c b/source3/client/client.c
index f49f3ff..034b48a 100644
--- a/source3/client/client.c
+++ b/source3/client/client.c
@@ -4656,6 +4656,124 @@ static int cmd_show_connect( void )
 	return 0;
 }
 
+/**
+ * set_remote_attr - set DOS attributes of a remote file
+ * @filename: path to the file name
+ * @new_attr: attribute bit mask to use
+ * @mode: one of ATTR_SET or ATTR_UNSET
+ *
+ * Update the file attributes with the one provided.
+ */
+int set_remote_attr(const char *filename, uint16_t new_attr, int mode)
+{
+	extern struct cli_state *cli;
+	uint16_t old_attr;
+	NTSTATUS status;
+
+	status = cli_getatr(cli, filename, &old_attr, NULL, NULL);
+	if (!NT_STATUS_IS_OK(status)) {
+		d_printf("cli_getatr failed: %s\n", nt_errstr(status));
+		return 1;
+	}
+
+	if (mode == ATTR_SET) {
+		new_attr |= old_attr;
+	} else {
+		new_attr = old_attr & ~new_attr;
+	}
+
+	status = cli_setatr(cli, filename, new_attr, 0);
+	if (!NT_STATUS_IS_OK(status)) {
+		d_printf("cli_setatr failed: %s\n", nt_errstr(status));
+		return 1;
+	}
+
+	return 0;
+}
+
+/**
+ * cmd_setmode - interactive command to set DOS attributes
+ *
+ * Read a filename and mode from the client command line and update
+ * the file DOS attributes.
+ */
+int cmd_setmode(void)
+{
+	const extern char *cmd_ptr;
+	char *buf;
+	char *fname = NULL;
+	uint16_t attr[2] = {0};
+	int mode = ATTR_SET;
+	int err = 0;
+	bool ok;
+	TALLOC_CTX *ctx = talloc_new(NULL);
+	if (ctx == NULL) {
+		return 1;
+	}
+
+	ok = next_token_talloc(ctx, &cmd_ptr, &buf, NULL);
+	if (!ok) {
+		d_printf("setmode <filename> <[+|-]rsha>\n");
+		err = 1;
+		goto out;
+	}
+
+	fname = talloc_asprintf(ctx,
+				"%s%s",
+				client_get_cur_dir(),
+				buf);
+	if (fname == NULL) {
+		err = 1;
+		goto out;
+	}
+
+	while (next_token_talloc(ctx, &cmd_ptr, &buf, NULL)) {
+		const char *s = buf;
+
+		while (*s) {
+			switch (*s++) {
+			case '+':
+				mode = ATTR_SET;
+				break;
+			case '-':
+				mode = ATTR_UNSET;
+				break;
+			case 'r':
+				attr[mode] |= FILE_ATTRIBUTE_READONLY;
+				break;
+			case 'h':
+				attr[mode] |= FILE_ATTRIBUTE_HIDDEN;
+				break;
+			case 's':
+				attr[mode] |= FILE_ATTRIBUTE_SYSTEM;
+				break;
+			case 'a':
+				attr[mode] |= FILE_ATTRIBUTE_ARCHIVE;
+				break;
+			default:
+				d_printf("setmode <filename> <perm=[+|-]rsha>\n");
+				err = 1;
+				goto out;
+			}
+		}
+	}
+
+	if (attr[ATTR_SET] == 0 && attr[ATTR_UNSET] == 0) {
+		d_printf("setmode <filename> <[+|-]rsha>\n");
+		err = 1;
+		goto out;
+	}
+
+	DEBUG(2, ("perm set %d %d\n", attr[ATTR_SET], attr[ATTR_UNSET]));
+
+	/* ignore return value: server might not store DOS attributes */
+	set_remote_attr(fname, attr[ATTR_SET], ATTR_SET);
+	set_remote_attr(fname, attr[ATTR_UNSET], ATTR_UNSET);
+out:
+	talloc_free(ctx);
+	return err;
+}
+
 /****************************************************************************
  iosize command
 ***************************************************************************/
diff --git a/source3/client/client_proto.h b/source3/client/client_proto.h
index 86f1d18..d3d4036 100644
--- a/source3/client/client_proto.h
+++ b/source3/client/client_proto.h
@@ -26,6 +26,11 @@
 struct cli_state;
 struct file_info;
 
+enum {
+        ATTR_UNSET,
+        ATTR_SET,
+};
+
 /* The following definitions come from client/client.c  */
 
 const char *client_get_cur_dir(void);
@@ -36,6 +41,7 @@ NTSTATUS do_list(const char *mask,
 				   const char *dir),
 			bool rec,
 			bool dirs);
+int set_remote_attr(const char *filename, uint16_t new_attr, int mode);
 int cmd_iosize(void);
 
 /* The following definitions come from client/dnsbrowse.c  */
diff --git a/source3/client/clitar.c b/source3/client/clitar.c
index 5561845..7eb3fa0 100644
--- a/source3/client/clitar.c
+++ b/source3/client/clitar.c
@@ -121,11 +121,6 @@ enum tar_selection {
 	TAR_EXCLUDE,       /* X flag */
 };
 
-enum {
-	ATTR_UNSET,
-	ATTR_SET,
-};
-
 struct tar {
 	TALLOC_CTX *talloc_ctx;
 
@@ -216,7 +211,6 @@ static int make_remote_path(const char *full_path);
 static int max_token (const char *str);
 static NTSTATUS is_subpath(const char *sub, const char *full,
 			   bool *_subpath_match);
-static int set_remote_attr(const char *filename, uint16_t new_attr, int mode);
 
 /**
  * tar_get_ctx - retrieve global tar context handle
@@ -383,88 +377,6 @@ out:
 	return err;
 }
 
-/**
- * cmd_setmode - interactive command to set DOS attributes
- *
- * Read a filename and mode from the client command line and update
- * the file DOS attributes.
- */
-int cmd_setmode(void)
-{
-	const extern char *cmd_ptr;
-	char *buf;
-	char *fname = NULL;
-	uint16_t attr[2] = {0};
-	int mode = ATTR_SET;
-	int err = 0;
-	bool ok;
-	TALLOC_CTX *ctx = talloc_new(NULL);
-	if (ctx == NULL) {
-		return 1;
-	}
-
-	ok = next_token_talloc(ctx, &cmd_ptr, &buf, NULL);
-	if (!ok) {
-		DBG(0, ("setmode <filename> <[+|-]rsha>\n"));
-		err = 1;
-		goto out;
-	}
-
-	fname = talloc_asprintf(ctx,
-				"%s%s",
-				client_get_cur_dir(),
-				buf);
-	if (fname == NULL) {
-		err = 1;
-		goto out;
-	}
-
-	while (next_token_talloc(ctx, &cmd_ptr, &buf, NULL)) {
-		const char *s = buf;
-
-		while (*s) {
-			switch (*s++) {
-			case '+':
-				mode = ATTR_SET;
-				break;
-			case '-':
-				mode = ATTR_UNSET;
-				break;
-			case 'r':
-				attr[mode] |= FILE_ATTRIBUTE_READONLY;
-				break;
-			case 'h':
-				attr[mode] |= FILE_ATTRIBUTE_HIDDEN;
-				break;
-			case 's':
-				attr[mode] |= FILE_ATTRIBUTE_SYSTEM;
-				break;
-			case 'a':
-				attr[mode] |= FILE_ATTRIBUTE_ARCHIVE;
-				break;
-			default:
-				DBG(0, ("setmode <filename> <perm=[+|-]rsha>\n"));
-				err = 1;
-				goto out;
-			}
-		}
-	}
-
-	if (attr[ATTR_SET] == 0 && attr[ATTR_UNSET] == 0) {
-		DBG(0, ("setmode <filename> <[+|-]rsha>\n"));
-		err = 1;
-		goto out;
-	}
-
-	DBG(2, ("perm set %d %d\n", attr[ATTR_SET], attr[ATTR_UNSET]));
-
-	/* ignore return value: server might not store DOS attributes */
-	set_remote_attr(fname, attr[ATTR_SET], ATTR_SET);
-	set_remote_attr(fname, attr[ATTR_UNSET], ATTR_UNSET);
-out:
-	talloc_free(ctx);
-	return err;
-}
 
 /**
  * tar_parse_args - parse and set tar command line arguments
@@ -1631,41 +1543,6 @@ out:
 	return status;
 }
 
-/**
- * set_remote_attr - set DOS attributes of a remote file
- * @filename: path to the file name
- * @new_attr: attribute bit mask to use
- * @mode: one of ATTR_SET or ATTR_UNSET
- *
- * Update the file attributes with the one provided.
- */
-static int set_remote_attr(const char *filename, uint16_t new_attr, int mode)
-{
-	extern struct cli_state *cli;
-	uint16_t old_attr;
-	NTSTATUS status;
-
-	status = cli_getatr(cli, filename, &old_attr, NULL, NULL);
-	if (!NT_STATUS_IS_OK(status)) {
-		DBG(0, ("cli_getatr failed: %s\n", nt_errstr(status)));
-		return 1;
-	}
-
-	if (mode == ATTR_SET) {
-		new_attr |= old_attr;
-	} else {
-		new_attr = old_attr & ~new_attr;
-	}
-
-	status = cli_setatr(cli, filename, new_attr, 0);
-	if (!NT_STATUS_IS_OK(status)) {
-		DBG(1, ("cli_setatr failed: %s\n", nt_errstr(status)));
-		return 1;
-	}
-
-	return 0;
-}
-
 
 /**
  * make_remote_path - recursively make remote dirs
@@ -1932,12 +1809,6 @@ int cmd_tarmode(void)
 	return 1;
 }
 
-int cmd_setmode(void)
-{
-	NOT_IMPLEMENTED;
-	return 1;
-}
-
 int cmd_tar(void)
 {
 	NOT_IMPLEMENTED;
-- 
2.6.0.rc0.131.gf624c3d


From f76fa5c17b42933835b73ddfc075917b1e97e8bc Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 16 Sep 2015 12:42:46 -0700
Subject: [PATCH 2/4] s3: smbd: Remove unused parameter from
 build_stream_path().
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Böhme <rb at sernet.de>
---
 source3/smbd/filename.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 6899d2a..e2c3fe4 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -32,7 +32,6 @@
 
 static NTSTATUS build_stream_path(TALLOC_CTX *mem_ctx,
 				  connection_struct *conn,
-				  const char *orig_path,
 				  struct smb_filename *smb_fname);
 
 /****************************************************************************
@@ -981,7 +980,7 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
 		smb_fname->stream_name = stream;
 
 		/* Check path now that the base_name has been converted. */
-		status = build_stream_path(ctx, conn, orig_path, smb_fname);
+		status = build_stream_path(ctx, conn, smb_fname);
 		if (!NT_STATUS_IS_OK(status)) {
 			goto fail;
 		}
@@ -1233,7 +1232,6 @@ int get_real_filename(connection_struct *conn, const char *path,
 
 static NTSTATUS build_stream_path(TALLOC_CTX *mem_ctx,
 				  connection_struct *conn,
-				  const char *orig_path,
 				  struct smb_filename *smb_fname)
 {
 	NTSTATUS status;
-- 
2.6.0.rc0.131.gf624c3d


From 23ba8469d746d62c7da63e21e260b17e75b5cc2e Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 16 Sep 2015 12:03:34 -0700
Subject: [PATCH 3/4] s3: smbd: Fix opening/creating :stream files on the root
 share directory.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Böhme <rb at sernet.de>
---
 source3/smbd/filename.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index e2c3fe4..3ed60e2 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -370,6 +370,29 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
 			 */
 			*stream = '\0';
 			stream = tmp;
+
+			if (smb_fname->base_name[0] == '\0') {
+				/*
+				 * orig_name was just a stream name.
+				 * This is a stream on the root of
+				 * the share. Replace base_name with
+				 * a "."
+				 */
+				smb_fname->base_name =
+					talloc_strdup(smb_fname, ".");
+				if (smb_fname->base_name == NULL) {
+					status = NT_STATUS_NO_MEMORY;
+					goto err;
+				}
+				if (SMB_VFS_STAT(conn, smb_fname) != 0) {
+					status = map_nt_error_from_unix(errno);
+					goto err;
+				}
+				DEBUG(5, ("conversion finished %s -> %s\n",
+					orig_path,
+					smb_fname->base_name));
+				goto done;
+			}
 		}
 	}
 
-- 
2.6.0.rc0.131.gf624c3d


From 5140d888c508425ecb3d6c05c3f17b6a04d00b8d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 16 Sep 2015 16:12:15 -0700
Subject: [PATCH 4/4] s3: tests: smbclient test to ensure we can create and see
 a :foobar stream on the top level directory in a share.

Regression test for:

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

Remember to remove the ARCHIVE attribute from the toplevel
share when done (can only be done over SMB2+).

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/script/tests/test_smbclient_s3.sh | 36 +++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index 69c7452..c073b43 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -971,6 +971,38 @@ EOF
     fi
 }
 
+# Test creating a stream on the root of the share directory filname - :foobar
+test_toplevel_stream()
+{
+    tmpfile=$PREFIX/smbclient_interactive_prompt_commands
+    cat > $tmpfile <<EOF
+put ${PREFIX}/smbclient_interactive_prompt_commands :foobar
+allinfo \\
+setmode \\ -a
+quit
+EOF
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/tmp -I $SERVER_IP -mSMB3 $ADDARGS < $tmpfile 2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+    rm -f $tmpfile
+
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed creating toplevel stream :foobar with error $ret"
+	false
+	return
+    fi
+
+    echo "$out" | grep '^stream:.*:foobar'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed creating toplevel stream :foobar"
+	false
+    fi
+}
+
 
 LOGDIR_PREFIX=test_smbclient_s3
 
@@ -1059,6 +1091,10 @@ testit "server-side file copy" \
     test_scopy || \
     failed=`expr $failed + 1`
 
+testit "creating a :stream at root of share" \
+    test_toplevel_stream || \
+    failed=`expr $failed + 1`
+
 testit "rm -rf $LOGDIR" \
     rm -rf $LOGDIR || \
     failed=`expr $failed + 1`
-- 
2.6.0.rc0.131.gf624c3d



More information about the samba-technical mailing list