arrayfire / arrayfire/arrayfire-rust

[BUG] Multiple soundness issues reachable from safe code

未关闭
#385 1 条评论 0 个 reaction 已指派 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
预计耗时
一周以上
活跃度
冷清
描述清晰度
描述清楚
新手友好度
35/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。