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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aPI9tNoh0I3KGDjl@google.com>
Date: Fri, 17 Oct 2025 12:59:32 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: gregkh@...uxfoundation.org, rafael@...nel.org, ojeda@...nel.org, 
	alex.gaynor@...il.com, boqun.feng@...il.com, gary@...yguo.net, 
	bjorn3_gh@...tonmail.com, lossin@...nel.org, a.hindborg@...nel.org, 
	tmgross@...ch.edu, mmaurer@...gle.com, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/7] rust: debugfs: support for binary large objects

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@...nel.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