The Samba-Bugzilla – Attachment 8719 Details for
Bug 9130
Certain xattrs cause Windows error 0x800700FF
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for 4.0.x that went into master.
bug-9130-4.0.patch (text/plain), 16.34 KB, created by
Jeremy Allison
on 2013-04-03 16:25:26 UTC
(
hide
)
Description:
git-am fix for 4.0.x that went into master.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2013-04-03 16:25:26 UTC
Size:
16.34 KB
patch
obsolete
>From bcd9fda0d302fa2472c3dc8513e614d34778732f Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 29 Mar 2013 10:07:20 -0700 >Subject: [PATCH 1/9] Ensure we can never return an uninitialized EA list. > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: David Disseldorp <ddiss@suse.de> >--- > source3/smbd/trans2.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > >diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c >index 062af25..7eb9ecf 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); >@@ -515,7 +516,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; >-- >1.8.1.3 > > >From e6b530d1e9bee237dbd8fb3f3cd6bbdbaaada5a0 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Mar 2013 15:46:06 -0700 >Subject: [PATCH 2/9] Modify fill_ea_chained_buffer() to be able to do size > calculation only, no marshalling. > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: David Disseldorp <ddiss@suse.de> >--- > source3/smbd/trans2.c | 27 +++++++++++++++------------ > 1 file changed, 15 insertions(+), 12 deletions(-) > >diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c >index 7eb9ecf..dd8f881 100644 >--- a/source3/smbd/trans2.c >+++ b/source3/smbd/trans2.c >@@ -458,6 +458,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; > >@@ -470,7 +471,7 @@ static NTSTATUS fill_ea_chained_buffer(TALLOC_CTX *mem_ctx, > fstring dos_ea_name; > size_t this_size; > >- if (last_start) { >+ if (last_start != NULL && do_store_data) { > SIVAL(last_start, 0, PTR_DIFF(p, last_start)); > } > last_start = p; >@@ -491,19 +492,21 @@ static NTSTATUS fill_ea_chained_buffer(TALLOC_CTX *mem_ctx, > 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); >+ 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; > } > >-- >1.8.1.3 > > >From eb781fd6afe7bc492bc88b8c03763041a501a5fc Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Mar 2013 15:54:31 -0700 >Subject: [PATCH 3/9] Change estimate_ea_size() to correctly estimate the EA > size over SMB2. > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: David Disseldorp <ddiss@suse.de> >--- > source3/smbd/trans2.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > >diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c >index dd8f881..e9ff321 100644 >--- a/source3/smbd/trans2.c >+++ b/source3/smbd/trans2.c >@@ -534,6 +534,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; > } >-- >1.8.1.3 > > >From d8dcf4f514ff8cc5e24707c21817354f7631a468 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Mar 2013 16:37:22 -0700 >Subject: [PATCH 4/9] Fix bug #9130 - Certain xattrs cause Windows error > 0x800700FF > >Ensure we never return any zero-length EA's. > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: David Disseldorp <ddiss@suse.de> >--- > source3/smbd/trans2.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > >diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c >index e9ff321..325f263 100644 >--- a/source3/smbd/trans2.c >+++ b/source3/smbd/trans2.c >@@ -357,6 +357,15 @@ static NTSTATUS get_ea_list_from_file_path(TALLOC_CTX *mem_ctx, connection_struc > 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 += >-- >1.8.1.3 > > >From 27768cfcbc56674143b158a252e6aa823ee20c45 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Mar 2013 16:38:00 -0700 >Subject: [PATCH 5/9] Fix bug #9130 - Certain xattrs cause Windows error > 0x800700FF > >Ensure ntvfs server never returns zero length EA's. > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: David Disseldorp <ddiss@suse.de> >--- > source4/ntvfs/posix/pvfs_qfileinfo.c | 6 ++++++ > 1 file changed, 6 insertions(+) > >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++; > } >-- >1.8.1.3 > > >From 531c63e1937075058c3eb412a1bdbb856096793e Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Mar 2013 13:26:49 -0700 >Subject: [PATCH 6/9] 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@samba.org> >Reviewed-by: David Disseldorp <ddiss@suse.de> >--- > selftest/knownfail | 1 + > source4/torture/smb2/setinfo.c | 121 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 122 insertions(+) > >diff --git a/selftest/knownfail b/selftest/knownfail >index 84eb769..5a20a12 100644 >--- a/selftest/knownfail >+++ b/selftest/knownfail >@@ -159,6 +159,7 @@ > ^samba4.rpc.lsa.forest.trust #Not fully provided by Samba4 > ^samba4.blackbox.kinit\(.*\).kinit with user password for expired password\(.*\) # We need to work out why this fails only during the pw change > ^samba4.blackbox.dbcheck\(vampire_dc\).dbcheck\(vampire_dc:local\) # Due to replicating with --domain-critical-only we fail dbcheck on this database >+^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/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)) { >-- >1.8.1.3 > > >From 703853780b3b6199f927a99352f286bb0e9c1e8e Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 26 Mar 2013 16:46:51 -0700 >Subject: [PATCH 7/9] Ensure we don't return uninitialized memory in the pad > bytes. > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: David Disseldorp <ddiss@suse.de> >--- > source3/smbd/trans2.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > >diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c >index 325f263..bc8f32c 100644 >--- a/source3/smbd/trans2.c >+++ b/source3/smbd/trans2.c >@@ -479,6 +479,7 @@ 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 != NULL && do_store_data) { > SIVAL(last_start, 0, PTR_DIFF(p, last_start)); >@@ -497,7 +498,7 @@ 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); > this_size += pad; > } > >@@ -513,6 +514,11 @@ static NTSTATUS fill_ea_chained_buffer(TALLOC_CTX *mem_ctx, > 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; > } > >-- >1.8.1.3 > > >From 10b1696f48dd9ae3033baf3af149f0bde1a7570f Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 27 Mar 2013 11:54:34 -0700 >Subject: [PATCH 8/9] 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@samba.org> >Reviewed-by: David Disseldorp <ddiss@suse.de> >--- > source3/smbd/trans2.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c >index bc8f32c..ce9b556 100644 >--- a/source3/smbd/trans2.c >+++ b/source3/smbd/trans2.c >@@ -498,7 +498,7 @@ 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) { >- pad = 4 - (this_size % 4); >+ pad = (4 - (this_size % 4)) % 4; > this_size += pad; > } > >-- >1.8.1.3 > > >From 855ba82b8726dc03f560d78e9ddd193def85e745 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 28 Mar 2013 08:55:11 -0700 >Subject: [PATCH 9/9] 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@samba.org> >Reviewed-by: David Disseldorp <ddiss@suse.de> > >Autobuild-User(master): David Disseldorp <ddiss@samba.org> >Autobuild-Date(master): Tue Apr 2 21:54:33 CEST 2013 on sn-devel-104 >--- > source3/smbd/trans2.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > >diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c >index ce9b556..c129946 100644 >--- a/source3/smbd/trans2.c >+++ b/source3/smbd/trans2.c >@@ -349,11 +349,12 @@ 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; > } > >-- >1.8.1.3 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=8719">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
ddiss
:
review+
Actions:
View
Attachments on
bug 9130
:
7855
|
8668
|
8669
|
8685
|
8686
|
8687
|
8688
|
8690
|
8691
|
8696
|
8699
|
8701
|
8719
|
8733