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

[David Disseldorp]

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.

Cheers, David