Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pcap control fixes #143

Merged
merged 3 commits into from
Sep 10, 2021
Merged

pcap control fixes #143

merged 3 commits into from
Sep 10, 2021

Conversation

benzea
Copy link
Collaborator

@benzea benzea commented Sep 8, 2021

Issues discovered while creating the uru4000 test in libfprint.

Benjamin Berg added 2 commits September 8, 2021 20:34
libusb always sets the endpoint to 0x00 for control transfers. Hower,
the setup data contains a direction, and the kernel will use this
direction bit in the endpoint so we cannot compare them.
Other control transfers will not be submitted by the kernel and do not
need to be ignored. This avoids replay issues as the transfers could be
skipped accidentally otherwise.
@martinpitt
Copy link
Owner

Tests timed out on ubuntu:devel, re-running.

@martinpitt
Copy link
Owner

@benzea : Timed out again, this smells systematic..

@benzea
Copy link
Collaborator Author

benzea commented Sep 9, 2021

Yeah, though not a regression in umockdev.

The test that is timing out is the only one that is running Xorg internally. So, probably xorg is not shutting down.

@@ -256,8 +272,11 @@ internal class IoctlUsbPcapHandler : IoctlBase {
urb_buffer_length -= 8;
}

/* libusb always sets URB_CONTROL endpoint to 0x00, but the
* kernel exposes it as 0x80/0x00 depending on the direction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, the direction is a flag, the endpoint is only 4 bits:
https://www.beyondlogic.org/usbnutshell/usb6.shtml

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. The funny thing is that bmRequestType and the endpoint byte of other packets are very similar yet different. So it would be correct to just copy the top bit from the setup data into the endpoint, but we can also just ignore it.

@benzea
Copy link
Collaborator Author

benzea commented Sep 9, 2021

OK, quite confusing. I added debug printf's, and we seem to be hanging inside the spawn_async call that launches umockdev-run and Xorg.

@martinpitt
Copy link
Owner

Confirmed that the ubuntu:devel hang is not a regression from this PR, it just failed the same way on master. @benzea has a hunch that it's related to libglib2.0-0 2.68.3 → 2.68.4 update.

Copy link
Owner

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@martinpitt martinpitt merged commit 8a949b9 into master Sep 10, 2021
@martinpitt martinpitt deleted the benzea/pcap-control-fixes branch September 10, 2021 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants