r23858: Added srvstr_pull_buf_talloc() and srvstr_pull_talloc()

calls and converted reply_tcon and reply_tconX to use
them - to show the boilerplate usage (valgrind tested).
In conjunction with Volker's srvstr_get_path_talloc()
work this should allow us to start eliminating all
pstrings/fstrings out of the main path processing
code.
I'll watch the build farm tonight...
Jeremy.
(This used to be commit b4eff3f68089f082781afcf90d43faa317949566)

4 files changed, 319 insertions, 58 deletions

diff --git a/source3/include/safe_string.h b/source3/include/safe_string.h
index 68be38df75a..8c4d90c44a3 100644
--- a/source3/include/safe_string.h
+++ b/source3/include/safe_string.h
@@ -164,6 +164,7 @@ size_t __unsafe_string_function_usage_here_char__(void);
 #define safe_strcat(dest,src,maxlength)	safe_strcat_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE,dest,src,maxlength)
 #define push_string(base_ptr, dest, src, dest_len, flags) push_string_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, base_ptr, dest, src, dest_len, flags)
 #define pull_string(base_ptr, smb_flags2, dest, src, dest_len, src_len, flags) pull_string_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, base_ptr, smb_flags2, dest, src, dest_len, src_len, flags)
+#define pull_string_talloc(ctx, base_ptr, smb_flags2, dest, src, src_len, flags) pull_string_talloc_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, ctx, base_ptr, smb_flags2, dest, src, src_len, flags)
 #define clistr_push(cli, dest, src, dest_len, flags) clistr_push_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, cli, dest, src, dest_len, flags)
 #define clistr_pull(cli, dest, src, dest_len, src_len, flags) clistr_pull_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, cli, dest, src, dest_len, src_len, flags)
 #define srvstr_push(base_ptr, dest, src, dest_len, flags) srvstr_push_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, base_ptr, dest, src, dest_len, flags)
@@ -197,6 +198,9 @@ size_t __unsafe_string_function_usage_here_char__(void);
     ? __unsafe_string_function_usage_here_size_t__() \
     : pull_string_fn(fn_name, fn_line, base_ptr, smb_flags2, dest, src, dest_len, src_len, flags))
 
