[PATCH] Fix bug 10775 - smbd crashes when accessing garbage filenames

Jeremy Allison jra at samba.org
Fri Aug 29 11:01:51 MDT 2014


On Fri, Aug 29, 2014 at 04:47:29PM +0200, David Disseldorp wrote:
> 
> smb_vfs_call_readlink() returns an int, so this specific type change
> isn't necessary.

Fixed.

> Looks like cur_pdata/*out_data leaks on error here.

Fixed - good catch, thanks !

> Can't say I'm a fan of the errno-catch error handling here, but it
> looks like propagating error values all the way through to
> convert_string_error_handle() would be a pretty massive change :-/

Yep. I wasn't so happy either, but IMHO this is a good
first step. Eventually we need to propagate all the
way through - but that's a bigger refactoring for another
day :-). I do plan to do it eventually to tidy up all our
iconv error handling - just not for this bug.

> s3: smbd: Change smbd_marshall_dir_entry() to return an NTSTATUS. Returns STATUS_MORE_ENTRIES on out of space.
> s3: smbd: smbd_marshall_dir_entry() no longer needs explicit 'out_of_space' parameter.
> 
> srvstr_push() EAGAIN, EINTR, ENOBUFS and EWOULDBLOCK errors would also
> be propagated through as STATUS_MORE_ENTRIES. Could we get these from
> the iconv layer?

Just checked inside the glibc iconv source. It's only
dealing with file access POSIX calls, not sockets
so that rules out EAGAIN, EINTR, EWOULDBLOCK,
even when hit by signals. I don't think ENOBUFS is possible,
but just to be certain I've added code to filter
STATUS_MORE_ENTRIES out of the srvstr_push() return
(map to NT_STATUS_UNSUCCESSFUL).

Even if we leave the bool parameter in place, we would
have to filter out STATUS_MORE_ENTRIES error returns in
the SMB1/SMB2 engines anyway, as that has a specific meaning when
returned to the client (see below).

> I think I'd prefer to keep the explicit out_of_space
> parameter, rather than audit all return paths for potential
> STATUS_MORE_ENTRIES values.

STATUS_MORE_ENTRIES has a long and established history
of being used in this way - from the description of the
error return:

"Returned by enumeration APIs to indicate more information is available to successive calls."

I can leave the explicit out_of_space 'magic' bool
parameter alone if you really insist, but to be honest
it's an *ugly* use of code - and we have to filter
STATUS_MORE_ENTRIES out anyway :-). One of the things
that convinced me this approach was correct was
the way returning an NTSTATUS allowed me to remove it :-).

We have better taste these days :-).

Updated patch for your review attached !

Jeremy.
-------------- next part --------------
From 51932fd5fe9c3697e344344628172ddd70cd9c9a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 25 Aug 2014 16:21:24 -0700
Subject: [PATCH 01/10] s3: smbd: srvstr_push() was changed to never return -1,
 so don't check for that as an error.

Bug 10775 - smbd crashes when accessing garbage filenames

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/srvstr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/smbd/srvstr.c b/source3/smbd/srvstr.c
index c069dd4..648c69f 100644
--- a/source3/smbd/srvstr.c
+++ b/source3/smbd/srvstr.c
@@ -65,7 +65,7 @@ ssize_t message_push_string(uint8 **outbuf, const char *str, int flags)
 	result = srvstr_push((char *)tmp, SVAL(tmp, smb_flg2),
 			     tmp + buf_size, str, grow_size, flags);
 
-	if (result == (size_t)-1) {
+	if (result == 0) {
 		DEBUG(0, ("srvstr_push failed\n"));
 		return -1;
 	}
-- 
2.1.0.rc2.206.gedb03e5


From 9249bbbec6c575ed34f36c357588720a866c410c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 25 Aug 2014 17:05:47 -0700
Subject: [PATCH 02/10] s3: smbd: Ensure types for all variables called 'len'
 used in srvstr_push() are correct.

Bug 10775 - smbd crashes when accessing garbage filenames

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/trans2.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 1e2c02e..70d29f2 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -1594,7 +1594,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	uint64_t file_size = 0;
 	uint64_t allocation_size = 0;
 	uint64_t file_index = 0;
-	uint32_t len;
+	size_t len = 0;
 	struct timespec mdate_ts, adate_ts, cdate_ts, create_date_ts;
 	time_t mdate = (time_t)0, adate = (time_t)0, create_date = (time_t)0;
 	char *nameptr;
@@ -3077,7 +3077,8 @@ NTSTATUS smbd_do_qfsinfo(struct smbXsrv_connection *xconn,
 			 int *ret_data_len)
 {
 	char *pdata, *end_data;
-	int data_len = 0, len;
+	int data_len = 0;
+	size_t len = 0;
 	const char *vname = volume_label(talloc_tos(), SNUM(conn));
 	int snum = SNUM(conn);
 	const char *fstype = lp_fstype(SNUM(conn));
@@ -3187,9 +3188,9 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)st.st_ex_dev, (u
 				STR_NOALIGN|STR_TERMINATE);
 			SCVAL(pdata,l2_vol_cch,len);
 			data_len = l2_vol_szVolLabel + len;
-			DEBUG(5,("smbd_do_qfsinfo : time = %x, namelen = %d, name = %s\n",
+			DEBUG(5,("smbd_do_qfsinfo : time = %x, namelen = %u, name = %s\n",
 				 (unsigned)convert_timespec_to_time_t(st.st_ex_ctime),
-				 len, vname));
+				 (unsigned)len, vname));
 			break;
 
 		case SMB_QUERY_FS_ATTRIBUTE_INFO:
@@ -4426,6 +4427,7 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 	uint64_t allocation_size = 0;
 	uint64_t file_index = 0;
 	uint32_t access_mask = 0;
+	size_t len = 0;
 
 	if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) {
 		return NT_STATUS_INVALID_LEVEL;
@@ -4727,7 +4729,6 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 		case SMB_QUERY_FILE_ALT_NAME_INFO:
 		case SMB_FILE_ALTERNATE_NAME_INFORMATION:
 		{
-			int len;
 			char mangled_name[13];
 			DEBUG(10,("smbd_do_qfilepathinfo: SMB_FILE_ALTERNATE_NAME_INFORMATION\n"));
 			if (!name_to_8_3(base_name,mangled_name,
@@ -4746,7 +4747,6 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 
 		case SMB_QUERY_FILE_NAME_INFO:
 		{
-			int len;
 			/*
 			  this must be *exactly* right for ACLs on mapped drives to work
 			 */
