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

Tracking Issue for stabilizing the sanitizers (e.g., AddressSanitizer, LeakSanitizer, MemorySanitizer, ThreadSanitizer) #123615

Open
3 tasks
rcvalle opened this issue Apr 8, 2024 · 9 comments
Labels
A-sanitizers Area: Sanitizers for correctness and code quality C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC PG-exploit-mitigations Project group: Exploit mitigations

Comments

@rcvalle
Copy link
Member

rcvalle commented Apr 8, 2024

This is a tracking issue for stabilizing the sanitizers (e.g., AddressSanitizer, LeakSanitizer, MemorySanitizer, ThreadSanitizer). The original tracking issue for the sanitizers support for Rust is #39699. This is part of our work to organize and stabilize support for the sanitizers. (See our roadmap at https://hackmd.io/@rcvalle/S1Ou9K6H6.)

Steps

  • Add support for separating sanitizers support in stable and unstable for supported targets.
  • Add core sanitizers to the stable sanitizers for Tier 1 targets.
  • TBD.

Unresolved Questions

Is there any concern or anything that the community would like to see implemented before their stabilization?

Implementation history

@rcvalle rcvalle added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Apr 8, 2024
@rcvalle
Copy link
Member Author

rcvalle commented Apr 8, 2024

@davidtwco and I would like to propose initially stabilizing AddressSanitizer and LeakSanitizer for the Tier 1 targets that support them. After stabilizing these sanitizers, the supported sanitizers will look like this--we'll keep this supported sanitizers table updated as we stabilize the sanitizers:

Target Supported Sanitizers (Stable) Supported Sanitizers (Unstable)
aarch64-apple-darwin address cfi, thread
aarch64-apple-ios address, thread
aarch64-apple-ios-macabi address, leak, thread
aarch64-apple-ios-sim address, thread
aarch64-apple-visionos address, thread
aarch64-apple-visionos-sim address, thread
aarch64-linux-android address, cfi, hwaddress, memtag, shadow-call-stack
aarch64-unknown-freebsd address, cfi, memory, thread
aarch64-unknown-fuchsia address, cfi, shadow-call-stack
aarch64-unknown-illumos address, cfi
aarch64-unknown-linux-gnu address, leak cfi, hwaddress, kcfi, memory, memtag, thread
aarch64-unknown-linux-musl address, cfi, leak, memory, thread
aarch64-unknown-linux-ohos address, cfi, hwaddress, leak, memory, memtag, thread
aarch64-unknown-none kcfi, kernel-address
arm-linux-androideabi address
arm64e-apple-darwin address, cfi, thread
arm64e-apple-ios address, thread
armv7-linux-androideabi address
i586-pc-windows-msvc address
i586-unknown-linux-gnu address
i686-linux-android address
i686-pc-windows-msvc address
i686-unknown-linux-gnu address
loongarch64-unknown-linux-gnu address, cfi, leak, memory, thread
loongarch64-unknown-linux-musl address, cfi, leak, memory, thread
loongarch64-unknown-linux-ohos address, cfi, leak, memory, thread
riscv64-linux-android address
riscv64gc-unknown-fuchsia shadow-call-stack
riscv64gc-unknown-none-elf kernel-address, shadow-call-stack
riscv64gc-unknown-nuttx-elf kernel-address
riscv64imac-unknown-none-elf kernel-address, shadow-call-stack
riscv64imac-unknown-nuttx-elf kernel-address
s390x-unknown-linux-gnu address, leak, memory, thread
s390x-unknown-linux-musl address, leak, memory, thread
thumbv7neon-linux-androideabi address
x86_64-apple-darwin address, leak cfi, thread
x86_64-apple-ios address, thread
x86_64-apple-ios-macabi address, leak, thread
x86_64-linux-android address
x86_64-pc-solaris address, cfi, thread
x86_64-pc-windows-msvc address
x86_64-unknown-freebsd address, cfi, memory, thread
x86_64-unknown-fuchsia address, cfi, leak
x86_64-unknown-illumos address, cfi, thread
x86_64-unknown-linux-gnu address, leak cfi, dataflow, kcfi, memory, safestack, thread
x86_64-unknown-linux-musl address, cfi, leak, memory, thread
x86_64-unknown-linux-ohos address, cfi, leak, memory, thread
x86_64-unknown-netbsd address, cfi, leak, memory, thread
x86_64-unknown-none kcfi, kernel-address
x86_64h-apple-darwin address, cfi, leak, thread

