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
| ||
|
Date: Sat, 5 Jun 2021 03:39:35 +0000 From: Liam Howlett <liam.howlett@...cle.com> To: Shakeel Butt <shakeelb@...gle.com> CC: Alistair Popple <apopple@...dia.com>, "linux-mm@...ck.org" <linux-mm@...ck.org>, "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>, "nouveau@...ts.freedesktop.org" <nouveau@...ts.freedesktop.org>, "bskeggs@...hat.com" <bskeggs@...hat.com>, "rcampbell@...dia.com" <rcampbell@...dia.com>, "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>, "jhubbard@...dia.com" <jhubbard@...dia.com>, "bsingharora@...il.com" <bsingharora@...il.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>, "hch@...radead.org" <hch@...radead.org>, "jglisse@...hat.com" <jglisse@...hat.com>, "willy@...radead.org" <willy@...radead.org>, "jgg@...dia.com" <jgg@...dia.com>, "peterx@...hat.com" <peterx@...hat.com>, "hughd@...gle.com" <hughd@...gle.com>, Christoph Hellwig <hch@....de> Subject: Re: [PATCH v9 03/10] mm/rmap: Split try_to_munlock from try_to_unmap * Shakeel Butt <shakeelb@...gle.com> [210604 20:41]: > On Fri, Jun 4, 2021 at 1:49 PM Liam Howlett <liam.howlett@...cle.com> wrote: > > > > * Shakeel Butt <shakeelb@...gle.com> [210525 19:45]: > > > On Tue, May 25, 2021 at 11:40 AM Liam Howlett <liam.howlett@...cle.com> wrote: > > > > > > > [...] > > > > > > > > > > +/* > > > > > + * Walks the vma's mapping a page and mlocks the page if any locked vma's are > > > > > + * found. Once one is found the page is locked and the scan can be terminated. > > > > > + */ > > > > > > > > Can you please add that this requires the mmap_sem() lock to the > > > > comments? > > > > > > > > > > Why does this require mmap_sem() lock? Also mmap_sem() lock of which mm_struct? > > > > > > Doesn't the mlock_vma_page() require the mmap_sem() for reading? The > > mm_struct in vma->vm_mm; > > > > We are traversing all the vmas where this page is mapped of possibly > different mm_structs. I don't think we want to take mmap_sem() of all > those mm_structs. The commit b87537d9e2fe ("mm: rmap use pte lock not > mmap_sem to set PageMlocked") removed exactly that. > > > > > From what I can see, at least the following paths have mmap_lock held > > for writing: > > > > munlock_vma_pages_range() from __do_munmap() > > munlokc_vma_pages_range() from remap_file_pages() > > > > The following path does not hold mmap_sem: > > exit_mmap() -> munlock_vma_pages_all() -> munlock_vma_pages_range(). Isn't this the benign race referenced by Hugh in the commit you point to below? > > I would really suggest all to carefully read the commit message of > b87537d9e2fe ("mm: rmap use pte lock not mmap_sem to set > PageMlocked"). > > Particularly the following paragraph: > ... > Vlastimil Babka points out another race which this patch protects against. > try_to_unmap_one() might reach its mlock_vma_page() TestSetPageMlocked a > moment after munlock_vma_pages_all() did its Phase 1 TestClearPageMlocked: > leaving PageMlocked and unevictable when it should be evictable. mmap_sem > is ineffective because exit_mmap() does not hold it; page lock ineffective > because __munlock_pagevec() only takes it afterwards, in Phase 2; pte lock > is effective because __munlock_pagevec_fill() takes it to get the page, > after VM_LOCKED was cleared from vm_flags, so visible to try_to_unmap_one. > ... So this is saying the race with exit_mmap() isn't benign after all? > > Alistair, please bring back the VM_LOCKED check with pte lock held and > the comment "Holding pte lock, we do *not* need mmap_lock here". > > One positive outcome of this cleanup patch is the removal of > unnecessary invalidation (unmapping for kvm case) of secondary mmus.
Powered by blists - more mailing lists