Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1294387 > unrolled thread

Rethinking sigcontext's xfeatures slightly for PKRU's benefit?

Back to article view | Back to linux.kernel

Contents

  Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@kernel.org> - 2015-12-18 02:50 +0100
    Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-18 03:20 +0100
      Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-18 03:40 +0100
        Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-18 04:00 +0100
          Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-18 06:30 +0100
            Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? "H. Peter Anvin" <hpa@zytor.com> - 2015-12-18 07:50 +0100
              Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-18 17:10 +0100
                Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-18 18:00 +0100
                Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-18 19:50 +0100
                  Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-18 20:30 +0100
                    Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-18 21:10 +0100
                      Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-18 21:30 +0100
                      Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Linus Torvalds <torvalds@linux-foundation.org> - 2015-12-18 21:40 +0100
                        Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-18 21:50 +0100
                          Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? "H. Peter Anvin" <hpa@zytor.com> - 2015-12-18 22:00 +0100
                            Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-18 22:10 +0100
                              Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-18 22:10 +0100
                          Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Linus Torvalds <torvalds@linux-foundation.org> - 2015-12-18 22:10 +0100
                            Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-18 22:20 +0100
                              Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Linus Torvalds <torvalds@linux-foundation.org> - 2015-12-18 22:50 +0100
                                Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-18 23:30 +0100
                                  Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Linus Torvalds <torvalds@linux-foundation.org> - 2015-12-19 00:10 +0100
                                    Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-19 00:20 +0100
                                      Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Linus Torvalds <torvalds@linux-foundation.org> - 2015-12-19 00:30 +0100
                                      Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-21 18:10 +0100
                                        Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-22 00:00 +0100
                                          Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-22 00:10 +0100
                                            Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-22 00:10 +0100
                                              Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-22 00:10 +0100
                                  Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-22 00:10 +0100
                                    Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Andy Lutomirski <luto@amacapital.net> - 2015-12-22 00:10 +0100
                                Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Dave Hansen <dave.hansen@linux.intel.com> - 2015-12-30 00:50 +0100
                            Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Linus Torvalds <torvalds@linux-foundation.org> - 2015-12-18 22:20 +0100
            Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Ingo Molnar <mingo@kernel.org> - 2015-12-18 09:40 +0100
    Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Christoph Hellwig <hch@infradead.org> - 2015-12-18 10:00 +0100
      Re: Rethinking sigcontext's xfeatures slightly for PKRU's benefit? Borislav Petkov <bp@alien8.de> - 2015-12-18 14:00 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1295220

On Fri, Dec 18, 2015 at 1:45 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Dec 18, 2015 at 1:12 PM, Dave Hansen
> <dave.hansen@linux.intel.com> wrote:
>>
>> But, if we are picking out an execute-only pkey more dynamically, we've
>> got to keep the default value for the entire process somewhere.
>
> How dynamic do we want to make this, though?
>
> I haven't looked at the details, and perhaps more importantly, I don't
> know what exactly are the requirements you've gotten from the people
> who are expected to actually use this.
>
> I think we might want to hardcode a couple of keys as "kernel
> reserved". And I'd rather reserve them up-front than have some user
> program be unhappy later when we want to use them.
>
> I guess we want to leave key #0 for "normal page", so my suggesting to
> use that for the execute-only was probably misguided.
>
> But I do think we might want to have that "no read access" as a real
> fixed key too, because I think the kernel itself would want to use it:
>
>  (a) to make sure that it gets the right fault when user space passes
> in a execute-only address to a system call.
>
>  (b) for much more efficient PAGEALLOC_DEBUG for kernel mappings.
>
> so I do think that we'd want to reserve two of the 16 keys up front.
>
> Would it be ok for the expected users to have those keys simply be
> fixed? With key 0 being used for all default pages, and key 1 being
> used for all execute-only pages? And then defaulting PKRU to 4,
> disallowing access to that key #1?
>
> I could imagine that some kernel person would want to use even more
> keys, but I think two fixed keys are kind of the minimal we'd want to
> use.

