huggingface / huggingface/candle
Unsound usages of unsafe function `slice::from_raw_parts`
- Dominant language
- Rust
- Stars
- 21k
- Forks
- 1.8k
- Avg merge
- 16h 42m
- Merged PRs (30d)
- 25
Description
Hi, we are researchers from [Sunlab](https://sunlab-gmu.github.io/). When we tried to scan Rust-based repositories with our own implemented bug detectors, we found that there are some potentially unsound usages of `slice::from_raw_parts` in the package `candle-core` at version 0.4.1.
### `quantized::ggml_file::from_raw_data`
https://github.com/huggingface/candle/blob/a4d5a414e3ae79642ecfd6b7bb410c26a8a62a06/candle-core/src/quantized/ggml_file.rs#L120-L134
At line 128, the reference to `u8` slice was casted to raw pointers of generic types that implemented traits such as `GgmlType` e.g., `BlockQ4_0` or `f32`. However, the pointer type conversion in such way would break the alignment requirement of `slice::from_raw_parts`, and it might lead to unsound behaviors.
Even though this API is private, but it is accessible from other public API such as `qtensor_from_ggml`.
### PoC
```rust
use candle_core::quantized::{ggml_file::qtensor_from_ggml, GgmlDType};
use candle_core::Device;
fn main() {
let raw_data: [u8; 1] = [0x1; 1];
let dim: Vec = Vec::from([1usize, 2, 3, 4]);
let res = qtensor_from_ggml(
GgmlDType::F32,
&raw_data,
dim,
&Device::Cpu
);
println!("{:?}", res.unwrap().data());
}
```
Run with miri and get the result,
```
error: Undefined Behavior: dereferencing pointer failed: alloc828 has size 1, so pointer to 96 bytes starting at offset 0 is out-of-bounds
--> /home/rafael/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/raw.rs:101:9
|
101 | &*ptr::slice_from_raw_parts(data, len)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dereferencing pointer failed: alloc828 has size 1, so pointer to 96 bytes starting at offset 0 is out-of-bounds
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
help: alloc828 was allocated here:
--> src/main.rs:5:9
|
5 | let raw_data: [u8; 1] = [0x1; 1];
| ^^^^^^^^
= note: BACKTRACE (of the first span):
= note: inside `std::slice::from_raw_parts::<'_, f32>` at /home/rafael/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/raw.rs:101:9: 101:47
= note: inside `candle_core::quantized::ggml_file::from_raw_data::` at /home/rafael/.cargo/registry/src/index.crates.io-6f17d22bba15001f/candle-core-0.4.1/src/quantized/ggml_file.rs:128:25: 128:87
= note: inside `candle_core::quantized::ggml_file::qtensor_from_ggml` at /home/rafael/.cargo/registry/src/index.crates.io-6f17d22bba15001f/candle-core-0.4.1/src/quantized/ggml_file.rs:154:27: 154:86
note: inside `main`
--> src/main.rs:7:15
|
7 | let res = qtensor_from_ggml(
| _______________^
8 | | GgmlDType::F32,
9 | | &raw_data,
10 | | dim,
11 | | &Device::Cpu
12 | | );
| |_____^
```
In addition to OOB, the potential consequence of this undefined behavior is inconsistency in different environment. For example, if we run this program with x86 targets:
```
Ok([1, 192, 51, 207, 86, 4, 0, 0, 0, 4, 0, 0, 0, 194, 48, 31, 10, 1, 0, 0, 0, 2, 0, 0, 0, 3, 0, 0, 0, 4, 0, 0, 0, 3, 0, 0, 0, 32, 1, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 49, 0, 0, 0, 7, 0, 0, 0, 0, 0, 0, 0, 32, 0, 0, 0, 20, 48, 207, 86, 91, 0, 0, 0, 192, 51, 207, 86, 4, 0, 0, 0, 4, 0, 0, 0, 124, 255, 255])
```
However, if we run this program with x86_64 targets:
```
Ok([1, 32, 185, 148, 192, 200, 85, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0])
```
This might be one impact when users rely on the consistency of API.
Some other usages of `slice::from_raw_parts` are also unsound:
* `safetensors::convert_slice`: https://github.com/huggingface/candle/blob/a0460cd2b13a396ff8545dc1bbffa741f2ec3d79/candle-core/src/safetensors.rs#L94
* `safetensors::convert_slice_with_cast`: https://github.com/huggingface/candle/blob/a0460cd2b13a396ff8545dc1bbffa741f2ec3d79/candle-core/src/safetensors.rs#L124
Would love to have more discussion about consequence and how to fix this issue:)
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with candle-core/src/quantized/ggml_file.rs, especially from_raw_data and its qtensor_from_ggml entry point, then inspect the convert_slice and convert_slice_with_cast usages in candle-core/src/safetensors.rs. Reproduce the provided example under Miri to observe the undefined behavior. Done means the listed conversions no longer trigger invalid slice construction or inconsistent results.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100