[SCM] Samba Shared Repository - branch v4-0-test updated

Karolin Seeger kseeger at samba.org
Mon Apr 8 02:35:05 MDT 2013


The branch, v4-0-test has been updated
       via  2584dd2 Ensure EA value is allocated on the right context.
       via  abac017 Final fix for bug #9130 - Certain xattrs cause Windows error 0x800700FF
       via  a5ddcbc Ensure we don't return uninitialized memory in the pad bytes.
       via  d0e5282 Add a test to show that zero-length EA's are never returned over SMB2.
       via  c9e43e7 Fix bug #9130 - Certain xattrs cause Windows error 0x800700FF
       via  2eea03c Fix bug #9130 - Certain xattrs cause Windows error 0x800700FF
       via  b710723 Change estimate_ea_size() to correctly estimate the EA size over SMB2.
       via  8c632ac Modify fill_ea_chained_buffer() to be able to do size calculation only, no marshalling.
       via  61055c2 Ensure we can never return an uninitialized EA list.
      from  6786a77 BUG 9758: Don't leak the epm_Map policy handle.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v4-0-test


- Log -----------------------------------------------------------------
commit 2584dd2278acf7ed4c3c9b5a156deddee3b963f6
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Mar 28 08:55:11 2013 -0700

    Ensure EA value is allocated on the right context.
    
    Ensure we free on error condition (tidyup, not a leak).
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Disseldorp <ddiss at suse.de>
    
    Autobuild-User(master): David Disseldorp <ddiss at samba.org>
    Autobuild-Date(master): Tue Apr  2 21:54:33 CEST 2013 on sn-devel-104
    
    The last 9 patches address bug #9130 - Certain xattrs cause Windows error
    0x800700FF.
    
    Autobuild-User(v4-0-test): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(v4-0-test): Mon Apr  8 10:34:37 CEST 2013 on sn-devel-104

commit abac017e22eeba24816b755c6a8910b819dcad4d
Author: Jeremy Allison <jra at samba.org>
Date:   Wed Mar 27 11:54:34 2013 -0700

    Final fix for bug #9130 - Certain xattrs cause Windows error 0x800700FF
    
    The spec lies when it says that NextEntryOffset is the only value
    considered when finding the next EA. We were adding 4 more extra
    pad bytes than needed (i.e. if the next entry already was on a 4
    byte boundary, then we were adding 4 additional pad bytes).
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Disseldorp <ddiss at suse.de>

commit a5ddcbc15be038b28213c4a095b8a327eee9833d
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Mar 26 16:46:51 2013 -0700

    Ensure we don't return uninitialized memory in the pad bytes.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Disseldorp <ddiss at suse.de>

commit d0e5282ae8f3c7f89b4a84557527ae7827407dcc
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Mar 26 13:26:49 2013 -0700

    Add a test to show that zero-length EA's are never returned over SMB2.
    
    Zero length EA's only delete an EA, never store. Proves we should
    never return zero-length EA's even if they have been set on the
    POSIX side.
    
    ntvfs server doesn't implement the FULL_EA_INFORMATION setinfo
    call, so add to selftest/knownfail.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Disseldorp <ddiss at suse.de>

commit c9e43e78d10af29ca13c73dcdbab950199088019
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Mar 26 16:38:00 2013 -0700

    Fix bug #9130 - Certain xattrs cause Windows error 0x800700FF
    
    Ensure ntvfs server never returns zero length EA's.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Disseldorp <ddiss at suse.de>

commit 2eea03c4540ea3c14927333f2e244ce5f4587d47
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Mar 26 16:37:22 2013 -0700

    Fix bug #9130 - Certain xattrs cause Windows error 0x800700FF
    
    Ensure we never return any zero-length EA's.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Disseldorp <ddiss at suse.de>

