[Bug 47832] New: FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

Bug ID: 47832
           Summary: FindFirstFileExW believes every directory entry has
                    been read if NtQueryDirectoryFile underfills buffer
           Product: Wine
           Version: unspecified
          Hardware: x86
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: kernel32
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

When using USVFS (through ModOrganizer 2), it will hook NtQueryDirectoryFile,
and its implementation will occasionally underfill the buffer, even if there
are more directory entries to be read.

Due to this check in FindFirstFileExW:

if (!has_wildcard || info->data_len < info->data_size - max_entry_size)
        {
            ...
            info->data_size = 0;  /* we read everything */
        }

It will stop FindNextFileW from reading more. Windows doesn't seem to exhibit
this behavior.

Removing the `info->data_len < info->data_size - max_entry_size` check allows
it to work properly.

Unfortunately I wasn't able to find any reference to why that check exists. My
best guess after looking through the source code is that it's an optimization,
as the code should still run fine if wine's NtQueryDirectoryFile implementation
underfills the buffer (therefore indicating that there are no more files needed
to be read).

So far I've tested many various applications with the check patched out, with
no obvious regressions.

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

--- Comment #1 from qsniyg  ---
Created attachment 65340
  --> https://bugs.winehq.org/attachment.cgi?id=65340
Patch to remove underfill check

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

qsniyg  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
                   |                            |, [e-mail]

--- Comment #2 from qsniyg  ---
CCing Alistair and Zebediah as https://wiki.winehq.org/Wine-Staging_Development
suggests, to have the patch considered for wine-staging.

Since this patch removes a check that exists in a core DLL, this could lead to
issues, which is partly why I'm submitting it for the staging branch.

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

qsniyg  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

Alistair Leslie-Hughes  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

--- Comment #3 from Alistair Leslie-Hughes  ---
(In reply to qsniyg from comment #2)
> CCing Alistair and Zebediah as
> https://wiki.winehq.org/Wine-Staging_Development suggests, to have the patch
> considered for wine-staging.
>
> Since this patch removes a check that exists in a core DLL, this could lead
> to issues, which is partly why I'm submitting it for the staging branch.

Thats cool,  Can you please attached a patch with Full name, so we can give
credit where credit is due.

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

--- Comment #4 from qsniyg  ---
(In reply to Alistair Leslie-Hughes from comment #3)
> Thats cool,  Can you please attached a patch with Full name, so we can give
> credit where credit is due.

For personal reasons (unrelated to wine), I'd prefer not, if possible. Both of
these patches (including https://bugs.winehq.org/show_bug.cgi?id=47833) are in
public domain however, so feel free to take credit instead. In any case, these
patches aren't very complex, so there isn't much to take credit for anyways :)

I understand (and completely respect) why wine has the real name policy, I
realize this might make it a bit more complicated, which is why I initially
avoided submitting these to wine directly (instead opting to submit it to
forks, hoping someone else would take over submitting these to mainline wine).

If there's any specific process I have to go through to transfer ownership or
properly license in public domain, please let me know :)

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

qsniyg  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #5 from qsniyg  ---
I just did a quick git blame of the check, it appears to have been added by
Alexandre Julliard back in 2013:

https://github.com/wine-mirror/wine/commit/7ce90cc7198a6dee75a999e9502ff997e...

Unfortunately it still doesn't shed much light on the purpose of the check.

I've CC'd him as I believe he would be the right person to take a look at this,
I hope it isn't an issue to do this :) (I wasn't able to find a mention about
CCing developers on the wine wiki pages)

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

--- Comment #6 from Mathew Hodson  ---
https://source.winehq.org/git/wine.git/commitdiff/1658d5a20de4b12df539f3e5ec...

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

Zebediah Figura  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED
      Fixed by SHA1|                            |1658d5a20de4b12df539f3e5ecc
                   |                            |11b81984684a4
            Version|unspecified                 |4.17

--- Comment #7 from Zebediah Figura  ---
Marking fixed then.

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #8 from Alexandre Julliard  ---
Closing bugs fixed in 4.21.

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

Michael Stefaniuc  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.0.x

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

Michael Stefaniuc  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.0.x                       |---

--- Comment #9 from Michael Stefaniuc  ---
Removing the 4.0.x milestone from bug fixes included in 4.0.4.

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

Joe Davison  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #10 from Joe Davison  ---
This was fixed in 4.21 but it appears there's been a regression since. The last
version of Wine that Mod Organizer's USVFS is functional in, as far as I can
tell, is 5.0.

In an initial attempt to figure out the cause I looked at kernelbase/file.c
where most of kernel32/file.c is now located and found some discrepancies. I
made a frankensteined file.c containing Myah's patch but it had no effect.

At any rate, I think this bug should be reopened as there definitely seems to
be a regression.

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

--- Comment #11 from Jeff Zaroyko  ---
(In reply to Joe Davison from comment #10)
> This was fixed in 4.21 but it appears there's been a regression since. The
> last version of Wine that Mod Organizer's USVFS is functional in, as far as
> I can tell, is 5.0.
> At any rate, I think this bug should be reopened as there definitely seems
> to be a regression.

Please file a new bug for the regression occurred after being fixed.  This one
would only be reopened if it was never actually fixed.   If you can help with
regression testing that would be super: 
https://wiki.winehq.org/Regression_Testing

[Bug 47832] FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=47832

--- Comment #12 from qsniyg  ---
(In reply to Joe Davison from comment #10)
> This was fixed in 4.21 but it appears there's been a regression since. The
> last version of Wine that Mod Organizer's USVFS is functional in, as far as
> I can tell, is 5.0.
>
> In an initial attempt to figure out the cause I looked at kernelbase/file.c
> where most of kernel32/file.c is now located and found some discrepancies. I
> made a frankensteined file.c containing Myah's patch but it had no effect.
>
> At any rate, I think this bug should be reopened as there definitely seems
> to be a regression.

The patch is still there, the important bit for this issue is that the
`info->data_len < info->data_size - max_entry_size` check has been removed,
which is still the case.

If USVFS fails, then something else is the cause. There could be a number of
different causes. An issue that was exhibited in one of the later staging
releases was that wine would call internal versions of the syscalls, rather
than the exported thunks, which meant that it would bypass USVFS's hooked
calls. This has since been fixed, but it's not unlikely that this might be
exhibited somewhere else by accident.