linux-kernel - Re: [PATCH 1/1] mm: prevent a race between process_mrelease and 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]

Message-ID: <YYrQ/hENQPn6Mk3v@dhcp22.suse.cz>
Date:   Tue, 9 Nov 2021 20:50:22 +0100
From:   Michal Hocko <mhocko@...e.com>
To:     Suren Baghdasaryan <surenb@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        David Rientjes <rientjes@...gle.com>,
        Matthew Wilcox <willy@...radead.org>,
        Johannes Weiner <hannes@...xchg.org>,
        Roman Gushchin <guro@...com>, Rik van Riel <riel@...riel.com>,
        Minchan Kim <minchan@...nel.org>,
        Christian Brauner <christian@...uner.io>,
        Christoph Hellwig <hch@...radead.org>,
        Oleg Nesterov <oleg@...hat.com>,
        David Hildenbrand <david@...hat.com>,
        Jann Horn <jannh@...gle.com>,
        Shakeel Butt <shakeelb@...gle.com>,
        Andy Lutomirski <luto@...nel.org>,
        Christian Brauner <christian.brauner@...ntu.com>,
        Florian Weimer <fweimer@...hat.com>,
        Jan Engelhardt <jengelh@...i.de>,
        Linux API <linux-api@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-team <kernel-team@...roid.com>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        Andrea Arcangeli <aarcange@...hat.com>
Subject: Re: [PATCH 1/1] mm: prevent a race between process_mrelease and
 exit_mmap

On Tue 09-11-21 11:37:06, Suren Baghdasaryan wrote:
> On Tue, Nov 9, 2021 at 11:26 AM Michal Hocko <mhocko@...e.com> wrote:
> >
> > On Tue 09-11-21 11:01:02, Suren Baghdasaryan wrote:
> > [...]
> > > Discussing how the patch I want to post works for maple trees that
> > > Matthew is working on, I've got a question:
> > >
> > > IIUC, according to Michal's post here:
> > > https://lore.kernel.org/all/20170725154514.GN26723@dhcp22.suse.cz,
> > > unmap_vmas() can race with other mmap_lock read holders (including
> > > oom_reap_task_mm()) with no issues.
> > > Maple tree patchset requires rcu read lock or the mmap semaphore be
> > > held (read or write side) when walking the tree, including inside
> > > unmap_vmas(). When asked, he told me that he is not sure why it's
> > > currently "safe" to walk the vma->vm_next list in unmap_vmas() while
> > > another thread is reaping the mm.
> > > Michal (or maybe someone else), could you please clarify why
> > > unmap_vmas() can safely race with oom_reap_task_mm()? Or maybe my
> > > understanding was wrong?
> >
> > I cannot really comment on the mapple tree part. But the existing
> > synchronization between oom reaper and exit_mmap is based on
> > - oom_reaper takes mmap_sem for reading
> > - exit_mmap sets MMF_OOM_SKIP and takes the exclusive mmap_sem before
> >   unmap_vmas.
> >
> > The oom_reaper therefore can either unmap the address space if the lock
> > is taken before exit_mmap or it would it would bale out on MMF_OOM_SKIP
> > if it takes the lock afterwards. So the reaper cannot race with
> > unmap_vmas.
> 
> I see. So, it's the combination of MMF_OOM_SKIP and mmap_lock working
> as a barrier which prevent them from racing with each other...
> I wasn't sure how
> https://lore.kernel.org/all/20170724072332.31903-1-mhocko@kernel.org/
> was implementing this synchronization because it would take mmap_sem
> write side after unmap_vmas() and IIUC there was no
> "mmap_lock_write(); mmap_unlock_write();" sequence in exit_mmap at
> that time. I'll need to checkout the old sources to figure this out.

My memory is rather dimm but AFAIR the main problem was freeing page
tables and freeing vmas not unmap_vmas. That one was no modifying the
vma list. Essentially it was just a slightly modified madvise don't
need. So that part was allowed to race with oom_reaper.
-- 
Michal Hocko
SUSE Labs