Re: [PATCH v9 8/8] task: rust: rework how current is accessed
From: Jann Horn
Date: Tue Nov 26 2024 - 12:15:37 EST
On Fri, Nov 22, 2024 at 4:41 PM Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
> +impl CurrentTask {
> + /// Access the address space of this task.
> + ///
> + /// To increment the refcount of the referenced `mm`, you can use `ARef::from`.
> + #[inline]
> + pub fn mm(&self) -> Option<&MmWithUser> {
> + let mm = unsafe { (*self.as_ptr()).mm };
> +
> + if mm.is_null() {
> + None
> + } else {
> + // SAFETY: If `current->mm` is non-null, then it references a valid mm with a non-zero
> + // value of `mm_users`. The returned `&MmWithUser` borrows from `CurrentTask`, so the
> + // `&MmWithUser` cannot escape the current task, meaning `mm_users` can't reach zero
> + // while the reference is still live.
> + Some(unsafe { MmWithUser::from_raw(mm) })
Maybe also add safety comments for these nitpicky details:
kthreads can use kthread_use_mm()/kthread_unuse_mm() to change
current->mm (which allows kthreads to access arbitrary userspace
address spaces with copy_from_user/copy_to_user), but as long as you
can't call into kthread_use_mm()/kthread_unuse_mm() from Rust code,
this should be correct. If you do want to allow calls into
kthread_use_mm()/kthread_unuse_mm() later on, you might have to gate
this on a check for PF_KTHREAD, or something like that.
Binary formats' .load_binary implementations can change current->mm by
calling begin_new_exec(), but that's not an issue as long as no binary
format loaders are implemented in Rust.
> + }
> + }
> +}
> +
> // SAFETY: The type invariants guarantee that `Task` is always refcounted.
> unsafe impl crate::types::AlwaysRefCounted for Task {
> fn inc_ref(&self) {
>
> --
> 2.47.0.371.ga323438b13-goog
>