Re: [PATCH 21/25] mm: implement new mprotect_key() system call

[Dave Hansen]

On 09/28/2015 11:39 PM, Michael Ellerman wrote:
> On Mon, 2015-09-28 at 12:18 -0700, Dave Hansen wrote:
>> From: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
>>
>> mprotect_key() is just like mprotect, except it also takes a
>> protection key as an argument.  On systems that do not support
>> protection keys, it still works, but requires that key=0.
> 
> I'm not sure how userspace is going to use the key=0 feature? ie. userspace
> will still have to detect that keys are not supported and use key 0 everywhere.
> At that point it could just as well skip the mprotect_key() syscalls entirely
> couldn't it?

Yep.

Or, a new architecture could just skip mprotect() itself entirely and
only wire up mprotect_pkey().  I don't see this pkey=0 thing as an
important feature or anything.  I just wanted to call out the behavior.

>> I expect it to get used like this, if you want to guarantee that
>> any mapping you create can *never* be accessed without the right
>> protection keys set up.
>>
>> 	pkey_deny_access(11); // random pkey
>> 	int real_prot = PROT_READ|PROT_WRITE;
>> 	ptr = mmap(NULL, PAGE_SIZE, PROT_NONE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
>> 	ret = mprotect_key(ptr, PAGE_SIZE, real_prot, 11);
>>
>> This way, there is *no* window where the mapping is accessible
>> since it was always either PROT_NONE or had a protection key set.
>>
>> We settled on 'unsigned long' for the type of the key here.  We
>> only need 4 bits on x86 today, but I figured that other
>> architectures might need some more space.
> 
> If the existing mprotect() syscall had a flags argument you could have just
> used that. So is it worth just adding mprotect2() now and using it for this? ie:
> 
> int mprotect2(unsigned long start, size_t len, unsigned long prot, unsigned long flags) ..
> 
> And then you define bit zero of flags to say you're passing a pkey, and it's in
> bits 1-63?
> 
> That way if other arches need to do something different you at least have the
> flags available?

But what problem does that solve?

mprotect() itself has plenty of space in prot.  Do any of the other
architectures need to pass in more than just an integer key to implement
storage/protection keys?

I'd much rather have a set of (relatively) arch-specific system calls
implementing protection keys rather than a single one with one
arch-specific argument.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/