I imagine we'd reserve key 0 for normal page and key 1 for deny-read.
Let me be a bit more concrete about what I'm suggesting:

We'd have thread_struct.baseline_pkru.  It would start with key 0
allowing all access and key 1 denying reads.

We'd have a syscall like set_protection_key that could allocate unused
keys and change the values of keys that have been allocated.  Those
changes would be reflected in baseline_pkru.  Changes to keys 0 and 1
in baseline_pkru would not be allowed.

Signal delivery would load baseline_pkru into the PKRU register.
Signal restore would restore PKRU to its previous value.

WRPKRU would, of course, override baseline_pkru, but it wouldn't
change baseline_pkru.  The set_protection_key syscall would modify
*both* real PKRU and baseline_pkru.

Apps that don't want to use the baseline_pkru mechanism could use
syscalls to claim ownership of protection keys but then manage them
purely with WRPKRU directly.  We could optionally disallow
mprotect_key on keys that weren't allocated in advance.

Does that seem sane?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1295237

On Fri, Dec 18, 2015 at 2:28 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> Apps that don't want to use the baseline_pkru mechanism could use
> syscalls to claim ownership of protection keys but then manage them
> purely with WRPKRU directly.  We could optionally disallow
> mprotect_key on keys that weren't allocated in advance.
>
> Does that seem sane?

So everything seems sane except for the need for that baseline_pkru.

I'm not seeing why it couldn't just be a fixed value. Is there any
real downside to it?

                    Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1295240

On Fri, Dec 18, 2015 at 3:08 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Dec 18, 2015 at 2:28 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>
>> Apps that don't want to use the baseline_pkru mechanism could use
>> syscalls to claim ownership of protection keys but then manage them
>> purely with WRPKRU directly.  We could optionally disallow
>> mprotect_key on keys that weren't allocated in advance.
>>
>> Does that seem sane?
>
> So everything seems sane except for the need for that baseline_pkru.
>
> I'm not seeing why it couldn't just be a fixed value. Is there any
> real downside to it?

Yes, I think.  If I'm using protection keys to protect some critical
data structure (important stuff in shared memory, important memory
mapped files, pmem, etc), then I'll allocate a protection key and set
PKRU to deny writes.  The problem is that I really, really want writes
denied except when explicitly enabled in narrow regions of code that
use wrpkru to enable them, and I don't want an asynchronous signal
delivered in those narrow regions of code or newly cloned threads to
pick up the write-allow value.  So I want baseline_pkru to have the
deny writes entry.

I think I would do exactly this in my production code here if my
server supported it.  Some day...

Hrm.  We might also want an option to change pkru and/or baseline_pkru
in all threads in the current mm.  That's optional but it could be
handy.  Maybe it would be as simple as having the allocate-a-pkey call
have an option to set an initial baseline value and an option to
propagate that initial value to pre-existing threads.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1295241

On Fri, Dec 18, 2015 at 3:16 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> Yes, I think.  If I'm using protection keys to protect some critical
> data structure (important stuff in shared memory, important memory
> mapped files, pmem, etc), then I'll allocate a protection key and set
> PKRU to deny writes.  The problem is that I really, really want writes
> denied except when explicitly enabled in narrow regions of code that
> use wrpkru to enable them, and I don't want an asynchronous signal
> delivered in those narrow regions of code or newly cloned threads to
> pick up the write-allow value.  So I want baseline_pkru to have the
> deny writes entry.

Hmm. Ok, that does sound like a valid and interesting usage case. Fair enough.

                Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296084

On 12/18/2015 03:16 PM, Andy Lutomirski wrote:
> Hrm.  We might also want an option to change pkru and/or baseline_pkru
> in all threads in the current mm.  That's optional but it could be
> handy.  Maybe it would be as simple as having the allocate-a-pkey call
> have an option to set an initial baseline value and an option to
> propagate that initial value to pre-existing threads.

