[PATCH] Fix bug #9130 - Certain xattrs cause Windows error 0x800700FF

[Jeremy Allison]

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.