[Bug 47832] New: FindFirstFileExW believes every directory entry has been read if NtQueryDirectoryFile underfills buffer
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: wine-bugs(a)winehq.org Reporter: qsniyg(a)mail.com 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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 --- Comment #1 from qsniyg <qsniyg(a)mail.com> --- Created attachment 65340 --> https://bugs.winehq.org/attachment.cgi?id=65340 Patch to remove underfill check -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 qsniyg <qsniyg(a)mail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |leslie_alistair(a)hotmail.com | |, z.figura12(a)gmail.com --- Comment #2 from qsniyg <qsniyg(a)mail.com> --- 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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 qsniyg <qsniyg(a)mail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |qsniyg(a)mail.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 --- Comment #3 from Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> --- (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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 --- Comment #4 from qsniyg <qsniyg(a)mail.com> --- (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 :) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 qsniyg <qsniyg(a)mail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julliard(a)winehq.org --- Comment #5 from qsniyg <qsniyg(a)mail.com> --- 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) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 --- Comment #6 from Mathew Hodson <mathew.hodson(a)gmail.com> --- https://source.winehq.org/git/wine.git/commitdiff/1658d5a20de4b12df539f3e5ec... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED Fixed by SHA1| |1658d5a20de4b12df539f3e5ecc | |11b81984684a4 Version|unspecified |4.17 --- Comment #7 from Zebediah Figura <z.figura12(a)gmail.com> --- Marking fixed then. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #8 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 4.21. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 Michael Stefaniuc <mstefani(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |4.0.x -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 Michael Stefaniuc <mstefani(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|4.0.x |--- --- Comment #9 from Michael Stefaniuc <mstefani(a)winehq.org> --- Removing the 4.0.x milestone from bug fixes included in 4.0.4. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 Joe Davison <joe(a)warhaggis.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joe(a)warhaggis.com --- Comment #10 from Joe Davison <joe(a)warhaggis.com> --- 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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 --- Comment #11 from Jeff Zaroyko <jeffz(a)jeffz.name> --- (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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=47832 --- Comment #12 from qsniyg <qsniyg(a)mail.com> --- (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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
WineHQ Bugzilla