From: Christian Brauner <brauner@kernel.org>
To: Alice Ryhl <aliceryhl@google.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Wedson Almeida Filho" <wedsonaf@gmail.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <benno.lossin@proton.me>,
"Andreas Hindborg" <a.hindborg@samsung.com>,
"Peter Zijlstra" <peterz@infradead.org>,
"Alexander Viro" <viro@zeniv.linux.org.uk>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Arve Hjønnevåg" <arve@android.com>,
"Todd Kjos" <tkjos@android.com>,
"Martijn Coenen" <maco@android.com>,
"Joel Fernandes" <joel@joelfernandes.org>,
"Carlos Llamas" <cmllamas@google.com>,
"Suren Baghdasaryan" <surenb@google.com>,
"Dan Williams" <dan.j.williams@intel.com>,
"Kees Cook" <keescook@chromium.org>,
"Matthew Wilcox" <willy@infradead.org>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Daniel Xu" <dxu@dxuuu.xyz>,
linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 5/7] rust: file: add `Kuid` wrapper
Date: Wed, 29 Nov 2023 17:28:27 +0100 [thread overview]
Message-ID: <20231129-etappen-knapp-08e2e3af539f@brauner> (raw)
In-Reply-To: <20231129-alice-file-v1-5-f81afe8c7261@google.com>
On Wed, Nov 29, 2023 at 01:12:17PM +0000, Alice Ryhl wrote:
> Adds a wrapper around `kuid_t` called `Kuid`. This allows us to define
> various operations on kuids such as equality and current_euid. It also
> lets us provide conversions from kuid into userspace values.
>
> Rust Binder needs these operations because it needs to compare kuids for
> equality, and it needs to tell userspace about the pid and uid of
> incoming transactions.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/bindings/bindings_helper.h | 1 +
> rust/helpers.c | 45 ++++++++++++++++++++++++++
> rust/kernel/cred.rs | 5 +--
> rust/kernel/task.rs | 71 ++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 119 insertions(+), 3 deletions(-)
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 81b13a953eae..700f01840188 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -11,6 +11,7 @@
> #include <linux/errname.h>
> #include <linux/file.h>
> #include <linux/fs.h>
> +#include <linux/pid_namespace.h>
> #include <linux/security.h>
> #include <linux/slab.h>
> #include <linux/refcount.h>
> diff --git a/rust/helpers.c b/rust/helpers.c
> index fd633d9db79a..58e3a9dff349 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -142,6 +142,51 @@ void rust_helper_put_task_struct(struct task_struct *t)
> }
> EXPORT_SYMBOL_GPL(rust_helper_put_task_struct);
>
> +kuid_t rust_helper_task_uid(struct task_struct *task)
> +{
> + return task_uid(task);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_task_uid);
> +
> +kuid_t rust_helper_task_euid(struct task_struct *task)
> +{
> + return task_euid(task);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_task_euid);
> +
> +#ifndef CONFIG_USER_NS
> +uid_t rust_helper_from_kuid(struct user_namespace *to, kuid_t uid)
> +{
> + return from_kuid(to, uid);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_from_kuid);
> +#endif /* CONFIG_USER_NS */
> +
> +bool rust_helper_uid_eq(kuid_t left, kuid_t right)
> +{
> + return uid_eq(left, right);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_uid_eq);
> +
> +kuid_t rust_helper_current_euid(void)
> +{
> + return current_euid();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_current_euid);
> +
> +struct user_namespace *rust_helper_current_user_ns(void)
> +{
> + return current_user_ns();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_current_user_ns);
> +
> +pid_t rust_helper_task_tgid_nr_ns(struct task_struct *tsk,
> + struct pid_namespace *ns)
> +{
> + return task_tgid_nr_ns(tsk, ns);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_task_tgid_nr_ns);
I'm a bit puzzled by all these rust_helper_*() calls. Can you explain
why they are needed? Because they are/can be static inlines and that
somehow doesn't work?
> +
> struct kunit *rust_helper_kunit_get_current_test(void)
> {
> return kunit_get_current_test();
> diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs
> index 3794937b5294..fbc749788bfa 100644
> --- a/rust/kernel/cred.rs
> +++ b/rust/kernel/cred.rs
> @@ -8,6 +8,7 @@
>
> use crate::{
> bindings,
> + task::Kuid,
> types::{AlwaysRefCounted, Opaque},
> };
>
> @@ -52,9 +53,9 @@ pub fn get_secid(&self) -> u32 {
> }
>
> /// Returns the effective UID of the given credential.
> - pub fn euid(&self) -> bindings::kuid_t {
> + pub fn euid(&self) -> Kuid {
> // SAFETY: By the type invariant, we know that `self.0` is valid.
> - unsafe { (*self.0.get()).euid }
> + Kuid::from_raw(unsafe { (*self.0.get()).euid })
> }
> }
>
> diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
> index b2299bc7ac1f..1a27b968a907 100644
> --- a/rust/kernel/task.rs
> +++ b/rust/kernel/task.rs
> @@ -5,7 +5,12 @@
> //! C header: [`include/linux/sched.h`](../../../../include/linux/sched.h).
>
> use crate::{bindings, types::Opaque};
> -use core::{marker::PhantomData, ops::Deref, ptr};
> +use core::{
> + cmp::{Eq, PartialEq},
> + marker::PhantomData,
> + ops::Deref,
> + ptr,
> +};
>
> /// Returns the currently running task.
> #[macro_export]
> @@ -78,6 +83,12 @@ unsafe impl Sync for Task {}
> /// The type of process identifiers (PIDs).
> type Pid = bindings::pid_t;
>
> +/// The type of user identifiers (UIDs).
> +#[derive(Copy, Clone)]
> +pub struct Kuid {
> + kuid: bindings::kuid_t,
> +}
> +
> impl Task {
> /// Returns a task reference for the currently executing task/thread.
> ///
> @@ -132,12 +143,34 @@ pub fn pid(&self) -> Pid {
> unsafe { *ptr::addr_of!((*self.0.get()).pid) }
> }
>
> + /// Returns the UID of the given task.
> + pub fn uid(&self) -> Kuid {
> + // SAFETY: By the type invariant, we know that `self.0` is valid.
> + Kuid::from_raw(unsafe { bindings::task_uid(self.0.get()) })
> + }
> +
> + /// Returns the effective UID of the given task.
> + pub fn euid(&self) -> Kuid {
> + // SAFETY: By the type invariant, we know that `self.0` is valid.
> + Kuid::from_raw(unsafe { bindings::task_euid(self.0.get()) })
> + }
> +
> /// Determines whether the given task has pending signals.
> pub fn signal_pending(&self) -> bool {
> // SAFETY: By the type invariant, we know that `self.0` is valid.
> unsafe { bindings::signal_pending(self.0.get()) != 0 }
> }
>
> + /// Returns the given task's pid in the current pid namespace.
> + pub fn pid_in_current_ns(&self) -> Pid {
> + // SAFETY: We know that `self.0.get()` is valid by the type invariant. The rest is just FFI
> + // calls.
> + unsafe {
> + let namespace = bindings::task_active_pid_ns(bindings::get_current());
> + bindings::task_tgid_nr_ns(self.0.get(), namespace)
> + }
> + }
> +
> /// Wakes up the task.
> pub fn wake_up(&self) {
> // SAFETY: By the type invariant, we know that `self.0.get()` is non-null and valid.
> @@ -147,6 +180,42 @@ pub fn wake_up(&self) {
> }
> }
>
> +impl Kuid {
> + /// Get the current euid.
> + pub fn current_euid() -> Kuid {
> + // SAFETY: Just an FFI call.
> + Self {
> + kuid: unsafe { bindings::current_euid() },
> + }
> + }
> +
> + /// Create a `Kuid` given the raw C type.
> + pub fn from_raw(kuid: bindings::kuid_t) -> Self {
> + Self { kuid }
> + }
> +
> + /// Turn this kuid into the raw C type.
> + pub fn into_raw(self) -> bindings::kuid_t {
> + self.kuid
> + }
> +
> + /// Converts this kernel UID into a UID that userspace understands. Uses the namespace of the
> + /// current task.
> + pub fn into_uid_in_current_ns(self) -> bindings::uid_t {
Hm, I wouldn't special-case this. Just expose from_kuid() and let it
take a namespace argument, no? You don't need to provide bindings for
namespaces ofc.
> + // SAFETY: Just an FFI call.
> + unsafe { bindings::from_kuid(bindings::current_user_ns(), self.kuid) }
> + }
> +}
> +
> +impl PartialEq for Kuid {
> + fn eq(&self, other: &Kuid) -> bool {
> + // SAFETY: Just an FFI call.
> + unsafe { bindings::uid_eq(self.kuid, other.kuid) }
> + }
> +}
> +
> +impl Eq for Kuid {}
Do you need that?
> +
> // SAFETY: The type invariants guarantee that `Task` is always ref-counted.
> unsafe impl crate::types::AlwaysRefCounted for Task {
> fn inc_ref(&self) {
>
> --
> 2.43.0.rc1.413.gea7ed67945-goog
>
next prev parent reply other threads:[~2023-11-29 16:28 UTC|newest]
Thread overview: 96+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-29 12:51 [PATCH 0/7] File abstractions needed by Rust Binder Alice Ryhl
2023-11-29 12:51 ` [PATCH 1/7] rust: file: add Rust abstraction for `struct file` Alice Ryhl
2023-11-29 15:13 ` Matthew Wilcox
2023-11-29 15:23 ` Peter Zijlstra
2023-11-29 17:08 ` Boqun Feng
2023-11-30 10:42 ` Peter Zijlstra
2023-11-30 15:25 ` Boqun Feng
2023-12-01 8:53 ` Peter Zijlstra
2023-12-01 9:19 ` Boqun Feng
2023-12-01 9:40 ` Peter Zijlstra
2023-12-01 10:36 ` Boqun Feng
2023-12-01 11:05 ` Peter Zijlstra
2023-12-01 9:00 ` Peter Zijlstra
2023-12-01 9:52 ` Boqun Feng
2023-11-29 16:42 ` Alice Ryhl
2023-11-29 16:45 ` Peter Zijlstra
2023-11-30 15:02 ` Benno Lossin
2023-11-29 17:06 ` Christian Brauner
2023-11-29 21:27 ` Alice Ryhl
2023-11-29 23:17 ` Benno Lossin
2023-11-30 10:48 ` Christian Brauner
2023-11-30 12:10 ` Alice Ryhl
2023-11-30 12:36 ` Christian Brauner
2023-11-30 14:53 ` Benno Lossin
2023-11-30 14:59 ` Greg Kroah-Hartman
2023-11-30 15:46 ` Benno Lossin
2023-11-30 15:56 ` Greg Kroah-Hartman
2023-11-30 15:58 ` Theodore Ts'o
2023-11-30 16:12 ` Benno Lossin
2023-12-01 1:16 ` Theodore Ts'o
2023-12-01 12:11 ` David Laight
2023-12-01 12:27 ` Alice Ryhl
2023-12-01 15:04 ` Theodore Ts'o
2023-12-01 15:14 ` Benno Lossin
2023-12-01 17:25 ` David Laight
2023-12-01 17:37 ` Benno Lossin
2023-11-29 12:51 ` [PATCH 2/7] rust: cred: add Rust abstraction for `struct cred` Alice Ryhl
2023-11-30 16:17 ` Benno Lossin
2023-12-01 9:06 ` Alice Ryhl
2023-12-01 10:27 ` Christian Brauner
2023-12-04 15:42 ` Alice Ryhl
2023-11-29 13:11 ` [PATCH 3/7] rust: security: add abstraction for secctx Alice Ryhl
2023-11-30 16:26 ` Benno Lossin
2023-12-01 10:48 ` Alice Ryhl
2023-12-02 10:03 ` Benno Lossin
2023-11-29 13:11 ` [PATCH 4/7] rust: file: add `FileDescriptorReservation` Alice Ryhl
2023-11-29 16:14 ` Christian Brauner
2023-11-29 16:55 ` Alice Ryhl
2023-11-29 17:14 ` Alice Ryhl
2023-11-30 9:12 ` Christian Brauner
2023-11-30 9:23 ` Alice Ryhl
2023-11-30 9:09 ` Christian Brauner
2023-11-30 9:17 ` Alice Ryhl
2023-11-30 10:51 ` Christian Brauner
2023-11-30 11:54 ` Alice Ryhl
2023-11-30 12:17 ` Benno Lossin
2023-11-30 12:33 ` Christian Brauner
2023-11-30 16:40 ` Benno Lossin
2023-12-01 11:32 ` Alice Ryhl
2023-11-29 13:12 ` [PATCH 5/7] rust: file: add `Kuid` wrapper Alice Ryhl
2023-11-29 16:28 ` Christian Brauner [this message]
2023-11-29 16:48 ` Peter Zijlstra
2023-11-30 12:46 ` Christian Brauner
2023-12-06 19:59 ` Kent Overstreet
2023-12-08 16:26 ` Peter Zijlstra
2023-12-08 19:58 ` Kent Overstreet
2023-12-08 5:28 ` comex
2023-12-08 16:19 ` Miguel Ojeda
2023-12-08 17:08 ` Nick Desaulniers
2023-12-08 17:37 ` Miguel Ojeda
2023-12-08 17:43 ` Boqun Feng
2023-12-08 20:43 ` Matthew Wilcox
2023-12-09 7:24 ` comex
2023-11-30 9:36 ` Alice Ryhl
2023-11-30 10:52 ` Christian Brauner
2023-11-30 10:36 ` Peter Zijlstra
2023-12-06 20:02 ` Kent Overstreet
2023-12-07 7:18 ` Greg Kroah-Hartman
2023-12-07 7:46 ` Kent Overstreet
2023-11-30 16:48 ` Benno Lossin
2023-11-29 13:12 ` [PATCH 6/7] rust: file: add `DeferredFdCloser` Alice Ryhl
2023-11-30 17:12 ` Benno Lossin
2023-12-01 11:35 ` Alice Ryhl
2023-12-02 10:16 ` Benno Lossin
2023-12-05 14:43 ` Alice Ryhl
2023-12-05 18:16 ` Alice Ryhl
2023-11-29 13:12 ` [PATCH 7/7] rust: file: add abstraction for `poll_table` Alice Ryhl
2023-11-30 17:42 ` Benno Lossin
2023-12-01 11:47 ` Alice Ryhl
2023-11-30 22:39 ` Boqun Feng
2023-12-01 11:50 ` Alice Ryhl
2023-11-30 22:50 ` Boqun Feng
2023-11-29 16:31 ` [PATCH 0/7] File abstractions needed by Rust Binder Christian Brauner
2023-11-29 16:48 ` Miguel Ojeda
2023-12-06 20:05 ` Kent Overstreet
2023-12-08 16:59 ` Miguel Ojeda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231129-etappen-knapp-08e2e3af539f@brauner \
--to=brauner@kernel.org \
--cc=a.hindborg@samsung.com \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=arve@android.com \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=cmllamas@google.com \
--cc=dan.j.williams@intel.com \
--cc=dxu@dxuuu.xyz \
--cc=gary@garyguo.net \
--cc=gregkh@linuxfoundation.org \
--cc=joel@joelfernandes.org \
--cc=keescook@chromium.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maco@android.com \
--cc=ojeda@kernel.org \
--cc=peterz@infradead.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=surenb@google.com \
--cc=tglx@linutronix.de \
--cc=tkjos@android.com \
--cc=viro@zeniv.linux.org.uk \
--cc=wedsonaf@gmail.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).