+#define pull_string_talloc_fn2(fn_name, fn_line, ctx, base_ptr, smb_flags2, dest, src, src_len, flags) \
+    pull_string_talloc_fn(fn_name, fn_line, ctx, base_ptr, smb_flags2, dest, src, src_len, flags)
+
 #define clistr_push_fn2(fn_name, fn_line, cli, dest, src, dest_len, flags) \
     (CHECK_STRING_SIZE(dest, dest_len) \
     ? __unsafe_string_function_usage_here_size_t__() \
@@ -218,6 +222,7 @@ size_t __unsafe_string_function_usage_here_char__(void);
 #define safe_strcat_fn2 safe_strcat_fn
 #define push_string_fn2 push_string_fn
 #define pull_string_fn2 pull_string_fn
+#define pull_string_talloc_fn2 pull_string_talloc_fn
 #define clistr_push_fn2 clistr_push_fn
 #define clistr_pull_fn2 clistr_pull_fn
 #define srvstr_push_fn2 srvstr_push_fn
diff --git a/source3/include/srvstr.h b/source3/include/srvstr.h
index c1bb5226e5c..588a807f64b 100644
--- a/source3/include/srvstr.h
+++ b/source3/include/srvstr.h
@@ -20,6 +20,10 @@
 #define srvstr_pull(base_ptr, smb_flags2, dest, src, dest_len, src_len, flags) \
     pull_string(base_ptr, smb_flags2, dest, src, dest_len, src_len, flags)
 
+/* talloc version of above. */
+#define srvstr_pull_talloc(ctx, base_ptr, smb_flags2, dest, src, src_len, flags) \
+    pull_string_talloc(ctx, base_ptr, smb_flags2, dest, src, src_len, flags)
+
 /* pull a string from the smb_buf part of a packet. In this case the
    string can either be null terminated or it can be terminated by the
    end of the smbbuf area 
@@ -28,3 +32,6 @@
 #define srvstr_pull_buf(inbuf, smb_flags2, dest, src, dest_len, flags) \
     pull_string(inbuf, smb_flags2, dest, src, dest_len, smb_bufrem(inbuf, src), flags)
 
+/* talloc version of above. */
+#define srvstr_pull_buf_talloc(ctx, inbuf, smb_flags2, dest, src, flags) \
+    pull_string_talloc(ctx, inbuf, smb_flags2, dest, src, smb_bufrem(inbuf, src), flags)
diff --git a/source3/lib/charcnv.c b/source3/lib/charcnv.c
index b48880a8ea9..b71e15f44d4 100644
--- a/source3/lib/charcnv.c
+++ b/source3/lib/charcnv.c
@@ -1,21 +1,21 @@
-/* 
+/*
    Unix SMB/CIFS implementation.
    Character set conversion Extensions
    Copyright (C) Igor Vergeichik <iverg@mail.ru> 2001
    Copyright (C) Andrew Tridgell 2001
    Copyright (C) Simo Sorce 2001
    Copyright (C) Martin Pool 2003
-   
+
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
    the Free Software Foundation; either version 3 of the License, or
    (at your option) any later version.
-   
+
    This program is distributed in the hope that it will be useful,
    but WITHOUT ANY WARRANTY; without even the implied warranty of
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    GNU General Public License for more details.
-   
+
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
@@ -33,7 +33,7 @@ char lp_failed_convert_char(void)
  * @file
  *
  * @brief Character-set conversion routines built on our iconv.
- * 
+ *
  * @note Samba's internal character set (at least in the 3.0 series)
  * is always the same as the one for the Unix filesystem.  It is
  * <b>not</b> necessarily UTF-8 and may be different on machines that
@@ -509,6 +509,7 @@ size_t convert_string(charset_t from, charset_t to,
  * Convert between character sets, allocating a new buffer for the result.
  *
  * @param ctx TALLOC_CTX to use to allocate with. If NULL use malloc.
+ * (this is a bad interface and needs fixing. JRA).
  * @param srclen length of source buffer.
  * @param dest always set at least to NULL
  * @note -1 is not accepted for srclen.
@@ -516,7 +517,7 @@ size_t convert_string(charset_t from, charset_t to,
  * @returns Size in bytes of the converted string; or -1 in case of error.
  *
  * Ensure the srclen contains the terminating zero.
- * 
+ *
  * I hate the goto's in this function. It's embarressing.....
  * There has to be a cleaner way to do this. JRA.
  **/
@@ -603,6 +604,11 @@ size_t convert_string_allocate(TALLOC_CTX *ctx, charset_t from, charset_t to,
 		if (!conv_silent)
 			DEBUG(0,("Conversion error: %s(%s)\n",reason,inbuf));
 		/* smb_panic(reason); */
+		if (ctx) {
+			TALLOC_FREE(ob);
+		} else {
+			SAFE_FREE(ob);
+		}
 		return (size_t)-1;
 	}
 
@@ -711,7 +717,7 @@ size_t convert_string_allocate(TALLOC_CTX *ctx, charset_t from, charset_t to,
  * Convert between character sets, allocating a new buffer using talloc for the result.
  *
  * @param srclen length of source buffer.
- * @param dest always set at least to NULL 
+ * @param dest always set at least to NULL
  * @note -1 is not accepted for srclen.
  *
  * @returns Size in bytes of the converted string; or -1 in case of error.
@@ -736,7 +742,7 @@ size_t unix_strupper(const char *src, size_t srclen, char *dest, size_t destlen)
 {
 	size_t size;
 	smb_ucs2_t *buffer;
-	
+
 	size = push_ucs2_allocate(&buffer, src);
 	if (size == (size_t)-1) {
 		smb_panic("failed to create UCS2 buffer");
@@ -745,7 +751,7 @@ size_t unix_strupper(const char *src, size_t srclen, char *dest, size_t destlen)
 		free(buffer);
 		return srclen;
 	}
-	
+
 	size = convert_string(CH_UTF16LE, CH_UNIX, buffer, size, dest, destlen, True);
 	free(buffer);
 	return size;
@@ -788,7 +794,7 @@ char *strdup_upper(const char *s)
 		}
 
 		strupper_w(buffer);
-	
+
 		size = convert_string(CH_UTF16LE, CH_UNIX, buffer, -1, out_buffer, sizeof(out_buffer), True);
 		if (size == (size_t)-1) {
 			return NULL;
@@ -802,7 +808,7 @@ size_t unix_strlower(const char *src, size_t srclen, char *dest, size_t destlen)
 {
 	size_t size;
 	smb_ucs2_t *buffer = NULL;
-	
+
 	size = convert_string_allocate(NULL, CH_UNIX, CH_UTF16LE, src, srclen,
 				       (void **)(void *)&buffer, True);
 	if (size == (size_t)-1 || !buffer) {
@@ -826,21 +832,21 @@ char *strdup_lower(const char *s)
 	size_t size;
 	smb_ucs2_t *buffer = NULL;
 	char *out_buffer;
-	
+
 	size = push_ucs2_allocate(&buffer, s);
 	if (size == -1 || !buffer) {
 		return NULL;
 	}
 
 	strlower_w(buffer);
-	
+
 	size = pull_ucs2_allocate(&out_buffer, buffer);
 	SAFE_FREE(buffer);
 
 	if (size == (size_t)-1) {
 		return NULL;
 	}
-	
+
 	return out_buffer;
 }
 
@@ -987,6 +993,84 @@ size_t pull_ascii(char *dest, const void *src, size_t dest_len, size_t src_len,
 	return src_len;
 }
 
+/**
+ * Copy a string from a dos codepage source to a unix char* destination.
+ Talloc version.
+ Uses malloc if TALLOC_CTX is NULL (this is a bad interface and
+ needs fixing. JRA).
+ *
+ * The resulting string in "dest" is always null terminated.
+ *
+ * @param flags can have:
+ * <dl>
+ * <dt>STR_TERMINATE</dt>
+ * <dd>STR_TERMINATE means the string in @p src
+ * is null terminated, and src_len is ignored.</dd>
+ * </dl>
+ *
+ * @param src_len is the length of the source area in bytes.
+ * @returns the number of bytes occupied by the string in @p src.
+ **/
+
+static size_t pull_ascii_base_talloc(TALLOC_CTX *ctx,
+					char **ppdest,
+					const void *src,
+					size_t src_len,
+					int flags)
+{
+	char *dest = NULL;
+	size_t dest_len = 0;
+
+#ifdef DEVELOPER
+	/* Ensure we never use the braindead "malloc" varient. */
+	if (ctx == NULL) {
+		smb_panic("NULL talloc CTX in pull_ascii_base_talloc\n");
+	}
+#endif
+
+	*ppdest = NULL;
+
+	if (flags & STR_TERMINATE) {
+		if (src_len == (size_t)-1) {
+			src_len = strlen((const char *)src) + 1;
+		} else {
+			size_t len = strnlen((const char *)src, src_len);
+			if (len < src_len)
+				len++;
+			src_len = len;
+		}
+		/* Ensure we don't use an insane length from the client. */
+		if (src_len >= 1024*1024) {
+			smb_panic("Bad src length in pull_ascii_base_talloc\n");
+		}
+	}
+
+	dest_len = convert_string_allocate(ctx,
+				CH_DOS,
+				CH_UNIX,
+				src,
+				src_len,
+				&dest,
+				True);
+
+	if (dest_len == (size_t)-1) {
+		return 0;
+	}
+
+	if (dest_len && dest) {
+		/* Did we already process the terminating zero ? */
+		if (dest[dest_len-1] != 0) {
+			dest[dest_len-1] = 0;
+		}
+	} else if (dest) {
+		dest[0] = 0;
+	}
+
+	*ppdest = dest;
+	return src_len;
+}
+
+
 size_t pull_ascii_pstring(char *dest, const void *src)
 {
 	return pull_ascii(dest, src, sizeof(pstring), -1, STR_TERMINATE);
@@ -1214,7 +1298,7 @@ size_t pull_ucs2(const void *base_ptr, char *dest, const void *src, size_t dest_
 	/* ucs2 is always a multiple of 2 bytes */
 	if (src_len != (size_t)-1)
 		src_len &= ~1;
-	
+
 	ret = convert_string(CH_UTF16LE, CH_UNIX, src, src_len, dest, dest_len, True);
 	if (ret == (size_t)-1) {
 		return 0;
@@ -1222,7 +1306,7 @@ size_t pull_ucs2(const void *base_ptr, char *dest, const void *src, size_t dest_
 
 	if (src_len == (size_t)-1)
 		src_len = ret*2;
-		
+
 	if (dest_len && ret) {
 		/* Did we already process the terminating zero ? */
 		if (dest[MIN(ret-1, dest_len-1)] != 0) {
@@ -1235,6 +1319,92 @@ size_t pull_ucs2(const void *base_ptr, char *dest, const void *src, size_t dest_
 	return src_len;
 }
 
+/**
+ Copy a string from a ucs2 source to a unix char* destination.
+ Talloc version with a base pointer.
+ Uses malloc if TALLOC_CTX is NULL (this is a bad interface and
+ needs fixing. JRA).
+ Flags can have:
+  STR_TERMINATE means the string in src is null terminated.
+  STR_NOALIGN   means don't try to align.
+ if STR_TERMINATE is set then src_len is ignored if it is -1.
+ src_len is the length of the source area in bytes
+ Return the number of bytes occupied by the string in src.
+ The resulting string in "dest" is always null terminated.
+**/
+
+static size_t pull_ucs2_base_talloc(TALLOC_CTX *ctx,
+			const void *base_ptr,
+			char **ppdest,
+			const void *src,
+			size_t src_len,
+			int flags)
+{
+	char *dest;
+	size_t dest_len;
+
+	*ppdest = NULL;
+
+#ifdef DEVELOPER
+	/* Ensure we never use the braindead "malloc" varient. */
+	if (ctx == NULL) {
+		smb_panic("NULL talloc CTX in pull_ucs2_base_talloc\n");
+	}
+#endif
+
+	if (ucs2_align(base_ptr, src, flags)) {
+		src = (const void *)((const char *)src + 1);
+		if (src_len != (size_t)-1)
+			src_len--;
+	}
+
+	if (flags & STR_TERMINATE) {
+		/* src_len -1 is the default for null terminated strings. */
+		if (src_len != (size_t)-1) {
+			size_t len = strnlen_w((const smb_ucs2_t *)src,
+						src_len/2);
+			if (len < src_len/2)
+				len++;
+			src_len = len*2;
+		}
+		/* Ensure we don't use an insane length from the client. */
+		if (src_len >= 1024*1024) {
+			smb_panic("Bad src length in pull_ucs2_base_talloc\n");
+		}
+	}
+
+	/* ucs2 is always a multiple of 2 bytes */
+	if (src_len != (size_t)-1) {
+		src_len &= ~1;
+	}
+
+	dest_len = convert_string_talloc(ctx,
+					CH_UTF16LE,
+					CH_UNIX,
+					src,
+					src_len,
+					(void **)&dest,
+					True);
+	if (dest_len == (size_t)-1) {
+		return 0;
+	}
+
+	if (src_len == (size_t)-1)
+		src_len = dest_len*2;
+
+	if (dest_len) {
+		/* Did we already process the terminating zero ? */
+		if (dest[dest_len-1] != 0) {
+			dest[dest_len-1] = 0;
+		}
+	} else if (dest) {
+		dest[0] = 0;
+	}
+
+	*ppdest = dest;
+	return src_len;
+}
+
 size_t pull_ucs2_pstring(char *dest, const void *src)
 {
 	return pull_ucs2(NULL, dest, src, sizeof(pstring), -1, STR_TERMINATE);
@@ -1398,6 +1568,54 @@ size_t pull_string_fn(const char *function, unsigned int line,
 	return pull_ascii(dest, src, dest_len, src_len, flags);
 }
 
+/**
+ Copy a string from a unicode or ascii source (depending on
+ the packet flags) to a char* destination.
+ Variant that uses talloc.
+ Flags can have:
+  STR_TERMINATE means the string in src is null terminated.
+  STR_UNICODE   means to force as unicode.
+  STR_ASCII     use ascii even with unicode packet.
+  STR_NOALIGN   means don't do alignment.
+ if STR_TERMINATE is set then src_len is ignored is it is -1
+ src_len is the length of the source area in bytes.
+ Return the number of bytes occupied by the string in src.
+ The resulting string in "dest" is always null terminated.
+**/
+
+size_t pull_string_talloc_fn(const char *function,
+			unsigned int line,
+			TALLOC_CTX *ctx,
+			const void *base_ptr,
+			uint16 smb_flags2,
+			char **ppdest,
+			const void *src,
+			size_t src_len,
+			int flags)
+{
+	if ((base_ptr == NULL) && ((flags & (STR_ASCII|STR_UNICODE)) == 0)) {
+		smb_panic("No base ptr to get flg2 and neither ASCII nor "
+			  "UNICODE defined");
+	}
+
+	if (!(flags & STR_ASCII) && \
+	    ((flags & STR_UNICODE || \
+	      (smb_flags2 & FLAGS2_UNICODE_STRINGS)))) {
+		return pull_ucs2_base_talloc(ctx,
+					base_ptr,
+					ppdest,
+					src,
+					src_len,
+					flags);
+	}
+	return pull_ascii_base_talloc(ctx,
+					ppdest,
+					src,
+					src_len,
+					flags);
+}
+
+
 size_t align_string(const void *base_ptr, const char *p, int flags)
 {
 	if (!(flags & STR_ASCII) && \
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 9a2dc19fa10..8b6a164a665 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -381,30 +381,40 @@ int reply_special(char *inbuf,char *outbuf)
 int reply_tcon(connection_struct *conn,
 	       char *inbuf,char *outbuf, int dum_size, int dum_buffsize)
 {
+	TALLOC_CTX *ctx;
 	const char *service;
-	pstring service_buf;
-	pstring password;
-	pstring dev;
+	char *service_buf = NULL;
+	char *password = NULL;
+	char *dev = NULL;
 	int outsize = 0;
 	uint16 vuid = SVAL(inbuf,smb_uid);
 	int pwlen=0;
 	NTSTATUS nt_status;
 	char *p;
 	DATA_BLOB password_blob;
-	
+
 	START_PROFILE(SMBtcon);
 
-	*service_buf = *password = *dev = 0;
+	ctx = talloc_init("reply_tcon");
+	if (!ctx) {
+		END_PROFILE(SMBtcon);
+		return ERROR_NT(NT_STATUS_NO_MEMORY);
+	}
 
 	p = smb_buf(inbuf)+1;
-	p += srvstr_pull_buf(inbuf, SVAL(inbuf, smb_flg2), service_buf, p,
-			     sizeof(service_buf), STR_TERMINATE) + 1;
-	pwlen = srvstr_pull_buf(inbuf, SVAL(inbuf, smb_flg2), password, p,
-				sizeof(password), STR_TERMINATE) + 1;
+	p += srvstr_pull_buf_talloc(ctx, inbuf, SVAL(inbuf, smb_flg2),
+			&service_buf, p, STR_TERMINATE) + 1;
+	pwlen = srvstr_pull_buf_talloc(ctx, inbuf, SVAL(inbuf, smb_flg2),
+			&password, p, STR_TERMINATE) + 1;
 	p += pwlen;
-	p += srvstr_pull_buf(inbuf, SVAL(inbuf, smb_flg2), dev, p, sizeof(dev),
-			     STR_TERMINATE) + 1;
+	p += srvstr_pull_buf_talloc(ctx, inbuf, SVAL(inbuf, smb_flg2),
+			&dev, p, STR_TERMINATE) + 1;
 
+	if (service_buf == NULL || password == NULL || dev == NULL) {
+		TALLOC_FREE(ctx);
+		END_PROFILE(SMBtcon);
+		return ERROR_NT(NT_STATUS_INVALID_PARAMETER);
+	}
 	p = strrchr_m(service_buf,'\\');
 	if (p) {
 		service = p+1;
@@ -417,21 +427,23 @@ int reply_tcon(connection_struct *conn,
 	conn = make_connection(service,password_blob,dev,vuid,&nt_status);
 
 	data_blob_clear_free(&password_blob);
-  
+
 	if (!conn) {
+		TALLOC_FREE(ctx);
 		END_PROFILE(SMBtcon);
 		return ERROR_NT(nt_status);
 	}
-  
+
 	outsize = set_message(inbuf,outbuf,2,0,True);
 	SSVAL(outbuf,smb_vwv0,max_recv);
 	SSVAL(outbuf,smb_vwv1,conn->cnum);
 	SSVAL(outbuf,smb_tid,conn->cnum);
-  
+
 	DEBUG(3,("tcon service=%s cnum=%d\n", 
 		 service, conn->cnum));
-  
+
 	END_PROFILE(SMBtcon);
+	TALLOC_FREE(ctx);
 	return(outsize);
 }
 
@@ -442,23 +454,22 @@ int reply_tcon(connection_struct *conn,
 
 int reply_tcon_and_X(connection_struct *conn, char *inbuf,char *outbuf,int length,int bufsize)
 {
-	fstring service;
+	char *service = NULL;
 	DATA_BLOB password;
 
+	TALLOC_CTX *ctx = NULL;
 	/* what the cleint thinks the device is */
-	fstring client_devicetype;
+	char *client_devicetype = NULL;
 	/* what the server tells the client the share represents */
 	const char *server_devicetype;
 	NTSTATUS nt_status;
 	uint16 vuid = SVAL(inbuf,smb_uid);
 	int passlen = SVAL(inbuf,smb_vwv3);
-	pstring path;
+	char *path = NULL;
 	char *p, *q;
 	uint16 tcon_flags = SVAL(inbuf,smb_vwv2);
-	
-	START_PROFILE(SMBtconX);	
 
-	*service = *client_devicetype = 0;
+	START_PROFILE(SMBtconX);
 
 	/* we might have to close an old one */
 	if ((SVAL(inbuf,smb_vwv2) & 0x1) && conn) {
@@ -468,7 +479,7 @@ int reply_tcon_and_X(connection_struct *conn, char *inbuf,char *outbuf,int lengt
 	if (passlen > MAX_PASS_LEN) {
 		return ERROR_DOS(ERRDOS,ERRbuftoosmall);
 	}
- 
+
 	if (global_encrypted_passwords_negotiated) {
 		password = data_blob(smb_buf(inbuf),passlen);
 		if (lp_security() == SEC_SHARE) {
@@ -487,34 +498,53 @@ int reply_tcon_and_X(connection_struct *conn, char *inbuf,char *outbuf,int lengt
 		p = smb_buf(inbuf) + passlen + 1;
 	}
 
-	p += srvstr_pull_buf(inbuf, SVAL(inbuf, smb_flg2), path, p,
-			     sizeof(path), STR_TERMINATE);
+	ctx = talloc_init("reply_tcon_and_X");
+	if (!ctx) {
+		END_PROFILE(SMBtconX);
+		return ERROR_NT(NT_STATUS_NO_MEMORY);
+	}
+	p += srvstr_pull_buf_talloc(ctx, inbuf, SVAL(inbuf, smb_flg2), &path, p,
+			     STR_TERMINATE);
+
+	if (path == NULL) {
+		TALLOC_FREE(ctx);
+		END_PROFILE(SMBtconX);
+		return ERROR_NT(NT_STATUS_INVALID_PARAMETER);
+	}
 
 	/*
 	 * the service name can be either: \\server\share
 	 * or share directly like on the DELL PowerVault 705
 	 */
-	if (*path=='\\') {	
+	if (*path=='\\') {
 		q = strchr_m(path+2,'\\');
 		if (!q) {
+			TALLOC_FREE(ctx);
 			END_PROFILE(SMBtconX);
 			return(ERROR_DOS(ERRDOS,ERRnosuchshare));
 		}
-		fstrcpy(service,q+1);
+		service = q+1;
+	} else {
+		service = path;
+	}
+
+	p += srvstr_pull_talloc(ctx, inbuf, SVAL(inbuf, smb_flg2), &client_devicetype, p,
+			 6, STR_ASCII);
+
+	if (client_devicetype == NULL) {
+		TALLOC_FREE(ctx);
+		END_PROFILE(SMBtconX);
+		return ERROR_NT(NT_STATUS_INVALID_PARAMETER);
 	}
-	else
-		fstrcpy(service,path);
-		
-	p += srvstr_pull(inbuf, SVAL(inbuf, smb_flg2), client_devicetype, p,
-			 sizeof(client_devicetype), 6, STR_ASCII);
 
 	DEBUG(4,("Client requested device type [%s] for share [%s]\n", client_devicetype, service));
 
 	conn = make_connection(service,password,client_devicetype,vuid,&nt_status);
-	
+
 	data_blob_clear_free(&password);
 
 	if (!conn) {
+		TALLOC_FREE(ctx);
 		END_PROFILE(SMBtconX);
 		return ERROR_NT(nt_status);
 	}
@@ -523,19 +553,19 @@ int reply_tcon_and_X(connection_struct *conn, char *inbuf,char *outbuf,int lengt
 		server_devicetype = "IPC";
 	else if ( IS_PRINT(conn) )
 		server_devicetype = "LPT1:";
-	else 
+	else
 		server_devicetype = "A:";
 
 	if (Protocol < PROTOCOL_NT1) {
 		set_message(inbuf,outbuf,2,0,True);
 		p = smb_buf(outbuf);
-		p += srvstr_push(outbuf, p, server_devicetype, -1, 
+		p += srvstr_push(outbuf, p, server_devicetype, -1,
 				 STR_TERMINATE|STR_ASCII);
 		set_message_end(inbuf,outbuf,p);
 	} else {
 		/* NT sets the fstype of IPC$ to the null string */
 		const char *fstype = IS_IPC(conn) ? "" : lp_fstype(SNUM(conn));
-		
+
 		if (tcon_flags & TCONX_FLAG_EXTENDED_RESPONSE) {
 			/* Return permissions. */
 			uint32 perm1 = 0;
@@ -559,29 +589,30 @@ int reply_tcon_and_X(connection_struct *conn, char *inbuf,char *outbuf,int lengt
 		}
 
 		p = smb_buf(outbuf);
-		p += srvstr_push(outbuf, p, server_devicetype, -1, 
+		p += srvstr_push(outbuf, p, server_devicetype, -1,
 				 STR_TERMINATE|STR_ASCII);
-		p += srvstr_push(outbuf, p, fstype, -1, 
+		p += srvstr_push(outbuf, p, fstype, -1,
 				 STR_TERMINATE);
-		
+
 		set_message_end(inbuf,outbuf,p);
-		
+
 		/* what does setting this bit do? It is set by NT4 and
 		   may affect the ability to autorun mounted cdroms */
 		SSVAL(outbuf, smb_vwv2, SMB_SUPPORT_SEARCH_BITS|
 				(lp_csc_policy(SNUM(conn)) << 2));
-		
+
 		init_dfsroot(conn, inbuf, outbuf);
 	}
 
-  
+
 	DEBUG(3,("tconX service=%s \n",
 		 service));
-  
+
 	/* set the incoming and outgoing tid to the just created one */
 	SSVAL(inbuf,smb_tid,conn->cnum);
 	SSVAL(outbuf,smb_tid,conn->cnum);
 
+	TALLOC_FREE(ctx);
 	END_PROFILE(SMBtconX);
 	return chain_reply(inbuf,outbuf,length,bufsize);
 }