[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: [e-mail]
          Reporter: [e-mail]
      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

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

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

Olivier F. R. Dierick  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download
                URL|                            |https://www.isobuster.com/d
                   |                            |ownload/
           Severity|critical                    |normal
                 CC|                            |[e-mail]
          Component|-unknown                    |ntdll

--- Comment #1 from Olivier F. R. Dierick  ---
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'.

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

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

--- Comment #2 from Peter  ---
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.

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

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

--- Comment #3 from Olivier F. R. Dierick  ---
(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.

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

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

--- Comment #4 from Peter  ---
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

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

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

Zebediah Figura  changed:

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

--- Comment #5 from Zebediah Figura  ---
(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?

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

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

--- Comment #6 from Peter  ---
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

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

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

--- Comment #7 from Zebediah Figura  ---
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.

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

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

--- Comment #8 from Peter  ---
AHA ... well that's just perfect then !
Cheers.

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

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

Zebediah Figura  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED
      Fixed by SHA1|                            |bf891f34af32839bf949e7ca4a1
                   |                            |921754aa5e0d1

--- Comment #9 from Zebediah Figura  ---
Fixed by
https://source.winehq.org/git/wine.git/commitdiff/bf891f34af32839bf949e7ca4a1921754aa5e0d1.

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

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

Alexandre Julliard  changed:

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

--- Comment #10 from Alexandre Julliard  ---
Closing bugs fixed in 4.12.

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

[WineHQ Bugzilla]

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

Michael Stefaniuc  changed:

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

[Bug 47424] DataTransferLength in SCSI_PASS_THROUGH and SCSI_PASS_THROUGH_DIRECT *must* have return value

[WineHQ Bugzilla]

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

Michael Stefaniuc  changed:

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

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