| 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
| ||
|
Message-ID: <aKVNh8HRe5ADbiSZ@archiso>
Date: Wed, 20 Aug 2025 04:22:31 +0000
From: Elle Rhumsaa <elle@...thered-steel.dev>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>,
Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>,
Alexandre Courbot <acourbot@...dia.com>,
linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
kernel@...labora.com, linux-media@...r.kernel.org
Subject: Re: [PATCH 6/7] rust: v4l2: add basic ioctl support
On Mon, Aug 18, 2025 at 02:49:52AM -0300, Daniel Almeida wrote:
> Most of the v4l2 API is implemented through ioctl(), so adding support
> for them is essential in order to be able to write v4l2 drivers.
>
> Hook up ioctl support by filling up an instance of v4l2_ioctl_ops. For
> now, we only support the most basic v4l2 ioctl: VIDIOC_QUERYCAPS. This
> is used by userspace to retrieve information about the device, and is
> considered enough to implement a simple v4l2 sample Rust driver.
>
> Signed-off-by: Daniel Almeida <daniel.almeida@...labora.com>
> ---
> rust/kernel/media/v4l2/file.rs | 1 -
> rust/kernel/media/v4l2/ioctl.rs | 92 +++++++++++++++++++++++++++++++++++++++++
> rust/kernel/media/v4l2/mod.rs | 3 ++
> rust/kernel/media/v4l2/video.rs | 13 +++++-
> 4 files changed, 107 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/media/v4l2/file.rs b/rust/kernel/media/v4l2/file.rs
> index 37b34f8e6f251fafde5f7e6b4bd654519d8247a5..8817051268323866f41fd56a0c7e8fa4b7537858 100644
> --- a/rust/kernel/media/v4l2/file.rs
> +++ b/rust/kernel/media/v4l2/file.rs
> @@ -50,7 +50,6 @@ impl<T: DriverFile> File<T> {
> ///
> /// - `ptr` must be a valid pointer to a `struct v4l2_file`.
> /// - `ptr` must be valid for 'a.
> - #[expect(dead_code)]
> pub(super) unsafe fn from_raw<'a>(ptr: *mut bindings::v4l2_fh) -> &'a File<T> {
> // SAFETY: `ptr` is a valid pointer to a `struct v4l2_file` as per the
> // safety requirements of this function.
> diff --git a/rust/kernel/media/v4l2/ioctl.rs b/rust/kernel/media/v4l2/ioctl.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..e8d20d4cb70f5722c0109ea5bad36041355fc7a1
> --- /dev/null
> +++ b/rust/kernel/media/v4l2/ioctl.rs
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-copyrightText: Copyright (C) 2025 Collabora Ltd.
> +
> +//! V4L2 device node ioctl support.
> +//!
> +//! Most of the V4L2 API is implemented through the ioctl system call. This
> +//! module provides support for ioctl operations on V4L2 device nodes.
> +
> +use core::{marker::PhantomData, mem::MaybeUninit};
> +
> +use crate::{
> + media::v4l2::{
> + self,
> + video::{self, Driver},
> + },
> + prelude::*,
> +};
> +
> +/// The vtable for the ioctls of a registered Rust [`video::Device`].
> +///
> +/// # Invariants
> +///
> +/// - All the callbacks in [`IoctlVtable`] are called after the underlying
> +/// [`video::Device`] has been registered.
> +pub(super) struct IoctlVtable<T: Driver>(PhantomData<T>);
> +
> +impl<T: Driver> IoctlVtable<T> {
> + /// # Safety
> + ///
> + /// This should only be called from the ioctl callbacks and the returned
> + /// reference should not outlive the callback itself.
> + unsafe fn data<'a>(file: *mut bindings::file) -> &'a <T as Driver>::Data
> + where
> + T: 'a,
> + {
> + // SAFETY: This was set during the video device registration process.
> + let vdev = unsafe { bindings::video_devdata(file) };
> +
> + // SAFETY: `video_device` is a valid pointer to a `struct video_device`
> + // returned by `bindings::video_devdata` and it is valid while the
> + // reference is alive.
> + unsafe { video::Device::<T>::from_raw(vdev) }
> + }
> +
> + /// # Safety
> + ///
> + /// This should only be used as the `videoc_querycap` callback.
> + unsafe extern "C" fn vidioc_querycap_callback(
> + file: *mut bindings::file,
> + _fh: *mut c_void,
> + cap: *mut bindings::v4l2_capability,
> + ) -> core::ffi::c_int {
> + // SAFETY: this is being called from an ioctl callback and the returned
> + // reference does not outlive it.
> + let data = unsafe { Self::data(file) };
> +
> + // SAFETY: the fact that this is being called from an ioctl callback means that:
> + //
> + // - the video device has been registered.
> + // - `open()` has been called (as you cannot call ioctl() on a file that
> + // has not been previously opened).
> + // - as a result from the statement above, a valid `v4l2_fh` was
> + // installed in `bindings::file::private_data`, which we then convert
> + // into `File<T>` here.
> + // - `ptr` is valid for 'a, i.e.: for the scope of this function.
> + let file = unsafe { v4l2::file::File::<T::File>::from_raw((*file).private_data.cast()) };
> +
> + // SAFETY: the safety requirements ensure that `cap` is a valid pointer
> + // to a `struct v4l2_capability`, which fulfills the requirements of the
> + // `Capabilities::from_raw`.
> + let cap = unsafe { v4l2::caps::Capabilities::from_raw(cap) };
> +
> + match T::querycap(file, data, cap) {
> + Ok(()) => 0,
> + Err(err) => err.to_errno(),
> + }
> + }
> +
> + const VTABLE: bindings::v4l2_ioctl_ops = bindings::v4l2_ioctl_ops {
> + vidioc_querycap: if T::HAS_QUERYCAP {
> + Some(Self::vidioc_querycap_callback)
> + } else {
> + None
> + },
> + // SAFETY: All zeros is a valid value for `bindings::v4l2_ioctl_ops`.
> + ..unsafe { MaybeUninit::zeroed().assume_init() }
> + };
> +
> + pub(super) const fn build() -> &'static bindings::v4l2_ioctl_ops {
> + &Self::VTABLE
> + }
> +}
> diff --git a/rust/kernel/media/v4l2/mod.rs b/rust/kernel/media/v4l2/mod.rs
> index 1d8241f8a2230954371965bb91b20e726f144dce..4caa9fdc1ff3ed2bd6145cc653b49a84873ecac2 100644
> --- a/rust/kernel/media/v4l2/mod.rs
> +++ b/rust/kernel/media/v4l2/mod.rs
> @@ -15,3 +15,6 @@
> pub mod caps;
> /// Support for Video for Linux 2 (V4L2) file handles.
> pub mod file;
> +
> +/// Support for Video for Linux 2 (V4L2) ioctls.
> +pub mod ioctl;
> diff --git a/rust/kernel/media/v4l2/video.rs b/rust/kernel/media/v4l2/video.rs
> index c0ac99a8234d2f7a8effd4701b9f7440236540c8..2390a93a39925dbff3f809bc65adfac1d881309c 100644
> --- a/rust/kernel/media/v4l2/video.rs
> +++ b/rust/kernel/media/v4l2/video.rs
> @@ -19,7 +19,7 @@
>
> use crate::{
> alloc,
> - error::to_result,
> + error::{to_result, VTABLE_DEFAULT_ERROR},
> media::v4l2::{self, caps::DeviceCaps, file::DriverFile, video},
> prelude::*,
> types::{ARef, AlwaysRefCounted, Opaque},
> @@ -143,6 +143,7 @@ unsafe impl<T: Driver> Sync for Device<T> {}
>
> /// The interface that must be implemented by structs that would otherwise embed
> /// a C [`struct video_device`](srctree/include/media/v4l2-dev.h).
> +#[vtable]
> pub trait Driver: v4l2::device::Driver {
> /// The type of the driver's private data.
> type Data;
> @@ -161,6 +162,15 @@ pub trait Driver: v4l2::device::Driver {
>
> /// The capabilities offered by this device node.
> const CAPS: DeviceCaps;
> +
> + /// Driver implementation for the `querycap` ioctl.
> + fn querycap(
> + _file: &v4l2::file::File<<Self as Driver>::File>,
> + _data: &<Self as Driver>::Data,
> + _cap: &mut v4l2::caps::Capabilities,
> + ) -> Result {
> + build_error!(VTABLE_DEFAULT_ERROR)
> + }
> }
>
> struct DeviceOptions<'a, T: Driver> {
> @@ -184,6 +194,7 @@ fn into_raw(self) -> bindings::video_device {
> release: Some(Device::<T>::release_callback),
> fops: super::file::FileVtable::<T::File>::build(),
> device_caps: T::CAPS.as_raw(),
> + ioctl_ops: super::ioctl::IoctlVtable::<T>::build(),
> // SAFETY: All zeros is valid for the rest of the fields in this C
> // type.
> ..unsafe { MaybeUninit::zeroed().assume_init() }
>
> --
> 2.50.1
Reviewed-by: Elle Rhumsaa <elle@...thered-steel.dev>
Powered by blists - more mailing lists