linux-kernel - Re: [PATCH v9 8/8] task: rust: rework how current is accessed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAG48ez1an_cbUinfCvukQe=X-veygFLe-twQq5zMKTVcMpPv7g@mail.gmail.com>
Date: Wed, 27 Nov 2024 17:18:21 +0100
From: Jann Horn <jannh@...gle.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Matthew Wilcox <willy@...radead.org>, 
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Vlastimil Babka <vbabka@...e.cz>, 
	John Hubbard <jhubbard@...dia.com>, "Liam R. Howlett" <Liam.Howlett@...cle.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Arnd Bergmann <arnd@...db.de>, Christian Brauner <brauner@...nel.org>, 
	Suren Baghdasaryan <surenb@...gle.com>, Alex Gaynor <alex.gaynor@...il.com>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, linux-kernel@...r.kernel.org, linux-mm@...ck.org, 
	rust-for-linux@...r.kernel.org, Andreas Hindborg <a.hindborg@...nel.org>
Subject: Re: [PATCH v9 8/8] task: rust: rework how current is accessed

On Wed, Nov 27, 2024 at 4:57 PM Alice Ryhl <aliceryhl@...gle.com> wrote:
> On Wed, Nov 27, 2024 at 4:52 PM Jann Horn <jannh@...gle.com> wrote:
> > On Wed, Nov 27, 2024 at 1:36 PM Alice Ryhl <aliceryhl@...gle.com> wrote:
> > > On Tue, Nov 26, 2024 at 6:15 PM Jann Horn <jannh@...gle.com> wrote:
> > > >
> > > > On Fri, Nov 22, 2024 at 4:41 PM Alice Ryhl <aliceryhl@...gle.com> 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.
> > >
> > > Huh ... is it possible to use kthread_use_mm() to create a situation
> > > where current->mm has mm_users equal to zero? If not, then I don't
> > > think it's a problem.
> >
> > Ah, no, I don't think so. I think the only problematic scenario would
> > be if rust code created a borrow of current->mm, then called
> > kthread_unuse_mm() and dropped the reference that was held on the MM,
> > and then accessed the borrowed old mm_struct. Which isn't possible
> > unless a Rust binding is created for
> > kthread_use_mm()/kthread_unuse_mm().
>
> Ah, ok.
>
> The way that the current abstraction works is that it enforces that
> the current pointer cannot escape the scope you were in when you
> obtained it. If we enforce that kthread_use_mm() and
> kthread_unuse_mm() involve a scope, then that should solve that.

Oooh, that's neat, thanks for the explanation.