linux-kernel - Re: [PATCH 15/25] x86, pkeys: check VMAs and PTEs for protection keys

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] [day] [month] [year] [list]

Date:	Thu, 22 Oct 2015 17:49:36 -0700
From:	Dave Hansen <dave@...1.net>
To:	Jerome Glisse <j.glisse@...il.com>
Cc:	borntraeger@...ibm.com, x86@...nel.org,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	dave.hansen@...ux.intel.com
Subject: Re: [PATCH 15/25] x86, pkeys: check VMAs and PTEs for protection keys

On 10/22/2015 03:25 PM, Jerome Glisse wrote:
> On Thu, Oct 22, 2015 at 02:23:08PM -0700, Dave Hansen wrote:
...
>> Can you give an example of where a process might be doing a gup and it
>> is completely separate from the CPU context that it's being executed under?
> 
> In drivers/iommu/amd_iommu_v2.c thought this is on AMD platform. I also
> believe that in infiniband one can have GUP call from workqueue that can
> run at any time. In GPU driver we also use GUP thought at this point we
> do not allow another process from accessing a buffer that is populated
> by GUP from another process.

>From quick grepping, there are only a couple of callers that do
get_user_pages() on something that isn't current->mm.

We can fairly easily introduce something new, like

	get_foreign_user_pages()

That sets a flag to tell us to ignore the current PKRU state.

I've attached a patch that at creates a variant of get_user_pages() for
when you're going after another process's mm.  This even makes a few of
the gup call sites look nicer because they're not passing 'current,
current->mm'.

>>> So as first i would just allow GUP to always work and then come up with
>>> syscall to allow to set pkey on device file. This obviously is a lot more
>>> work as you need to go over all device driver using GUP.
>>
>> I wouldn't be opposed to adding some context to the thread (like
>> pagefault_disable()) that indicates whether we should enforce protection
>> keys.  If we are in some asynchronous context, disassociated from the
>> running CPU's protection keys, we could set a flag.
> 
> I was simply thinking of having a global set of pkeys against the process
> mm struct which would be the default global setting for all device GUP
> access. This global set could be override by userspace on a per device
> basis allowing some device to have more access than others.

For now, I think leaving it permissive by default is probably OK.  A
device's access to memory is permissive after a gup anyway.

As you note, doing this is going to require another whole set of user
interfaces, so I'd rather revisit it later once we have a more concrete
need for it.

1. Store a common PKRU value somewhere and activate when servicing work
   outside of the context of the actual process.  Set this PKRU value
   with input from userspace and new user APIs.
2. When work is queued, copy the PKRU value and use it while servicing
   the work.
3. Do all out-of-context work with PKRU=0, or by disabling the PKRU
   checks conditionally.

>> I'd really appreciate if you could point to some concrete examples here
>> which could actually cause a problem, like workqueues doing gups.
> 
> Well i could grep for all current user of GUP, but i can tell you that this
> is gonna be the model for GPU thread ie a kernel workqueue gonna handle
> page fault on behalf of GPU and will perform equivalent of GUP. Also apply
> for infiniband ODP thing which is upstream.



View attachment "get_current_user_pages.patch" of type "text/x-patch" (17102 bytes)