Skip to content
New issue

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

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

Already on GitHub? # to your account

Unsound Send/Sync bound for Device and DeviceHandle #44

Closed
Qwaz opened this issue Dec 19, 2020 · 1 comment · Fixed by #45
Closed

Unsound Send/Sync bound for Device and DeviceHandle #44

Qwaz opened this issue Dec 19, 2020 · 1 comment · Fixed by #45
Assignees

Comments

@Qwaz
Copy link

Qwaz commented Dec 19, 2020

Hello fellow Rustacean,
we (Rust group @sslab-gatech) found a memory-safety/soundness issue in this crate while scanning Rust code on crates.io for potential vulnerabilities.

Issue Description

rusb/src/device.rs

Lines 34 to 35 in 12ee91d

unsafe impl<T: UsbContext> Send for Device<T> {}
unsafe impl<T: UsbContext> Sync for Device<T> {}

rusb/src/device_handle.rs

Lines 127 to 128 in 12ee91d

unsafe impl<T: UsbContext> Send for DeviceHandle<T> {}
unsafe impl<T: UsbContext> Sync for DeviceHandle<T> {}

Device and DeviceHandle implement Send and Sync trait for all T types that implement UsbContext. UsbContext trait has neither Send nor Sync bound and can be implemented from the user side. This permits writing a custom non-thread safe UsbContext implementation in safe Rust code, which can cause a data race when used with Device or DeviceHandle.

If UsbContext is not expected to be implemented by users, making UsbContext a sealed trait can solve this problem. Otherwise, a proper bound should be added to Send/Sync implementations of Device and DeviceHandle (<T: Send/Sync + UsbContext>) or to the definition of UsbContext (UsbContext: Send + Sync).

@a1ien a1ien self-assigned this Dec 19, 2020
a1ien added a commit that referenced this issue Dec 22, 2020
As mention #44 UsbContext trait has neither Send nor Sync bound and can be implemented from the user side. This permits writing a custom non-thread safe UsbContext implementation in safe Rust code, which can cause a data race when used with Device or DeviceHandle.
So required UsbContext implement Send + Sync
@a1ien a1ien closed this as completed in #45 Jan 4, 2021
@Qwaz
Copy link
Author

Qwaz commented Jan 4, 2021

Thanks for the fix!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants