[PATCH] Fix nasty vfs_fruit bug 13646

[Jeremy Allison]

On Wed, Oct 31, 2018 at 04:15:37PM +0100, Ralph Böhme via samba-technical wrote:
> > 
> > PATCH 28/34] vfs_fruit: prepare fruit_pread_meta() for reading on fake-fd
> > 
> > in this hunk:
> > 
> > +       if (nread == -1 && fio->created) {
> > +               AfpInfo *ai = NULL;
> > +               char afpinfo_buf[AFP_INFO_SIZE];
> > +
> > +               ai = afpinfo_new(talloc_tos());
> > +               if (ai == NULL) {
> > +                       return -1;
> > +               }
> > +
> > +               nread = afpinfo_pack(ai, afpinfo_buf);
> > +               TALLOC_FREE(ai);
> > +               if (nread != AFP_INFO_SIZE) {
> > +                       return -1;
> > +               }
> > +
> > +               memcpy(data, afpinfo_buf, to_return);
> > +               return nread;
> > +       }
> > 
> > 'to_return' is not defined here. I think the last
> > lines shoud read:
> > 
> > +               nread = MIN(n, nread);
> > +               memcpy(data, afpinfo_buf, nread);
> > +               return nread;
> > +       }
> > 
> > can you confirm ?
> 
> you just have to apply the other fruit patchset from the list in the correct
> order, then it works. :) Sorry...
> 
> Attached is a patchset with the patches for bugs 13642 and 13646 combined.

Sorry Ralph but I think that this:

 Subject: [PATCH 36/42] vfs_fruit: prepare fruit_pread_meta() for reading on fake-fd

is still wrong. In fruit_pread_meta() the requested
amount to be read is size_t n into void *data.

Inside fruit_pread_meta() we have:

        to_return = MIN(n, AFP_INFO_SIZE);

so to_return is now the correct amount to be
returned  - allowing for the fact the caller
might have asked for less than AFP_INFO_SIZE.

However your patch does:

+       if (nread == -1 && fio->created) {
+               AfpInfo *ai = NULL;
+               char afpinfo_buf[AFP_INFO_SIZE];
+
+               ai = afpinfo_new(talloc_tos());
+               if (ai == NULL) {
+                       return -1;
+               }
+
+               nread = afpinfo_pack(ai, afpinfo_buf);
+               TALLOC_FREE(ai);
+               if (nread != AFP_INFO_SIZE) {
+                       return -1;
+               }
+
+               memcpy(data, afpinfo_buf, to_return);
+               return nread;
+       }

Which will return nread == AFP_INFO_SIZE *even
if the caller asked for *less* (remember, to_read
is MIN(n, AFP_INFO_SIZE)) !

This should instead be:

		return to_return;

as that is the amount we just copied into data, and
may be less than nread.

If you agree with this fix, LGTM.

Jeremy.