Re: [PATCH 5/7] rust: file: add `Kuid` wrapper

From: Benno Lossin
Date: Thu Nov 30 2023 - 11:48:59 EST


On 11/29/23 14:12, Alice Ryhl wrote:
> + /// 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)
> + }

I would split this into two `unsafe` blocks.

> + }
> +
> /// 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() },
> + }

Would expect a call to `from_raw` here instead of `Self {}`.

> + }
> +
> + /// Create a `Kuid` given the raw C type.
> + pub fn from_raw(kuid: bindings::kuid_t) -> Self {
> + Self { kuid }
> + }

Is there a reason that this is named `from_raw` and not just a normal
`From` impl? AFAICT any `bindings::kuid_t` is a valid `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.

Why not:

/// Converts this kernel UID into a userspace UID.
///
/// Uses the namespace of the current task.

--
Cheers,
Benno

> + pub fn into_uid_in_current_ns(self) -> bindings::uid_t {
> + // SAFETY: Just an FFI call.
> + unsafe { bindings::from_kuid(bindings::current_user_ns(), self.kuid) }
> + }
> +}