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

Broken PartialEq impl for sigset_t on Linux #2700

Closed
magicant opened this issue Feb 27, 2022 · 1 comment · May be fixed by #3692
Closed

Broken PartialEq impl for sigset_t on Linux #2700

magicant opened this issue Feb 27, 2022 · 1 comment · May be fixed by #3692
Labels
C-bug Category: bug
Milestone

Comments

@magicant
Copy link
Contributor

On Linux, sigset_t is defined as a 1024-bit struct whereas only its first 64 bits are used. The sigemptyset function leaves the other 960 bits uninitialized.

The derived implementation of PartialEq for sigset_t naively compares all the bits of the struct and wrongly returns false depending on the uninitialized part.

$ cargo version -v
cargo 1.59.0 (49d8809dc 2022-02-10)
release: 1.59.0
commit-hash: 49d8809dc2d3e6e0d5ec634fcf26d8e2aab67130
commit-date: 2022-02-10
host: x86_64-unknown-linux-gnu
libgit2: 1.3.0 (sys:0.13.23 vendored)
libcurl: 7.80.0-DEV (sys:0.4.51+curl-7.80.0 vendored ssl:OpenSSL/1.1.1l)
os: OracleLinux 35.0.0 [64-bit]
$ cat Cargo.toml
[package]
name = "sigset-test"
version = "0.1.0"
edition = "2021"

[dependencies]
libc = { version = "0.2.119", features = ["extra_traits"] }
$ cat src/main.rs
fn main() {
    use std::mem::MaybeUninit;
    unsafe {
        let mut s1 = MaybeUninit::<libc::sigset_t>::uninit();
        let mut s2 = MaybeUninit::<libc::sigset_t>::uninit();
        libc::sigemptyset(s1.as_mut_ptr());
        libc::sigemptyset(s2.as_mut_ptr());
        let s1 = s1.assume_init();
        let s2 = s2.assume_init();
        // The two empty sets should compare equal
        assert_eq!(s1, s2);
    }
}
$ cargo r
   Compiling sigset-test v0.1.0 (/home/magicant/Desktop/sigset-test)
    Finished dev [unoptimized + debuginfo] target(s) in 0.20s
     Running `target/debug/sigset-test`
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `sigset_t { __val: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] }`,
 right: `sigset_t { __val: [0, 0, 0, 0, 0, 0, 0, 0, 0, 139700099176444, 139700099131200, 139700096850408, 1736415839, 1, 139700096840524, 139700099177503] }`', src/main.rs:11:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
@magicant magicant added the C-bug Category: bug label Feb 27, 2022
@tgross35 tgross35 added this to the 1.0 milestone Aug 29, 2024
@tgross35 tgross35 added I-unsound 💥 A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness and removed I-unsound 💥 A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness labels Aug 29, 2024
@tgross35
Copy link
Contributor

Wait, this isn't actually unsound on libc's side. The example is incorrectly assuming that sigemptyset initializes the whole struct, which is no good when you assume_init - bad assumption :)

I am going to close this since there isn't something to be done for this specific issue. However, we are considering removing these traits in 1.0 to avoid this kind of footgun. This is being tracked at #3880, feel free to add any comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants