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

crossbeam-epoch's use of offsetof is unsound #395

Closed
Centril opened this issue Jun 21, 2019 · 8 comments
Closed

crossbeam-epoch's use of offsetof is unsound #395

Centril opened this issue Jun 21, 2019 · 8 comments

Comments

@Centril
Copy link

Centril commented Jun 21, 2019

See:

cc @RalfJung

@RalfJung
Copy link
Contributor

In particular, on Rust 1.33+, the way offset_of is unsound causes real misbehavior.

@jeehoonkang
Copy link
Contributor

Thank you for reporting this. May I ask how urgent this issue is for the Rust compiler team? Is it okay for us to wait for @Gilnaa fixing the memoffset crate, or it'd be better for us to fix it now?

@Centril
Copy link
Author

Centril commented Jun 26, 2019

Looking at Cargo.lock rustc's reverse dependency tree to memoffset is:

  • memoffset 0.2.1
    • crossbeam-epoch 0.3.1
      • crossbeam-deque 0.2.0
        • rayon-core 1.4.1
          • rayon 1.0.1
            • installer 0.0.0
            • rls 1.37.0
        • rustc-rayon 0.2.0
          • rustc 0.0.0
          • rustc-ap-rustc_data_structures 491.0.0
          • rustc_data_structures 0.0.0
          • rustc_driver 0.0.0
          • rustc_interface 0.0.0
          • rustdoc 0.0.0
        • rustc-rayon-core 0.2.0
          • rustc 0.0.0
          • rustc-ap-rustc_data_structures 491.0.0
          • rustc_data_structures 0.0.0
          • rustc-rayon 0.2.0 [elided rev deps]
    • crossbeam-epoch 0.7.0
      • crossbeam-deque 0.6.3
        • tokio-threadpool 0.1.10
          • tokio 0.1.14
            • rls 1.37.0
          • tokio-fs 0.1.15
            • tokio 0.1.14
              • rls 1.37.0

cc some of these authors:

This is going to take a bit of time to coordinate, I think.

While std::mem::MaybeUninit<T> will hit stable on 2019-07-04 and will make the probability of exploitable UB lower, the macro will still remain unsound. That said, we should mitigate where we can.

EDIT: As @RalfJung points out below, the macro will naturally need to be updated to use MaybeUninit<T> or else it won't have any effect.

@RalfJung
Copy link
Contributor

While std::mem::MaybeUninit will hit stable on 2019-07-04 and will make the probability of exploitable UB lower

Only if the macro gets changed to use it though.

@cuviper
Copy link
Contributor

cuviper commented Jun 26, 2019

FWIW the latest rayon uses newer crossbeam, but as tokio shows, that memoffset dependency remains.

@jeehoonkang
Copy link
Contributor

Hi @RalfJung, is this issue closed by #402?

@RalfJung
Copy link
Contributor

@jeehoonkang yes it is, thanks!

Would be nice to get a release of the fixed crossbeam-epoch so that we can get the affected memoffset out of the compiler's dependency tree.

@jeehoonkang
Copy link
Contributor

jeehoonkang commented Jul 16, 2019

I agree. Let's track the issue of a new release at #401.

Closing this issue. Thank you!

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

No branches or pull requests

4 participants