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: Wed, 18 Nov 2015 13:29:38 +0100 From: Jan Kara <jack@...e.cz> To: Dave Hansen <dave@...1.net> Cc: linux-kernel@...r.kernel.org, x86@...nel.org, dave.hansen@...ux.intel.com, jack@...e.cz, akpm@...ux-foundation.org, aarcange@...hat.com Subject: Re: [PATCH 02/37] mm, frame_vector: do not use get_user_pages_locked() On Mon 16-11-15 19:35:14, Dave Hansen wrote: > > From: Dave Hansen <dave.hansen@...ux.intel.com> > > get_user_pages_locked() appears to be for use when a caller needs > to know that its lock on mmap_sem was invalidated by the gup > call. > > But, get_vaddr_frames() is not one of those users. It > unconditionally locks the mmap_sem and unconditionally unlocks it > after the gup call. It takes no special action and does not need > to know whether its lock was invalidated or not. > > Replace get_user_pages_locked() with a vanilla get_user_pages() > and save a few lines of code. > > Note that this was the *ONLY* use of get_user_pages_locked() in > the entire kernel tree. I've used get_user_pages_locked() because of a comment before that function saying: * We can leverage the VM_FAULT_RETRY functionality in the page fault * paths better by using either get_user_pages_locked() or * get_user_pages_unlocked(). * * get_user_pages_locked() is suitable to replace the form: * * down_read(&mm->mmap_sem); * do_something() * get_user_pages(tsk, mm, ..., pages, NULL); * up_read(&mm->mmap_sem); * * to: * * int locked = 1; * down_read(&mm->mmap_sem); * do_something() * get_user_pages_locked(tsk, mm, ..., pages, &locked); * if (locked) * up_read(&mm->mmap_sem); So I understood it as a way to reduce mmap_sem hold time by doing a try first. Did I understand that comment wrong? Honza > Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com> > Cc: Jan Kara <jack@...e.cz> > Cc: Andrew Morton <akpm@...ux-foundation.org> > Cc: Andrea Arcangeli <aarcange@...hat.com> > --- > > b/mm/frame_vector.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff -puN mm/frame_vector.c~remove-get_user_pages_locked-user mm/frame_vector.c > --- a/mm/frame_vector.c~remove-get_user_pages_locked-user 2015-11-16 12:35:35.282169293 -0800 > +++ b/mm/frame_vector.c 2015-11-16 12:35:35.285169429 -0800 > @@ -40,7 +40,6 @@ int get_vaddr_frames(unsigned long start > struct vm_area_struct *vma; > int ret = 0; > int err; > - int locked; > > if (nr_frames == 0) > return 0; > @@ -49,7 +48,6 @@ int get_vaddr_frames(unsigned long start > nr_frames = vec->nr_allocated; > > down_read(&mm->mmap_sem); > - locked = 1; > vma = find_vma_intersection(mm, start, start + 1); > if (!vma) { > ret = -EFAULT; > @@ -58,8 +56,8 @@ int get_vaddr_frames(unsigned long start > if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) { > vec->got_ref = true; > vec->is_pfns = false; > - ret = get_user_pages_locked(current, mm, start, nr_frames, > - write, force, (struct page **)(vec->ptrs), &locked); > + ret = get_user_pages(current, mm, start, nr_frames, > + write, force, (struct page **)(vec->ptrs), NULL); > goto out; > } > > @@ -87,8 +85,7 @@ int get_vaddr_frames(unsigned long start > vma = find_vma_intersection(mm, start, start + 1); > } while (vma && vma->vm_flags & (VM_IO | VM_PFNMAP)); > out: > - if (locked) > - up_read(&mm->mmap_sem); > + up_read(&mm->mmap_sem); > if (!ret) > ret = -EFAULT; > if (ret > 0) > _ -- Jan Kara <jack@...e.com> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists