Re: [PATCH hotfix 6.12 v2 4/8] mm: resolve faulty mmap_region() error path behaviour

["Liam R. Howlett" <Liam.Howlett@oracle.com>]

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [241028 15:14]:
> On Mon, Oct 28, 2024 at 09:05:44AM -1000, Linus Torvalds wrote:
> > On Mon, 28 Oct 2024 at 08:57, Lorenzo Stoakes
> > <lorenzo.stoakes@oracle.com> wrote:
> > >
> > > So likely hook on your mapping changes flags to set VM_MTE | VM_MTE_ALLOWED and
> > > expects this to be checked after (ugh).
> >
> > Gaah. Yes. mm/shmem.c: shmem_mmap() does
> >
> >         /* arm64 - allow memory tagging on RAM-based files */
> >         vm_flags_set(vma, VM_MTE_ALLOWED);
> >
> > and while I found the equivalent hack for the VM_SPARC_ADI case, I
> > hadn't noticed that MTE thing.
> >
> > How very annoying.
> >
> > So the arch_validate_flags() case does need to be done after the ->mmap() call.
> >
> > How about just finalizing everything, and then doing a regular
> > munmap() afterwards and returning an error (all still holding the mmap
> > semaphore, of course).
> >
> > That still avoids the whole "partially completed mmap" case.
> >
> >              Linus
> 
> Yeah I was thinking the same... just bite the bullet, go through the whole damn
> process and revert if arch_validate_flags() chokes. It also removes the ugly
> #ifdef CONFIG_SPARC64 hack...
> 
> This will litearlly only be applicable for these two cases and (hopefully) most
> of the time you'd not fail it.
> 
> I mean by then it'll be added into the rmap and such but nothing will be
> populated yet and we shouldn't be able to fault as vma_start_write() should have
> incremented the vma lock seqnum.
> 
> Any issues from the RCU visibility stuff Liam?

It is probably fine?  We would see a mapping appear then disappear.
We'd have a (benign) race with rmap for truncating the PTEs (but it's
safe).  Page faults would be stopped though.

Unfortunately, we'd have to write to the vma tree so that we could...
write to the vma tree.  We'd have to somehow ensure munmap() is done
with a gfp flag to ensure no failures as well...

Maybe we should just call close on the vma again (and do whatever
call_mmap() needs to undo)?

> 
> Any security problems Jann...?
> 
> It'd suck to have to bring back a partial complete case. Though I do note we
> handle errors from mmap_file() ok so we could still potentially handle that
> there, but would sort of semi-undo some of the point of the series.