Changes derived from RE Windows driver #3
Conversation
- Increased default Timeout between retries - Updated default packet size to 8024 - Send SLEEP instead of RESET after each read Documentation from reversing the Windows driver and USB Packet capture analysis in HARDWARE_SPECS.md Information on RE work in REVERSE_ENGINEERING_NOTES.md
archeYR
left a comment
archeYR
left a comment
There was a problem hiding this comment.
Overall very nice work with reverse engineering this. I've some questions/minor suggestions because I would like to understand some changes better before merging, but in any case I appreciate the effort.
| #define CS9711_SENSOR_HEIGHT 236 | ||
|
|
||
| #define CS9711_DEFAULT_WAIT_TIMEOUT 300 | ||
| #define CS9711_DEFAULT_WAIT_TIMEOUT 500 |
There was a problem hiding this comment.
Why? Is this what Windows driver does?
There was a problem hiding this comment.
So the windows driver uses a configurable (registry) delay of 300 to 500 ms. I had a Python test application that was using a value of 250ms to detect the Chip and it consistently needed 3 tries (failed twice) until I upped it to 500ms; then it would get on the first try...
My machine is a i9-13900K (which should be fast), but it is running the scanner over a KVM-usb-hub (so maybe not?)...
There was a problem hiding this comment.
Would be nice to mention that in a comment or hardware spec document.
| * With cmd 0x03: arrives as 8024 bytes in one transfer | ||
| */ | ||
| #define CS9711_FP_RECV_LEN_1 8024 | ||
| #define CS9711_FP_RECV_LEN_2 0 |
There was a problem hiding this comment.
That constant and the M_SCAN_READ_CB_BULK_UD_SECOND_BLOCK thing are not needed then?
There was a problem hiding this comment.
Here's what my AI said about it
The USB capture showed:
- cmd 0x04: Data arrives as 8000 + 24 bytes (two transfers)
- cmd 0x03: Would arrive as 8024 bytes in one transfer (per documentation)
Since we use cmd 0x04 and now receive all 8024 bytes in the first read (CS9711_FP_RECV_LEN_1=8024), the second block machinery is technically dead code.
However: I kept them for two reasons:
- Future-proofing: If someone tests with cmd 0x03 or a different chip variant
- Minimal code churn: The dead code path is harmless and removing it would require restructuring the state machine
It also mentioned it was complete legit to nuke those lines, so can do that too...
There was a problem hiding this comment.
I think I would prefer for the unused code to be removed. Having less code will likely make a review process upstream easier.
| #define CS9711_FP_RECV_LEN_1 8024 | ||
| #define CS9711_FP_RECV_LEN_2 0 | ||
| /* Max buffer size to handle high-res CS62A0 (288x208=59904) if ever enabled */ | ||
| #define CS9711_FP_RECV_LEN_MAX 65536 |
There was a problem hiding this comment.
Does that alone help with anything? If the scanner actually returned a large image, then it would have to be handled in m_scan_submit_image too?
There was a problem hiding this comment.
This one came from trying to reconcile detected specifications vs manufacture's documentation...
- Windows driver mentioned a device type 5 showed the chip was capable of a max resolution of 288x208
- The booklet that came with my FP100-LK indicates that this device has a resolution of 112x112 at 508DPI
- The original CS9711 driver said it was (correctly) only 112x62 (8000 bytes)...
I thought this low resolution might have been the reason for my low scan success rate, so initial efforts were focused on trying to tell the device to take a "full" resolution scan, with the maximum possible size expected to be almost 64kb
As the driver is now we shouldn't need more than 8024, but if some other CS9711 type 5 device does actually exist, then it would need more buffer space...
There was a problem hiding this comment.
Yeah, but the image decoding code only handles the small (118x68) images so it is not going to work either way. I think full read of the large image would only ever make sense if the driver can handle it.
| else | ||
| { | ||
| fp_dbg ("Init response valid"); | ||
| fp_info ("Init response valid, ChipID: 0x62A0"); |
There was a problem hiding this comment.
I don't think that change is necessary.
|
|
||
| case M_SCAN_SEND_POST_SCAN: | ||
| usb_send_out_sync (_dev, CS9711_FP_CMD_TYPE_RESET, &error); | ||
| /* Windows driver sends SLEEP_WAKE (0x02) after capture, not RESET (0x07) */ |
There was a problem hiding this comment.
Is this somehow unusual that it warrants a comment?
There was a problem hiding this comment.
Originally the CS9711 driver was sending a reset... I think this comment was intended as more of a call-out to draw attention to the change, we can ditch it if you like.
There was a problem hiding this comment.
Yeah, in that case I don't think the comment is necessary.
Apply Chip ID logging suggestion Co-authored-by: Adam Słaboń <asaillen@protonmail.com>
|
This PR is AI generated with a LOT of guidance. I'm not an expert in C or reverse engineering - I know a little of both, enough to mostly know BS when I see it, but not enough to do this stuff quickly. Here are the amp-cli threads of what I was doing; if you need something to put you to sleep check them out 🤣
I also have another PR that I'll push after this to help address USB disconnect and reconnects - this can happen from sleeping or in my case from switching a KVM between machines. That one involves some disconnect logic to the driver, some udev reconnect rules and a systemd unit to kill |
Sounds quite hacky. Do you know if Windows driver fares any better with the KVM switching/suspend? |
The reader is seen as a new USB device, so fprintd needs to figure out the old device isn't there and init the new one and it needs to do that before kscreen locker starts with a handle to the old one, it's a race. Windows likely has a different Lock Screen behaviour, I did test unlocking Windows with the reader but not the kvm switch, reconnecting or suspending and resuming. All Windows testing was done via a Linux KVM virtual machine passing the USB device through rather than on a real Windows computer (I don't have any) The hacky solution was a riff on NixOS/nixpkgs#432276 The net result of the hack is if the USB device reconnects, the lock screen might flicker after half a second, but then fingerprint auth works (when it never worked in that scenario for me previously), I was pretty happy it worked at all... if someone else digs deeper and finds another way that'd be awesome too. |
2 participants
Changes derived from RE Windows driver:
Documentation from reversing the Windows driver and USB Packet capture analysis in HARDWARE_SPECS.md
Information on RE work in REVERSE_ENGINEERING_NOTES.md