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 <j...@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 <j...@samba.org> Reviewed-by: David Disseldorp <dd...@suse.de> Autobuild-User(master): David Disseldorp <dd...@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 <ksee...@samba.org> Autobuild-Date(v4-0-test): Mon Apr 8 10:34:37 CEST 2013 on sn-devel-104 commit abac017e22eeba24816b755c6a8910b819dcad4d Author: Jeremy Allison <j...@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 <j...@samba.org> Reviewed-by: David Disseldorp <dd...@suse.de> commit a5ddcbc15be038b28213c4a095b8a327eee9833d Author: Jeremy Allison <j...@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 <j...@samba.org> Reviewed-by: David Disseldorp <dd...@suse.de> commit d0e5282ae8f3c7f89b4a84557527ae7827407dcc Author: Jeremy Allison <j...@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 <j...@samba.org> Reviewed-by: David Disseldorp <dd...@suse.de> commit c9e43e78d10af29ca13c73dcdbab950199088019 Author: Jeremy Allison <j...@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 <j...@samba.org> Reviewed-by: David Disseldorp <dd...@suse.de> commit 2eea03c4540ea3c14927333f2e244ce5f4587d47 Author: Jeremy Allison <j...@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 <j...@samba.org> Reviewed-by: David Disseldorp <dd...@suse.de> commit b71072301d37378c45d6a5639d1aeb51f80ea0e0 Author: Jeremy Allison <j...@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 <j...@samba.org> Reviewed-by: David Disseldorp <dd...@suse.de> commit 8c632acc66ef39a2ec5cbd3eda859cfa91ea382a Author: Jeremy Allison <j...@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 <j...@samba.org> Reviewed-by: David Disseldorp <dd...@suse.de> commit 61055c25785ad00db06ece2fd804bca5bdcd4431 Author: Jeremy Allison <j...@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 <j...@samba.org> Reviewed-by: David Disseldorp <dd...@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