Re: [tip:x86/pkeys] mm/gup: Introduce get_user_pages_remote()

2016-02-19 Thread Konstantin Khlebnikov
On Tue, Feb 16, 2016 at 3:14 PM, tip-bot for Dave Hansen
 wrote:
> Commit-ID:  1e9877902dc7e11d2be038371c6fbf2dfcd469d7
> Gitweb: http://git.kernel.org/tip/1e9877902dc7e11d2be038371c6fbf2dfcd469d7
> Author: Dave Hansen 
> AuthorDate: Fri, 12 Feb 2016 13:01:54 -0800
> Committer:  Ingo Molnar 
> CommitDate: Tue, 16 Feb 2016 10:04:09 +0100
>
> mm/gup: Introduce get_user_pages_remote()
>
> 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 "remote" operations.
>
> This patch introduces a new get_user_pages() variant:
>
> get_user_pages_remote()
>
> Which is a replacement for when get_user_pages() is called on
> non-current tsk/mm.

As I see task-struct argument could be NULL as well as in old api.
They only usage for it is updating task->maj_flt/min_flt.

May be just remove arg and always account major/minor faults into
current task - currently counters are plain unsigned long, so remote
access could corrupt them.

>
> We also introduce a new gup flag: FOLL_REMOTE which can be used
> for the "__" gup variants to get this new behavior.
>
> 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 'remote' access where protection keys will not
> be enforced.
>
> Without protection keys, this patch should not change any behavior.
>
> Signed-off-by: Dave Hansen 
> Reviewed-by: Thomas Gleixner 
> Cc: Andrea Arcangeli 
> Cc: Andrew Morton 
> Cc: Andy Lutomirski 
> Cc: Borislav Petkov 
> Cc: Brian Gerst 
> Cc: Dave Hansen 
> Cc: Denys Vlasenko 
> Cc: H. Peter Anvin 
> Cc: Kirill A. Shutemov 
> Cc: Linus Torvalds 
> Cc: Naoya Horiguchi 
> Cc: Peter Zijlstra 
> Cc: Rik van Riel 
> Cc: Srikar Dronamraju 
> Cc: Vlastimil Babka 
> Cc: j...@suse.cz
> Cc: linux...@kvack.org
> Link: http://lkml.kernel.org/r/20160212210154.3f0e5...@viggo.jf.intel.com
> Signed-off-by: Ingo Molnar 
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  6 +++---
>  drivers/gpu/drm/i915/i915_gem_userptr.c | 10 +-
>  drivers/infiniband/core/umem_odp.c  |  8 
>  fs/exec.c   |  8 ++--
>  include/linux/mm.h  |  5 +
>  kernel/events/uprobes.c | 10 --
>  mm/gup.c| 27 ++-
>  mm/memory.c |  2 +-
>  mm/process_vm_access.c  | 11 ---
>  security/tomoyo/domain.c|  9 -
>  virt/kvm/async_pf.c |  8 +++-
>  11 files changed, 77 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 4b519e4..97d4457 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -753,9 +753,9 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
>
> down_read(&mm->mmap_sem);
> while (pinned < npages) {
> -   ret = get_user_pages(task, mm, ptr, npages - pinned,
> -!etnaviv_obj->userptr.ro, 0,
> -pvec + pinned, NULL);
> +   ret = get_user_pages_remote(task, mm, ptr, npages - pinned,
> +   !etnaviv_obj->userptr.ro, 0,
> +   pvec + pinned, NULL);
> if (ret < 0)
> break;
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
> b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index 59e45b3..90dbf81 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -584,11 +584,11 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
> *_work)
>
> down_read(&mm->mmap_sem);
> while (pinned < npages) {
> -   ret = get_user_pages(work->task, mm,
> -obj->userptr.ptr + pinned * 
> PAGE_SIZE,
> -npages - pinned,
> -!obj->userptr.read_only, 0,
> -pvec + pinned, NULL);
> +   ret = get_user_pages_remote(work->task, mm,
> +   obj->userptr.ptr + pinned * PAGE_SIZE,
> +   npages - pinned,
> +   !obj->userptr.read_only, 0,
> +   pvec + pinned, NULL);
> if (ret < 0)
> break;
>
> diff --git a/drivers/infiniban

[tip:x86/pkeys] mm/gup: Introduce get_user_pages_remote()

2016-02-16 Thread tip-bot for Dave Hansen
Commit-ID:  1e9877902dc7e11d2be038371c6fbf2dfcd469d7
Gitweb: http://git.kernel.org/tip/1e9877902dc7e11d2be038371c6fbf2dfcd469d7
Author: Dave Hansen 
AuthorDate: Fri, 12 Feb 2016 13:01:54 -0800
Committer:  Ingo Molnar 
CommitDate: Tue, 16 Feb 2016 10:04:09 +0100

mm/gup: Introduce get_user_pages_remote()

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 "remote" operations.

This patch introduces a new get_user_pages() variant:

get_user_pages_remote()

Which is a replacement for when get_user_pages() is called on
non-current tsk/mm.

We also introduce a new gup flag: FOLL_REMOTE which can be used
for the "__" gup variants to get this new behavior.

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 'remote' access where protection keys will not
be enforced.

Without protection keys, this patch should not change any behavior.

Signed-off-by: Dave Hansen 
Reviewed-by: Thomas Gleixner 
Cc: Andrea Arcangeli 
Cc: Andrew Morton 
Cc: Andy Lutomirski 
Cc: Borislav Petkov 
Cc: Brian Gerst 
Cc: Dave Hansen 
Cc: Denys Vlasenko 
Cc: H. Peter Anvin 
Cc: Kirill A. Shutemov 
Cc: Linus Torvalds 
Cc: Naoya Horiguchi 
Cc: Peter Zijlstra 
Cc: Rik van Riel 
Cc: Srikar Dronamraju 
Cc: Vlastimil Babka 
Cc: j...@suse.cz
Cc: linux...@kvack.org
Link: http://lkml.kernel.org/r/20160212210154.3f0e5...@viggo.jf.intel.com
Signed-off-by: Ingo Molnar 
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  6 +++---
 drivers/gpu/drm/i915/i915_gem_userptr.c | 10 +-
 drivers/infiniband/core/umem_odp.c  |  8 
 fs/exec.c   |  8 ++--
 include/linux/mm.h  |  5 +
 kernel/events/uprobes.c | 10 --
 mm/gup.c| 27 ++-
 mm/memory.c |  2 +-
 mm/process_vm_access.c  | 11 ---
 security/tomoyo/domain.c|  9 -
 virt/kvm/async_pf.c |  8 +++-
 11 files changed, 77 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 4b519e4..97d4457 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -753,9 +753,9 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
 
down_read(&mm->mmap_sem);
while (pinned < npages) {
-   ret = get_user_pages(task, mm, ptr, npages - pinned,
-!etnaviv_obj->userptr.ro, 0,
-pvec + pinned, NULL);
+   ret = get_user_pages_remote(task, mm, ptr, npages - pinned,
+   !etnaviv_obj->userptr.ro, 0,
+   pvec + pinned, NULL);
if (ret < 0)
break;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 59e45b3..90dbf81 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -584,11 +584,11 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
*_work)
 
down_read(&mm->mmap_sem);
while (pinned < npages) {
-   ret = get_user_pages(work->task, mm,
-obj->userptr.ptr + pinned * 
PAGE_SIZE,
-npages - pinned,
-!obj->userptr.read_only, 0,
-pvec + pinned, NULL);
+   ret = get_user_pages_remote(work->task, mm,
+   obj->userptr.ptr + pinned * PAGE_SIZE,
+   npages - pinned,
+   !obj->userptr.read_only, 0,
+   pvec + pinned, NULL);
if (ret < 0)
break;
 
diff --git a/drivers/infiniband/core/umem_odp.c 
b/drivers/infiniband/core/umem_odp.c
index e69bf26..75077a0 100644
--- a/drivers/infiniband/core/umem_odp.c
+++ b/drivers/infiniband/core/umem_odp.c
@@ -572,10 +572,10 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 
user_virt, u64 bcnt,
 * complex (and doesn't gain us much performance in most use
 * cases).
 */
-   npages = get_user_pages(owning_process, owning_mm, user_virt,
-   gup_num_pages,
-