[Bug 47424] New: DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value
https://bugs.winehq.org/show_bug.cgi?id=47424 Bug ID: 47424 Summary: DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value Product: Wine Version: 4.0 Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: critical Priority: P2 Component: -unknown Assignee: wine-bugs(a)winehq.org Reporter: peter(a)smart-projects.net Distribution: --- I develop Windows software ( fyi: www.isobuster.com ) I have no clue about Linux though advice people to use Wine when needed ( fyi: https://www.isobuster.com/linux-mac.php ) I do not use Linux nor wine myself. So, this is based on user feedback, but I asked to make a log file and based on that feedback I understand the issue and can report it here: The user runs IsoBuster 4.4 on Linux Mint 19 Cinnamon using Wine 4.0 (which simulates Windows 7 (2.6.1.7601)). FYI: https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/ntddsc... When using the SCSI_PASS_THROUGH_DIRECT (or SCSI_PASS_THROUGH) structures I find that the 'DataTransferLength' field contains '0' after a successful call. That is not OK, it should contain the amount of bytes that were put in the buffer during command execution. To easily reproduce in a debug environment, run IsoBuster in free mode (Get it here: https://www.isobuster.com/download/ ) and use it on optical media (CD, DVD, BD). You will see that no file systems etc. are found. This is because IsoBuster internally fails the read commands, and this due to the fact that it thinks no data was transferred ( DataTransferLength == 0 ). Older IsoBuster versions did not encounter this issue because they didn't care about DataTransferLength. However I found that there are bad behaving (most likely USB bridge) Windows drivers that return less data than requested (yet without any error or warning). For instance a read of 27 * 2352-byte blocks would succeed but the buffer would only contain some 22 good blocks, and the rest was nonsense. I found that especially for read commands I had to double check DataTransferLength to make sure the requested data was indeed transferred. If not, further retries and/or breaking up the reads then avoids data corruption. I'm certain this is a bug and I hope it is fixed soon. If not I will need to try and detect Wine and make a work-around, which is less desirable. Best, Peter -- 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=47424 Olivier F. R. Dierick <o.dierick(a)piezo-forte.be> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download URL| |https://www.isobuster.com/d | |ownload/ Severity|critical |normal CC| |o.dierick(a)piezo-forte.be Component|-unknown |ntdll --- Comment #1 from Olivier F. R. Dierick <o.dierick(a)piezo-forte.be> --- Bug triage notification: Issue in a single application is severity 'normal'. Read more about severity levels descriptions there: https://wiki.winehq.org/Bugs#severity Copied URL in URL field. Added keyword 'download'. 'SCSI_PASS_THROUGH', 'SCSI_PASS_THROUGH_DIRECT' and 'DataTransferLength' are used in 'ntdll/cdrom.c'. Component is 'ntdll'. -- 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=47424 --- Comment #2 from Peter <peter(a)smart-projects.net> --- This is my first ever reported bug here so I don't know what to expect. Should I have seen some concrete feedback by now or not ? As in yes we will fix it by then, or I intend to look into it then, or no we don't care or we need more info or ... Or how long does it generally take for somebody with in depth knowledge to look at it and provide feedback ? Without testing an educated guess tells me that other tools that do low level access to optical media will run into this issue as well, whether they fail the command or not because of it is something else, they should but may ignore it for Wine. -- 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=47424 --- Comment #3 from Olivier F. R. Dierick <o.dierick(a)piezo-forte.be> --- (In reply to Peter from comment #2)
This is my first ever reported bug here so I don't know what to expect.
Should I have seen some concrete feedback by now or not ?
As in yes we will fix it by then, or I intend to look into it then, or no we don't care or we need more info or ...
Or how long does it generally take for somebody with in depth knowledge to look at it and provide feedback ?
Without testing an educated guess tells me that other tools that do low level access to optical media will run into this issue as well, whether they fail the command or not because of it is something else, they should but may ignore it for Wine.
Hello, You usually don't get any feedback from developers until they already looked into the issue and have something to report or to ask. It can take mere minutes or many years if the bug was overlooked. But you're lucky as I just checked and I can tell you that the issue is being worked on: https://www.winehq.org/pipermail/wine-devel/2019-July/148327.html Regards. -- 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=47424 --- Comment #4 from Peter <peter(a)smart-projects.net> --- Thank you. Proposed solution will work. I don't believe it is 100% correct, but it is good enough. I understand that this is due to Linux' implementation of 'resid'. Not 100% correct because the input value 'DataTransferLength' may be, and often is, more than what is really returned. Meaning it is possible to setup a buffer of (for instance) 64KB, put that value in 'DataTransferLength' and send a command that will only return a few bytes. In current suggested fix a buffer under-run may not be properly detected if the input buffer was bigger than the expected return data, because 'resid' may be smaller than what the buffer was bigger than required. On the other hand, software could detect the delta between input and output and use that to detect the buffer under-run. However since the mechanism is different on Windows, that may not happen. Still, this is as good as it gets, and certainly the way to go for the time being. Cheers, Peter -- 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=47424 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12(a)gmail.com --- Comment #5 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to Peter from comment #4)
I don't believe it is 100% correct, but it is good enough. I understand that this is due to Linux' implementation of 'resid'.
Not 100% correct because the input value 'DataTransferLength' may be, and often is, more than what is really returned. Meaning it is possible to setup a buffer of (for instance) 64KB, put that value in 'DataTransferLength' and send a command that will only return a few bytes.
In current suggested fix a buffer under-run may not be properly detected if the input buffer was bigger than the expected return data, because 'resid' may be smaller than what the buffer was bigger than required.
I'm not sure I follow. Under what circumstances is the patch wrong? -- 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=47424 --- Comment #6 from Peter <peter(a)smart-projects.net> --- Perhaps I'm not understanding the true return value of 'resid' and I cannot test to check but the way I understand it it will return the amount of requested bytes that could *not* be returned (due to a under-run). So let's assume this hypothetical situation: An application sends a read10 command of 2 blocks user data, to be read from a data CD. The drive (in case the read10 command succeeds) will return exactly 2*2048 (=4096) bytes. To do this the application may use a buffer in memory that is bigger, for instance 20480 bytes (space for 10*2048-byte blocks). The applicatin can set 'DataTransferLength' to 20480 bytes That is perfectly OK and quite common. So 5 times bigger than what is expected to be returned. Assume the command succeeds but there is a buffer under-run on driver level and for whatever reason the driver only passes through 2048 bytes, not the requested 4096 bytes. IF I understand the use of 'resid' correctly, it will contain a value of 2048 bytes, because 2048 bytes of the requested 4096 bytes could not be transferred. Suggested code/fix will then set 'DataTransferLength' to 20480-2048 (=18432) bytes. The application however, when it checks 'DataTransferLength' after the command succeeds will think that 9 blocks of data were put in the buffer instead of the requested 2. Now ... latter statement is *highly unlikely* to happen. An application wants to find value 4096 in 'DataTransferLength' but if the value is higher, will simply not care. Only when the value is lower than 4096 in this case will alarm bells will go off because less data than expected was returned (so a buffer under-run was detected). I hope this explanation makes sense ? I do not know the exact working of 'resid', so perhaps the fix is perfect. In any case, the now suggested fix *will* work for me and I expect also for other apps than run into this (if at all they run into it). This is -- 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=47424 --- Comment #7 from Zebediah Figura <z.figura12(a)gmail.com> --- I don't think that's how it works, no. The documentation clearly states that resid "is 'dxfer_len' less the number of bytes actually transferred." In your example dxfer_len is 20480, so the driver should set resid to 20480 - 2048 = 18432, causing us to return 20480 - 18432 = 2048 in DataTransferLength. A quick skim of the Linux source suggests this is indeed how scsi drivers behave, though I won't claim to have any familiarity with that code. -- 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=47424 --- Comment #8 from Peter <peter(a)smart-projects.net> --- AHA ... well that's just perfect then ! Cheers. -- 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=47424 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED Fixed by SHA1| |bf891f34af32839bf949e7ca4a1 | |921754aa5e0d1 --- Comment #9 from Zebediah Figura <z.figura12(a)gmail.com> --- Fixed by <https://source.winehq.org/git/wine.git/commitdiff/bf891f34af32839bf949e7ca4a1921754aa5e0d1>. -- 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=47424 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #10 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 4.12. -- 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=47424 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=47424 Michael Stefaniuc <mstefani(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|4.0.x |--- --- Comment #11 from Michael Stefaniuc <mstefani(a)winehq.org> --- Removing the 4.0.x milestone from bug fixes included in 4.0.3. -- 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 (2)
-
wine-bugs＠winehq.org -
WineHQ Bugzilla