Skip to content

Use ManuallyDrop in queues #184

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

Merged
1 commit merged into from Jul 25, 2018
Merged

Use ManuallyDrop in queues #184

1 commit merged into from Jul 25, 2018

Conversation

ghost
Copy link

@ghost ghost commented Jul 25, 2018

This will hopefully fix crossbeam-rs/crossbeam-epoch#82

@c0gent
Copy link

c0gent commented Jul 25, 2018

Will test.

@c0gent
Copy link

c0gent commented Jul 25, 2018

Looks good. Yank time?

@ghost ghost mentioned this pull request Jul 25, 2018
@ghost ghost merged commit e6b3b98 into crossbeam-rs:master Jul 25, 2018
@ghost ghost deleted the use-manually-drop branch July 25, 2018 15:41
@ghost
Copy link
Author

ghost commented Jul 25, 2018

@c0gent Published v0.4.1 with this fix.

c0gent added a commit to poanetwork/hydrabadger that referenced this pull request Jul 25, 2018
@Thomasdezeeuw
Copy link
Contributor

Maybe I'm missing something, but shouldn't this now manually drop the values? Doesn't the current code leak values?

@ghost
Copy link
Author

ghost commented Jul 25, 2018

@Thomasdezeeuw There are no leaks.

Queue nodes (more precisely, Segment and Payload) are just temporary storage for values between the moment they are pushed and the moment they are popped. Note that we only destroy empty nodes (not containing any values). Therefore, dropping a node mustn't run any value destructors (we enforce this by wrapping values into ManuallyDrop). In other words, just because we have a ManuallyDrop<T>, that doesn't necessarily mean a value is currently written there.

@Thomasdezeeuw
Copy link
Contributor

@stjepang Ok thanks for the reply.

@Shnatsel
Copy link
Contributor

Shnatsel commented Dec 9, 2018

Thanks for the quick resolution and the yank!

This seems to be some kind of a memory safety issue, perhaps a use-after-free, and those are usually exploitable. Could you file this issue in the Rust security advisory database so that people who still have 0.4.0 in Cargo.lock could find out that they're using a vulnerable version and upgrade?

@ghost
Copy link
Author

ghost commented Dec 9, 2018

@Shnatsel Reported: rustsec/advisory-db#75

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Segfault
4 participants