Re: [PATCH 3/7] rust: debugfs: support for binary large objects

[Alice Ryhl <aliceryhl@google.com>]

On Sat, Oct 04, 2025 at 12:26:40AM +0200, Danilo Krummrich wrote:
> Introduce support for read-only, write-only, and read-write binary files
> in Rust debugfs. This adds:
> 
> - BinaryWriter and BinaryReader traits for writing to and reading from
>   user slices in binary form.
> - New Dir methods: read_binary_file(), write_binary_file(),
>   `read_write_binary_file`.
> - Corresponding FileOps implementations: BinaryReadFile,
>   BinaryWriteFile, BinaryReadWriteFile.
> 
> This allows kernel modules to expose arbitrary binary data through
> debugfs, with proper support for offsets and partial reads/writes.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

> +extern "C" fn blob_write<T: BinaryReader>(
> +    file: *mut bindings::file,
> +    buf: *const c_char,
> +    count: usize,
> +    ppos: *mut bindings::loff_t,
> +) -> isize {
> +    // SAFETY:
> +    // - `file` is a valid pointer to a `struct file`.
> +    // - The type invariant of `FileOps` guarantees that `private_data` points to a valid `T`.
> +    let this = unsafe { &*((*file).private_data.cast::<T>()) };
> +
> +    // SAFETY: `ppos` is a valid `loff_t` pointer.
> +    let pos = unsafe { &mut *ppos };
> +
> +    let mut reader = UserSlice::new(UserPtr::from_ptr(buf.cast_mut().cast()), count).reader();
> +
> +    let ret = || -> Result<isize> {
> +        let offset = (*pos).try_into()?;

So offsets larger than the buffer result in Ok(0) unless the offset
doesn't fit in an usize, in which case it's an error instead? I think we
should treat offsets that are too large in the same manner no matter
how large they are.

> +        let read = this.read_from_slice(&mut reader, offset)?;
> +        *pos += bindings::loff_t::try_from(read)?;

This addition could overflow and panic the kernel.

> +        Ok(read.try_into()?)
> +    }();
> +
> +    match ret {
> +        Ok(n) => n,
> +        Err(e) => e.to_errno() as isize,
> +    }
> +}
> +
> +pub(crate) trait BinaryWriteFile<T> {
> +    const FILE_OPS: FileOps<T>;
> +}

Hmm ... this is inconsistent with how we do vtables in other parts of
`kernel`. Normally a struct is used instead of a trait (see e.g.
miscdevice or block). But the inconsistency is already present.

Alice