[PATCH] Fix reversal of symlink command parameters when creating reparse points on a Windows server.

Jeremy Allison jra at samba.org
Wed Nov 29 21:33:18 UTC 2017


Another discovery in my work to make reparse points work
over SMB2 (in order to get some tests so I can implement
the server side in smbd :-).

The smbclient 'symlink' command reversed the sense of
newname and link_taget when creating a reparse point
on a Windows server, vs the sense of the parameters
when creating a UNIX extensions symlink against smbd.

Patch fixes this, plus renames the 'oldname' paramters
to 'link_target' so we have a chance of getting this
right in future :-).

Please review and push if happy !

Jeremy.
-------------- next part --------------
From 8da3c50b9c9fa05675f4caf8b552ec18f9d618bc Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 29 Nov 2017 13:10:25 -0800
Subject: [PATCH 1/3] s3: client: Rename <oldname> to <link_target> in
 cmd_symlink() and cli_posix_symlink().

Stops us from mixing up the old and new names. Only behavior change
is correcting the names printed in the error messages.

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/client/client.c  | 15 ++++++++-------
 source3/libsmb/clifile.c | 15 ++++++++-------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/source3/client/client.c b/source3/client/client.c
index 754907d2e09..ad10a5381dd 100644
--- a/source3/client/client.c
+++ b/source3/client/client.c
@@ -3523,7 +3523,7 @@ static int cmd_readlink(void)
 static int cmd_symlink(void)
 {
 	TALLOC_CTX *ctx = talloc_tos();
-	char *oldname = NULL;
+	char *link_target = NULL;
 	char *newname = NULL;
 	char *buf = NULL;
 	char *buf2 = NULL;
@@ -3532,11 +3532,11 @@ static int cmd_symlink(void)
 
 	if (!next_token_talloc(ctx, &cmd_ptr,&buf,NULL) ||
 	    !next_token_talloc(ctx, &cmd_ptr,&buf2,NULL)) {
-		d_printf("symlink <oldname> <newname>\n");
+		d_printf("symlink <link_target> <newname>\n");
 		return 1;
 	}
 	/* Oldname (link target) must be an untouched blob. */
-	oldname = buf;
+	link_target = buf;
 
 	if (SERVER_HAS_UNIX_CIFS(cli)) {
 		newname = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(),
@@ -3553,19 +3553,20 @@ static int cmd_symlink(void)
 				popt_get_cmdline_auth_info(), cli, newname,
 				&newcli, &newname);
 		if (!NT_STATUS_IS_OK(status)) {
-			d_printf("link %s: %s\n", oldname, nt_errstr(status));
+			d_printf("link %s: %s\n", newname,
+				nt_errstr(status));
 			return 1;
 		}
-		status = cli_posix_symlink(newcli, oldname, newname);
+		status = cli_posix_symlink(newcli, link_target, newname);
 	} else {
 		status = cli_symlink(
-			cli, oldname, buf2,
+			cli, link_target, buf2,
 			buf2[0] == '\\' ? 0 : SYMLINK_FLAG_RELATIVE);
 	}
 
 	if (!NT_STATUS_IS_OK(status)) {
 		d_printf("%s symlinking files (%s -> %s)\n",
-			 nt_errstr(status), oldname, newname);
+			 nt_errstr(status), newname, link_target);
 		return 1;
 	}
 
diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c
index a6a0bafa2cd..e942b27e175 100644
--- a/source3/libsmb/clifile.c
+++ b/source3/libsmb/clifile.c
@@ -151,7 +151,7 @@ NTSTATUS cli_setpathinfo(struct cli_state *cli,
 
 /****************************************************************************
  Hard/Symlink a file (UNIX extensions).
- Creates new name (sym)linked to oldname.
+ Creates new name (sym)linked to link_target.
 ****************************************************************************/
 
 struct cli_posix_link_internal_state {
@@ -164,7 +164,7 @@ static struct tevent_req *cli_posix_link_internal_send(TALLOC_CTX *mem_ctx,
 					struct tevent_context *ev,
 					struct cli_state *cli,
 					uint16_t level,
-					const char *oldname,
+					const char *link_target,
 					const char *newname)
 {
 	struct tevent_req *req = NULL, *subreq = NULL;
@@ -182,7 +182,8 @@ static struct tevent_req *cli_posix_link_internal_send(TALLOC_CTX *mem_ctx,
 		return tevent_req_post(req, ev);
 	}
 	state->data = trans2_bytes_push_str(
-		state->data, smbXcli_conn_use_unicode(cli->conn), oldname, strlen(oldname)+1, NULL);
+		state->data, smbXcli_conn_use_unicode(cli->conn),
+		link_target, strlen(link_target)+1, NULL);
 
 	subreq = cli_setpathinfo_send(
 		state, ev, cli, level, newname,
@@ -207,11 +208,11 @@ static void cli_posix_link_internal_done(struct tevent_req *subreq)
 struct tevent_req *cli_posix_symlink_send(TALLOC_CTX *mem_ctx,
 					struct tevent_context *ev,
 					struct cli_state *cli,
-					const char *oldname,
+					const char *link_target,
 					const char *newname)
 {
 	return cli_posix_link_internal_send(
-		mem_ctx, ev, cli, SMB_SET_FILE_UNIX_LINK, oldname, newname);
+		mem_ctx, ev, cli, SMB_SET_FILE_UNIX_LINK, link_target, newname);
 }
 
 NTSTATUS cli_posix_symlink_recv(struct tevent_req *req)
@@ -220,7 +221,7 @@ NTSTATUS cli_posix_symlink_recv(struct tevent_req *req)
 }
 
 NTSTATUS cli_posix_symlink(struct cli_state *cli,
-			const char *oldname,
+			const char *link_target,
 			const char *newname)
 {
 	TALLOC_CTX *frame = talloc_stackframe();
@@ -245,7 +246,7 @@ NTSTATUS cli_posix_symlink(struct cli_state *cli,
 	req = cli_posix_symlink_send(frame,
 				ev,
 				cli,
-				oldname,
+				link_target,
 				newname);
 	if (req == NULL) {
 		status = NT_STATUS_NO_MEMORY;
-- 
2.15.0.531.g2ccb3012c9-goog


From d0edcaacfad522a99a1381c7d0445f30ff58cb72 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 29 Nov 2017 13:16:43 -0800
Subject: [PATCH 2/3] s3: libsmb: Fix reversing of oldname/newname paths when
 creating a reparse point symlink on Windows from smbclient.

This happened as smbd doesn't support reparse points so we couldn't test.
This was the reverse of the (tested) symlink parameters in the unix extensions
symlink command.

Rename parameters to link_target instead of oldname so this is clearer.

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/libsmb/clisymlink.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/source3/libsmb/clisymlink.c b/source3/libsmb/clisymlink.c
index 496e3e1d953..a52f6ff7f6d 100644
--- a/source3/libsmb/clisymlink.c
+++ b/source3/libsmb/clisymlink.c
@@ -31,7 +31,7 @@
 struct cli_symlink_state {
 	struct tevent_context *ev;
 	struct cli_state *cli;
-	const char *oldpath;
+	const char *link_target;
 	const char *newpath;
 	uint32_t flags;
 
@@ -49,7 +49,7 @@ static void cli_symlink_close_done(struct tevent_req *subreq);
 struct tevent_req *cli_symlink_send(TALLOC_CTX *mem_ctx,
 				    struct tevent_context *ev,
 				    struct cli_state *cli,
-				    const char *oldpath,
+				    const char *link_target,
 				    const char *newpath,
 				    uint32_t flags)
 {
@@ -62,12 +62,12 @@ struct tevent_req *cli_symlink_send(TALLOC_CTX *mem_ctx,
 	}
 	state->ev = ev;
 	state->cli = cli;
-	state->oldpath = oldpath;
+	state->link_target = link_target;
 	state->newpath = newpath;
 	state->flags = flags;
 
 	subreq = cli_ntcreate_send(
-		state, ev, cli, state->oldpath, 0,
+		state, ev, cli, state->newpath, 0,
 		SYNCHRONIZE_ACCESS|DELETE_ACCESS|
 		FILE_READ_ATTRIBUTES|FILE_WRITE_ATTRIBUTES,
 		FILE_ATTRIBUTE_NORMAL, FILE_SHARE_NONE, FILE_CREATE,
@@ -102,7 +102,7 @@ static void cli_symlink_create_done(struct tevent_req *subreq)
 	SCVAL(state->setup, 7, 0); /* IsFlags */
 
 	if (!symlink_reparse_buffer_marshall(
-		    state->newpath, NULL, state->flags, state,
+		    state->link_target, NULL, state->flags, state,
 		    &data, &data_len)) {
 		tevent_req_oom(req);
 		return;
@@ -197,7 +197,7 @@ NTSTATUS cli_symlink_recv(struct tevent_req *req)
 	return tevent_req_simple_recv_ntstatus(req);
 }
 
-NTSTATUS cli_symlink(struct cli_state *cli, const char *oldname,
+NTSTATUS cli_symlink(struct cli_state *cli, const char *link_target,
 		     const char *newname, uint32_t flags)
 {
 	TALLOC_CTX *frame = talloc_stackframe();
@@ -213,7 +213,7 @@ NTSTATUS cli_symlink(struct cli_state *cli, const char *oldname,
 	if (ev == NULL) {
 		goto fail;
 	}
-	req = cli_symlink_send(frame, ev, cli, oldname, newname, flags);
+	req = cli_symlink_send(frame, ev, cli, link_target, newname, flags);
 	if (req == NULL) {
 		goto fail;
 	}
-- 
2.15.0.531.g2ccb3012c9-goog


From f95db94a2ef6713c927c9b14b9ab3d05f0384c55 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 29 Nov 2017 13:22:25 -0800
Subject: [PATCH 3/3] WHATSNEW: Explain reversal of smbclient symlink
 parameters against Windows server.

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 WHATSNEW.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index 4265627d774..4d42599ebe9 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -87,6 +87,17 @@ assumptions. Samba implements async I/O based on threads by default,
 there is no special module required to see benefits of read and write
 request being sent do the disk in parallel.
 
+smbclient reparse point symlink parameters reversed
+===================================================
+
+A bug in smbclient caused the 'symlink' command to reverse the
+meaning of the new name and link target paramters when creating a
+reparse point symlink against a Windows server. As this is a
+little used feature the ordering of these parameters has been
+reversed to match the parameter ordering of the UNIX extensions
+'symlink' command. The usage message for this command has also
+been improved to remove confusion.
+
 KNOWN ISSUES
 ============
 
-- 
2.15.0.531.g2ccb3012c9-goog



More information about the samba-technical mailing list