https://bugs.winehq.org/show_bug.cgi?id=42225
Bug ID: 42225 Summary: HID device not initialize properly Product: Wine Version: 2.0-rc5 Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: hid Assignee: wine-bugs@winehq.org Reporter: sguerrini97@gmail.com Distribution: ---
Created attachment 56865 --> https://bugs.winehq.org/attachment.cgi?id=56865 commandline output
When running a Windows software which is supposed to talk to a RAW HID device, the software reports the error "External Exception 80000100". The software talks to a magnetic stripe reader/writer device and can be found here (the hardware needs to be connected to reproduce the issue) http://www.card-device.com/files/201511/20151123214221196.zip
Cmdline shows unimplemented call to hid.dll.HidD_GetNumInputBuffers
https://bugs.winehq.org/show_bug.cgi?id=42225
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |aric@codeweavers.com
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #1 from Aric Stewart aric@codeweavers.com --- Created attachment 56868 --> https://bugs.winehq.org/attachment.cgi?id=56868 Implementaion of stub function
Here is the implementation of that function.
This is not a strict bug fix so submitting the patch will wait until after 2.0 However you can test it and see if there are more stub function missing.
thanks! -aric
https://bugs.winehq.org/show_bug.cgi?id=42225
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Ever confirmed|0 |1 Keywords| |download CC| |focht@gmx.net Status|UNCONFIRMED |NEW URL| |http://www.card-device.com/ | |files/201511/20151123214221 | |196.zip Summary|HID device not initialize |MSRX v2015a (magnetic |properly |stripe reader app) crashed | |on unimplemented function | |hid.dll.HidD_GetNumInputBuf | |fers
--- Comment #2 from Anastasius Focht focht@gmx.net --- Hello folks,
filling fields...
Regards
https://bugs.winehq.org/show_bug.cgi?id=42225
Samuele sguerrini97@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #56865|0 |1 is obsolete| |
--- Comment #3 from Samuele sguerrini97@gmail.com --- Created attachment 56870 --> https://bugs.winehq.org/attachment.cgi?id=56870 cmdline output after implementation of stub call
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #4 from Samuele sguerrini97@gmail.com --- (In reply to Aric Stewart from comment #1)
Created attachment 56868 [details] Implementaion of stub function
Here is the implementation of that function.
This is not a strict bug fix so submitting the patch will wait until after 2.0 However you can test it and see if there are more stub function missing.
thanks! -aric
Thanks, I've applied the patch to 2.0-rc5 source and built wine again. Function seems to be implemented fine, the error is different now.
https://bugs.winehq.org/show_bug.cgi?id=42225
Samuele sguerrini97@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|MSRX v2015a (magnetic |MSRX v2015a (magnetic |stripe reader app) crashed |stripe reader app) crashed |on unimplemented function |on |hid.dll.HidD_GetNumInputBuf |plugplay:hidraw_set_feature |fers |_report Output feature | |buffer too small
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #5 from Samuele sguerrini97@gmail.com --- Created attachment 56872 --> https://bugs.winehq.org/attachment.cgi?id=56872 hid debug output
hid debug output after running the software, choosing option to read card, swiping a card, closing the software.
https://bugs.winehq.org/show_bug.cgi?id=42225
Samuele sguerrini97@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Version|2.0-rc5 |unspecified
--- Comment #6 from Samuele sguerrini97@gmail.com --- in dlls/winebus.sys/bus_udev.c line 354 if (length + 1 > sizeof(feature_buffer)) should be if (length + 1 > sizeof(buffer))
(?) Device get recognized after this change but it cannot read any card even if no error is displayed. Running with WINEDEBUG=+hid I don't see anything strange.
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #7 from Aric Stewart aric@codeweavers.com --- (In reply to Samuele from comment #6)
in dlls/winebus.sys/bus_udev.c line 354 if (length + 1 > sizeof(feature_buffer)) should be if (length + 1 > sizeof(buffer))
(?) Device get recognized after this change but it cannot read any card even if no error is displayed. Running with WINEDEBUG=+hid I don't see anything strange.
Nice find! Please submit a patch and I will sign off.
It may be helpful to have +hid_minidriver, +plugplay to see if that shows more of what is going on.
-aric
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #8 from Samuele sguerrini97@gmail.com --- Created attachment 56883 --> https://bugs.winehq.org/attachment.cgi?id=56883 Buffer len check fix
Here's a patch. Please double check that because this is the first time I'm looking at wine source code and I'm not sure of what I did, I just checked the semantics to fix it..
I will provide the debug output ASAP. Yesterday I've tried also "+hid_report" and when I swipe a card a packet is received but it looks like the packet is not handled by the application (the software keeps waiting for a card swipe).
https://bugs.winehq.org/show_bug.cgi?id=42225
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |austinenglish@gmail.com
--- Comment #9 from Austin English austinenglish@gmail.com --- (In reply to Samuele from comment #8)
Created attachment 56883 [details] Buffer len check fix
Here's a patch. Please double check that because this is the first time I'm looking at wine source code and I'm not sure of what I did, I just checked the semantics to fix it..
The code change is correct, but you'll want to use git to generate a patch with a sign off instead of diff.
See https://wiki.winehq.org/SubmittingPatches for more info
https://bugs.winehq.org/show_bug.cgi?id=42225
Bruno Jesus 00cpxxx@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |2.0-rc5 Keywords| |hardware
https://bugs.winehq.org/show_bug.cgi?id=42225
Samuele sguerrini97@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #56883|0 |1 is obsolete| |
--- Comment #10 from Samuele sguerrini97@gmail.com --- Created attachment 56886 --> https://bugs.winehq.org/attachment.cgi?id=56886 Buffer len check fix
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #11 from Austin English austinenglish@gmail.com --- (In reply to Samuele from comment #10)
Created attachment 56886 [details] Buffer len check fix
Looks good, please submit to wine-patches
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #12 from Bruno Jesus 00cpxxx@gmail.com --- (In reply to Samuele from comment #10)
Created attachment 56886 [details] Buffer len check fix
Subject should be prefixed with component name that is affected, in this case:
[PATCH] winebus.sys: Buffer length check fix
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #13 from Samuele sguerrini97@gmail.com --- Created attachment 56889 --> https://bugs.winehq.org/attachment.cgi?id=56889 more debug output
I've sent the patch. Debug output
-Opening software -Connecting device -Selecting "read" operation -Swiping card -Canceling "read" operation -Closing software
WINEDEBUG="+hid,+hid_minidriver,+plugplay,+hid_report"
I had to remove some lines to make things more readable
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #14 from Aric Stewart aric@codeweavers.com --- On a business trip right now so not as many cycles So Just a few random thoughts...
For the feature report, it has a report ID of 0, which is not a case I have tested. Because of this we have code that strips the leading zero in hidclass.sus/device.c: HID_set_to_device and then re-adds the leading zero in winebus.sys/bus_udev.c:hidraw_set_feature_report
You could check to make sure the report data in question actually is leading with a zero and then we add that zero back.
Then maybe look at the report coming back from the device in winebus.sys/main.c:process_hid_report and see what data is coming back. does that data change when you swipe something.
-aric
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #15 from Aric Stewart aric@codeweavers.com --- Any chance to look deeper into this?
-aric
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #16 from Samuele sguerrini97@gmail.com --- (In reply to Aric Stewart from comment #15)
Any chance to look deeper into this?
-aric
Yeah I would love to be able to use this software, I'm willing to test sorry if I didn't reply sooner. I'm not sure I understood correctly what you were asking but I did this to print first and second byte of the buffer before and after the code that strips the leading zero and adds it back. Data seems to match:
trace:hid_report:HID_set_to_device [FIRST] Leading 0,nextchar: 0,194 trace:hid_report:hidraw_set_feature_report [SECOND] Leading 0,nextchar: 0,194
Then in process_hid_report I've added code to print the report buffer and I get different reports depending on which card I swipe. It's hard to see if the data is correct because it uses magstripe encoding and I need to convert the bytes to binary data and check it manually.
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #17 from Aric Stewart aric@codeweavers.com --- (In reply to Samuele from comment #16)
(In reply to Aric Stewart from comment #15)
Any chance to look deeper into this?
-aric
Yeah I would love to be able to use this software, I'm willing to test sorry if I didn't reply sooner. I'm not sure I understood correctly what you were asking but I did this to print first and second byte of the buffer before and after the code that strips the leading zero and adds it back. Data seems to match:
trace:hid_report:HID_set_to_device [FIRST] Leading 0,nextchar: 0,194 trace:hid_report:hidraw_set_feature_report [SECOND] Leading 0,nextchar: 0,194
Then in process_hid_report I've added code to print the report buffer and I get different reports depending on which card I swipe. It's hard to see if the data is correct because it uses magstripe encoding and I need to convert the bytes to binary data and check it manually.
Actually I was more thinking of just dumping the raw data and looking to see if thing changed.
Code like this
{
}
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #18 from Aric Stewart aric@codeweavers.com --- (In reply to Samuele from comment #16)
(In reply to Aric Stewart from comment #15)
Any chance to look deeper into this?
-aric
Yeah I would love to be able to use this software, I'm willing to test sorry if I didn't reply sooner. I'm not sure I understood correctly what you were asking but I did this to print first and second byte of the buffer before and after the code that strips the leading zero and adds it back. Data seems to match:
trace:hid_report:HID_set_to_device [FIRST] Leading 0,nextchar: 0,194 trace:hid_report:hidraw_set_feature_report [SECOND] Leading 0,nextchar: 0,194
Is this meaning there are 2 leading 0s on the feature report (0,0,194) or is it just (0, 194) and that 0 is being removed and re-added?
Then in process_hid_report I've added code to print the report buffer and I get different reports depending on which card I swipe. It's hard to see if the data is correct because it uses magstripe encoding and I need to convert the bytes to binary data and check it manually.
Actually I was more thinking of just dumping the raw data and looking to see if thing changed. Something like this for the reports
diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 1902d516c6..23ab790682 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -640,6 +640,14 @@ void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD length) if (!length || !report) return;
+{ + int ii; + ERR("Report Data: "); + for (ii = 0; ii < length; ii++) + ERR("%x ", report[ii]); + ERR("\n"); +} + EnterCriticalSection(&ext->report_cs); if (length > ext->buffer_size) {
thanks -aric
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #19 from Samuele sguerrini97@gmail.com --- (In reply to Aric Stewart from comment #18)
Is this meaning there are 2 leading 0s on the feature report (0,0,194) or is it just (0, 194) and that 0 is being removed and re-added?
First two bytes are 0 and 194, so that 0 is being removed and re-added
Actually I was more thinking of just dumping the raw data and looking to see if thing changed. Something like this for the reports
I think that's what I did.. In process_hid_report I'm printing an hex string of the report buffer. Using this table http://www.hhhh.org/~joeboy/resources/magcards/trackdata_ANSI-ISO_ALPHA.html I can see that the data in that buffer is what's supposed to be on the magstripe (with the addition some bytes at the beginning and some bytes at the end). And yes the content of that buffer changes when I swipe a different card. So it looks like the data is sent by the reader back to the application, but the application keeps waiting like nothing happened.
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #20 from Bruno Jesus 00cpxxx@gmail.com --- I believe Andrew is requesting the raw data from the HID report, not the processed track data from the swiped card.
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #21 from Aric Stewart aric@codeweavers.com --- (In reply to Samuele from comment #19)
(In reply to Aric Stewart from comment #18)
Is this meaning there are 2 leading 0s on the feature report (0,0,194) or is it just (0, 194) and that 0 is being removed and re-added?
First two bytes are 0 and 194, so that 0 is being removed and re-added
Humm, ok that seem right. At least according to the spec.
Actually I was more thinking of just dumping the raw data and looking to see if thing changed. Something like this for the reports
I think that's what I did.. In process_hid_report I'm printing an hex string of the report buffer. Using this table http://www.hhhh.org/~joeboy/resources/magcards/trackdata_ANSI-ISO_ALPHA.html I can see that the data in that buffer is what's supposed to be on the magstripe (with the addition some bytes at the beginning and some bytes at the end). And yes the content of that buffer changes when I swipe a different card. So it looks like the data is sent by the reader back to the application, but the application keeps waiting like nothing happened.
Raw data is good, Curious if there is a report ID leading the byte string. You said there where some extra bytes at the beginning, What would those be? are the same with different cards? The report descriptor just seems to state it will be 64 bytes. The reportID is 0, so I wonder if there is a leading 0 or not. Shouldn't be.
-aric
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #22 from Aric Stewart aric@codeweavers.com --- Another helpful step may be to move a level up, add +file to the log and look for the ReadFile calls that are reading the data.
I see no use of any of the HidP functions to process the data that route.
-aric
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #23 from Samuele sguerrini97@gmail.com --- Created attachment 57015 --> https://bugs.winehq.org/attachment.cgi?id=57015 log WINEDEBUG="+hid_report,+file"
Common pattern at the beginning of the buffer seems to be
<1st byte> 1b 73 1b 01
The first byte it's 0xbf on most of the cards I've tried but it changes with some (it's 0xff in this log) Data at the end of the buffer is different for every card, I'm not sure if it's part of the magstripe data or some kind of checksum.
I've attached the full log with +file enabled and report buffers (i've censured only the report buffer with my card data)
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #24 from Aric Stewart aric@codeweavers.com --- All that looks correct.
That is just +hid_report,+file? Could you include a +hid,+hid_minidriver
You probably do not need the hex dump from the reads.
It may be that the data is reaching the application and other problems are coming up. So a +relay or something will help tell us what the application is doing once the read returns data.
-aric
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #25 from Samuele sguerrini97@gmail.com --- Created attachment 57022 --> https://bugs.winehq.org/attachment.cgi?id=57022 log WINEDEBUG="+hid_report,+file,+hid,+hid_minidriver"
New log attached. I've removed the code that prints buffer data If I add relay the output gets very noisy. I've uploaded it on another host.
log WINEDEBUG="+hid_report,+file,+hid,+hid_minidriver,+relay" https://drive.google.com/open?id=0B-zsdl1TlWZ_cWFzMXA0OEx2NXc
By the way I've noticed your implementation of stub funciton is not present on the wine git.
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #26 from Aric Stewart aric@codeweavers.com --- Created attachment 57034 --> https://bugs.winehq.org/attachment.cgi?id=57034 Experimental patch
You are correct the stub function was not in wine yet. Now that the code freeze is over I have just submitted it.
I am attaching a new test patch. What I am noticing is that the application is doing quite a few asynchronous reads in a row from the same thread. Then when we get data, we only are sending the data back to the first read request. I am not sure if that is correct or not (I need to write some tests) But the test patch will send the data back to all the queued requests. Just to see if that changes behavior.
-aric
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #27 from Samuele sguerrini97@gmail.com --- Created attachment 57035 --> https://bugs.winehq.org/attachment.cgi?id=57035 success read log WINEDEBUG="+hid_report,+file,+hid,+hid_minidriver"
I've applied your experimental patch and I'm able to read data from magstripe cards!
However erase and write operations seems to have problems: after the card is swiped the application has a strange behavior. Sometimes it says the reader is disconnected, sometimes it gets to the next step but if I read the card after the data is corrupted. I'm attaching some logs.
success read log WINEDEBUG="+hid_report,+file,+hid,+hid_minidriver,+relay" https://drive.google.com/file/d/0B-zsdl1TlWZ_NVp0SnVqeTVWX2s/view?usp=sharin...
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #28 from Samuele sguerrini97@gmail.com --- Created attachment 57036 --> https://bugs.winehq.org/attachment.cgi?id=57036 erase log WINEDEBUG="+hid_report,+file,+hid,+hid_minidriver"
Here's the log of an erase operation
erase log WINEDEBUG="+hid_report,+file,+hid,+hid_minidriver,+relay" https://drive.google.com/file/d/0B-zsdl1TlWZ_VFdIanZiWGdIMHc/view?usp=sharin...
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #29 from Aric Stewart aric@codeweavers.com --- Ok nice, progress.
So just to check, that has reading correct. It is just writing and erasing that are causing problems.
The proper fix for the reading and multiple reads is a bit trickier, I am trying to dig into it and figure out how to do it correctly. So a patch may be a bit further out. But i am working on it.
-aric
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #30 from Samuele sguerrini97@gmail.com --- (In reply to Aric Stewart from comment #29)
Ok nice, progress.
So just to check, that has reading correct. It is just writing and erasing that are causing problems.
The proper fix for the reading and multiple reads is a bit trickier, I am trying to dig into it and figure out how to do it correctly. So a patch may be a bit further out. But i am working on it.
-aric
Yes that's right, reading works good so far. Writing and erasing have strange behaviour. Take your time, thanks for your work
https://bugs.winehq.org/show_bug.cgi?id=42225
Aric Stewart aric@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #57034|0 |1 is obsolete| |
--- Comment #31 from Aric Stewart aric@codeweavers.com --- Created attachment 57038 --> https://bugs.winehq.org/attachment.cgi?id=57038 Another experiment for reading
Another approach to the reading bug.
Could you remove the old test.patch and apply this one instead and see if reading continues to work?
thanks! -aric
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #32 from Samuele sguerrini97@gmail.com --- (In reply to Aric Stewart from comment #31)
Created attachment 57038 [details] Another experiment for reading
Another approach to the reading bug.
Could you remove the old test.patch and apply this one instead and see if reading continues to work?
thanks! -aric
Yes reading still works with the last experimental patch
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #33 from Aric Stewart aric@codeweavers.com --- The initial patches for this are all now in WINE. So reading should now work with the tip.
I still have on my plate what is up with writing/erasing when I get a chance to circle back.
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #34 from Aric Stewart aric@codeweavers.com --- (In reply to Samuele from comment #28)
Created attachment 57036 [details] erase log WINEDEBUG="+hid_report,+file,+hid,+hid_minidriver"
Here's the log of an erase operation
erase log WINEDEBUG="+hid_report,+file,+hid,+hid_minidriver,+relay" https://drive.google.com/file/d/0B-zsdl1TlWZ_VFdIanZiWGdIMHc/view?usp=sharin...
What is interesting is that the erase log has no Write statements to the device. I only set SetFeature calls, which it is already doing.
If you try to do an erase you say it fails, Does it actually do anything to the card? Is there any change in behavior?
You said that writes also fail. Could you do a +hid,+file log for an attempted Write operation?
thanks! -aric
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #35 from Samuele sguerrini97@gmail.com --- Created attachment 57105 --> https://bugs.winehq.org/attachment.cgi?id=57105 success erase log WINEDEBUG="+hid_report,+file,+hid,+hid_minidriver"
After an erase operation I got read errors if I try to read the card, usually this means the data is corrupted (CRC/VRC is wrong). So something actually changed on the magstripe.
BUT.. I was testing again now and it seems that erase and write are working fine.. Nothing changed in my wine tree since my last build. I've just realized that there are problems only with a single card (the one I was using for my erase/write tests) which is probably broken. I've tried different cards and it's ok. I'll try to write on that card using Windows just to be sure
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #36 from Samuele sguerrini97@gmail.com --- Created attachment 57106 --> https://bugs.winehq.org/attachment.cgi?id=57106 success write log WINEDEBUG="+hid_report,+file,+hid,+hid_minidriver"
Log of a write operation
Track1: "CIAO CIAO" Track2: "12345" Track3: "12345"
Data entered manually inside the software (not read from file).
https://bugs.winehq.org/show_bug.cgi?id=42225
--- Comment #37 from Samuele sguerrini97@gmail.com --- Created attachment 57107 --> https://bugs.winehq.org/attachment.cgi?id=57107 connection lost log WINEDEBUG="+hid_report,+file,+hid,+hid_minidriver"
The non-working card was dirty, I'm sorry I didn't realize before. Everything is working as it should now!
Sometimes the software looses connection with the device but this seems to be random and an easy workaround is selecting the device again from the top down menu. I'm attaching a log just in case (I did some reading and one erasing before the connecion lost, then a write after reconnecting).
https://bugs.winehq.org/show_bug.cgi?id=42225
Aric Stewart aric@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |FIXED
--- Comment #38 from Aric Stewart aric@codeweavers.com --- Ok, I would consider this bug closed then.
https://bugs.winehq.org/show_bug.cgi?id=42225
Bruno Jesus 00cpxxx@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |fbd3525e249b858b3f811499c4b | |38cdf1f2694f1
--- Comment #39 from Bruno Jesus 00cpxxx@gmail.com --- Assuming http://source.winehq.org/git/wine.git/commitdiff/fbd3525e249b858b3f811499c4b... as SHA1 because it was the first problem, if not please fix ;-)
https://bugs.winehq.org/show_bug.cgi?id=42225
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #40 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 2.1.
-
wine-bugs＠winehq.org