Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

unsafe precondition(s) violated: ptr::write requires that the pointer argument is aligned and non-null #21

Closed
kalzoo opened this issue Dec 14, 2023 · 2 comments · Fixed by #24

Comments

@kalzoo
Copy link

kalzoo commented Dec 14, 2023

First report on this - happy to make an MRE if that helps. It may not even be a bug, but I don't know that yet.

Situation:

  1. I'm adding threading support to a library compiled to WASM for use in the browser. The app uses serde-yaml. The app has worked quite well for some time, and this issue appeared when adding the required build flags from the docs:
[unstable]
build-std = ['std', 'panic_abort']

[build]
target = "wasm32-unknown-unknown"
rustflags = '-Ctarget-feature=+atomics,+bulk-memory'

My first thought is that this panic might be "expected" and otherwise caught somewhere in the stack above with unwind and that's broken with the use of abort, but I'm not seeing a panic recovery here or in serde-yaml. Maybe I'm just missing it.

  1. Build succeeds, but the binary panics and emits the following error at runtime:
lib_bg.wasm:0x14dab71 Uncaught (in promise) RuntimeError: unreachable
    at std::sys::wasm::common::abort_internal::hb32c013b124a63c5
    at std::panicking::rust_panic_with_hook::h8d83516cdb4f9a91
    at std::panicking::begin_panic_handler::{{closure}}::hf16a9ba57012eb4
    at std::sys_common::backtrace::__rust_end_short_backtrace::h16a9293adcf4d4fb
    at rust_begin_unwind
    at core::panicking::panic_nounwind_fmt::ha0e10cdd15fcadb1
    at core::panicking::panic_nounwind::h277c78959cb6049e
    at core::ptr::write::hcbc9dd9a199faf58
    at unsafe_libyaml::scanner::yaml_parser_fetch_stream_start::hecc19e1d6f4b1357
    at unsafe_libyaml::scanner::yaml_parser_fetch_next_token::h5f7a3b945d3253ad

So, it appears to be in here. If this is expected or intractable, I can always migrate the app off of YAML serialization, but it'd be nice to keep it in there if I can.

Versions:

➜  cargo tree -i unsafe-libyaml
unsafe-libyaml v0.2.9
└── serde_yaml v0.9.25
      ...

Thanks!

@dtolnay
Copy link
Owner

dtolnay commented Dec 14, 2023

Thanks for the report! This is definitely a serious bug in this crate.

I found one bug that I think would cause this crash when running on a 32-bit architecture such as wasm. I tried reproducing the crash using MIRIFLAGS='-Zmiri-disable-isolation' cargo miri test --target i686-unknown-linux-gnu which also should be 32-bit. I didn't manage to observe a crash this way, but I think that is a coincidence of miri's allocator working differently than wasm's.

#24 has the fix and is published in unsafe-libyaml 0.2.10. Could you try this in your use case and let me know whether it's fixed?

@kalzoo
Copy link
Author

kalzoo commented Dec 17, 2023

Worked like a charm, thank you for the quick fix!

gitlab-dfinity pushed a commit to dfinity/ic that referenced this issue Dec 28, 2023
fix: fix multiple advisory warnings and 1 error found by cargo-deny

Openssl is removed (again) from Cargo.toml.

The following warnings are removed from the repository.
```
error[xxx]: Tungstenite allows remote attackers to cause a denial of service
     ┌─ /ic/Cargo.lock:1331:1
     │
1331 │ tungstenite 0.17.3 registry+https://github.com/rust-lang/crates.io-index
     │ ------------------------------------------------------------------------ security xxx detected
     │
     = ID: RUSTSEC-2023-0065
     = Advisory: https://rustsec.org/advisories/RUSTSEC-2023-0065
     = The Tungstenite crate through 0.20.0 for Rust allows remote attackers to cause
       a denial of service (minutes of CPU consumption) via an excessive length of an
       HTTP header in a client handshake. The length affects both how many times a parse
       is attempted (e.g., thousands of times) and the average amount of data for each
       parse attempt (e.g., millions of bytes).
     = Announcement: snapview/tungstenite-rs#376
     = Solution: Upgrade to >=0.20.1 (try `cargo update -p tungstenite`)


warning[unmaintained]: difference is unmaintained
    ┌─ /ic/Cargo.lock:267:1
    │
267 │ difference 2.0.0 registry+https://github.com/rust-lang/crates.io-index
    │ ---------------------------------------------------------------------- unmaintained advisory detected
    │
    = ID: RUSTSEC-2020-0095
    = Advisory: https://rustsec.org/advisories/RUSTSEC-2020-0095
    = The author of the `difference` crate is unresponsive.
      
      Maintained alternatives:
      
      - [`dissimilar`](https://crates.io/crates/dissimilar)
      
      - [`similar`](https://crates.io/crates/similar)
      
      - [`treediff`](https://crates.io/crates/treediff)
      
      - [`diffus`](https://crates.io/crates/diffus)
    = Announcement: johannhof/difference.rs#45
    = Solution: No safe upgrade is available!


warning[unsound]: Unaligned write of u64 on 32-bit and 16-bit platforms
     ┌─ /ic/Cargo.lock:1355:1
     │
1355 │ unsafe-libyaml 0.2.9 registry+https://github.com/rust-lang/crates.io-index
     │ -------------------------------------------------------------------------- unsound advisory detected
     │
     = ID: RUSTSEC-2023-0075
     = Advisory: https://rustsec.org/advisories/RUSTSEC-2023-0075
     = Affected versions allocate memory using the alignment of `usize` and write data
       to it of type `u64`, without using `core::ptr::write_unaligned`. In platforms
       with sub-64bit alignment for `usize` (including wasm32 and x86) these writes
       are insufficiently aligned some of the time.
       
       If using an ordinary optimized standard library, the bug exhibits Undefined
       Behavior so may or may not behave in any sensible way, depending on
       optimization settings and hardware and other things. If using a Rust standard
       library built with debug assertions enabled, the bug manifests deterministically
       in a crash (non-unwinding panic) saying _"ptr::write requires that the pointer
       argument is aligned and non-null"_.
       
       No 64-bit platform is impacted by the bug.
       
       The flaw was corrected by allocating with adequately high alignment on all
       platforms.
     = Announcement: dtolnay/unsafe-libyaml#21
     = Solution: Upgrade to >=0.2.10 (try `cargo update -p unsafe-libyaml`)



warning[yanked]: detected yanked crate (try `cargo update -p ahash`)
   ┌─ /ic/Cargo.lock:20:1
   │
20 │ ahash 0.7.6 registry+https://github.com/rust-lang/crates.io-index
   │ ----------------------------------------------------------------- yanked version

warning[yanked]: detected yanked crate (try `cargo update -p ahash`)
   ┌─ /ic/Cargo.lock:21:1
   │
21 │ ahash 0.8.3 registry+https://github.com/rust-lang/crates.io-index
   │ ----------------------------------------------------------------- yanked version

warning[yanked]: detected yanked crate (try `cargo update -p hermit-abi`)
    ┌─ /ic/Cargo.lock:385:1
    │
385 │ hermit-abi 0.3.1 registry+https://github.com/rust-lang/crates.io-index
    │ ---------------------------------------------------------------------- yanked version
``` 

See merge request dfinity-lab/public/ic!16899
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants