linux-kernel - Re: [RFC PATCH] mm/mmap: Fix uprobe anon page be overwritten when expanding vma during mremap

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: <b84f00c8-966c-46f2-8afe-d09465153217@lucifer.local>
Date: Tue, 27 May 2025 12:42:38 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: David Hildenbrand <david@...hat.com>
Cc: Oleg Nesterov <oleg@...hat.com>, Pu Lehui <pulehui@...weicloud.com>,
        mhiramat@...nel.org, peterz@...radead.org, Liam.Howlett@...cle.com,
        akpm@...ux-foundation.org, vbabka@...e.cz, jannh@...gle.com,
        pfalcato@...e.de, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        pulehui@...wei.com
Subject: Re: [RFC PATCH] mm/mmap: Fix uprobe anon page be overwritten when
 expanding vma during mremap

On Mon, May 26, 2025 at 08:46:07PM +0200, David Hildenbrand wrote:
> On 26.05.25 17:48, Oleg Nesterov wrote:
> > Hi Lehui,
> >
> > As I said, I don't understand mm/, so can't comment, but...
> >
> > On 05/26, Pu Lehui wrote:
> > >
> > > To make things simpler, perhaps we could try post-processing, that is:
> > >
> > > diff --git a/mm/mremap.c b/mm/mremap.c
> > > index 83e359754961..46a757fd26dc 100644
> > > --- a/mm/mremap.c
> > > +++ b/mm/mremap.c
> > > @@ -240,6 +240,11 @@ static int move_ptes(struct pagetable_move_control
> > > *pmc,
> > >                  if (pte_none(ptep_get(old_pte)))
> > >                          continue;
> > >
> > > +               /* skip move pte when expanded range has uprobe */
> > > +               if (unlikely(pte_present(*new_pte) &&
> > > +                            vma_has_uprobes(pmc->new, new_addr, new_addr +
> > > PAGE_SIZE)))

This feels like a horrible hack, note that we also move page tables at higher
page table levels _anyway_ so this would be broken by that (unless uprobes split
huge mappings).

If it's uprobe code that's putting the new PTE in place, then this is
just... yeah. I'm with David's suggestion of just disallowing this scenario, I
really dislike the idea that we're ok with an invalid condition being ok, only
to cover off this one specific case.


> > > +                       continue;
> > > +
> >
> > I was thinking about
> >
> > 	WARN_ON(!pte_none(*new_pte))
> >
> > at the start of the main loop.
> >
> > Obviously not to fix the problem, but rather to make it more explicit.
>
> Yeah, WARN_ON_ONCE().
>
> We really should fix the code to not install uprobes into the area we are
> moving.
>
> Likely, the correct fix will be to pass the range as well to uprobe_mmap(),
> and passing that range to build_probe_list().
>
> Only when growing using mremap(), we want to call it on the extended range
> only.

We might be able to implement a simpler version of the proposed patch though
which might avoid us needing to do something like this.

Having a look...

>
> --
> Cheers,
>
> David / dhildenb
>