Do you mean actively going in and changing PKRU in other threads?  I
fear that will be dangerous.

IMNHO, whatever we do, I think we need to ensure that _raw_ PKRU calls
are allowed (somehow).  Raw in this case would mean a thread calling
WRPKRU without a system call and without checking in with what any other
threads are doing.

Let's say baseline_pkru=0x004 (we're access-disabling PKEY[1] and using
it for execute-only).  Now, a thread is trying to do this:

	pkey2 = sys_pkey_alloc(); // now pkey2=2
	tmp = rdpkru(); // 0x004
	tmp |= 0x10; // set PKRU[2].AD=1
	wrpkru(tmp);

While another thread does:

	pkey4 = pkey_alloc(); // pkey4=4
	sys_pkey_set(pkey4, ACCESS_DISABLE, SET_BASELINE_ALL_THREADS);

Without some kind of locking, that's going to race.  We could do all the
locking in the kernel, but that requires that the kernel do all the PKRU
writing, which I'd really like to avoid.

I think the closest we can get reasonably is to have the kernel track
the baseline_pkru and then allow userspace to query it in case userspace
decides that thread needs to update its thread-local PKRU from the baseline.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296275

On Dec 22, 2015 2:04 AM, "Dave Hansen" <dave.hansen@linux.intel.com> wrote:
>
> On 12/18/2015 03:16 PM, Andy Lutomirski wrote:
> > Hrm.  We might also want an option to change pkru and/or baseline_pkru
> > in all threads in the current mm.  That's optional but it could be
> > handy.  Maybe it would be as simple as having the allocate-a-pkey call
> > have an option to set an initial baseline value and an option to
> > propagate that initial value to pre-existing threads.
>
> Do you mean actively going in and changing PKRU in other threads?  I
> fear that will be dangerous.
>
> IMNHO, whatever we do, I think we need to ensure that _raw_ PKRU calls
> are allowed (somehow).  Raw in this case would mean a thread calling
> WRPKRU without a system call and without checking in with what any other
> threads are doing.
>
> Let's say baseline_pkru=0x004 (we're access-disabling PKEY[1] and using
> it for execute-only).  Now, a thread is trying to do this:
>
>         pkey2 = sys_pkey_alloc(); // now pkey2=2
>         tmp = rdpkru(); // 0x004
>         tmp |= 0x10; // set PKRU[2].AD=1
>         wrpkru(tmp);
>
> While another thread does:
>
>         pkey4 = pkey_alloc(); // pkey4=4
>         sys_pkey_set(pkey4, ACCESS_DISABLE, SET_BASELINE_ALL_THREADS);
>
> Without some kind of locking, that's going to race.  We could do all the
> locking in the kernel, but that requires that the kernel do all the PKRU
> writing, which I'd really like to avoid.
>
> I think the closest we can get reasonably is to have the kernel track
> the baseline_pkru and then allow userspace to query it in case userspace
> decides that thread needs to update its thread-local PKRU from the baseline.

Yeah, fair point.  Let's skip the modify-other-threads thing.

Perhaps this is silly, but what if the default were changed to deny
reads and writes for unallocated keys?  Is there a use case that
breaks?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296281

On 12/21/2015 02:52 PM, Andy Lutomirski wrote:
> Perhaps this is silly, but what if the default were changed to deny
> reads and writes for unallocated keys?  Is there a use case that
> breaks?

It's probably a reasonable debugging feature.

But, anything that takes an XSAVE feature out of its "init state" has
the potential to do a bit of harm because it increases the potential
size of writes during XSAVE.  XSAVEOPT will _help_ here, but we probably
don't want to go out of our way to take things out of the init state
when we're unsure of the benefits.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296282

