Linux-api Archive on lore.kernel.org
 help / color / Atom feed
From: Michael Sammler <msammler@mpi-sws.org>
To: Dave Hansen <dave.hansen@intel.com>, Jann Horn <jannh@google.com>
Cc: wad@chromium.org, Kees Cook <keescook@chromium.org>,
	Linux API <linux-api@vger.kernel.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	linuxram@us.ibm.com, Andy Lutomirski <luto@amacapital.net>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [RFC PATCH] seccomp: Add protection keys into seccomp_data
Date: Tue, 30 Oct 2018 11:55:17 +0100
Message-ID: <257eca22-8ef4-2621-8a62-d65d611abf61@mpi-sws.org> (raw)
In-Reply-To: <6c7cd491-ab12-b553-68cf-f66b2eaa25de@intel.com>

On 10/29/2018 11:33 PM, Dave Hansen wrote:
> But, that's really an implementation detail.  The effect on the ABI and
> how this might constrain future pkeys use is my bigger worry.
> 
> I'd also want to make sure that your specific use-case is compatible
> with all the oddities of pkeys, like the 'clone' and signal behavior.
> Some of that is spelled out here:
> 
> 	http://man7.org/linux/man-pages/man7/pkeys.7.html
> 

I think my specific use case is compatible with these oddities since the 
untrusted code is not allowed to install signal handlers or spawn 
threads himself but only through the trusted code, which ensures, that 
the PKRU has the correct value when control passes back to the untrusted 
code. But I agree that these oddities are something a programmer needs 
to take into account if he uses pkeys in general (and this patch adds 
new considerations to it if the program installs a seccomp filter and 
e.g. wants to do system calls from a signal handler).

> One thing that's a worry is that we have never said that you *can't*
> write to arbitrary permissions in PKRU.  I can totally see some really
> paranoid code saying, "I'm about to do something risky, so I'll turn off
> access to *all* pkeys", or " turn off all access except my current
> stack".  If they did that, they might also inadvertently disable access
> to certain seccomp-restricted syscalls.
> 
> We can fix that up by documenting restrictions like "code should never
> change the access rights of any pkey other than those that it
> allocated", but that doesn't help any old code (of which I hope there is
> relatively little).
> 

I can also see paranoid code wanting to do something like you describe 
and I think, that we should try to not forbid this behaviour. 
Documenting restrictions on code which writes to the PKRU as you 
describe would be one way to go, but this would disallow this paranoid 
use case if I understand it correctly because the code would not be 
allowed to disable access to all except of one pkey.

My idea would be to not put the blame on the code which writes to the 
PKRU but on the code which installs the seccomp filter based on the 
PKRU: It has to make sure, that it allows the system calls which the 
paranoid code should be allowed to do when the pkey of the paranoid code 
is the (only) enabled pkey. This could be ensured e.g. by the paranoid 
code providing the pkey it intends to use to the code which installs the 
seccomp filter. This of course means, that you need to know what you are 
doing, when you install a seccomp filter which depends on the PKRU, but 
I think this is already the case when you install a seccomp filter 
without this patch (but maybe someone with more seccomp experience than 
me can say better how much reasoning is needed to install a seccomp 
filter without and with this patch).

-- Michael

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-29 11:23 Michael Sammler
2018-10-29 16:25 ` Kees Cook
2018-10-29 16:37   ` Dave Hansen
2018-10-29 16:48     ` Jann Horn
2018-10-29 17:02       ` Michael Sammler
2018-10-29 17:07         ` Dave Hansen
2018-10-29 17:29       ` Dave Hansen
2018-10-29 21:55         ` Michael Sammler
2018-10-29 22:33           ` Dave Hansen
2018-10-30 10:55             ` Michael Sammler [this message]
2018-10-29 16:42   ` Jann Horn
2018-10-29 16:48   ` Ram Pai
2018-10-29 17:05     ` Michael Sammler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=257eca22-8ef4-2621-8a62-d65d611abf61@mpi-sws.org \
    --to=msammler@mpi-sws.org \
    --cc=dave.hansen@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=jannh@google.com \
    --cc=keescook@chromium.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=linuxram@us.ibm.com \
    --cc=luto@amacapital.net \
    --cc=wad@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-api Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-api/0 linux-api/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-api linux-api/ https://lore.kernel.org/linux-api \
		linux-api@vger.kernel.org
	public-inbox-index linux-api

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-api


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git