linux-kernel - Re: [PATCH 1/2] mm: protect free_pgtables with mmap_lock write lock in exit

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]

Date:   Tue, 23 Nov 2021 09:56:41 -0800
From:   Suren Baghdasaryan <surenb@...gle.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     akpm@...ux-foundation.org, mhocko@...nel.org, mhocko@...e.com,
        rientjes@...gle.com, hannes@...xchg.org, guro@...com,
        riel@...riel.com, minchan@...nel.org, kirill@...temov.name,
        aarcange@...hat.com, christian@...uner.io, hch@...radead.org,
        oleg@...hat.com, david@...hat.com, jannh@...gle.com,
        shakeelb@...gle.com, luto@...nel.org, christian.brauner@...ntu.com,
        fweimer@...hat.com, jengelh@...i.de, timmurray@...gle.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        kernel-team@...roid.com
Subject: Re: [PATCH 1/2] mm: protect free_pgtables with mmap_lock write lock
 in exit_mmap

On Tue, Nov 23, 2021 at 5:19 AM Matthew Wilcox <willy@...radead.org> wrote:
>
> On Tue, Nov 16, 2021 at 01:57:14PM -0800, Suren Baghdasaryan wrote:
> > @@ -3170,6 +3172,7 @@ void exit_mmap(struct mm_struct *mm)
> >       unmap_vmas(&tlb, vma, 0, -1);
> >       free_pgtables(&tlb, vma, FIRST_USER_ADDRESS, USER_PGTABLES_CEILING);
> >       tlb_finish_mmu(&tlb);
> > +     mmap_write_unlock(mm);
> >
> >       /*
> >        * Walk the list again, actually closing and freeing it,
>
> Is there a reason to unlock here instead of after the remove_vma loop?
> We'll need the mmap sem held during that loop when VMAs are stored in
> the maple tree.

I didn't realize remove_vma() would need to be protected as well. I
think I can move mmap_write_unlock down to cover the last walk too
with no impact.
Does anyone know if there was any specific reason to perform that last
walk with no locks held (as the comment states)? I can track that
comment back to Linux-2.6.12-rc2 merge with no earlier history, so not
sure if it's critical not to hold any locks at this point. Seems to me
it's ok to hold mmap_write_unlock but maybe I'm missing something?