linux-kernel - Re: [PATCH v2 3/3] rust: sync: Add SpinLockIrq

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: <991a7dd2-f8a8-402d-a651-eafd857c540d@proton.me>
Date: Thu, 01 Aug 2024 10:29:43 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Lyude Paul <lyude@...hat.com>, rust-for-linux@...r.kernel.org
Cc: Danilo Krummrich <dakr@...hat.com>, airlied@...hat.com, Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>, Waiman Long <longman@...hat.com>, Peter Zijlstra <peterz@...radead.org>, Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Wedson Almeida Filho <wedsonaf@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, Andreas Hindborg <a.hindborg@...sung.com>, Alice Ryhl <aliceryhl@...gle.com>, Martin Rodriguez Reboredo <yakoyoku@...il.com>, Valentin Obst <kernel@...entinobst.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] rust: sync: Add SpinLockIrq

On 01.08.24 00:35, Lyude Paul wrote:
> +unsafe impl super::Backend for SpinLockIrqBackend {
> +    type State = bindings::spinlock_t;
> +    type GuardState = ();
> +    type Context<'a> = IrqDisabled<'a>;
> +
> +    unsafe fn init(
> +        ptr: *mut Self::State,
> +        name: *const core::ffi::c_char,
> +        key: *mut bindings::lock_class_key,
> +    ) {
> +        // SAFETY: The safety requirements ensure that `ptr` is valid for writes, and `name` and
> +        // `key` are valid for read indefinitely.
> +        unsafe { bindings::__spin_lock_init(ptr, name, key) }
> +    }
> +
> +    unsafe fn lock(ptr: *mut Self::State) -> Self::GuardState {
> +        // SAFETY: The safety requirements of this function ensure that `ptr` points to valid
> +        // memory, and that it has been initialised before.
> +        unsafe { bindings::spin_lock(ptr) }

Should this really be the same function as `SpinLock`? (ie is there not
a special version that expects IRQ to already be disabled? eg this could
add additional debug calls)

If there is such a function and you decide to use it, you need to adjust
the safety comment, since you need to justify that IRQs are disabled.
For that you need additional safety requirements on `lock` namely that
when `lock` is called there exists a object of type `Context<'a>` and
that object is alive at least until `unlock` is called.

---
Cheers,
Benno

> +    }
> +
> +    unsafe fn unlock(ptr: *mut Self::State, _guard_state: &Self::GuardState) {
> +        // SAFETY: The safety requirements of this function ensure that `ptr` is valid and that the
> +        // caller is the owner of the spinlock.
> +        unsafe { bindings::spin_unlock(ptr) }
> +    }
> +}
> --
> 2.45.2
>