linux-kernel - Re: [RFC 1/2] rust: introduce abstractions for fwctl

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: <20251030162207.GS1018328@nvidia.com>
Date: Thu, 30 Oct 2025 13:22:07 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Zhi Wang <zhiw@...dia.com>
Cc: rust-for-linux@...r.kernel.org, dakr@...nel.org, bhelgaas@...gle.com,
	kwilczynski@...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, aliceryhl@...gle.com,
	tmgross@...ch.edu, linux-kernel@...r.kernel.org, cjia@...dia.com,
	smitra@...dia.com, ankita@...dia.com, aniketa@...dia.com,
	kwankhede@...dia.com, targupta@...dia.com, zhiwang@...nel.org,
	alwilliamson@...dia.com, acourbot@...dia.com, joelagnelf@...dia.com,
	jhubbard@...dia.com
Subject: Re: [RFC 1/2] rust: introduce abstractions for fwctl

On Thu, Oct 30, 2025 at 04:03:12PM +0000, Zhi Wang wrote:
> +impl<T: FwCtlOps> Registration<T> {
> +    /// Allocate and register a new fwctl device under the given parent device.
> +    pub fn new(parent: &device::Device) -> Result<Self> {
> +        let ops = &FwCtlVTable::<T>::VTABLE as *const _ as *mut _;
> +
> +        // SAFETY: `_fwctl_alloc_device()` allocates a new `fwctl_device`
> +        // and initializes its embedded `struct device`.
> +        let dev = unsafe {
> +            bindings::_fwctl_alloc_device(
> +                parent.as_raw(),
> +                ops,
> +                core::mem::size_of::<bindings::fwctl_device>(),
> +            )
> +        };
> +
> +        let dev = NonNull::new(dev).ok_or(ENOMEM)?;
> +
> +        // SAFETY: `fwctl_register()` expects a valid device from `_fwctl_alloc_device()`.
> +        let ret = unsafe { bindings::fwctl_register(dev.as_ptr()) };

This is a Bound device, not just any device.

> +        if ret != 0 {
> +            // SAFETY: If registration fails, release the allocated fwctl_device().
> +            unsafe {
> +                bindings::put_device(core::ptr::addr_of_mut!((*dev.as_ptr()).dev));

?? Don't open code fwctl_put() - it should be called directly?

> +            }
> +            return Err(Error::from_errno(ret));
> +        }
> +
> +        Ok(Self {
> +            fwctl_dev: dev,
> +            _marker: PhantomData,
> +        })
> +    }
> +
> +    fn as_raw(&self) -> *mut bindings::fwctl_device {
> +        self.fwctl_dev.as_ptr()
> +    }
> +}
> +
> +impl<T: FwCtlOps> Drop for Registration<T> {
> +    fn drop(&mut self) {
> +        // SAFETY: `fwctl_unregister()` expects a valid device from `_fwctl_alloc_device()`.

Incomplete safety statement, the device passed to fwctl_alloc_device must
still be bound prior to calling fwctl_unregister

> +        unsafe {
> +            bindings::fwctl_unregister(self.as_raw());
> +            bindings::put_device(core::ptr::addr_of_mut!((*self.as_raw()).dev));

There for Drop can only do fwctl_put() since otherwise there is no way
to guarantee a Bound device.

unregister has to happen before remove() completes, Danilo had some
approach to this I think he told me?

Jason