@@ -4777,7 +4777,6 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 		case SMB_QUERY_FILE_ALL_INFO:
 		case SMB_FILE_ALL_INFORMATION:
 		{
-			int len;
 			unsigned int ea_size =
 			    estimate_ea_size(conn, fsp, smb_fname);
 			DEBUG(10,("smbd_do_qfilepathinfo: SMB_FILE_ALL_INFORMATION\n"));
@@ -4810,7 +4809,6 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 
 		case 0xFF12:/*SMB2_FILE_ALL_INFORMATION*/
 		{
-			int len;
 			unsigned int ea_size =
 			    estimate_ea_size(conn, fsp, smb_fname);
 			DEBUG(10,("smbd_do_qfilepathinfo: SMB2_FILE_ALL_INFORMATION\n"));
@@ -5010,7 +5008,7 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 
 		case SMB_QUERY_FILE_UNIX_LINK:
 			{
-				int len;
+				int link_len = 0;
 				char *buffer = talloc_array(mem_ctx, char, PATH_MAX+1);
 
 				if (!buffer) {
@@ -5025,13 +5023,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 #else
 				return NT_STATUS_DOS(ERRDOS, ERRbadlink);
 #endif
-				len = SMB_VFS_READLINK(conn,
+				link_len = SMB_VFS_READLINK(conn,
 						       smb_fname->base_name,
 						       buffer, PATH_MAX);
-				if (len == -1) {
+				if (link_len == -1) {
 					return map_nt_error_from_unix(errno);
 				}
-				buffer[len] = 0;
+				buffer[link_len] = 0;
 				len = srvstr_push(dstart, flags2,
 						  pdata, buffer,
 						  PTR_DIFF(dend, pdata),
-- 
2.1.0.rc2.206.gedb03e5


From 83ee798e7697af2110b1701eadb32490edc8285f Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 25 Aug 2014 17:11:58 -0700
Subject: [PATCH 03/10] s3: smbd: Change the function signature of
 srvstr_push() from returning a length to returning an NTSTATUS with a length
 param.

srvstr_push_fn() now returns an NTSTATUS reporting any
string conversion failure.

We need to get serious about returning character set conversion errors
inside smbd.

Bug 10775 - smbd crashes when accessing garbage filenames

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 lib/util/samba_util.h         |   1 +
 lib/util/string_wrappers.h    |   8 +-
 source3/include/safe_string.h |   1 +
 source3/modules/vfs_default.c |  10 ++-
 source3/smbd/lanman.c         |   9 ++-
 source3/smbd/proto.h          |   4 +-
 source3/smbd/reply.c          |  30 ++++++--
 source3/smbd/srvstr.c         |  57 ++++++++++++--
 source3/smbd/trans2.c         | 174 ++++++++++++++++++++++++++++++------------
 9 files changed, 221 insertions(+), 73 deletions(-)

diff --git a/lib/util/samba_util.h b/lib/util/samba_util.h
index f1f4c2d..528d373 100644
--- a/lib/util/samba_util.h
+++ b/lib/util/samba_util.h
@@ -63,6 +63,7 @@ do { \
 
 #include "lib/util/memory.h"
 
+#include "../libcli/util/ntstatus.h"
 #include "lib/util/string_wrappers.h"
 
 /**
diff --git a/lib/util/string_wrappers.h b/lib/util/string_wrappers.h
index fcc088c..1feea8c 100644
--- a/lib/util/string_wrappers.h
+++ b/lib/util/string_wrappers.h
@@ -57,6 +57,8 @@ char * __unsafe_string_function_usage_here__(void);
 
 size_t __unsafe_string_function_usage_here_size_t__(void);
 
+NTSTATUS __unsafe_string_function_usage_here_NTSTATUS__(void);
+
 #define CHECK_STRING_SIZE(d, len) (sizeof(d) != (len) && sizeof(d) != sizeof(char *))
 
 /* if the compiler will optimize out function calls, then use this to tell if we are
@@ -68,10 +70,10 @@ size_t __unsafe_string_function_usage_here_size_t__(void);
     ? __unsafe_string_function_usage_here_size_t__() \
     : push_string_check_fn(dest, src, dest_len, flags))
 
-#define srvstr_push(base_ptr, smb_flags2, dest, src, dest_len, flags) \
+#define srvstr_push(base_ptr, smb_flags2, dest, src, dest_len, flags, ret_len) \
     (CHECK_STRING_SIZE(dest, dest_len) \
-    ? __unsafe_string_function_usage_here_size_t__() \
-    : srvstr_push_fn(base_ptr, smb_flags2, dest, src, dest_len, flags))
+    ? __unsafe_string_function_usage_here_NTSTATUS__() \
+    : srvstr_push_fn(base_ptr, smb_flags2, dest, src, dest_len, flags, ret_len))
 
 /* This allows the developer to choose to check the arguments to
    strlcpy.  if the compiler will optimize out function calls, then
diff --git a/source3/include/safe_string.h b/source3/include/safe_string.h
index 03878b4..e77017c 100644
--- a/source3/include/safe_string.h
+++ b/source3/include/safe_string.h
@@ -62,6 +62,7 @@
 
 #endif /* !_SPLINT_ */
 
+#include "../libcli/util/ntstatus.h"
 #include "lib/util/string_wrappers.h"
 
 #endif
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 3430cd0..3a3943b 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -1178,10 +1178,16 @@ static NTSTATUS vfswrap_fsctl(struct vfs_handle_struct *handle,
 			  shadow_data->num_volumes, fsp_str_dbg(fsp)));
 		if (labels && shadow_data->labels) {
 			for (i=0; i<shadow_data->num_volumes; i++) {
-				srvstr_push(cur_pdata, req_flags,
+				size_t len = 0;
+				status = srvstr_push(cur_pdata, req_flags,
 					    cur_pdata, shadow_data->labels[i],
 					    2 * sizeof(SHADOW_COPY_LABEL),
-					    STR_UNICODE|STR_TERMINATE);
+					    STR_UNICODE|STR_TERMINATE, &len);
+				if (!NT_STATUS_IS_OK(status)) {
+					TALLOC_FREE(*out_data);
+					TALLOC_FREE(shadow_data);
+					return status;
+				}
 				cur_pdata += 2 * sizeof(SHADOW_COPY_LABEL);
 				DEBUGADD(10,("Label[%u]: '%s'\n",i,shadow_data->labels[i]));
 			}
diff --git a/source3/smbd/lanman.c b/source3/smbd/lanman.c
index b7c74e9..ac4873d 100644
--- a/source3/smbd/lanman.c
+++ b/source3/smbd/lanman.c
@@ -3655,8 +3655,13 @@ static bool api_RNetServerGetInfo(struct smbd_server_connection *sconn,
 	}
 
 	if (uLevel != 20) {
-		srvstr_push(NULL, 0, p, info.info101->server_name, 16,
-			STR_ASCII|STR_UPPER|STR_TERMINATE);
+		size_t len = 0;
+		status = srvstr_push(NULL, 0, p, info.info101->server_name, 16,
+			STR_ASCII|STR_UPPER|STR_TERMINATE, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			errcode = W_ERROR_V(ntstatus_to_werror(status));
+			goto out;
+		}
   	}
 	p += 16;
 	if (uLevel > 0) {
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 692f582..bb4d5f6 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1019,8 +1019,8 @@ bool is_share_read_only_for_token(const char *username,
 
 /* The following definitions come from smbd/srvstr.c  */
 
-size_t srvstr_push_fn(const char *base_ptr, uint16 smb_flags2, void *dest,
-		      const char *src, int dest_len, int flags);
+NTSTATUS srvstr_push_fn(const char *base_ptr, uint16 smb_flags2, void *dest,
+		      const char *src, int dest_len, int flags, size_t *ret_len);
 ssize_t message_push_string(uint8 **outbuf, const char *str, int flags);
 
 /* The following definitions come from smbd/statcache.c  */
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 4cb446f..8dba79b 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -1099,6 +1099,8 @@ void reply_ioctl(struct smb_request *req)
 	switch (ioctl_code) {
 		case IOCTL_QUERY_JOB_INFO:		    
 		{
+			NTSTATUS status;
+			size_t len = 0;
 			files_struct *fsp = file_fsp(
 				req, SVAL(req->vwv+0, 0));
 			if (!fsp) {
@@ -1109,15 +1111,25 @@ void reply_ioctl(struct smb_request *req)
 			/* Job number */
 			SSVAL(p, 0, print_spool_rap_jobid(fsp->print_file));
 
-			srvstr_push((char *)req->outbuf, req->flags2, p+2,
+			status = srvstr_push((char *)req->outbuf, req->flags2, p+2,
 				    lp_netbios_name(), 15,
-				    STR_TERMINATE|STR_ASCII);
+				    STR_TERMINATE|STR_ASCII, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				reply_nterror(req, status);
+				END_PROFILE(SMBioctl);
+				return;
+			}
 			if (conn) {
-				srvstr_push((char *)req->outbuf, req->flags2,
+				status = srvstr_push((char *)req->outbuf, req->flags2,
 					    p+18,
 					    lp_servicename(talloc_tos(),
 							   SNUM(conn)),
-					    13, STR_TERMINATE|STR_ASCII);
+					    13, STR_TERMINATE|STR_ASCII, &len);
+				if (!NT_STATUS_IS_OK(status)) {
+					reply_nterror(req, status);
+					END_PROFILE(SMBioctl);
+					return;
+				}
 			} else {
 				memset(p+18, 0, 13);
 			}
@@ -5740,6 +5752,7 @@ void reply_printqueue(struct smb_request *req)
 			char *p = blob;
 			time_t qtime = spoolss_Time_to_time_t(&info[i].info2.submitted);
 			int qstatus;
+			size_t len = 0;
 			uint16_t qrapjobid = pjobid_to_rap(sharename,
 							info[i].info2.job_id);
 
@@ -5754,9 +5767,12 @@ void reply_printqueue(struct smb_request *req)
 			SSVAL(p, 5, qrapjobid);
 			SIVAL(p, 7, info[i].info2.size);
 			SCVAL(p, 11, 0);
-			srvstr_push(blob, req->flags2, p+12,
-				    info[i].info2.notify_name, 16, STR_ASCII);
-
+			status = srvstr_push(blob, req->flags2, p+12,
+				    info[i].info2.notify_name, 16, STR_ASCII, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				reply_nterror(req, status);
+				goto out;
+			}
 			if (message_push_blob(
 				    &req->outbuf,
 				    data_blob_const(
diff --git a/source3/smbd/srvstr.c b/source3/smbd/srvstr.c
index 648c69f..e6a8541 100644
--- a/source3/smbd/srvstr.c
+++ b/source3/smbd/srvstr.c
@@ -24,16 +24,56 @@
 
 /* Make sure we can't write a string past the end of the buffer */
 
-size_t srvstr_push_fn(const char *base_ptr, uint16 smb_flags2, void *dest,
-		      const char *src, int dest_len, int flags)
+NTSTATUS srvstr_push_fn(const char *base_ptr, uint16 smb_flags2, void *dest,
+		      const char *src, int dest_len, int flags, size_t *ret_len)
 {
+	size_t len;
+	int saved_errno;
+	NTSTATUS status;
+
 	if (dest_len < 0) {
-		return 0;
+		return NT_STATUS_INVALID_PARAMETER;
 	}
 
+	saved_errno = errno;
+	errno = 0;
+
 	/* 'normal' push into size-specified buffer */
-	return push_string_base(base_ptr, smb_flags2, dest, src,
+	len = push_string_base(base_ptr, smb_flags2, dest, src,
 				dest_len, flags);
+
+	if (errno != 0) {
+		/*
+		 * Special case E2BIG, EILSEQ, EINVAL
+		 * as they mean conversion errors here,
+		 * but we don't generically map them as
+		 * they can mean different things in
+		 * generic filesystem calls (such as
+		 * read xattrs).
+		 */
+		if (errno == E2BIG || errno == EILSEQ || errno == EINVAL) {
+			status = NT_STATUS_ILLEGAL_CHARACTER;
+		} else {
+			status = map_nt_error_from_unix_common(errno);
+			/*
+			 * Paranoia - Filter out STATUS_MORE_ENTRIES.
+			 * I don't think we can get this but it has a
+			 * specific meaning to the client.
+			 */
+			if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
+				status = NT_STATUS_UNSUCCESSFUL;
+			}
+		}
+		DEBUG(10,("character conversion failure "
+			"on string (%s) (%s)\n",
+			src, strerror(errno)));
+	} else {
+		/* Success - restore untouched errno. */
+		errno = saved_errno;
+		*ret_len = len;
+		status = NT_STATUS_OK;
+	}
+	return status;
 }
 
 /*******************************************************************
@@ -45,8 +85,9 @@ ssize_t message_push_string(uint8 **outbuf, const char *str, int flags)
 {
 	size_t buf_size = smb_len(*outbuf) + 4;
 	size_t grow_size;
-	size_t result;
+	size_t result = 0;
 	uint8 *tmp;
+	NTSTATUS status;
 
 	/*
 	 * We need to over-allocate, now knowing what srvstr_push will
@@ -62,10 +103,10 @@ ssize_t message_push_string(uint8 **outbuf, const char *str, int flags)
 		return -1;
 	}
 
-	result = srvstr_push((char *)tmp, SVAL(tmp, smb_flg2),
-			     tmp + buf_size, str, grow_size, flags);
+	status = srvstr_push((char *)tmp, SVAL(tmp, smb_flg2),
+			     tmp + buf_size, str, grow_size, flags, &result);
 
-	if (result == 0) {
+	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(0, ("srvstr_push failed\n"));
 		return -1;
 	}
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 70d29f2..bdecc60 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -1602,6 +1602,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	bool was_8_3;
 	int off;
 	int pad = 0;
+	NTSTATUS status;
 
 	*out_of_space = false;
 
@@ -1684,9 +1685,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			p += ucs2_align(base_data, p, 0);
 		}
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE);
+				  STR_TERMINATE, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
 				SCVAL(nameptr, -1, len - 2);
@@ -1722,9 +1726,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		}
 		p += 27;
 		nameptr = p - 1;
-		len = srvstr_push(base_data, flags2,
+		status = srvstr_push(base_data, flags2,
 				  p, fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE | STR_NOALIGN);
+				  STR_TERMINATE | STR_NOALIGN, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
 				len -= 2;
@@ -1747,7 +1754,6 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	{
 		struct ea_list *file_list = NULL;
 		size_t ea_len = 0;
-		NTSTATUS status;
 
 		DEBUG(10,("smbd_marshall_dir_entry: SMB_FIND_EA_LIST\n"));
 		if (!name_list) {
@@ -1787,9 +1793,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		/* Push the ea_data followed by the name. */
 		p += fill_ea_buffer(ctx, p, space_remaining, conn, name_list);
 		nameptr = p;
-		len = srvstr_push(base_data, flags2,
+		status = srvstr_push(base_data, flags2,
 				  p + 1, fname, PTR_DIFF(end_data, p+1),
-				  STR_TERMINATE | STR_NOALIGN);
+				  STR_TERMINATE | STR_NOALIGN, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
 				len -= 2;
@@ -1842,9 +1851,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				memset(mangled_name,'\0',12);
 			}
 			mangled_name[12] = 0;
-			len = srvstr_push(base_data, flags2,
+			status = srvstr_push(base_data, flags2,
 					  p+2, mangled_name, 24,
-					  STR_UPPER|STR_UNICODE);
+					  STR_UPPER|STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return false;
+			}
 			if (len < 24) {
 				memset(p + 2 + len,'\0',24 - len);
 			}
@@ -1853,9 +1865,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			memset(p,'\0',26);
 		}
 		p += 2 + 24;
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(q,0,len);
 		p += len;
 
@@ -1889,9 +1904,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		SOFF_T(p,0,file_size); p += 8;
 		SOFF_T(p,0,allocation_size); p += 8;
 		SIVAL(p,0,mode); p += 4;
-		len = srvstr_push(base_data, flags2,
+		status = srvstr_push(base_data, flags2,
 				  p + 4, fname, PTR_DIFF(end_data, p+4),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(p,0,len);
 		p += 4 + len;
 
@@ -1932,9 +1950,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			SIVAL(p,0,ea_size); /* Extended attributes */
 			p +=4;
 		}
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(q, 0, len);
 		p += len;
 
@@ -1964,9 +1985,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		p += 4;
 		/* this must *not* be null terminated or w2k gets in a loop trying to set an
 		   acl on a dir (tridge) */
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(p, -4, len);
 		p += len;
 
@@ -2011,9 +2035,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		p += 4;
 		SIVAL(p,0,0); p += 4; /* Unknown - reserved ? */
 		SBVAL(p,0,file_index); p += 8;
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(q, 0, len);
 		p += len;
 
@@ -2069,9 +2096,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				memset(mangled_name,'\0',12);
 			}
 			mangled_name[12] = 0;
-			len = srvstr_push(base_data, flags2,
+			status = srvstr_push(base_data, flags2,
 					  p+2, mangled_name, 24,
-					  STR_UPPER|STR_UNICODE);
+					  STR_UPPER|STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return false;
+			}
 			SSVAL(p, 0, len);
 			if (len < 24) {
 				memset(p + 2 + len,'\0',24 - len);
@@ -2083,9 +2113,12 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		p += 26;
 		SSVAL(p,0,0); p += 2; /* Reserved ? */
 		SBVAL(p,0,file_index); p += 8;
-		len = srvstr_push(base_data, flags2, p,
+		status = srvstr_push(base_data, flags2, p,
 				  fname, PTR_DIFF(end_data, p),
-				  STR_TERMINATE_ASCII);
+				  STR_TERMINATE_ASCII, &len);
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
 		SIVAL(q,0,len);
 		p += len;
 
@@ -2121,17 +2154,23 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			DEBUG(10,("smbd_marshall_dir_entry: SMB_FIND_FILE_UNIX\n"));
 			p = store_file_unix_basic(conn, p,
 						NULL, &smb_fname->st);
-			len = srvstr_push(base_data, flags2, p,
+			status = srvstr_push(base_data, flags2, p,
 					  fname, PTR_DIFF(end_data, p),
-					  STR_TERMINATE);
+					  STR_TERMINATE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return false;
+			}
 		} else {
 			DEBUG(10,("smbd_marshall_dir_entry: SMB_FIND_FILE_UNIX_INFO2\n"));
 			p = store_file_unix_basic_info2(conn, p,
 						NULL, &smb_fname->st);
 			nameptr = p;
 			p += 4;
-			len = srvstr_push(base_data, flags2, p, fname,
-					  PTR_DIFF(end_data, p), 0);
+			status = srvstr_push(base_data, flags2, p, fname,
+					  PTR_DIFF(end_data, p), 0, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return false;
+			}
 			SIVAL(nameptr, 0, len);
 		}
 
@@ -3181,11 +3220,14 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)st.st_ex_dev, (u
 			 * this call so try fixing this by adding a terminating null to
 			 * the pushed string. The change here was adding the STR_TERMINATE. JRA.
 			 */
-			len = srvstr_push(
+			status = srvstr_push(
 				pdata, flags2,
 				pdata+l2_vol_szVolLabel, vname,
 				PTR_DIFF(end_data, pdata+l2_vol_szVolLabel),
-				STR_NOALIGN|STR_TERMINATE);
+				STR_NOALIGN|STR_TERMINATE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			SCVAL(pdata,l2_vol_cch,len);
 			data_len = l2_vol_szVolLabel + len;
 			DEBUG(5,("smbd_do_qfsinfo : time = %x, namelen = %u, name = %s\n",
@@ -3218,9 +3260,12 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)st.st_ex_dev, (u
 			SIVAL(pdata,4,255); /* Max filename component length */
 			/* NOTE! the fstype must *not* be null terminated or win98 won't recognise it
 				and will think we can't do long filenames */
-			len = srvstr_push(pdata, flags2, pdata+12, fstype,
+			status = srvstr_push(pdata, flags2, pdata+12, fstype,
 					  PTR_DIFF(end_data, pdata+12),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			SIVAL(pdata,8,len);
 			data_len = 12 + len;
 			if (max_data_bytes >= 16 && data_len > max_data_bytes) {
@@ -3234,8 +3279,11 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)st.st_ex_dev, (u
 
 		case SMB_QUERY_FS_LABEL_INFO:
 		case SMB_FS_LABEL_INFORMATION:
-			len = srvstr_push(pdata, flags2, pdata+4, vname,
-					  PTR_DIFF(end_data, pdata+4), 0);
+			status = srvstr_push(pdata, flags2, pdata+4, vname,
+					  PTR_DIFF(end_data, pdata+4), 0, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			data_len = 4 + len;
 			SIVAL(pdata,0,len);
 			break;
@@ -3251,9 +3299,12 @@ cBytesSector=%u, cUnitTotal=%u, cUnitAvail=%d\n", (unsigned int)st.st_ex_dev, (u
 				(str_checksum(get_local_machine_name())<<16));
 
 			/* Max label len is 32 characters. */
-			len = srvstr_push(pdata, flags2, pdata+18, vname,
+			status = srvstr_push(pdata, flags2, pdata+18, vname,
 					  PTR_DIFF(end_data, pdata+18),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			SIVAL(pdata,12,len);
 			data_len = 18+len;
 
@@ -4735,10 +4786,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 						True,conn->params)) {
 				return NT_STATUS_NO_MEMORY;
 			}
-			len = srvstr_push(dstart, flags2,
+			status = srvstr_push(dstart, flags2,
 					  pdata+4, mangled_name,
 					  PTR_DIFF(dend, pdata+4),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			data_size = 4 + len;
 			SIVAL(pdata,0,len);
 			*fixed_portion = 8;
@@ -4750,10 +4804,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 			/*
 			  this must be *exactly* right for ACLs on mapped drives to work
 			 */
-			len = srvstr_push(dstart, flags2,
+			status = srvstr_push(dstart, flags2,
 					  pdata+4, dos_fname,
 					  PTR_DIFF(dend, pdata+4),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			DEBUG(10,("smbd_do_qfilepathinfo: SMB_QUERY_FILE_NAME_INFO\n"));
 			data_size = 4 + len;
 			SIVAL(pdata,0,len);
@@ -4796,10 +4853,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 			pdata += 24;
 			SIVAL(pdata,0,ea_size);
 			pdata += 4; /* EA info */
-			len = srvstr_push(dstart, flags2,
+			status = srvstr_push(dstart, flags2,
 					  pdata+4, dos_fname,
 					  PTR_DIFF(dend, pdata+4),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			SIVAL(pdata,0,len);
 			pdata += 4 + len;
 			data_size = PTR_DIFF(pdata,(*ppdata));
@@ -4833,10 +4893,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 
 			pdata += 0x60;
 
-			len = srvstr_push(dstart, flags2,
+			status = srvstr_push(dstart, flags2,
 					  pdata+4, dos_fname,
 					  PTR_DIFF(dend, pdata+4),
-					  STR_UNICODE);
+					  STR_UNICODE, &len);
+			if (!NT_STATUS_IS_OK(status)) {
+				return status;
+			}
 			SIVAL(pdata,0,len);
 			pdata += 4 + len;
 			data_size = PTR_DIFF(pdata,(*ppdata));
@@ -5030,10 +5093,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn,
 					return map_nt_error_from_unix(errno);
 				}
 				buffer[link_len] = 0;
-				len = srvstr_push(dstart, flags2,
+				status = srvstr_push(dstart, flags2,
 						  pdata, buffer,
 						  PTR_DIFF(dend, pdata),
-						  STR_TERMINATE);
+						  STR_TERMINATE, &len);
+				if (!NT_STATUS_IS_OK(status)) {
+					return status;
+				}
 				pdata += len;
 				data_size = PTR_DIFF(pdata,(*ppdata));
 
@@ -8602,6 +8668,8 @@ static void call_trans2ioctl(connection_struct *conn,
 {
 	char *pdata = *ppdata;
 	files_struct *fsp = file_fsp(req, SVAL(req->vwv+15, 0));
+	NTSTATUS status;
+	size_t len = 0;
 
 	/* check for an invalid fid before proceeding */
 
@@ -8625,12 +8693,20 @@ static void call_trans2ioctl(connection_struct *conn,
 		/* Job number */
 		SSVAL(pdata, 0, print_spool_rap_jobid(fsp->print_file));
 
-		srvstr_push(pdata, req->flags2, pdata + 2,
+		status = srvstr_push(pdata, req->flags2, pdata + 2,
 			    lp_netbios_name(), 15,
-			    STR_ASCII|STR_TERMINATE); /* Our NetBIOS name */
-		srvstr_push(pdata, req->flags2, pdata+18,
+			    STR_ASCII|STR_TERMINATE, &len); /* Our NetBIOS name */
+		if (!NT_STATUS_IS_OK(status)) {
+			reply_nterror(req, status);
+			return;
+		}
+		status = srvstr_push(pdata, req->flags2, pdata+18,
 			    lp_servicename(talloc_tos(), SNUM(conn)), 13,
-			    STR_ASCII|STR_TERMINATE); /* Service name */
+			    STR_ASCII|STR_TERMINATE, &len); /* Service name */
+		if (!NT_STATUS_IS_OK(status)) {
+			reply_nterror(req, status);
+			return;
+		}
 		send_trans2_replies(conn, req, NT_STATUS_OK, *pparams, 0, *ppdata, 32,
 				    max_data_bytes);
 		return;
-- 
2.1.0.rc2.206.gedb03e5


From aec6e8a5a4a41a38a37c18e4936db236e9f42199 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 11:36:41 -0700
Subject: [PATCH 04/10] s3: smbd: Change smbd_marshall_dir_entry() to return an
 NTSTATUS. Returns STATUS_MORE_ENTRIES on out of space.

Bug 10775 - smbd crashes when accessing garbage filenames

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/trans2.c | 47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index bdecc60..1a145c7 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -1570,7 +1570,7 @@ static bool smbd_dirptr_lanman2_mode_fn(TALLOC_CTX *ctx,
 	return true;
 }
 
-static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
+static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				    connection_struct *conn,
 				    uint16_t flags2,
 				    uint32_t info_level,
@@ -1647,7 +1647,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			"for padding (wanted %u, had %d)\n",
 			(unsigned int)pad,
 			space_remaining ));
-		return false; /* Not finished - just out of space */
+		return STATUS_MORE_ENTRIES; /* Not finished - just out of space */
 	}
 
 	off += pad;
@@ -1689,7 +1689,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
@@ -1730,7 +1730,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  p, fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE | STR_NOALIGN, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
@@ -1757,7 +1757,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 
 		DEBUG(10,("smbd_marshall_dir_entry: SMB_FIND_EA_LIST\n"));
 		if (!name_list) {
-			return false;
+			return NT_STATUS_INVALID_PARAMETER;
 		}
 		if (requires_resume_key) {
 			SIVAL(p,0,reskey);
@@ -1787,7 +1787,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				"(wanted %u, had %d)\n",
 				(unsigned int)PTR_DIFF(p + 255 + ea_len,pdata),
 				space_remaining ));
-			return False; /* Not finished - just out of space */
+			return STATUS_MORE_ENTRIES; /* Not finished - just out of space */
 		}
 
 		/* Push the ea_data followed by the name. */
@@ -1797,7 +1797,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  p + 1, fname, PTR_DIFF(end_data, p+1),
 				  STR_TERMINATE | STR_NOALIGN, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		if (flags2 & FLAGS2_UNICODE_STRINGS) {
 			if (len > 2) {
@@ -1855,7 +1855,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 					  p+2, mangled_name, 24,
 					  STR_UPPER|STR_UNICODE, &len);
 			if (!NT_STATUS_IS_OK(status)) {
-				return false;
+				return status;
 			}
 			if (len < 24) {
 				memset(p + 2 + len,'\0',24 - len);
@@ -1869,7 +1869,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(q,0,len);
 		p += len;
@@ -1908,7 +1908,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  p + 4, fname, PTR_DIFF(end_data, p+4),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(p,0,len);
 		p += 4 + len;
@@ -1954,7 +1954,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(q, 0, len);
 		p += len;
@@ -1989,7 +1989,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(p, -4, len);
 		p += len;
@@ -2039,7 +2039,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(q, 0, len);
 		p += len;
@@ -2100,7 +2100,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 					  p+2, mangled_name, 24,
 					  STR_UPPER|STR_UNICODE, &len);
 			if (!NT_STATUS_IS_OK(status)) {
-				return false;
+				return status;
 			}
 			SSVAL(p, 0, len);
 			if (len < 24) {
@@ -2117,7 +2117,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				  fname, PTR_DIFF(end_data, p),
 				  STR_TERMINATE_ASCII, &len);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			return status;
 		}
 		SIVAL(q,0,len);
 		p += len;
@@ -2158,7 +2158,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 					  fname, PTR_DIFF(end_data, p),
 					  STR_TERMINATE, &len);
 			if (!NT_STATUS_IS_OK(status)) {
-				return false;
+				return status;
 			}
 		} else {
 			DEBUG(10,("smbd_marshall_dir_entry: SMB_FIND_FILE_UNIX_INFO2\n"));
@@ -2169,7 +2169,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			status = srvstr_push(base_data, flags2, p, fname,
 					  PTR_DIFF(end_data, p), 0, &len);
 			if (!NT_STATUS_IS_OK(status)) {
-				return false;
+				return status;
 			}
 			SIVAL(nameptr, 0, len);
 		}
@@ -2198,7 +2198,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		break;
 
 	default:
-		return false;
+		return NT_STATUS_INVALID_LEVEL;
 	}
 
 	if (PTR_DIFF(p,pdata) > space_remaining) {
@@ -2207,7 +2207,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 			"(wanted %u, had %d)\n",
 			(unsigned int)PTR_DIFF(p,pdata),
 			space_remaining ));
-		return false; /* Not finished - just out of space */
+		return STATUS_MORE_ENTRIES; /* Not finished - just out of space */
 	}
 
 	/* Setup the last entry pointer, as an offset from base_data */
@@ -2215,7 +2215,7 @@ static bool smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	/* Advance the data pointer to the next slot */
 	*ppdata = p;
 
-	return true;
+	return NT_STATUS_OK;
 }
 
 bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
@@ -2248,6 +2248,7 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 	struct smbd_dirptr_lanman2_state state;
 	bool ok;
 	uint64_t last_entry_off = 0;
+	NTSTATUS status;
 
 	ZERO_STRUCT(state);
 	state.conn = conn;
@@ -2289,7 +2290,7 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 
 	*got_exact_match = state.got_exact_match;
 
-	ok = smbd_marshall_dir_entry(ctx,
+	status = smbd_marshall_dir_entry(ctx,
 				     conn,
 				     flags2,
 				     info_level,
@@ -2309,11 +2310,11 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 				     &last_entry_off);
 	TALLOC_FREE(fname);
 	TALLOC_FREE(smb_fname);
-	if (*out_of_space) {
+	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
 		dptr_SeekDir(dirptr, prev_dirpos);
 		return false;
 	}
-	if (!ok) {
+	if (!NT_STATUS_IS_OK(status)) {
 		return false;
 	}
 
-- 
2.1.0.rc2.206.gedb03e5


From c9849794a0b54511a13be9e4c56d1b4cbd6e3487 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 11:40:19 -0700
Subject: [PATCH 05/10] s3: smbd: smbd_marshall_dir_entry() no longer needs
 explicit 'out_of_space' parameter.

Handle this in the caller when it returns STATUS_MORE_ENTRIES.

Bug 10775 - smbd crashes when accessing garbage filenames

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/trans2.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 1a145c7..b950820 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -1586,7 +1586,6 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 				    char *base_data,
 				    char **ppdata,
 				    char *end_data,
-				    bool *out_of_space,
 				    uint64_t *last_entry_off)
 {
 	char *p, *q, *pdata = *ppdata;
@@ -1604,8 +1603,6 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	int pad = 0;
 	NTSTATUS status;
 
-	*out_of_space = false;
-
 	ZERO_STRUCT(mdate_ts);
 	ZERO_STRUCT(adate_ts);
 	ZERO_STRUCT(create_date_ts);
@@ -1642,7 +1639,6 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	pad -= off;
 
 	if (pad && pad > space_remaining) {
-		*out_of_space = true;
 		DEBUG(9,("smbd_marshall_dir_entry: out of space "
 			"for padding (wanted %u, had %d)\n",
 			(unsigned int)pad,
@@ -1782,7 +1778,6 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 		/* We need to determine if this entry will fit in the space available. */
 		/* Max string size is 255 bytes. */
 		if (PTR_DIFF(p + 255 + ea_len,pdata) > space_remaining) {
-			*out_of_space = true;
 			DEBUG(9,("smbd_marshall_dir_entry: out of space "
 				"(wanted %u, had %d)\n",
 				(unsigned int)PTR_DIFF(p + 255 + ea_len,pdata),
@@ -2202,7 +2197,6 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	}
 
 	if (PTR_DIFF(p,pdata) > space_remaining) {
-		*out_of_space = true;
 		DEBUG(9,("smbd_marshall_dir_entry: out of space "
 			"(wanted %u, had %d)\n",
 			(unsigned int)PTR_DIFF(p,pdata),
@@ -2306,11 +2300,11 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 				     base_data,
 				     ppdata,
 				     end_data,
-				     out_of_space,
 				     &last_entry_off);
 	TALLOC_FREE(fname);
 	TALLOC_FREE(smb_fname);
 	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
+		*out_of_space = true;
 		dptr_SeekDir(dirptr, prev_dirpos);
 		return false;
 	}
-- 
2.1.0.rc2.206.gedb03e5


From 11b40f42b714f52ef3cac7a004c83d8ccb04d3ca Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 14:49:37 -0700
Subject: [PATCH 06/10] s3: smbd: SMB2 - change smbd_dirptr_lanman2_entry() to
 return an NTSTATUS.

Handle the errors correctly at the top level inside the SMB2 server.

Bug 10775 - smbd crashes when accessing garbage filenames

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/globals.h   |  2 +-
 source3/smbd/smb2_find.c | 15 ++++++++++-----
 source3/smbd/trans2.c    | 14 ++++++++------
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index f78ce45..20ab75d 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -185,7 +185,7 @@ bool smbd_dirptr_get_entry(TALLOC_CTX *ctx,
 			   uint32_t *_mode,
 			   long *_prev_offset);
 
-bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
+NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 			       connection_struct *conn,
 			       struct dptr_struct *dirptr,
 			       uint16 flags2,
diff --git a/source3/smbd/smb2_find.c b/source3/smbd/smb2_find.c
index 45b0890..af9995e 100644
--- a/source3/smbd/smb2_find.c
+++ b/source3/smbd/smb2_find.c
@@ -432,14 +432,13 @@ static struct tevent_req *smbd_smb2_find_send(TALLOC_CTX *mem_ctx,
 				     true);
 
 	while (true) {
-		bool ok;
 		bool got_exact_match = false;
 		bool out_of_space = false;
 		int space_remaining = in_output_buffer_length - off;
 
 		SMB_ASSERT(space_remaining >= 0);
 
-		ok = smbd_dirptr_lanman2_entry(state,
+		status = smbd_dirptr_lanman2_entry(state,
 					       conn,
 					       fsp->dptr,
 					       smbreq->flags2,
@@ -462,12 +461,18 @@ static struct tevent_req *smbd_smb2_find_send(TALLOC_CTX *mem_ctx,
 
 		off = (int)PTR_DIFF(pdata, base_data);
 
-		if (!ok) {
-			if (num > 0) {
+		if (!NT_STATUS_IS_OK(status)) {
+			if (NT_STATUS_EQUAL(status, NT_STATUS_ILLEGAL_CHARACTER)) {
+				/*
+				 * Bad character conversion on name. Ignore this
+				 * entry.
+				 */
+				continue;
+			} else if (num > 0) {
 				SIVAL(state->out_output_buffer.data, last_entry_off, 0);
 				tevent_req_done(req);
 				return tevent_req_post(req, ev);
-			} else if (out_of_space) {
+			} else if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
 				tevent_req_nterror(req, NT_STATUS_INFO_LENGTH_MISMATCH);
 				return tevent_req_post(req, ev);
 			} else {
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index b950820..2d6c261 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -2212,7 +2212,7 @@ static NTSTATUS smbd_marshall_dir_entry(TALLOC_CTX *ctx,
 	return NT_STATUS_OK;
 }
 
-bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
+NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 			       connection_struct *conn,
 			       struct dptr_struct *dirptr,
 			       uint16 flags2,
@@ -2279,7 +2279,7 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 				   &mode,
 				   &prev_dirpos);
 	if (!ok) {
-		return false;
+		return NT_STATUS_END_OF_FILE;
 	}
 
 	*got_exact_match = state.got_exact_match;
@@ -2306,14 +2306,14 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
 		*out_of_space = true;
 		dptr_SeekDir(dirptr, prev_dirpos);
-		return false;
+		return status;
 	}
 	if (!NT_STATUS_IS_OK(status)) {
-		return false;
+		return status;
 	}
 
 	*_last_entry_off = last_entry_off;
-	return true;
+	return NT_STATUS_OK;
 }
 
 static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
@@ -2337,13 +2337,14 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 {
 	uint8_t align = 4;
 	const bool do_pad = true;
+	NTSTATUS status;
 
 	if (info_level >= 1 && info_level <= 3) {
 		/* No alignment on earlier info levels. */
 		align = 1;
 	}
 
-	return smbd_dirptr_lanman2_entry(ctx, conn, dirptr, flags2,
+	status = smbd_dirptr_lanman2_entry(ctx, conn, dirptr, flags2,
 					 path_mask, dirtype, info_level,
 					 requires_resume_key, dont_descend, ask_sharemode,
 					 align, do_pad,
@@ -2351,6 +2352,7 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 					 space_remaining,
 					 out_of_space, got_exact_match,
 					 last_entry_off, name_list);
+	return NT_STATUS_IS_OK(status);
 }
 
 /****************************************************************************
-- 
2.1.0.rc2.206.gedb03e5


From dbab1bd7ad67c3ff0e8c50f9d3f7fba1043e5f91 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 14:54:56 -0700
Subject: [PATCH 07/10] s3: smbd: Remove unneeded 'out_of_space' parameter from
 smbd_dirptr_lanman2_entry().

This can now be handled by checking for the STATUS_MORE_ENTRIES error return.

Bug 10775 - smbd crashes when accessing garbage filenames

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/globals.h   |  1 -
 source3/smbd/smb2_find.c |  2 --
 source3/smbd/trans2.c    | 10 ++++++----
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index 20ab75d..5a8e3bd 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -201,7 +201,6 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 			       char *base_data,
 			       char *end_data,
 			       int space_remaining,
-			       bool *out_of_space,
 			       bool *got_exact_match,
 			       int *_last_entry_off,
 			       struct ea_list *name_list);
diff --git a/source3/smbd/smb2_find.c b/source3/smbd/smb2_find.c
index af9995e..2dab86b 100644
--- a/source3/smbd/smb2_find.c
+++ b/source3/smbd/smb2_find.c
@@ -433,7 +433,6 @@ static struct tevent_req *smbd_smb2_find_send(TALLOC_CTX *mem_ctx,
 
 	while (true) {
 		bool got_exact_match = false;
-		bool out_of_space = false;
 		int space_remaining = in_output_buffer_length - off;
 
 		SMB_ASSERT(space_remaining >= 0);
@@ -454,7 +453,6 @@ static struct tevent_req *smbd_smb2_find_send(TALLOC_CTX *mem_ctx,
 					       base_data,
 					       end_data,
 					       space_remaining,
-					       &out_of_space,
 					       &got_exact_match,
 					       &last_entry_off,
 					       NULL);
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 2d6c261..e4d64e8 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -2228,7 +2228,6 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 			       char *base_data,
 			       char *end_data,
 			       int space_remaining,
-			       bool *out_of_space,
 			       bool *got_exact_match,
 			       int *_last_entry_off,
 			       struct ea_list *name_list)
@@ -2251,7 +2250,6 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 	state.has_wild = dptr_has_wild(dirptr);
 	state.got_exact_match = false;
 
-	*out_of_space = false;
 	*got_exact_match = false;
 
 	p = strrchr_m(path_mask,'/');
@@ -2304,7 +2302,6 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 	TALLOC_FREE(fname);
 	TALLOC_FREE(smb_fname);
 	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
-		*out_of_space = true;
 		dptr_SeekDir(dirptr, prev_dirpos);
 		return status;
 	}
@@ -2339,6 +2336,8 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 	const bool do_pad = true;
 	NTSTATUS status;
 
+	*out_of_space = false;
+
 	if (info_level >= 1 && info_level <= 3) {
 		/* No alignment on earlier info levels. */
 		align = 1;
@@ -2350,8 +2349,11 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 					 align, do_pad,
 					 ppdata, base_data, end_data,
 					 space_remaining,
-					 out_of_space, got_exact_match,
+					 got_exact_match,
 					 last_entry_off, name_list);
+	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
+		*out_of_space = true;
+	}
 	return NT_STATUS_IS_OK(status);
 }
 
-- 
2.1.0.rc2.206.gedb03e5


From 5b772ca741ea45d74ab04ea7b9c5b249fd927f7d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 15:05:24 -0700
Subject: [PATCH 08/10] s3: smbd: Change get_lanman2_dir_entry() to return the
 full NTSTATUS.

Handle the errors correctly at the level above inside the SMB1 server.

Bug 10775 - smbd crashes when accessing garbage filenames

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/trans2.c | 52 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index e4d64e8..6a66f3b 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -2313,7 +2313,7 @@ NTSTATUS smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx,
 	return NT_STATUS_OK;
 }
 
-static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
+static NTSTATUS get_lanman2_dir_entry(TALLOC_CTX *ctx,
 				connection_struct *conn,
 				struct dptr_struct *dirptr,
 				uint16 flags2,
@@ -2327,23 +2327,19 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 				char *base_data,
 				char *end_data,
 				int space_remaining,
-				bool *out_of_space,
 				bool *got_exact_match,
 				int *last_entry_off,
 				struct ea_list *name_list)
 {
 	uint8_t align = 4;
 	const bool do_pad = true;
-	NTSTATUS status;
-
-	*out_of_space = false;
 
 	if (info_level >= 1 && info_level <= 3) {
 		/* No alignment on earlier info levels. */
 		align = 1;
 	}
 
-	status = smbd_dirptr_lanman2_entry(ctx, conn, dirptr, flags2,
+	return smbd_dirptr_lanman2_entry(ctx, conn, dirptr, flags2,
 					 path_mask, dirtype, info_level,
 					 requires_resume_key, dont_descend, ask_sharemode,
 					 align, do_pad,
@@ -2351,10 +2347,6 @@ static bool get_lanman2_dir_entry(TALLOC_CTX *ctx,
 					 space_remaining,
 					 got_exact_match,
 					 last_entry_off, name_list);
-	if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
-		*out_of_space = true;
-	}
-	return NT_STATUS_IS_OK(status);
 }
 
 /****************************************************************************
@@ -2628,7 +2620,7 @@ total_data=%u (should be %u)\n", (unsigned int)total_data, (unsigned int)IVAL(pd
 			out_of_space = True;
 			finished = False;
 		} else {
-			finished = !get_lanman2_dir_entry(ctx,
+			ntstatus = get_lanman2_dir_entry(ctx,
 					conn,
 					dirptr,
 					req->flags2,
@@ -2636,14 +2628,24 @@ total_data=%u (should be %u)\n", (unsigned int)total_data, (unsigned int)IVAL(pd
 					requires_resume_key,dont_descend,
 					ask_sharemode,
 					&p,pdata,data_end,
-					space_remaining, &out_of_space,
+					space_remaining,
 					&got_exact_match,
 					&last_entry_off, ea_list);
+			if (NT_STATUS_EQUAL(ntstatus,
+					NT_STATUS_ILLEGAL_CHARACTER)) {
+				/*
+				 * Bad character conversion on name. Ignore this
+				 * entry.
+				 */
+				continue;
+			}
+			if (NT_STATUS_EQUAL(ntstatus, STATUS_MORE_ENTRIES)) {
+				out_of_space = true;
+			} else {
+				finished = !NT_STATUS_IS_OK(ntstatus);
+			}
 		}
 
-		if (finished && out_of_space)
-			finished = False;
-
 		if (!finished && !out_of_space)
 			numentries++;
 
@@ -3004,7 +3006,7 @@ total_data=%u (should be %u)\n", (unsigned int)total_data, (unsigned int)IVAL(pd
 			out_of_space = True;
 			finished = False;
 		} else {
-			finished = !get_lanman2_dir_entry(ctx,
+			ntstatus = get_lanman2_dir_entry(ctx,
 						conn,
 						dirptr,
 						req->flags2,
@@ -3012,14 +3014,24 @@ total_data=%u (should be %u)\n", (unsigned int)total_data, (unsigned int)IVAL(pd
 						requires_resume_key,dont_descend,
 						ask_sharemode,
 						&p,pdata,data_end,
-						space_remaining, &out_of_space,
+						space_remaining,
 						&got_exact_match,
 						&last_entry_off, ea_list);
+			if (NT_STATUS_EQUAL(ntstatus,
+					NT_STATUS_ILLEGAL_CHARACTER)) {
+				/*
+				 * Bad character conversion on name. Ignore this
+				 * entry.
+				 */
+				continue;
+			}
+			if (NT_STATUS_EQUAL(ntstatus, STATUS_MORE_ENTRIES)) {
+				out_of_space = true;
+			} else {
+				finished = !NT_STATUS_IS_OK(ntstatus);
+			}
 		}
 
-		if (finished && out_of_space)
-			finished = False;
-
 		if (!finished && !out_of_space)
 			numentries++;
 
-- 
2.1.0.rc2.206.gedb03e5


From 23abf89763eed3178b6ccf2f7724d73e03c10033 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 26 Aug 2014 16:39:56 -0700
Subject: [PATCH 09/10] s3: smbd: Fix a tricky slow-path case - don't return a
 mangled name for a name that cannot be converted.

For a name that contains an illegal Windows character, the
directory listing code returns the mangled 8.3 name as the
primary name for the file.

If the original (non-mangled) filename cannot be converted
to UCS2 on the wire via iconv due to conversion error, we
should skip that name when returning a directory listing,
as we can't map back from a returned 8.3 name to a usable
non-mangled filename if the client sends it back to us.

As this is only done in a very slow path (name must be mangled)
I don't feel too bad about using a talloc/free pair here.

Bug 10775 - smbd crashes when accessing garbage filenames

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/trans2.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 6a66f3b..f1ccc7e 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -1469,6 +1469,31 @@ static bool smbd_dirptr_lanman2_match_fn(TALLOC_CTX *ctx,
 
 	/* Mangle fname if it's an illegal name. */
 	if (mangle_must_mangle(dname, state->conn->params)) {
+		/*
+		 * Slow path - ensure we can push the original name as UCS2. If
+		 * not, then just don't return this name.
+		 */
+		NTSTATUS status;
+		size_t ret_len = 0;
+		size_t len = (strlen(dname) + 2) * 4; /* Allow enough space. */
+		uint8 *tmp = talloc_array(talloc_tos(),
+					uint8,
+					len);
+
+		status = srvstr_push(NULL,
+			FLAGS2_UNICODE_STRINGS,
+			tmp,
+			dname,
+			len,
+			STR_TERMINATE,
+			&ret_len);
+
+		TALLOC_FREE(tmp);
+
+		if (!NT_STATUS_IS_OK(status)) {
+			return false;
+		}
+
 		ok = name_to_8_3(dname, mangled_name,
 				 true, state->conn->params);
 		if (!ok) {
-- 
2.1.0.rc2.206.gedb03e5


From 90dbca9247e5a800dfc837d3d1ba582868c20d75 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 27 Aug 2014 13:15:29 -0700
Subject: [PATCH 10/10] Add test suite for iconv conversion fail of bad names
 over SMB1/SMB3.

Bug 10775 - smbd crashes when accessing garbage filenames

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 selftest/target/Samba3.pm                 |  37 ++++++++
 source3/script/tests/test_smbclient_s3.sh | 144 ++++++++++++++++++++++++++++++
 2 files changed, 181 insertions(+)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 5544105..de40ced 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -873,6 +873,9 @@ sub provision($$$$$$)
 	my $msdfs_deeppath="$msdfs_shrdir/deeppath";
 	push(@dirs,$msdfs_deeppath);
 
+	my $badnames_shrdir="$shrdir/badnames";
+	push(@dirs,$badnames_shrdir);
+
 	# this gets autocreated by winbindd
 	my $wbsockdir="$prefix_abs/winbindd";
 	my $wbsockprivdir="$lockdir/winbindd_privileged";
@@ -925,6 +928,36 @@ sub provision($$$$$$)
 	symlink "msdfs:$server_ip\\ro-tmp", "$msdfs_shrdir/msdfs-src1";
 	symlink "msdfs:$server_ipv6\\ro-tmp", "$msdfs_shrdir/deeppath/msdfs-src2";
 
+	##
+	## create bad names in $badnames_shrdir
+	##
+	## (An invalid name, would be mangled to 8.3).
+        my $badname_target = "$badnames_shrdir/\340|\231\216\377\177";
+        unless (open(BADNAME_TARGET, ">$badname_target")) {
+                warn("Unable to open $badname_target");
+                return undef;
+        }
+        close(BADNAME_TARGET);
+        chmod 0666, $badname_target;
+
+	## (A bad name, would not be mangled to 8.3).
+        my $badname_target = "$badnames_shrdir/\240\276\346\327\377\177";
+        unless (open(BADNAME_TARGET, ">$badname_target")) {
+                warn("Unable to open $badname_target");
+                return undef;
+        }
+        close(BADNAME_TARGET);
+        chmod 0666, $badname_target;
+
+	## (A bad good name).
+        my $badname_target = "$badnames_shrdir/blank.txt";
+        unless (open(BADNAME_TARGET, ">$badname_target")) {
+                warn("Unable to open $badname_target");
+                return undef;
+        }
+        close(BADNAME_TARGET);
+        chmod 0666, $badname_target;
+
 	my $conffile="$libdir/server.conf";
 
 	my $nss_wrapper_pl = "$ENV{PERL} $self->{srcdir}/lib/nss_wrapper/nss_wrapper.pl";
@@ -1182,6 +1215,10 @@ sub provision($$$$$$)
 	fruit:metadata = netatalk
 	fruit:locking = netatalk
 	fruit:encoding = native
+
+[badname-tmp]
+	path = $badnames_shrdir
+	guest ok = yes
 	";
 	close(CONF);
 
diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index 596cd42..67ac94a 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -722,6 +722,146 @@ EOF
     fi
 }
 
+# Test accessing an share with bad names (won't convert).
+test_bad_names()
+{
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/badname-tmp -I $SERVER_IP $ADDARGS -c ls 2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed accessing badname-tmp (SMB1) with error $ret"
+	false
+	return
+    fi
+
+    echo "$out" | wc -l 2>&1 | grep 6
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep of number of lines (1) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep 'Domain=.*OS=.*Server='
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (1) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  \. *D'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (2) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  \.\. *D'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (3) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  blank.txt *N'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (4) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^ *$'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (5) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep 'blocks of size.*blocks available'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - grep (6) failed with $ret"
+	false
+    fi
+
+    # Now check again with -mSMB3
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/badname-tmp -I $SERVER_IP -mSMB3 $ADDARGS -c ls 2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed accessing badname-tmp (SMB3) with error $ret"
+	false
+	return
+    fi
+
+    echo "$out" | wc -l 2>&1 | grep 6
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep of number of lines (1) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep 'Domain=.*OS=.*Server='
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (1) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  \. *D'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (2) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  \.\. *D'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (3) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^  blank.txt *N'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (4) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep '^ *$'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (5) failed with $ret"
+	false
+    fi
+
+    echo "$out" | grep 'blocks of size.*blocks available'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed listing \\badname-tmp - SMB3 grep (6) failed with $ret"
+	false
+    fi
+}
 
 LOGDIR_PREFIX=test_smbclient_s3
 
@@ -798,6 +938,10 @@ testit "list with backup privilege" \
     test_backup_privilege_list || \
     failed=`expr $failed + 1`
 
+testit "list a share with bad names (won't convert)" \
+    test_bad_names || \
+    failed=`expr $failed + 1`
+
 testit "rm -rf $LOGDIR" \
     rm -rf $LOGDIR || \
     failed=`expr $failed + 1`
-- 
2.1.0.rc2.206.gedb03e5



More information about the samba-technical mailing list