Message-Id: <20220906055828.10451-1-yangff1@gmail.com>
Date:   Tue,  6 Sep 2022 00:58:28 -0500
From:   Fangfei Yang <yangff1@...il.com>
To:     luto@...nel.org
Cc:     dave.hansen@...el.com, dave.hansen@...ux.intel.com,
        keescook@...omium.org, linux-kernel@...r.kernel.org,
        sroettger@...gle.com, x86@...nel.org, yangff1@...il.com
Subject: Re: PKU usage improvements for threads

> How is PKRU a “privilege” and what do you mean my “immediately”?  I can’t follow this.

If you use PKRU, then you add a layer of permissions controlled by the user 
to the existing memory management to determine whether a particular PKEY's 
memory can be read or written by the current context. 
Then, we don't want this PKRU to be accidentally modified. 

>From this perspective, PKRU is a privilege in user space.

In addition to the restrictions on the user program itself, 
some system calls can also cause the PKRU to be modified. 
For the kernel, this user space is designed to be agnostic 
(and can be anything, having all kinds of guarantees), 
and only a few policies can be sent to the kernel via seccomp-bpf or other security apporches.

Examples of "immediately", e.g., sigreturn-oriented programming, 
can change the current PKRU value while achieving the effect of ROP, 
thus allowing an attacker to access memory that was not read or written in the original context.
This compared to attacks that takes time to work, sigaltstack once, and then wait the signal get triggered.

> Why would the entry point check a source?  Or change PKRU?  What would its PKRU logic be and why?

> As I see it, the handler can (awkwardly, perhaps) manage PKRU just fine for all purposes except kernel access to the signal stack.

Yes, it can be correct and safe, but it'd be better if kernel can just provide 
the PKRU desired by the user when sending signal to the user.

> Can you elaborate?  Of course WRPKRU can be abused to fully bypass PKRU protection. 

This is determined by the characteristics of MPK and signal.
First, currently, the kernel uses a default PKRU value of 0x55555554 when it signals, 
which means that only PKEY=0 is writable. 
However, the program that actually handles the signal may need other permissions, 
for example it may need to access PKEY=0,1,2 at the same time.
Then, I would need to write the following assembly code in the preamble of the handling function.
```
mov eax, 0x55555540
xor ecx, ecx
xor edx, edx
wrpkru
cmp eax, 0x55555540
jne fail
```
However, this leads to another problem, if, for example, 
I am an attacker (assuming you have gone through some method that prevents sigreturn-oriented programming), 
I can modify the RSP to jump to this signal handling function. 
Then, you will see that in the code afterwards, 
you have no way to tell if there is actually a signal currently being sent by the kernel to our handler function, 
or if this was jumped over by the attacker since the previous PKRU has gone.

Of course this is not insurmountable. 
For example, we can save a token to the memory that PKEY=0 before the very first `mov eax, 0x55555540`, 
thus ensuring that PKEY=0 is writable before executing wrpkru 
(assuming here that PKEY=0 indicates some higher privilege). 
However, in a multi-threaded environment, saving such a token also becomes more complicated.

If the kernel can give a correct PKRU according to the user's specific design needs, 
then there is no need for additional token here. 
Then, even if the attacker jumps here, its PKRU unchanged, 
and thus its privileges can be restricted by other methods 
(e.g., it has been proposed to add a function to read pkru in seccomp-bpf[1]).

> This isn’t logical at all to me. It makes some sense as an API simplification to avoid a new syscall, and it makes sense in a bizarre (to me) world in which user code can control access to PKRU but not to sigaltstack(), but why do we live in that world?

If we can control WRPKRU, of course sigaltstack can also be controlled, 
whether by seccomp or something else. 
My point is that doing so would reduce the exposure of risky syscalls 
by ensuring that no matter how the syscall is used, 
it does not expand the privileges of the caller. 
There is always a way to make the these system calls secure, 
as long as the availability of the signal is addressed.

> Do you mean in current kernels?

No, what I mean is that if you save the current PKRU, 
rather than allowing the caller to specify one at will. 
Then, in the future, when doing a signal deliver, 
if it tries to write to a stack address that is not allowed to be accessed by this saved PKRU, 
it will segfault. 
This is not necessary, as you can restrict access to sigaltstack, 
I'm just saying that it makes sigaltstack itself a little more secure.
(Because PKRU is also in effect in the kernel, in the vast majority of cases, system calls 
still follow the restrictions of the current context's PKRU when accessing user pointers. 
This assumption is not absolute, but it is not a bad thing 
if syscalls can satisfy this requirement)

> What do you mean “accurate”?

If the accessibility of the memory itself is only checked at the time of the system call, 
then there is a possibility that this address is unmap and mapped again, 
at which point the PKEY check based on is inaccurate.

> Sorry, what code does XSAVE here?

I simplified the process of signal delivery a bit.

For xsave here the process is: handle_signal->setup_rt_frame -> __setup_rt_frame->get_sigframe->copy_fpstate_to_sigframe->copy_fpregs_to_sigframe-> copy_xregs_to_user->xsave

It will be slightly different if other instructions are used.

Because this xsave will use the user's stack address directly, 
the PKRU must be changed to the PKRU value obtained 
by any of the methods discussed here before the xsave is made.

However, this results in the xsave saving the fpu registers with the new PKRU value,
but the xsave should save the old PKRU value (before the siganl).

So, we must backup the before write to it, and then after xsave, 
the old value is used to replace the value in the signal frame.

Something similar needs to be done at `rt_sigreturn`, 
because the restore_sigcontext will modify the PKRU register 
to the PKRU value in the frame, but the `restore_altstack` that 
follows will immediately access `&frame->uc.uc_stack`. 
If, however, the restored PKRU does not have access to the `&frame->uc.uc_stack`, 
it will cause a kernel panic. 

[1] https://lore.kernel.org/linux-api/20181029112343.27454-1-msammler@mpi-sws.org/

Powered by blists - more mailing lists