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

Alice Ryhl
2024-02-02T15:56:27Z

On Fri, Feb 2, 2024 at 4:36 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Feb 02, 2024 at 10:55:41AM +0000, Alice Ryhl wrote:
> > + /// Returns the given task's pid in the current pid namespace.
> > + pub fn pid_in_current_ns(&self) -> Pid {
> > + let current = Task::current_raw();
> > + // SAFETY: Calling `task_active_pid_ns` with the current task is always safe.
> > + let namespace = unsafe { bindings::task_active_pid_ns(current) };
> > + // SAFETY: We know that `self.0.get()` is valid by the type invariant, and the namespace
> > + // pointer is not dangling since it points at this task's namespace.
> > + unsafe { bindings::task_tgid_nr_ns(self.0.get(), namespace) }
> > + }
>
> pids are reference counted in the kernel, how does this deal with that?
> Are they just ignored somehow? Where is the reference count given back?

The intention is that it will be used to replicate the following line
of code from C binder:

trd->sender_pid = task_tgid_nr_ns(sender, task_active_pid_ns(current));

The context of this is an ioctl where the `trd` struct contains what
will be copied into userspace as the output of the ioctl. So, the pid
here is just a number that is given to userspace immediately, and
userspace can then do with it as it likes. It is true that the pid is
stale immediately, as the remote process could die and the pid could
get reused. But it is up to userspace to handle that properly. Binder
has a different mechanism than pids that userspace can use for a
trusted way of verifying credentials (see the 5th patch).

If this implementation of pid_in_current_ns actually takes a refcount
on the pid, then it is incorrect because it will leak the refcount in
that case.

Alice