[PATCH] Fix bug #9130 - Certain xattrs cause Windows error 0x800700FF
Jeremy Allison
jra at samba.org
Thu Mar 28 09:47:54 MDT 2013
On Thu, Mar 28, 2013 at 03:44:40PM +0100, David Disseldorp wrote:
> Hi Jeremy,
>
> A few minor comments on the patches below...
>
> On Wed, 27 Mar 2013 15:05:05 -0700
> Jeremy Allison <jra at samba.org> wrote:
>
> > Patchset for master that fixes copying files with
> > extended attributes from Linux or Solaris onto Windows
> > over SMB2.
> >
> > You can read the details here:
> >
> > https://bugzilla.samba.org/show_bug.cgi?id=9130
> >
> > Fix has been confirmed by the OEM who reported
> > it on both 4.0.x and 4.6.x platforms. Includes
> > an enhancement to the SMB2 torture tests to
> > test the change.
> >
> > Please review and push so I can get the fix
> > into release branches.
>
> 8f75904 Modify fill_ea_chained_buffer() to be able to do size calculation only, no marshalling.
> fill_ea_chained_buffer() continues to do pointer arithmetic based on
> the potentially NULL pdata. I'd prefer it to be split into a separate
> function.
>
> 96e1776 Fix bug #9130 - Certain xattrs cause Windows error 0x800700FF
> 316 static NTSTATUS get_ea_list_from_file_path(
> ...
> 352 status = get_ea_value(mem_ctx, conn, fsp,
> 353 fname, names[i],
> 354 &listp->ea);
> 355
> 356 if (!NT_STATUS_IS_OK(status)) {
> 357 return status;
> 358 }
> 359
> 360 if (listp->ea.value.length == 0) {
> 361 /*
> 362 * We can never return a zero length EA.
> 363 * Windows reports the EA's as corrupted.
> 364 */
> 365 TALLOC_FREE(listp);
> 366 continue;
> 367 }
>
> AFAICT, listp should be used as the memory context for get_ea_value()
> to avoid leaking ea.value.data on continue.
>
> 4342bfc Fix bug #9130 - Certain xattrs cause Windows error 0x800700FF
> 77 NTSTATUS pvfs_query_ea_list(struct pvfs_state *pvfs, TALLOC_CTX *mem_ctx,
> ...
> 96 eas->num_eas = num_names;
> 97 for (i=0;i<num_names;i++) {
> 98 int j;
> 99 eas->eas[i].flags = 0;
> 100 eas->eas[i].name.s = names[i].name.s;
> 101 eas->eas[i].value = data_blob(NULL, 0);
> 102 for (j=0;j<ealist->num_eas;j++) {
> 103 if (strcasecmp_m(eas->eas[i].name.s,
> 104 ealist->eas[j].name) == 0) {
> 105 if (ealist->eas[i].value.length == 0) {
> 106 continue;
> 107 }
>
> This doesn't look right, as the data_blob(NULL, 0) EA would remain in
> the array. num_eas should be disjoint from num_names, and calculated
> on the fly IIUC.
Good points. I'll update and re-submit - thanks !
Jeremy.
More information about the samba-technical
mailing list