arrayfire / arrayfire/arrayfire-rust

[BUG] Multiple soundness issues reachable from safe code

オープン
#385 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る
Bug
主要言語
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

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。