On Mon, Dec 21, 2015 at 3:00 PM, Dave Hansen
<dave.hansen@linux.intel.com> wrote:
> On 12/21/2015 02:52 PM, Andy Lutomirski wrote:
>> Perhaps this is silly, but what if the default were changed to deny
>> reads and writes for unallocated keys?  Is there a use case that
>> breaks?
>
> It's probably a reasonable debugging feature.
>
> But, anything that takes an XSAVE feature out of its "init state" has
> the potential to do a bit of harm because it increases the potential
> size of writes during XSAVE.  XSAVEOPT will _help_ here, but we probably
> don't want to go out of our way to take things out of the init state
> when we're unsure of the benefits.

Aren't you already doing that with your magic execute-only thing?

Also, if we ever do the deferred-xstate-restore thing that Rik was
playing with awhile back, then we'll want to switch to using rdpkru
and wrpkru in-kernel directly, and we'll explicitly mask PKRU out of
the XRSTOR and XSAVEOPT state, and this particular issue will become
irrelevant.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296285

On 12/21/2015 03:02 PM, Andy Lutomirski wrote:
> On Mon, Dec 21, 2015 at 3:00 PM, Dave Hansen
> <dave.hansen@linux.intel.com> wrote:
>> On 12/21/2015 02:52 PM, Andy Lutomirski wrote:
>>> Perhaps this is silly, but what if the default were changed to deny
>>> reads and writes for unallocated keys?  Is there a use case that
>>> breaks?
>>
>> It's probably a reasonable debugging feature.
>>
>> But, anything that takes an XSAVE feature out of its "init state" has
>> the potential to do a bit of harm because it increases the potential
>> size of writes during XSAVE.  XSAVEOPT will _help_ here, but we probably
>> don't want to go out of our way to take things out of the init state
>> when we're unsure of the benefits.
> 
> Aren't you already doing that with your magic execute-only thing?

Yep, but that's with a concrete benefit in mind.

> Also, if we ever do the deferred-xstate-restore thing that Rik was
> playing with awhile back, then we'll want to switch to using rdpkru
> and wrpkru in-kernel directly, and we'll explicitly mask PKRU out of
> the XRSTOR and XSAVEOPT state, and this particular issue will become
> irrelevant.

Yep, agreed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296284

On 12/18/2015 02:28 PM, Andy Lutomirski wrote:
...
>> I could imagine that some kernel person would want to use even more
>> keys, but I think two fixed keys are kind of the minimal we'd want to
>> use.
> 
> I imagine we'd reserve key 0 for normal page and key 1 for deny-read.
> Let me be a bit more concrete about what I'm suggesting:
> 
> We'd have thread_struct.baseline_pkru.  It would start with key 0
> allowing all access and key 1 denying reads.

Are you sure thread_struct is the right place for this?  I think of
signal handlers as a process-wide thing, and it seems a bit goofy if we
have the PKRU value in a signal handler depend on the PKRU of the thread
that got interrupted.

> We'd have a syscall like set_protection_key that could allocate unused
> keys and change the values of keys that have been allocated.  Those
> changes would be reflected in baseline_pkru.  Changes to keys 0 and 1
> in baseline_pkru would not be allowed.

FWIW, I think we can do this without *actually* dedicating key 1 to
execute-only.  But that's a side issue.

> Signal delivery would load baseline_pkru into the PKRU register.
> Signal restore would restore PKRU to its previous value.

Do you really mean "its previous value" or are you OK with the existing
behavior which restores PKRU from the XSAVE buffer in the sigcontext?

> WRPKRU would, of course, override baseline_pkru, but it wouldn't
> change baseline_pkru.  The set_protection_key syscall would modify
> *both* real PKRU and baseline_pkru.

How about this:

We make baseline_pkru a process-wide baseline and store it in
mm->context.  That way, no matter which thread gets interrupted for a
signal, they see consistent values.  We only write to it when an app
_specifically_ asks for it to be updated with a special flag to
sys_pkey_set().

When an app uses the execute-only support, we implicitly set the
read-disable bit in baseline_pkru for the execute-only pkey.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296286

