[2/7] rust: cred: add Rust abstraction for `struct cred`
Commit Message
From: Wedson Almeida Filho <wedsonaf@gmail.com>
Add a wrapper around `struct cred` called `Credential`, and provide
functionality to get the `Credential` associated with a `File`.
Rust Binder must check the credentials of processes when they attempt to
perform various operations, and these checks usually take a
`&Credential` as parameter. The security_binder_set_context_mgr function
would be one example. This patch is necessary to access these security_*
methods from Rust.
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Co-developed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/bindings/bindings_helper.h | 1 +
rust/helpers.c | 13 +++++++++
rust/kernel/cred.rs | 64 +++++++++++++++++++++++++++++++++++++++++
rust/kernel/file.rs | 16 +++++++++++
rust/kernel/lib.rs | 1 +
5 files changed, 95 insertions(+)
Comments
On 11/29/23 13:51, Alice Ryhl wrote:
> + /// Returns the credentials of the task that originally opened the file.
> + pub fn cred(&self) -> &Credential {
> + // This `read_volatile` is intended to correspond to a READ_ONCE call.
> + //
> + // SAFETY: The file is valid because the shared reference guarantees a nonzero refcount.
> + //
> + // TODO: Replace with `read_once` when available on the Rust side.
> + let ptr = unsafe { core::ptr::addr_of!((*self.0.get()).f_cred).read_volatile() };
> +
> + // SAFETY: The signature of this function ensures that the caller will only access the
> + // returned credential while the file is still valid, and the credential must stay valid
> + // while the file is valid.
About the last part of this safety comment, is this a guarantee from the
C side? If yes, then I would phrase it that way:
... while the file is still valid, and the C side ensures that the
credentials stay valid while the file is valid.
Benno Lossin <benno.lossin@proton.me> writes:
> On 11/29/23 13:51, Alice Ryhl wrote:
>> + /// Returns the credentials of the task that originally opened the file.
>> + pub fn cred(&self) -> &Credential {
>> + // This `read_volatile` is intended to correspond to a READ_ONCE call.
>> + //
>> + // SAFETY: The file is valid because the shared reference guarantees a nonzero refcount.
>> + //
>> + // TODO: Replace with `read_once` when available on the Rust side.
>> + let ptr = unsafe { core::ptr::addr_of!((*self.0.get()).f_cred).read_volatile() };
>> +
>> + // SAFETY: The signature of this function ensures that the caller will only access the
>> + // returned credential while the file is still valid, and the credential must stay valid
>> + // while the file is valid.
>
> About the last part of this safety comment, is this a guarantee from the
> C side? If yes, then I would phrase it that way:
>
> ... while the file is still valid, and the C side ensures that the
> credentials stay valid while the file is valid.
Yes, that's my intention with this code.
But I guess this is a good question for Christian Brauner to confirm:
If I read the credential from the `f_cred` field, is it guaranteed that
the pointer remains valid for at least as long as the file?
Or should I do some dance along the lines of "lock file, increment
refcount on credential, unlock file"?
Alice
On Fri, Dec 01, 2023 at 09:06:35AM +0000, Alice Ryhl wrote:
> Benno Lossin <benno.lossin@proton.me> writes:
> > On 11/29/23 13:51, Alice Ryhl wrote:
> >> + /// Returns the credentials of the task that originally opened the file.
> >> + pub fn cred(&self) -> &Credential {
> >> + // This `read_volatile` is intended to correspond to a READ_ONCE call.
> >> + //
> >> + // SAFETY: The file is valid because the shared reference guarantees a nonzero refcount.
> >> + //
> >> + // TODO: Replace with `read_once` when available on the Rust side.
> >> + let ptr = unsafe { core::ptr::addr_of!((*self.0.get()).f_cred).read_volatile() };
> >> +
> >> + // SAFETY: The signature of this function ensures that the caller will only access the
> >> + // returned credential while the file is still valid, and the credential must stay valid
> >> + // while the file is valid.
> >
> > About the last part of this safety comment, is this a guarantee from the
> > C side? If yes, then I would phrase it that way:
> >
> > ... while the file is still valid, and the C side ensures that the
> > credentials stay valid while the file is valid.
>
> Yes, that's my intention with this code.
>
> But I guess this is a good question for Christian Brauner to confirm:
>
> If I read the credential from the `f_cred` field, is it guaranteed that
> the pointer remains valid for at least as long as the file?
>
> Or should I do some dance along the lines of "lock file, increment
> refcount on credential, unlock file"?
The lifetime of the f_cred reference is at least as long as the lifetime
of the file:
// file not yet visible anywhere
some_file = alloc_file*()
-> init_file()
{
file->f_cred = get_cred(cred /* usually current_cred() */)
}
// install into fd_table -> irreversible, thing visible, possibly shared
fd_install(1234, some_file)
// last fput
fput()
// atomic_dec_and_test() dance:
-> file_free() // either "delayed" through task work, workqueue, or
// sometimes freed right away if file hasn't been opened,
// i.e., if fd_install() wasn't called
-> put_cred(file->f_cred)
In order to access anything you must hold a reference to the file or
files->file_lock. IOW, no poking around in f->f_cred or any field for
that matter just under rcu_read_lock() for example. Because files are
SLAB_TYPESAFE_BY_RCU. You might be poking in someone else's creds then.
Christian Brauner <brauner@kernel.org> writes:
> On Fri, Dec 01, 2023 at 09:06:35AM +0000, Alice Ryhl wrote:
> > Benno Lossin <benno.lossin@proton.me> writes:
> > > On 11/29/23 13:51, Alice Ryhl wrote:
> > >> + /// Returns the credentials of the task that originally opened the file.
> > >> + pub fn cred(&self) -> &Credential {
> > >> + // This `read_volatile` is intended to correspond to a READ_ONCE call.
> > >> + //
> > >> + // SAFETY: The file is valid because the shared reference guarantees a nonzero refcount.
> > >> + //
> > >> + // TODO: Replace with `read_once` when available on the Rust side.
> > >> + let ptr = unsafe { core::ptr::addr_of!((*self.0.get()).f_cred).read_volatile() };
> > >> +
> > >> + // SAFETY: The signature of this function ensures that the caller will only access the
> > >> + // returned credential while the file is still valid, and the credential must stay valid
> > >> + // while the file is valid.
> > >
> > > About the last part of this safety comment, is this a guarantee from the
> > > C side? If yes, then I would phrase it that way:
> > >
> > > ... while the file is still valid, and the C side ensures that the
> > > credentials stay valid while the file is valid.
> >
> > Yes, that's my intention with this code.
> >
> > But I guess this is a good question for Christian Brauner to confirm:
> >
> > If I read the credential from the `f_cred` field, is it guaranteed that
> > the pointer remains valid for at least as long as the file?
> >
> > Or should I do some dance along the lines of "lock file, increment
> > refcount on credential, unlock file"?
>
> The lifetime of the f_cred reference is at least as long as the lifetime
> of the file:
>
> // file not yet visible anywhere
> some_file = alloc_file*()
> -> init_file()
> {
> file->f_cred = get_cred(cred /* usually current_cred() */)
> }
>
>
> // install into fd_table -> irreversible, thing visible, possibly shared
> fd_install(1234, some_file)
>
> // last fput
> fput()
> // atomic_dec_and_test() dance:
> -> file_free() // either "delayed" through task work, workqueue, or
> // sometimes freed right away if file hasn't been opened,
> // i.e., if fd_install() wasn't called
> -> put_cred(file->f_cred)
>
> In order to access anything you must hold a reference to the file or
> files->file_lock. IOW, no poking around in f->f_cred or any field for
> that matter just under rcu_read_lock() for example. Because files are
> SLAB_TYPESAFE_BY_RCU. You might be poking in someone else's creds then.
Okay, we aren't dealing with the rcu case in this patchset, so we know
that it wont be freed while we're accessing it.
I guess this means that the `f_cred` field is immutable, which means
that I don't need READ_ONCE to read it? I'll use an ordinary load in the
next version.
Alice
@@ -7,6 +7,7 @@
*/
#include <kunit/test.h>
+#include <linux/cred.h>
#include <linux/errname.h>
#include <linux/file.h>
#include <linux/fs.h>
@@ -23,6 +23,7 @@
#include <kunit/test-bug.h>
#include <linux/bug.h>
#include <linux/build_bug.h>
+#include <linux/cred.h>
#include <linux/err.h>
#include <linux/errname.h>
#include <linux/fs.h>
@@ -164,6 +165,18 @@ struct file *rust_helper_get_file(struct file *f)
}
EXPORT_SYMBOL_GPL(rust_helper_get_file);
+const struct cred *rust_helper_get_cred(const struct cred *cred)
+{
+ return get_cred(cred);
+}
+EXPORT_SYMBOL_GPL(rust_helper_get_cred);
+
+void rust_helper_put_cred(const struct cred *cred)
+{
+ put_cred(cred);
+}
+EXPORT_SYMBOL_GPL(rust_helper_put_cred);
+
/*
* `bindgen` binds the C `size_t` type as the Rust `usize` type, so we can
* use it in contexts where Rust expects a `usize` like slice (array) indices.
new file mode 100644
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Credentials management.
+//!
+//! C header: [`include/linux/cred.h`](../../../../include/linux/cred.h)
+//!
+//! Reference: <https://www.kernel.org/doc/html/latest/security/credentials.html>
+
+use crate::{
+ bindings,
+ types::{AlwaysRefCounted, Opaque},
+};
+
+/// Wraps the kernel's `struct cred`.
+///
+/// # Invariants
+///
+/// Instances of this type are always ref-counted, that is, a call to `get_cred` ensures that the
+/// allocation remains valid at least until the matching call to `put_cred`.
+#[repr(transparent)]
+pub struct Credential(pub(crate) Opaque<bindings::cred>);
+
+// SAFETY: By design, the only way to access a `Credential` is via an immutable reference or an
+// `ARef`. This means that the only situation in which a `Credential` can be accessed mutably is
+// when the refcount drops to zero and the destructor runs. It is safe for that to happen on any
+// thread, so it is ok for this type to be `Send`.
+unsafe impl Send for Credential {}
+
+// SAFETY: It's OK to access `Credential` through shared references from other threads because
+// we're either accessing properties that don't change or that are properly synchronised by C code.
+unsafe impl Sync for Credential {}
+
+impl Credential {
+ /// Creates a reference to a [`Credential`] from a valid pointer.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
+ /// returned [`Credential`] reference.
+ pub unsafe fn from_ptr<'a>(ptr: *const bindings::cred) -> &'a Credential {
+ // SAFETY: The safety requirements guarantee the validity of the dereference, while the
+ // `Credential` type being transparent makes the cast ok.
+ unsafe { &*ptr.cast() }
+ }
+
+ /// Returns the effective UID of the given credential.
+ pub fn euid(&self) -> bindings::kuid_t {
+ // SAFETY: By the type invariant, we know that `self.0` is valid.
+ unsafe { (*self.0.get()).euid }
+ }
+}
+
+// SAFETY: The type invariants guarantee that `Credential` is always ref-counted.
+unsafe impl AlwaysRefCounted for Credential {
+ fn inc_ref(&self) {
+ // SAFETY: The existence of a shared reference means that the refcount is nonzero.
+ unsafe { bindings::get_cred(self.0.get()) };
+ }
+
+ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
+ // SAFETY: The safety requirements guarantee that the refcount is nonzero.
+ unsafe { bindings::put_cred(obj.cast().as_ptr()) };
+ }
+}
@@ -7,6 +7,7 @@
use crate::{
bindings,
+ cred::Credential,
error::{code::*, Error, Result},
types::{ARef, AlwaysRefCounted, Opaque},
};
@@ -138,6 +139,21 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::file) -> &'a File {
unsafe { &*ptr.cast() }
}
+ /// Returns the credentials of the task that originally opened the file.
+ pub fn cred(&self) -> &Credential {
+ // This `read_volatile` is intended to correspond to a READ_ONCE call.
+ //
+ // SAFETY: The file is valid because the shared reference guarantees a nonzero refcount.
+ //
+ // TODO: Replace with `read_once` when available on the Rust side.
+ let ptr = unsafe { core::ptr::addr_of!((*self.0.get()).f_cred).read_volatile() };
+
+ // SAFETY: The signature of this function ensures that the caller will only access the
+ // returned credential while the file is still valid, and the credential must stay valid
+ // while the file is valid.
+ unsafe { Credential::from_ptr(ptr) }
+ }
+
/// Returns the flags associated with the file.
///
/// The flags are a combination of the constants in [`flags`].
@@ -33,6 +33,7 @@
#[cfg(not(testlib))]
mod allocator;
mod build_assert;
+pub mod cred;
pub mod error;
pub mod file;
pub mod init;