arrayfire / arrayfire/arrayfire-rust
[BUG] Multiple soundness issues reachable from safe code
- 主要言語
- Rust
- スター
- 827
- フォーク
- 59
- PR マージ指標
- 30日以内にマージされた PR はありません
説明
Five separate soundness problems, all reachable from safe code. Grouping them since they're related in character; happy to split into individual issues if that's easier to triage.
*Found by Claude Opus 5 after prompting it to look for potential causes for 0xC0000005 errors on Windows.*
Description
===========
## 1. `AfError::from` transmutes values that are not valid discriminants
[`src/core/util.rs:69-74`](https://github.com/arrayfire/arrayfire-rust/blob/master/src/core/util.rs#L69-L74):
```rust
impl From for AfError {
fn from(t: i32) -> Self {
assert!(AfError::SUCCESS as i32 <= t && t <= AfError::ERR_UNKNOWN as i32);
unsafe { mem::transmute(t) }
}
}
```
The assert bounds `t` to `[0, 999]`, but `AfError` has **17 sparse variants** in that range. A range check is not a validity check for a sparse enum, so any in-range value that isn't a declared discriminant becomes an invalid `#[repr(u32)]` enum value — immediate UB.
Five error codes that ArrayFire genuinely returns have no Rust variant (verified against `include/af/defines.h` at v3.8.0; unchanged in 3.10):
| C value | C name |
|---|---|
| 303 | `AF_ERR_NONFREE` |
| 403 | `AF_ERR_NO_HALF` |
| **501** | **`AF_ERR_LOAD_LIB`** |
| 502 | `AF_ERR_LOAD_SYM` |
| 503 | `AF_ERR_ARR_BKND_MISMATCH` |
`AF_ERR_LOAD_LIB = 501` is what the unified backend's `CALL` macro returns when it cannot locate a backend (`src/api/unified/symbol_manager.hpp:155`, "ArrayFire couldn't locate any backends."). Since `build.rs` links the unified `af` library by default, this is live on every call for anyone whose backend DLL fails to load.
`AF_ERR_ARR_BKND_MISMATCH = 503` is returned whenever an `Array` from one backend is used after `set_backend` switched to another — which `examples/unified.rs` does.
The value then flows into `Display for AfError` ([`src/core/defines.rs:84-105`](https://github.com/arrayfire/arrayfire-rust/blob/master/src/core/defines.rs#L84-L105)), an exhaustive `match` with no wildcard arm, which rustc lowers to a switch with an `unreachable` default.
Observed behaviour with a faithful standalone repro of the enum + `Display` + `From`:
- Debug: `panicked: trying to construct an enum from an invalid value 0x1f5`, then `thread caused non-unwinding panic. aborting.`
- Release (`-O`): survives, and reports **`"Unknown Error"`** — LLVM happens to bounds-guard the jump table.
So on current codegen this is more often a *diagnostic* failure than a crash: users are told "Unknown Error" instead of "ArrayFire couldn't locate any backends," which I suspect is a large part of why #285 and #311 have gone undiagnosed for years. But it is UB regardless and the release-mode behaviour is not guaranteed.
Note this is the same failure mode as RUSTSEC-2018-0011; the `#[repr(u32)]` half of that fix landed, but this conversion still constructs invalid values.
**Fix:** replace with an exhaustive `match` mapping unknown codes to `ERR_UNKNOWN`, and add the five missing variants. `RandomEngineType::from` ([`util.rs:429-437`](https://github.com/arrayfire/arrayfire-rust/blob/master/src/core/util.rs#L429-L437)) has the same sparse-range hole, though I couldn't find a reachable trigger for it.
---
## 2. `MatProp::from` transmutes with no validation at all, and `BitOr` manufactures invalid values
[`src/core/util.rs:829-841`](https://github.com/arrayfire/arrayfire-rust/blob/master/src/core/util.rs#L829-L841):
```rust
impl From for MatProp {
fn from(t: u32) -> Self {
unsafe { mem::transmute(t) }
}
}
impl BitOr for MatProp {
type Output = Self;
fn bitor(self, rhs: Self) -> Self {
Self::from(self as u32 | rhs as u32)
}
}
```
`MatProp` is a bit-flag enum with variants `0, 1, 2, 4, 32, 64, 128, 512, 1024, 2048, 4096, 8192`. Combining any two non-adjacent flags produces a value with no corresponding variant:
```rust
let p = MatProp::UPPER | MatProp::DIAGUNIT; // 32 | 128 = 160 -> UB
```
This is 100% safe code, using the API the docs point at for `matmul`/`solve`/LAPACK routines.
**Fix:** `MatProp` should be a `bitflags`-style newtype over `u32` rather than an enum. A minimal stopgap is to drop the `From`/`BitOr` impls and have callers pass `u32`.
---
## 3. `#[derive(Clone)]` on `Window` alongside `Drop` is a double free
[`src/graphics/mod.rs:180-199`](https://github.com/arrayfire/arrayfire-rust/blob/master/src/graphics/mod.rs#L180-L199):
```rust
#[derive(Clone)]
pub struct Window {
handle: af_window,
...
}
impl Drop for Window {
fn drop(&mut self) {
let err_val = unsafe { af_destroy_window(self.handle) };
```
The derived `Clone` bit-copies the raw `af_window`; both copies then call `af_destroy_window` on the same handle.
Every other handle wrapper in the crate gets this right with a hand-written `Clone` that calls the corresponding retain function — `Array` (`af_retain_array`, `array.rs:706-718`), `Features` (`af_retain_features`, `vision/mod.rs:194-200`), `RandomEngine` (`af_retain_random_engine`, `random.rs:197-205`). `Window` looks like it was simply missed.
**Fix:** replace the derive with a manual `Clone` that retains, or remove `Clone` if Forge has no retain equivalent for windows.
---
## 4. `alloc_host` always returns NULL and leaks
[`src/core/util.rs:53-61`](https://github.com/arrayfire/arrayfire-rust/blob/master/src/core/util.rs#L53-L61):
```rust
pub fn alloc_host(elements: usize, _type: DType) -> *const T {
let ptr: *const T = ::std::ptr::null();
let bytes = (elements * get_size(_type)) as dim_t;
let err_val = unsafe { af_alloc_host(&mut (ptr as *const c_void), bytes) };
HANDLE_ERROR(AfError::from(err_val));
ptr
}
```
`&mut (ptr as *const c_void)` takes a mutable borrow of the **temporary produced by the cast**, not of `ptr`. ArrayFire writes the allocated pointer into that temporary, which is discarded at the end of the statement. `ptr` isn't even declared `mut`, so it cannot be written to.
The function therefore returns NULL unconditionally and leaks the host allocation on every call. Any caller dereferencing the result gets a null-pointer access violation.
**Fix:**
```rust
pub fn alloc_host(elements: usize, _type: DType) -> *const T {
let mut ptr: *mut c_void = ::std::ptr::null_mut();
let bytes = (elements * get_size(_type)) as dim_t;
let err_val = unsafe { af_alloc_host(&mut ptr, bytes) };
HANDLE_ERROR(AfError::from(err_val));
ptr as *const T
}
```
---
## 5. `Array::set` is safe but installs an arbitrary handle
[`src/core/array.rs:493-496`](https://github.com/arrayfire/arrayfire-rust/blob/master/src/core/array.rs#L493-L496):
```rust
/// Set the native FFI handle for Rust object `Array`
pub fn set(&mut self, handle: af_array) {
self.handle = handle;
}
```
Safe code can store any pointer value here; `Drop` (`array.rs:721-729`) then calls `af_release_array` on it. It also leaks the previously held handle.
The neighbouring getter `get()` at `array.rs:489` *is* correctly marked `unsafe`, so this looks like an oversight rather than a deliberate choice.
**Fix:** make it `unsafe fn`, and document the invariant that the handle must be a valid, owned `af_array`.
Reproducible Code and/or Steps
------------------------------
System Information
------------------
Checklist
---------
- [ ] Using the latest available ArrayFire release
- [ ] GPU drivers are up to date
コントリビューションガイド
このリポジトリのコントリビューションガイドは索引されていません
調査の方向性
Start with the cited locations in src/core/util.rs and src/core/defines.rs, then inspect src/graphics/mod.rs and src/core/array.rs, including the neighbouring handle methods. Use examples/unified.rs to understand backend switching and the documented failure path. Done means each reported safe-code path preserves valid enum and native-handle invariants without leaks or double releases.
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- rust
- 領域
- api, backend
- issue の種類
- バグ
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 活発さ
- 静か
- 明瞭さ
- 明確に書かれている
- 初心者へのやさしさ
- 35/100