@Jules-Bertholet
Copy link
Contributor

@rustbot label A-sanitizers

@rustbot rustbot added the A-sanitizers Area: Sanitizers for correctness and code quality label Apr 8, 2024
@briansmith
Copy link
Contributor

Is there any concern or anything that the community would like to see implemented before their stabilization?

  • cfg(sanitizer = "known-but-unstable-sanitizer") should trigger a warning that can be disabled on non-Nightly Rust, instead of triggering the "use of an unstable feature". Otherwise crates that want to support stable Rust need to add feature flags or equivalent to guard their #[cfg(sanizier)] usage whenever they do something to support an unstable sanitizer.
  • cfg(sanitizer = "unknown-sanitizer") should trigger a warning that can be disabled on non-Nightly Rust, for the same reason. Perhaps it should be a separate lint.

@briansmith
Copy link
Contributor

briansmith commented Jun 7, 2024

Is there any concern or anything that the community would like to see implemented before their stabilization?

  • We are lacking an official way to __msan_unpoison, which is necessary for many things that skip libc and directly invoke syscalls, e.g. via inline assembly or libc::syscall or equivalent. Similarly it is needed for inline assembly because MSAN (at least with Clang's inline assembly) quite reasonably doesn't track writes via inline assembly. Currently I just declare __msan_unpoison as extern "C" and call it. Ideally there would be an official API for this.
  • When sanitizers are active, what should we assume about how functions like MaybeUninit::{assume_init, assume_init_drop, assume_init_mut, assume_init_ref, slice::{from_raw_parts,from_raw_parts_mut} will interact with msan? Note that these are all functions that serve to indicate that memory is readable/writable but don't actually read to the memory, so msan won't immediately trigger when they are called. However, I do think we should reserve the right for these functions to call __msan_check_mem_is_initialized, perhaps in some future opt-in or perhaps automatically. At a minimum, we need to document that calling assume_init and the like will NOT call __msan_unpoison.
  • Similarly, will we reserve the right for pointer-to-reference conversion (unsafe { &*ptr } and unsafe { &mut *p }) to immediately trigger msan via __msan_check_mem_is_initialized in the future, in some mode, perhaps the default?
  • To address these last two points, we could say in the Memory Sanitizer documentation that, although it is not the case now, any construction of a reference to uninitialized memory may trigger MSAN at some point in the future.

The above is specifically regarding MSAN but I guess analogous concerns for ASAN at least.

@rcvalle
Copy link
Member Author

rcvalle commented Jul 18, 2024

Is there any concern or anything that the community would like to see implemented before their stabilization?

* `cfg(sanitizer = "known-but-unstable-sanitizer")` should trigger a warning that can be disabled on non-Nightly Rust, instead of triggering the "use of an unstable feature". Otherwise crates that want to support stable Rust need to add feature flags or equivalent to guard their `#[cfg(sanizier)]` usage whenever they do something to support an unstable sanitizer.

* `cfg(sanitizer = "unknown-sanitizer")` should trigger a warning that can be disabled on non-Nightly Rust, for the same reason. Perhaps it should be a separate lint.

Do you have an example of when it would be need that #[cfg(sanitize = "sanitizer")] could not be used instead? If the sanitizer is not stable yet, it wouldn't be enabled and the code not compiled/evaluated by non-nightly builds of the compiler. Would it be for warning purposes only?

@rcvalle
Copy link
Member Author

rcvalle commented Jul 18, 2024

Is there any concern or anything that the community would like to see implemented before their stabilization?

  • We are lacking an official way to __msan_unpoison, which is necessary for many things that skip libc and directly invoke syscalls, e.g. via inline assembly or libc::syscall or equivalent. Similarly it is needed for inline assembly because MSAN (at least with Clang's inline assembly) quite reasonably doesn't track writes via inline assembly. Currently I just declare __msan_unpoison as extern "C" and call it. Ideally there would be an official API for this.

I've been working on this for the past few days and have an early/draft implementation of a create that provides (safe) interfaces and FFI bindings for the sanitizers interfaces (see https://crates.io/crates/sanitizers and https://github.com/rcvalle/rust-crate-sanitizers). It currently reflects the current state of the sanitizers interfaces API and documentation, and I think the crate and the sanitizers interfaces themselves would benefit from some standardization of their API and documentation and is something I'd like to improve in the future. Any contributions are greatly appreciated!

  • When sanitizers are active, what should we assume about how functions like MaybeUninit::{assume_init, assume_init_drop, assume_init_mut, assume_init_ref, slice::{from_raw_parts,from_raw_parts_mut} will interact with msan? Note that these are all functions that serve to indicate that memory is readable/writable but don't actually read to the memory, so msan won't immediately trigger when they are called. However, I do think we should reserve the right for these functions to call __msan_check_mem_is_initialized, perhaps in some future opt-in or perhaps automatically. At a minimum, we need to document that calling assume_init and the like will NOT call __msan_unpoison.

  • Similarly, will we reserve the right for pointer-to-reference conversion (unsafe { &*ptr } and unsafe { &mut *p }) to immediately trigger msan via __msan_check_mem_is_initialized in the future, in some mode, perhaps the default?

* To address these last two points, we could say in the Memory Sanitizer documentation that, although it is not the case now, any construction of a reference to uninitialized memory may trigger MSAN at some point in the future.

The above is specifically regarding MSAN but I guess analogous concerns for ASAN at least.

Thanks for pointing this out! I don't know the answer for this yet, but I think we could definitely reserve the right to (as you suggested) implement additional/specific checks for Rust in the future, similarly to how we could improve these sanitizers generally, and these would not be classified as breaking changes but improvements. What do you think?

@rorth
Copy link

rorth commented Aug 19, 2024

There are issues with the entry for x86_64-pc-solaris (and also x86_64-unknown-illumos):

  • While ASan is supported on Solaris (I should know, I did the port), right now it's 32-bit-only on both x86_64 and sparcv9 (the latter only with gcc, clang is buggy here).
  • CFI is not supported in upstream compiler-rt, but I read that rust cfi support doesn't require libclang_rt.cfi-*.a.
  • TSan isn't supported upstream either: PR compiler: update solaris/illumos to enable tsan support. #112039 that initially enabled it is mistaken: the reference to config-ix.cmake is irrelevant since while COMPILER_RT_TSAN_HAS_STATIC_RUNTIME may be TRUE (for whatever reason), COMPILER_RT_HAS_TSAN is still FALSE. To the best of my knowledge there's never been a tsan port to Solaris/Illumos; certainly there's none upstream.

@davidtwco
Copy link
Member

Do you have an example of when it would be need that #[cfg(sanitize = "sanitizer")] could not be used instead? If the sanitizer is not stable yet, it wouldn't be enabled and the code not compiled/evaluated by non-nightly builds of the compiler. Would it be for warning purposes only?

I think what @briansmith is imagining is a scenario where a crate has cfgs for unstable sanitisers (which that crates supports on nightly), but because of those cfgs, the crate now has warnings on stable. On stable, these cfgs would never evaluate to true, so it would be useful to be able to silence the "what is this sanitizer" warnings since you know it's just a yet-to-be-stabilized sanitizer.

@briansmith
Copy link
Contributor

I think what @briansmith is imagining is a scenario where a crate has cfgs for unstable sanitisers (which that crates supports on nightly), but because of those cfgs, the crate now has warnings on stable. On stable, these cfgs would never evaluate to true, so it would be useful to be able to silence the "what is this sanitizer" warnings since you know it's just a yet-to-be-stabilized sanitizer.

Exactly.

josephlr added a commit to rust-random/getrandom that referenced this issue Dec 18, 2024
This allows msan detection to "just-work" whenever someone passes
`-Zsanitizer=memory`. Users no longer need to do any
`getrandom`-specific configuration.

This will also continue working once
rust-lang/rust#123615 is merged which
stabilizes some sanitizers (but not MemorySanitizer).

This is the approch taken by other low-level crates:
  - [`parking_lot_core`](https://github.com/Amanieu/parking_lot/blob/ca920b31312839013b4455aba1d53a4aede21b2f/core/build.rs)
  - [`crossbeam-utils`](https://github.com/crossbeam-rs/crossbeam/blob/00283fb1818174c25b02d7f1c883c5e19f8506a4/crossbeam-utils/build.rs#L42)

The only downside is that this adds a build-script, but it's as small as
possible, doesn't seem to impact build times, and is only a temporary
workaround.

Signed-off-by: Joe Richey <[email protected]>
josephlr added a commit to rust-random/getrandom that referenced this issue Dec 18, 2024
This allows msan detection to "just-work" whenever someone passes
`-Zsanitizer=memory`. Users no longer need to do any
`getrandom`-specific configuration.

This will also continue working once
rust-lang/rust#123615 is merged which
stabilizes some sanitizers (but not MemorySanitizer).

This is the approch taken by other low-level crates:
  - [`parking_lot_core`](https://github.com/Amanieu/parking_lot/blob/ca920b31312839013b4455aba1d53a4aede21b2f/core/build.rs)
  - [`crossbeam-utils`](https://github.com/crossbeam-rs/crossbeam/blob/00283fb1818174c25b02d7f1c883c5e19f8506a4/crossbeam-utils/build.rs#L42)

The only downside is that this adds a build-script, but it's as small as
possible, doesn't seem to impact build times, and is only a temporary
workaround.

Signed-off-by: Joe Richey <[email protected]>
josephlr added a commit to rust-random/getrandom that referenced this issue Dec 18, 2024
This allows msan detection to "just-work" whenever someone passes
`-Zsanitizer=memory`. Users no longer need to do any
`getrandom`-specific configuration.

This will also continue working once
rust-lang/rust#123615 is merged which
stabilizes some sanitizers (but not MemorySanitizer).

This is the approch taken by other low-level crates:
  - [`parking_lot_core`](https://github.com/Amanieu/parking_lot/blob/ca920b31312839013b4455aba1d53a4aede21b2f/core/build.rs)
  - [`crossbeam-utils`](https://github.com/crossbeam-rs/crossbeam/blob/00283fb1818174c25b02d7f1c883c5e19f8506a4/crossbeam-utils/build.rs#L42)

The only downside is that this adds a build-script, but it's as small as
possible, doesn't seem to impact build times, and is only a temporary
workaround.

Signed-off-by: Joe Richey <[email protected]>
josephlr added a commit to rust-random/getrandom that referenced this issue Dec 18, 2024
This allows msan detection to "just-work" whenever someone passes
`-Zsanitizer=memory`. Users no longer need to do any
`getrandom`-specific configuration.

This will also continue working once
rust-lang/rust#123615 is merged, which
stabilizes some sanitizers (but not MemorySanitizer).

This is the approch taken by other low-level crates:
-
[`parking_lot_core`](https://github.com/Amanieu/parking_lot/blob/ca920b31312839013b4455aba1d53a4aede21b2f/core/build.rs)
-
[`crossbeam-utils`](https://github.com/crossbeam-rs/crossbeam/blob/00283fb1818174c25b02d7f1c883c5e19f8506a4/crossbeam-utils/build.rs#L42)

The only downside is that this adds a build-script, but it's as small as
possible, doesn't seem to impact build times, and is only a temporary
workaround.

---------

Signed-off-by: Joe Richey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sanitizers Area: Sanitizers for correctness and code quality C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC PG-exploit-mitigations Project group: Exploit mitigations
Projects
None yet
Development

No branches or pull requests

6 participants