On Mon, Dec 21, 2015 at 3:04 PM, Dave Hansen
<dave.hansen@linux.intel.com> wrote:
> On 12/18/2015 02:28 PM, Andy Lutomirski wrote:
> ...
>>> I could imagine that some kernel person would want to use even more
>>> keys, but I think two fixed keys are kind of the minimal we'd want to
>>> use.
>>
>> I imagine we'd reserve key 0 for normal page and key 1 for deny-read.
>> Let me be a bit more concrete about what I'm suggesting:
>>
>> We'd have thread_struct.baseline_pkru.  It would start with key 0
>> allowing all access and key 1 denying reads.
>
> Are you sure thread_struct is the right place for this?  I think of
> signal handlers as a process-wide thing, and it seems a bit goofy if we
> have the PKRU value in a signal handler depend on the PKRU of the thread
> that got interrupted.

I think you're right.  mmu_context_t might be a better choice.

>
>> We'd have a syscall like set_protection_key that could allocate unused
>> keys and change the values of keys that have been allocated.  Those
>> changes would be reflected in baseline_pkru.  Changes to keys 0 and 1
>> in baseline_pkru would not be allowed.
>
> FWIW, I think we can do this without *actually* dedicating key 1 to
> execute-only.  But that's a side issue.
>
>> Signal delivery would load baseline_pkru into the PKRU register.
>> Signal restore would restore PKRU to its previous value.
>
> Do you really mean "its previous value" or are you OK with the existing
> behavior which restores PKRU from the XSAVE buffer in the sigcontext?

By "its previous value" I meant the value in the XSAVE buffer in the
sigcontext.  So I think I'm okay with that :)

>
>> WRPKRU would, of course, override baseline_pkru, but it wouldn't
>> change baseline_pkru.  The set_protection_key syscall would modify
>> *both* real PKRU and baseline_pkru.
>
> How about this:
>
> We make baseline_pkru a process-wide baseline and store it in
> mm->context.  That way, no matter which thread gets interrupted for a
> signal, they see consistent values.  We only write to it when an app
> _specifically_ asks for it to be updated with a special flag to
> sys_pkey_set().
>
> When an app uses the execute-only support, we implicitly set the
> read-disable bit in baseline_pkru for the execute-only pkey.

Sounds good, I think.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1299168

On 12/18/2015 01:45 PM, Linus Torvalds wrote:
> On Fri, Dec 18, 2015 at 1:12 PM, Dave Hansen
> <dave.hansen@linux.intel.com> wrote:
>>
>> But, if we are picking out an execute-only pkey more dynamically, we've
>> got to keep the default value for the entire process somewhere.
> 
> How dynamic do we want to make this, though?

Right now, all I plan to do is make it a one-way trip: if a process does
a prot=PROT_EXEC mapping, we dedicate a key local to that process, and
it gets 14 usable keys.  If it doesn't use prot=PROT_EXEC, then it gets
15 usable keys.

> I haven't looked at the details, and perhaps more importantly, I don't
> know what exactly are the requirements you've gotten from the people
> who are expected to actually use this.
> 
> I think we might want to hardcode a couple of keys as "kernel
> reserved". And I'd rather reserve them up-front than have some user
> program be unhappy later when we want to use them.

The one constant I've heard from the folks that are going to use this is
that 15 keys is not enough.  That's why I'm hesitant to remove _any_ more.

> But I do think we might want to have that "no read access" as a real
> fixed key too, because I think the kernel itself would want to use it:
> 
>  (a) to make sure that it gets the right fault when user space passes
> in a execute-only address to a system call.

Having a dedicated or static key for execute-only doesn't really change
this code.  We just have one extra step to go look in the mm->context
and see which pkey (if any) is assigned to be execute-only in the fault
code.

>  (b) for much more efficient PAGEALLOC_DEBUG for kernel mappings.

The current hardware only applies the keys on _PAGE_USER mappings, so we
can't use it for kernel mappings.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1295175

On Fri, Dec 18, 2015 at 1:04 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I do wonder if you need an explicit value, though. I think it's
> reasonable to say that PKRU value 0 is special. It's what we'd start
> processes with, and why not just say that it's what we run signal
> handlers in?
>
> Would any other value ever make sense, really?

Ahh. Your point about the PROT_EXEC handling means that maybe we don't
want to default to zero. Maybe we want to make the default PKRU
startup value be 1 instead, enabling access disable key for key 0?

         Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1294497

* Andy Lutomirski <luto@amacapital.net> wrote:

> > >> But what about the register state when delivering a signal?  Don't we
> > >> set the registers to the init state?  Do we need to preserve PKRU state
> > >> instead of init'ing it?  The init state _is_ nice here because it is
> > >> permissive allows us to do something useful no matter what PKRU gets set to.
> > >
> > > I think we leave the extended regs alone.  Don't we?
> > >
> > > I think that, for most signals, we want to leave PKRU as is,
> > > especially for things that aren't SIGSEGV.  For SIGSEGV, maybe we want
> > > an option to reset PKRU on delivery (and then set the flag to restore
> > > on return?).
> >
> > Is there some precedent for doing the state differently for different
> > signals?
> 
> Yes, to a very limited extent: SA_ONSTACK.
> 
> >
> > >> Well, the signal handler isn't necessarily going to clobber it, but the
> > >> delivery code already clobbers it when going to the init state.
> > >
> > > Can you point to that code?
> >
> > handle_signal() -> fpu__clear()
> >
> > The comment around it says:
> >
> > "Ensure the signal handler starts with the new fpu state."
> >
> 
> You win this round :)
> 
> So maybe we should have a mask of xfeatures that aren't cleared on
> signal delivery (e.g. PKRU, perhaps) and that are, by default,
> preserved across signal returns.

So the principle is this: signals are conceptually like creating a new thread of 
execution, and that's a very powerful programming concept, like fork() or 
pthread_create() are powerful concepts. So we definitely want to keep that default 
behavior, and I don't think we want to deviate from that for typical new extended 
CPU context features, even if signal delivery slows down as a result.

But we've been arguing about 'lightweight signals' for up to two decades that I 
can remember. (The first such suggestion was to not save the FPU state, back when 
FPU saves were ridiculously slow compared to other parts of saving/restoring a 
context.)

So having a well-enumerated, extensible opt-in mask (which defaults to 'all state 
saved') that allows smart signal handlers to skip the save/restore of certain CPU 
context components would be acceptable I think.

But I'd still expect this to be limited to closely coded, specialistic signal 
handlers - as the trend goes against such type of specialization: compilers and 
runtime environments do take advantage of new CPU features so if you want to have 
an 'easy to use' signal handler, you should use the default one.

I'd not be surprised if large-scale signal users like Valgrind could benefit.

> > I'm sure we can preserve it, we just need to be _careful_.
> 
> Right.
> 
> How much does XSAVEOPT help here?  IOW if we're careful to save to the same 
> place we restored from and we don't modify the state in the mean time, how much 
> of the time do we save?  In the best case, I guess we save the memory writes but 
> not the reads?

So I'd not design new signal interfaces around current behavior, I'd design them 
for the existing patterns (which center around programming ease of use) - with 
opt-in, performance-enhancing specializations.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1294534

On Thu, Dec 17, 2015 at 05:48:56PM -0800, Andy Lutomirski wrote:
> Hi all-
> 
> I think that, for PKRU in particular, we want the default signal
> handling behavior to be a bit unusual.

Stupid question, but what the heck is PKRU?  A grep of the kernel tree
shows no results, and a web search returns mostly Thai language results.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1294793

On Fri, Dec 18, 2015 at 12:59:14AM -0800, Christoph Hellwig wrote:
> Stupid question, but what the heck is PKRU?  A grep of the kernel tree
> shows no results, and a web search returns mostly Thai language results.

That should explain it:

https://lkml.kernel.org/r/20151214190542.39C4886D@viggo.jf.intel.com

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web