commit b71072301d37378c45d6a5639d1aeb51f80ea0e0
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Mar 26 15:54:31 2013 -0700

    Change estimate_ea_size() to correctly estimate the EA size over SMB2.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Disseldorp <ddiss at suse.de>

commit 8c632acc66ef39a2ec5cbd3eda859cfa91ea382a
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Mar 26 15:46:06 2013 -0700

    Modify fill_ea_chained_buffer() to be able to do size calculation only, no marshalling.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Disseldorp <ddiss at suse.de>

commit 61055c25785ad00db06ece2fd804bca5bdcd4431
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Mar 29 10:07:20 2013 -0700

    Ensure we can never return an uninitialized EA list.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: David Disseldorp <ddiss at suse.de>

-----------------------------------------------------------------------

Summary of changes:
 selftest/knownfail                   |    1 +
 source3/smbd/trans2.c                |   70 +++++++++++++++----
 source4/ntvfs/posix/pvfs_qfileinfo.c |    6 ++
 source4/torture/smb2/setinfo.c       |  121 ++++++++++++++++++++++++++++++++++
 4 files changed, 183 insertions(+), 15 deletions(-)


Changeset truncated at 500 lines:

diff --git a/selftest/knownfail b/selftest/knownfail
index ecb1934..e3964d6 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -162,6 +162,7 @@
 ^samba4.blackbox.upgradeprovision.alpha13.ldapcmp_sd\(none\) # Due to something rewriting the NT ACL on DNS objects
 ^samba4.blackbox.upgradeprovision.alpha13.ldapcmp_full_sd\(none\) # Due to something rewriting the NT ACL on DNS objects
 ^samba4.blackbox.upgradeprovision.release-4-0-0.ldapcmp_sd\(none\) # Due to something rewriting the NT ACL on DNS objects
