| 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: Tue, 9 Feb 2016 13:46:49 +0100 From: Ingo Molnar <mingo@...nel.org> To: Dave Hansen <dave@...1.net> Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org, x86@...nel.org, torvalds@...ux-foundation.org, dave.hansen@...ux.intel.com, srikar@...ux.vnet.ibm.com, vbabka@...e.cz, akpm@...ux-foundation.org, kirill.shutemov@...ux.intel.com, aarcange@...hat.com, n-horiguchi@...jp.nec.com, jack@...e.cz Subject: Re: [PATCH 01/31] mm, gup: introduce concept of "foreign" get_user_pages() * Dave Hansen <dave@...1.net> wrote: > > OK, so I've fixed up my build process to _actually_ build the > nommu code. > > One of Vlastimil's comments made me go dig back in to the uprobes > code's use of get_user_pages(). I decided to change both of them > to be "foreign" accesses. > > This also fixes the nommu breakage that Vlastimil noted last time. > > Srikar, I'd appreciate if you can have a look at the uprobes.c > modifications, especially the comment. I don't think this will > change any behavior, but I want to make sure the comment is > accurate. > > --- > > From: Dave Hansen <dave.hansen@...ux.intel.com> > > For protection keys, we need to understand whether protections > should be enforced in software or not. In general, we enforce > protections when working on our own task, but not when on others. > We call these "current" and "foreign" operations. > > This patch introduces a new get_user_pages() variant: > > get_user_pages_foreign() > > We modify the vanilla get_user_pages() so it can no longer be > used on mm/tasks other than 'current/current->mm', which is by > far the most common way it is called. Using it makes a few of > the call sites look a bit nicer. > > In other words, get_user_pages_foreign() is a replacement for > when get_user_pages() is called on non-current tsk/mm. > > This also switches get_user_pages_(un)locked() over to be like > get_user_pages() and not take a tsk/mm. There is no > get_user_pages_foreign_(un)locked(). If someone wants that > behavior they just have to use "__" variant and pass in > FOLL_FOREIGN explicitly. > > The uprobes is_trap_at_addr() location holds mmap_sem and > calls get_user_pages(current->mm) on an instruction address. This > makes it a pretty unique gup caller. Being an instruction access > and also really originating from the kernel (vs. the app), I opted > to consider this a 'foreign' access where protection keys will not > be enforced. > > Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com> > Acked-by: Srikar Dronamraju <srikar@...ux.vnet.ibm.com> > Acked-by: Vlastimil Babka <vbabka@...e.cz> > Cc: Andrew Morton <akpm@...ux-foundation.org> > Cc: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com> > Cc: Andrea Arcangeli <aarcange@...hat.com> > Cc: Naoya Horiguchi <n-horiguchi@...jp.nec.com> > Cc: jack@...e.cz > --- > > b/arch/cris/arch-v32/drivers/cryptocop.c | 8 --- > b/arch/ia64/kernel/err_inject.c | 3 - > b/arch/mips/mm/gup.c | 3 - > b/arch/s390/mm/gup.c | 4 - > b/arch/sh/mm/gup.c | 2 > b/arch/sparc/mm/gup.c | 2 > b/arch/x86/mm/gup.c | 2 > b/arch/x86/mm/mpx.c | 4 - > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 - > b/drivers/gpu/drm/etnaviv/etnaviv_gem.c | 2 > b/drivers/gpu/drm/i915/i915_gem_userptr.c | 2 > b/drivers/gpu/drm/radeon/radeon_ttm.c | 3 - > b/drivers/gpu/drm/via/via_dmablit.c | 3 - > b/drivers/infiniband/core/umem.c | 2 > b/drivers/infiniband/core/umem_odp.c | 8 +-- > b/drivers/infiniband/hw/mthca/mthca_memfree.c | 3 - > b/drivers/infiniband/hw/qib/qib_user_pages.c | 3 - > b/drivers/infiniband/hw/usnic/usnic_uiom.c | 2 > b/drivers/media/pci/ivtv/ivtv-udma.c | 4 - > b/drivers/media/pci/ivtv/ivtv-yuv.c | 10 +--- > b/drivers/media/v4l2-core/videobuf-dma-sg.c | 3 - > b/drivers/misc/mic/scif/scif_rma.c | 2 > b/drivers/misc/sgi-gru/grufault.c | 3 - > b/drivers/scsi/st.c | 2 > b/drivers/staging/rdma/ipath/ipath_user_pages.c | 3 - > b/drivers/video/fbdev/pvr2fb.c | 4 - > b/drivers/virt/fsl_hypervisor.c | 5 -- > b/fs/exec.c | 8 ++- > b/include/linux/mm.h | 21 +++++---- > b/kernel/events/uprobes.c | 10 +++- > b/mm/frame_vector.c | 2 > b/mm/gup.c | 52 +++++++++++++++--------- > b/mm/ksm.c | 2 > b/mm/memory.c | 2 > b/mm/mempolicy.c | 6 +- > b/mm/nommu.c | 30 ++++++++----- > b/mm/process_vm_access.c | 11 +++-- > b/mm/util.c | 4 - > b/net/ceph/pagevec.c | 2 > b/security/tomoyo/domain.c | 9 +++- > b/virt/kvm/async_pf.c | 7 ++- > b/virt/kvm/kvm_main.c | 10 ++-- > 42 files changed, 148 insertions(+), 123 deletions(-) So this patch conflicts with recent upstream changes: patching file drivers/scsi/st.c can't find file to patch at input line 463 mind respinning it against v4.5-rc3 or so? Also, please split this into three patches: - one patch adds the _foreign() GUP variant and applies it to code that uses it on remote tasks. - introduce the new get_user_pages() but also add macros so that both 8-parameter and 7-parameter variants work without breaking the build. We can remove the compatibility wrapping on v4.6 or so. - the third will be a large but trivial patch, which will change 8-parameter GUP usage to 7-parameter usage. ... this should reduce the pain from the GUP interface change churn. Agreed? Thanks, Ingo
Powered by blists - more mailing lists