Claude review: rust: Add dma_fence abstractions

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Author: Philipp Stanner**

This is the core patch and contains the most substance. The design is ambitious and well-motivated. Several issues:

#### Bug: Positive errno passed to `dma_fence_set_error`

In `DriverFence::drop()`:
```rust
bindings::dma_fence_set_error(fence, ECANCELED as i32);
```

`bindings::ECANCELED` is the positive value `125` (from `include/uapi/asm-generic/errno.h`). The C `dma_fence_set_error` requires a **negative** errno and will trigger `WARN_ON(error >= 0)`:

```c
static inline void dma_fence_set_error(struct dma_fence *fence, int error)
{
    WARN_ON(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags));
    WARN_ON(error >= 0 || error < -MAX_ERRNO);
    fence->error = error;
}
```

Furthermore, `fence->error` is set to the positive value `125`, which code checking `fence->error < 0` will not recognize as an error. This should be:
```rust
bindings::dma_fence_set_error(fence, -(ECANCELED as i32));
```
Or better, use the kernel error infrastructure consistently (e.g., `kernel::error::code::ECANCELED.to_errno()`).

Note: the `signal()` method correctly uses `err.to_errno()` (which returns a negative value), so only the drop path is affected.

#### Potential soundness concern: FenceCtx lifetime after DriverFence drop

When `DriverFence::drop()` runs, it:
1. Signals the fence (setting `DMA_FENCE_FLAG_SIGNALED_BIT`)
2. Calls `drop_in_place(self.data.as_ptr())` — drops the `Arc<FenceCtx>`, the user data, etc.
3. Calls `dma_fence_put(fence)`

The C backend's `dma_fence_driver_name()` and `dma_fence_timeline_name()` check the signaled flag:
```c
if (!dma_fence_test_signaled_flag(fence))
    return (const char __rcu *)ops->get_driver_name(fence);
else
    return (const char __rcu *)"detached-driver";
```

This prevents NEW callers from invoking the callback after signaling. However, there's a TOCTOU race: a caller that reads `signaled_flag == false` before step 1, but executes the callback after step 2, will access the dropped `Arc<FenceCtx>`. The `get_driver_name` callback (`FenceCtx::get_driver_name`) does:
```rust
extern "C" fn get_driver_name(ptr: *mut bindings::dma_fence) -> *const c_char {
    let fctx = unsafe { Self::from_raw_fence(ptr) };
    fctx.driver_name.as_char_ptr()
}
```

Where `from_raw_fence` accesses `fence_data.fctx` — a field that was logically dropped by `drop_in_place`. If this was the last `Arc<FenceCtx>`, the `FenceCtx` memory (including `driver_name` and `timeline_name`) is freed. The `rcu_barrier()` in `FenceCtx::drop` waits for pending `call_rcu` callbacks, but does **not** wait for in-flight RCU read sections (that would require `synchronize_rcu()`).

The C code has the same race pattern but is safe because `get_driver_name` returns static strings. The Rust abstraction uses dynamically allocated `CString`s, making this a potential UAF.

Possible fixes:
- Add `synchronize_rcu()` between signaling and `drop_in_place` to ensure all in-flight RCU readers have completed.
- Store the name strings in the `DriverFenceData` directly instead of going through `FenceCtx`.
- Use `RcuBox` (or similar deferred-free wrapper) for the `Arc<FenceCtx>` stored in `DriverFenceData`.

#### `__GFP_ZERO` is ineffective in `DriverFenceAllocation::new`

```rust
let data = KBox::new(fence_data, GFP_KERNEL | __GFP_ZERO)?;
```

`KBox::new` allocates memory (zeroed with `__GFP_ZERO`), then **moves** the `fence_data` struct on top of it, overwriting the zeros. The `Opaque::uninit()` field in the struct carries indeterminate bytes from the stack, so the dma_fence part of the allocation is NOT actually zeroed despite the flag. The `__GFP_ZERO` gives a false sense of security — it should either be removed (since `dma_fence_init` fully initializes the struct) or replaced with an approach that actually zeroes the `Opaque` field.

#### `FenceCtxOps` trait is unused

```rust
pub trait FenceCtxOps {}
```

Declared but never referenced. It should either be implemented as part of this series or removed to avoid dead code in the public API.

#### Missing `#[repr(transparent)]` on `Fence`

`Fence::from_raw` casts `*mut bindings::dma_fence` directly to `*const Fence`:
```rust
pub unsafe fn from_raw<'a>(ptr: *mut bindings::dma_fence) -> &'a Self {
    unsafe { &*ptr.cast() }
}
```

And the `DriverFenceData` `#[repr(C)]` layout assumes `Fence` starts at offset 0 and has the same address as its inner `Opaque<bindings::dma_fence>`. Without `#[repr(transparent)]` on `Fence`, this layout is not guaranteed by Rust's ABI (though it holds in practice for single-field structs). Adding `#[repr(transparent)]` would make the safety argument explicit.

#### `DriverFence::as_raw` creates `&mut` from shared state

```rust
fn as_raw(&self) -> *mut bindings::dma_fence {
    let fence_data = unsafe { &mut *self.data.as_ptr() };
    fence_data.inner.inner.get()
}
```

This creates a `&mut DriverFenceData` from `&self`, which is unsound if any other reference exists. Since `Opaque::get()` only requires `&self` (it returns `*mut T` through `UnsafeCell`), this should be `&*self.data.as_ptr()` instead of `&mut *self.data.as_ptr()`. The same issue exists in `DriverFenceBorrow::as_raw`.

#### Incomplete re-exports in `dma_buf/mod.rs`

The module exports:
```rust
pub use self::dma_fence::{
    DriverFence, Fence, FenceCb, FenceCbRegistration, FenceCtx,
};
```

But users also need `DriverFenceAllocation`, `CallbackError`, and `DriverFenceAllowedData` to use the API. These should be re-exported.

#### No `cfg` gate for `dma_buf` module

```rust
pub mod dma_buf;
```

Unlike `pub mod drm` (which is gated on `CONFIG_DRM`), `dma_buf` is unconditionally compiled. This is probably fine since dma-fence is core infrastructure, but it should be verified that the helpers and bindings compile in minimal kernel configurations.

---

---
Generated by Claude Code Patch Reviewer