+^samba4.smb2.setinfo.setinfo # ntvfs doesn't support FULL_EA_INFORMATION set.
 ^samba3.smb2.create.gentest
 ^samba3.smb2.create.blob
 ^samba3.smb2.create.open
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index 062af25..c129946 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -322,6 +322,7 @@ static NTSTATUS get_ea_list_from_file_path(TALLOC_CTX *mem_ctx, connection_struc
 	NTSTATUS status;
 
 	*pea_total_len = 0;
+	*ea_list = NULL;
 
 	status = get_ea_names_from_file(talloc_tos(), conn, fsp, fname,
 					&names, &num_names);
@@ -348,14 +349,24 @@ static NTSTATUS get_ea_list_from_file_path(TALLOC_CTX *mem_ctx, connection_struc
 			return NT_STATUS_NO_MEMORY;
 		}
 
-		status = get_ea_value(mem_ctx, conn, fsp,
+		status = get_ea_value(listp, conn, fsp,
 				      fname, names[i],
 				      &listp->ea);
 
 		if (!NT_STATUS_IS_OK(status)) {
+			TALLOC_FREE(listp);
 			return status;
 		}
 
+		if (listp->ea.value.length == 0) {
+			/*
+			 * We can never return a zero length EA.
+			 * Windows reports the EA's as corrupted.
+			 */
+			TALLOC_FREE(listp);
+			continue;
+		}
+
 		push_ascii_fstring(dos_ea_name, listp->ea.name);
 
 		*pea_total_len +=
@@ -457,6 +468,7 @@ static NTSTATUS fill_ea_chained_buffer(TALLOC_CTX *mem_ctx,
 {
 	uint8_t *p = (uint8_t *)pdata;
 	uint8_t *last_start = NULL;
+	bool do_store_data = (pdata != NULL);
 
 	*ret_data_size = 0;
 
@@ -468,8 +480,9 @@ static NTSTATUS fill_ea_chained_buffer(TALLOC_CTX *mem_ctx,
 		size_t dos_namelen;
 		fstring dos_ea_name;
 		size_t this_size;
+		size_t pad = 0;
 
-		if (last_start) {
+		if (last_start != NULL && do_store_data) {
 			SIVAL(last_start, 0, PTR_DIFF(p, last_start));
 		}
 		last_start = p;
@@ -486,23 +499,30 @@ static NTSTATUS fill_ea_chained_buffer(TALLOC_CTX *mem_ctx,
 		this_size = 0x08 + dos_namelen + 1 + ea_list->ea.value.length;
 
 		if (ea_list->next) {
-			size_t pad = 4 - (this_size % 4);
+			pad = (4 - (this_size % 4)) % 4;
 			this_size += pad;
 		}
 
-		if (this_size > total_data_size) {
-			return NT_STATUS_INFO_LENGTH_MISMATCH;
+		if (do_store_data) {
+			if (this_size > total_data_size) {
+				return NT_STATUS_INFO_LENGTH_MISMATCH;
+			}
+
+			/* We know we have room. */
+			SIVAL(p, 0x00, 0); /* next offset */
+			SCVAL(p, 0x04, ea_list->ea.flags);
+			SCVAL(p, 0x05, dos_namelen);
+			SSVAL(p, 0x06, ea_list->ea.value.length);
+			strlcpy((char *)(p+0x08), dos_ea_name, dos_namelen+1);
+			memcpy(p + 0x08 + dos_namelen + 1, ea_list->ea.value.data, ea_list->ea.value.length);
+			if (pad) {
+				memset(p + 0x08 + dos_namelen + 1 + ea_list->ea.value.length,
+					'\0',
+					pad);
+			}
+			total_data_size -= this_size;
 		}
 
-		/* We know we have room. */
-		SIVAL(p, 0x00, 0); /* next offset */
-		SCVAL(p, 0x04, ea_list->ea.flags);
-		SCVAL(p, 0x05, dos_namelen);
-		SSVAL(p, 0x06, ea_list->ea.value.length);
-		strlcpy((char *)(p+0x08), dos_ea_name, dos_namelen+1);
-		memcpy(p + 0x08 + dos_namelen + 1, ea_list->ea.value.data, ea_list->ea.value.length);
-
-		total_data_size -= this_size;
 		p += this_size;
 	}
 
@@ -515,7 +535,7 @@ static unsigned int estimate_ea_size(connection_struct *conn, files_struct *fsp,
 {
 	size_t total_ea_len = 0;
 	TALLOC_CTX *mem_ctx;
-	struct ea_list *ea_list;
+	struct ea_list *ea_list = NULL;
 
 	if (!lp_ea_support(SNUM(conn))) {
 		return 0;
@@ -530,6 +550,26 @@ static unsigned int estimate_ea_size(connection_struct *conn, files_struct *fsp,
 		fsp = NULL;
 	}
 	(void)get_ea_list_from_file_path(mem_ctx, conn, fsp, smb_fname->base_name, &total_ea_len, &ea_list);
+	if(conn->sconn->using_smb2) {
+		NTSTATUS status;
+		unsigned int ret_data_size;
+		/*
+		 * We're going to be using fill_ea_chained_buffer() to
+		 * marshall EA's - this size is significantly larger
+		 * than the SMB1 buffer. Re-calculate the size without
+		 * marshalling.
+		 */
+		status = fill_ea_chained_buffer(mem_ctx,
+						NULL,
+						0,
+						&ret_data_size,
+						conn,
+						ea_list);
+		if (!NT_STATUS_IS_OK(status)) {
+			ret_data_size = 0;
+		}
+		total_ea_len = ret_data_size;
+	}
 	TALLOC_FREE(mem_ctx);
 	return total_ea_len;
 }
diff --git a/source4/ntvfs/posix/pvfs_qfileinfo.c b/source4/ntvfs/posix/pvfs_qfileinfo.c
index ac3e6a6..33ff9ce 100644
--- a/source4/ntvfs/posix/pvfs_qfileinfo.c
+++ b/source4/ntvfs/posix/pvfs_qfileinfo.c
@@ -102,6 +102,9 @@ NTSTATUS pvfs_query_ea_list(struct pvfs_state *pvfs, TALLOC_CTX *mem_ctx,
 		for (j=0;j<ealist->num_eas;j++) {
 			if (strcasecmp_m(eas->eas[i].name.s, 
 				       ealist->eas[j].name) == 0) {
+				if (ealist->eas[j].value.length == 0) {
+					continue;
+				}
 				eas->eas[i].value = ealist->eas[j].value;
 				break;
 			}
@@ -134,6 +137,9 @@ static NTSTATUS pvfs_query_all_eas(struct pvfs_state *pvfs, TALLOC_CTX *mem_ctx,
 	for (i=0;i<ealist->num_eas;i++) {
 		eas->eas[eas->num_eas].flags = 0;
 		eas->eas[eas->num_eas].name.s = ealist->eas[i].name;
+		if (ealist->eas[i].value.length == 0) {
+			continue;
+		}
 		eas->eas[eas->num_eas].value = ealist->eas[i].value;
 		eas->num_eas++;
 	}
diff --git a/source4/torture/smb2/setinfo.c b/source4/torture/smb2/setinfo.c
index 719e8f0..fee7293 100644
--- a/source4/torture/smb2/setinfo.c
+++ b/source4/torture/smb2/setinfo.c
@@ -30,6 +30,36 @@
 #include "libcli/security/security.h"
 #include "librpc/gen_ndr/ndr_security.h"
 
+static bool find_returned_ea(union smb_fileinfo *finfo2,
+				const char *eaname,
+				const char *eavalue)
+{
+	unsigned int i;
+	unsigned int num_eas = finfo2->all_eas.out.num_eas;
+	struct ea_struct *eas = finfo2->all_eas.out.eas;
+
+	for (i = 0; i < num_eas; i++) {
+		if (eas[i].name.s == NULL) {
+			continue;
+		}
+		/* Windows capitalizes returned EA names. */
+		if (strcasecmp_m(eas[i].name.s, eaname)) {
+			continue;
+		}
+		if (eavalue == NULL && eas[i].value.length == 0) {
+			/* Null value, found it ! */
+			return true;
+		}
+		if (eas[i].value.length == strlen(eavalue) &&
+				memcmp(eas[i].value.data,
+					eavalue,
+					strlen(eavalue)) == 0) {
+			return true;
+		}
+	}
+	return false;
+}
+
 #define BASEDIR ""
 
 #define FAIL_UNLESS(__cond)					\
@@ -60,6 +90,7 @@ bool torture_smb2_setinfo(struct torture_context *tctx)
 	const char *call_name;
 	time_t basetime = (time(NULL) - 86400) & ~1;
 	int n = time(NULL) % 100;
+	struct ea_struct ea;
 	
 	ZERO_STRUCT(handle);
 	
@@ -276,6 +307,96 @@ bool torture_smb2_setinfo(struct torture_context *tctx)
 	CHECK_CALL(SEC_DESC, NT_STATUS_OK);
 	FAIL_UNLESS(smb2_util_verify_sd(tctx, tree, handle, sd));
 
+	torture_comment(tctx, "Check zero length EA's behavior\n");
+
+	/* Set a new EA. */
+	sfinfo.full_ea_information.in.eas.num_eas = 1;
+	ea.flags = 0;
+	ea.name.private_length = 6;
+	ea.name.s = "NewEA";
+	ea.value = data_blob_string_const("testme");
+	sfinfo.full_ea_information.in.eas.eas = &ea;
+	CHECK_CALL(FULL_EA_INFORMATION, NT_STATUS_OK);
+
+	/* Does it still exist ? */
+	finfo2.generic.level = RAW_FILEINFO_SMB2_ALL_EAS;
+	finfo2.generic.in.file.handle = handle;
+	finfo2.all_eas.in.continue_flags = 1;
+	status2 = smb2_getinfo_file(tree, tctx, &finfo2);
+	if (!NT_STATUS_IS_OK(status2)) {
+		torture_result(tctx, TORTURE_FAIL, "(%s) %s - %s\n", __location__,
+			"SMB2_ALL_EAS", nt_errstr(status2));
+		ret = false;
+		goto done;
+	}
+
+	/* Note on Windows EA name is returned capitalized. */
+	if (!find_returned_ea(&finfo2, "NewEA", "testme")) {
+		torture_result(tctx, TORTURE_FAIL, "(%s) Missing EA 'NewEA'\n", __location__);
+		ret = false;
+	}
+
+	/* Now zero it out (should delete it) */
+	sfinfo.full_ea_information.in.eas.num_eas = 1;
+	ea.flags = 0;
+	ea.name.private_length = 6;
+	ea.name.s = "NewEA";
+	ea.value = data_blob_null;
+	sfinfo.full_ea_information.in.eas.eas = &ea;
+	CHECK_CALL(FULL_EA_INFORMATION, NT_STATUS_OK);
+
+	/* Does it still exist ? */
+	finfo2.generic.level = RAW_FILEINFO_SMB2_ALL_EAS;
+	finfo2.generic.in.file.handle = handle;
+	finfo2.all_eas.in.continue_flags = 1;
+	status2 = smb2_getinfo_file(tree, tctx, &finfo2);
+	if (!NT_STATUS_IS_OK(status2)) {
+		torture_result(tctx, TORTURE_FAIL, "(%s) %s - %s\n", __location__,
+			"SMB2_ALL_EAS", nt_errstr(status2));
+		ret = false;
+		goto done;
+	}
+
+	if (find_returned_ea(&finfo2, "NewEA", NULL)) {
+		torture_result(tctx, TORTURE_FAIL, "(%s) EA 'NewEA' should be deleted\n", __location__);
+		ret = false;
+	}
+
+	/* Set a zero length EA. */
+	sfinfo.full_ea_information.in.eas.num_eas = 1;
+	ea.flags = 0;
+	ea.name.private_length = 6;
+	ea.name.s = "ZeroEA";
+	ea.value = data_blob_null;
+	sfinfo.full_ea_information.in.eas.eas = &ea;
+	CHECK_CALL(FULL_EA_INFORMATION, NT_STATUS_OK);
+
+	/* Does it still exist ? */
+	finfo2.generic.level = RAW_FILEINFO_SMB2_ALL_EAS;
+	finfo2.generic.in.file.handle = handle;
+	finfo2.all_eas.in.continue_flags = 1;
+	status2 = smb2_getinfo_file(tree, tctx, &finfo2);
+	if (!NT_STATUS_IS_OK(status2)) {
+		torture_result(tctx, TORTURE_FAIL, "(%s) %s - %s\n", __location__,
+			"SMB2_ALL_EAS", nt_errstr(status2));
+		ret = false;
+		goto done;
+	}
+
+	/* Over SMB2 ZeroEA should not exist. */
+	if (!find_returned_ea(&finfo2, "EAONE", "VALUE1")) {
+		torture_result(tctx, TORTURE_FAIL, "(%s) Missing EA 'EAONE'\n", __location__);
+		ret = false;
+	}
+	if (!find_returned_ea(&finfo2, "SECONDEA", "ValueTwo")) {
+		torture_result(tctx, TORTURE_FAIL, "(%s) Missing EA 'SECONDEA'\n", __location__);
+		ret = false;
+	}
+	if (find_returned_ea(&finfo2, "ZeroEA", NULL)) {
+		torture_result(tctx, TORTURE_FAIL, "(%s) Found null EA 'ZeroEA'\n", __location__);
+		ret = false;
+	}
+
 done:
 	status = smb2_util_close(tree, handle);
 	if (NT_STATUS_IS_ERR(status)) {


-- 
Samba Shared Repository


More information about the samba-cvs mailing list