Claude review: dma-buf: add DMA_BUF_IOCTL_DERIVE for reduced-permission aliases

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

Patch Review

**Bug (security): O_WRONLY escalation not blocked**

The escalation check only blocks `O_RDWR`:

```c
/* Escalating permissions is not allowed. */
if ((params.flags & O_ACCMODE) == O_RDWR &&
    !(file->f_mode & FMODE_WRITE))
    return -EACCES;
```

Since `O_RDONLY = 0`, `O_WRONLY = 1`, `O_RDWR = 2`: when `params.flags & O_ACCMODE == O_WRONLY` (1), the condition `(1) == O_RDWR (2)` is false, and the ioctl proceeds to create a write-capable fd from a read-only one. The mmap check added to `dma_buf_mmap_internal()` doesn't help here — the derived fd would have `FMODE_WRITE` set, passing the mmap check too.

The fix should check that *any* write access is not being escalated:

```c
if ((params.flags & O_ACCMODE) != O_RDONLY &&
    !(file->f_mode & FMODE_WRITE))
    return -EACCES;
```

For full correctness, the reverse should also be checked (deriving `O_RDONLY` from an `O_WRONLY` fd would grant read access), though `O_WRONLY` dma-bufs don't exist in practice:

```c
if ((params.flags & O_ACCMODE) != O_WRONLY &&
    !(file->f_mode & FMODE_READ))
    return -EACCES;
```

**Minor: invalid access mode O_ACCMODE (3) not rejected**

The flags validation:

```c
if (params.flags & ~(O_ACCMODE | O_CLOEXEC))
    return -EINVAL;
```

permits the bit pattern `3` for the access mode. `OPEN_FMODE(3)` yields `(3+1) & 3 = 0`, creating a file with neither `FMODE_READ` nor `FMODE_WRITE`. While not a security issue, it's invalid input that should be rejected with `-EINVAL`.

**Design concern: DMA_BUF_IOCTL_SYNC not restricted on read-only fds**

`dma_buf_ioctl` dispatches `DMA_BUF_IOCTL_SYNC` on the `struct dma_buf` without checking file access mode. A read-only fd can call `begin_cpu_access` / `end_cpu_access` with `DMA_BUF_SYNC_WRITE` direction. This doesn't itself enable writing, but it's inconsistent with the read-only intent and could trigger unnecessary cache operations.

**Design concern: kernel-side DMA access is unrestricted**

A kernel driver that receives the derived read-only fd via `dma_buf_get()` gets back the same `struct dma_buf` with no indication that it came from a reduced-permission fd. It can `dma_buf_attach` + `dma_buf_map_attachment(DMA_BIDIRECTIONAL)` and get write DMA access. This is probably acceptable for v1 — the trust boundary is between user-space components — but worth documenting as a known limitation.

**Style nit: early private_data access**

```c
static int dma_buf_file_release(struct inode *inode, struct file *file)
{
	struct dma_buf *dmabuf = file->private_data;

	if (!is_dma_buf_file(file))
		return -EINVAL;

	if (file != dmabuf->file)
		dma_buf_put(dmabuf);
```

`dmabuf` is read before `is_dma_buf_file()` validates the file. While harmless (not dereferenced until after the check), moving the assignment after the check would be clearer.

**Lifecycle correctness**

The refcounting logic is correct: `get_dma_buf()` on creation increments the primary file's refcount, and `dma_buf_put()` on derived-file release decrements it. The move of `__dma_buf_list_del` to `dma_buf_release()` (dentry destruction) is necessary and correct — without it, closing the first derived fd would `list_del` the node, and closing a second would corrupt memory.

**Forward declaration of dma_buf_fops**

```c
static const struct file_operations dma_buf_fops;
```

This is needed so `dma_buf_ioctl_derive` (above the definition) can reference it. It works, but an alternative would be to move `dma_buf_ioctl_derive` below the definition to avoid the forward declaration. Minor style preference.

---

---
Generated by Claude Code Patch Reviewer