Windows report descriptor reconstructor reengineered pp struct #306
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…eport_descriptor_reconstructor_v2
…ptor debug printout
…eport_descriptor_reconstructor
- macOS implementation for it; - stubs for all other platforms;
…ws_report_descriptor_reconstructor
…ate to the hid_get_report_descriptor API function of PR libusb#303
…eport_descriptor_reconstructor
Added debug output for possible failure cases in button array code
…s_report_descriptor_reconstructor_reengineered_pp_struct
…ich contains additional elements).
|
Nice. This is the output for my devices and the results look better than #262 at the first glance. click to expand |
|
Result for Microsoft USB Nano Receiver. click to expand |
|
Microchip PICKit 2: checked, good Logitech M557 Bluetooth Mouse: at least report ID, report size and report count are all correct with a quick check. Logitech USB receiver: to be checked Microsoft USB Nano Receiver: to be checked |
|
Which Windows version did you use for the test? I think i is important, to check this with various Windows versions. |
|
Windows 10 Enterprise x64 (version 20H2) |
Improved comments
|
@JoergAtGithub I think that your code could be moved to separate file. |
|
@DJm00n I could seperate it, but than we need also a header file, which contains all the typedefs and declarations used by both. |
Not yet. See your README:
That is currently our design. I do want to change that, so we can decouple the implementation, move some common parts, etc. |
Thanks! I'll go over it over the weekend (I hope). |
Youw
reviewed
Dec 29, 2021
There was a problem hiding this comment.
I have too many comments to the implementation/code style.
So instead of leaving them on GH, I'll create a separate commit instead with "fixes".
And after giving it a good though - it seems like a good idea to try to separate the implementation out of hid.c.
I'll work on that too.
|
Thanks for working on this! If you restructure the code, please keep the testability in mind. For the unit test in #343 it's neccessary:
|
Yeah, maybe I'll update that too. |
|
I still have (none-written) comments regarding the implementation but lets have a |
|
Thanks for cleaning up my code! |
|
@JoergAtGithub Just wanted to add a little piece of information since I am currently myself trying to reconstruct what For obvious reasons we can't be sure about the names -- except for items that correspond 1:1 to another documented struct -- but compared to this one I came up with this: So in conclusion you can use the info about Additionally, as commented inline above, it's possible that the report info structs (your But my initial hunch was to have those separate, probably because I hadn't looked at as many functions as I have now. However, that said, the array hypothesis isn't too far-fetched if we look at how a number of functions taking I'd say chances are that this is hidden behind a macro, as it's repeated all over the place. Curiously That check is then followed by something that takes the Based on your information I can probably speed up my effort in what call One more note: you wrote:
and while that may be true, customarily Microsoft uses PS: If I find out anything further that can complete the idea of the involved structs, I'll let you know. |
@DJm00n Ouch, I'm not really allowed to look at leaked proprietary code. Clean room reverse engineering and all. Either way, I found that IDA curiously knows about this, supposedly from the PDB. And indeed looking with https://github.com/horsicq/PDBRipper discloses that the PDB contains definitions.
This is what IDA makes of it, when exporting to a header: So finally I also have more sensible type and member names. And sort of a confirmation for those things where I wasn't a 100% certain. But I find The gist is, I should probably pay more attention to IDA's changelog and their blog and always consult the "Local Types" (Shift+F1) when dealing with code that has debug symbols available. |
|
|
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a variant of #262, but based on the Google Chromium approach using a reengineered struct of the preparsed data returned by HIDP_PREPARSED_DATA.
The output of hidtest can be read by visualization tools like: https://eleccelerator.com/usbdescreqparser/
In ALL cases known to me the generated report descriptor is logical identical to the original, which I read using the free version of USBlyszer. But it's not byte wise identical. The order of the bytes differ, handling of reundant information differs and padding bits can be defined in a different way. And theoretical there might be also a different in report length, but only if somebody adds more constant padding bits at the end of the report than neccessary.
I developed this code with Windows7 64Bit Service Pack1 Windows7, and mcuee tested it on Windows 10 Enterprise x64 (version 20H2).