From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Lutomirski Subject: Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics Date: Wed, 09 May 2018 14:41:23 +0000 Message-ID: References: <20180502132751.05B9F401F3041@oldenburg.str.redhat.com> <248faadb-e484-806f-1485-c34a72a9ca0b@intel.com> <822a28c9-5405-68c2-11bf-0c282887466d@redhat.com> <57459C6F-C8BA-4E2D-99BA-64F35C11FC05@amacapital.net> <6286ba0a-7e09-b4ec-e31f-bd091f5940ff@redhat.com> <20180503021058.GA5670@ram.oc3035372033.ibm.com> <927c8325-4c98-d7af-b921-6aafcf8fe992@redhat.com> <314e1a48-db94-9b37-8793-a95a2082c9e2@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <314e1a48-db94-9b37-8793-a95a2082c9e2@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org Sender: "Linuxppc-dev" To: Florian Weimer Cc: linux-arch , Linux-MM , Linux API , X86 ML , linuxram@us.ibm.com, Dave Hansen , linux-x86_64@vger.kernel.org, Andrew Lutomirski , linuxppc-dev List-Id: linux-api@vger.kernel.org On Tue, May 8, 2018 at 5:40 AM Florian Weimer wrote: > On 05/08/2018 04:49 AM, Andy Lutomirski wrote: > > On Mon, May 7, 2018 at 2:48 AM Florian Weimer wrote: > > > >> On 05/03/2018 06:05 AM, Andy Lutomirski wrote: > >>> On Wed, May 2, 2018 at 7:11 PM Ram Pai wrote: > >>> > >>>> On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote: > >>>>> > >>>>>> If I recall correctly, the POWER maintainer did express a strong > >>> desire > >>>>>> back then for (what is, I believe) their current semantics, which my > >>>>>> PKEY_ALLOC_SIGNALINHERIT patch implements for x86, too. > >>>>> > >>>>> Ram, I really really don't like the POWER semantics. Can you give > > some > >>>>> justification for them? Does POWER at least have an atomic way for > >>>>> userspace to modify just the key it wants to modify or, even better, > >>>>> special load and store instructions to use alternate keys? > >>> > >>>> I wouldn't call it POWER semantics. The way I implemented it on power > >>>> lead to the semantics, given that nothing was explicitly stated > >>>> about how the semantics should work within a signal handler. > >>> > >>> I think that this is further evidence that we should introduce a new > >>> pkey_alloc() mode and deprecate the old. To the extent possible, this > >>> thing should work the same way on x86 and POWER. > > > >> Do you propose to change POWER or to change x86? > > > > Sorry for being slow to reply. I propose to introduce a new > > PKEY_ALLOC_something variant on x86 and POWER and to make the behavior > > match on both. > So basically implement PKEY_ALLOC_SETSIGNAL for POWER, and keep the > existing (different) behavior without the flag? > Ram, would you be okay with that? Could you give me a hand if > necessary? (I assume we have silicon in-house because it's a > long-standing feature of the POWER platform which was simply dormant on > Linux until now.) > > It should at least update the values loaded when a signal > > is delivered and it should probably also update it for new threads. > I think we should keep inheritance for new threads and fork. pkey_alloc > only has a single access rights argument, which makes it hard to reuse > this interface if there are two (three) separate sets of access rights. Hmm. I can get on board with the idea that fork() / clone() / pthread_create() are all just special cases of the idea that the thread that *calls* them should have the right pkey values, and the latter is already busted given our inability to asynchronously propagate the new mode in pkey_alloc(). So let's so PKEY_ALLOC_SETSIGNAL as a starting point. One thing we could do, though: the current initual state on process creation is all access blocked on all keys. We could change it so that half the keys are fully blocked and half are read-only. Then we could add a PKEY_ALLOC_STRICT or similar that allocates a key with the correct initial state *and* does the setsignal thing. If there are no keys left with the correct initial state, then it fails. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org ([198.145.29.99]:60656 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933988AbeEIOlg (ORCPT ); Wed, 9 May 2018 10:41:36 -0400 Received: from mail-wr0-f174.google.com (mail-wr0-f174.google.com [209.85.128.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 4BB40217B4 for ; Wed, 9 May 2018 14:41:35 +0000 (UTC) Received: by mail-wr0-f174.google.com with SMTP id v60-v6so35860226wrc.7 for ; Wed, 09 May 2018 07:41:35 -0700 (PDT) MIME-Version: 1.0 References: <20180502132751.05B9F401F3041@oldenburg.str.redhat.com> <248faadb-e484-806f-1485-c34a72a9ca0b@intel.com> <822a28c9-5405-68c2-11bf-0c282887466d@redhat.com> <57459C6F-C8BA-4E2D-99BA-64F35C11FC05@amacapital.net> <6286ba0a-7e09-b4ec-e31f-bd091f5940ff@redhat.com> <20180503021058.GA5670@ram.oc3035372033.ibm.com> <927c8325-4c98-d7af-b921-6aafcf8fe992@redhat.com> <314e1a48-db94-9b37-8793-a95a2082c9e2@redhat.com> In-Reply-To: <314e1a48-db94-9b37-8793-a95a2082c9e2@redhat.com> From: Andy Lutomirski Date: Wed, 09 May 2018 14:41:23 +0000 Message-ID: Subject: Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics Content-Type: text/plain; charset="UTF-8" Sender: linux-arch-owner@vger.kernel.org List-ID: To: Florian Weimer Cc: Andrew Lutomirski , linuxram@us.ibm.com, Dave Hansen , Linux-MM , Linux API , linux-x86_64@vger.kernel.org, linux-arch , X86 ML , linuxppc-dev Message-ID: <20180509144123.MSnXYB_guXevIXXZBaLjftAzpNkWG-ea9Fo-OEm9NbI@z> On Tue, May 8, 2018 at 5:40 AM Florian Weimer wrote: > On 05/08/2018 04:49 AM, Andy Lutomirski wrote: > > On Mon, May 7, 2018 at 2:48 AM Florian Weimer wrote: > > > >> On 05/03/2018 06:05 AM, Andy Lutomirski wrote: > >>> On Wed, May 2, 2018 at 7:11 PM Ram Pai wrote: > >>> > >>>> On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote: > >>>>> > >>>>>> If I recall correctly, the POWER maintainer did express a strong > >>> desire > >>>>>> back then for (what is, I believe) their current semantics, which my > >>>>>> PKEY_ALLOC_SIGNALINHERIT patch implements for x86, too. > >>>>> > >>>>> Ram, I really really don't like the POWER semantics. Can you give > > some > >>>>> justification for them? Does POWER at least have an atomic way for > >>>>> userspace to modify just the key it wants to modify or, even better, > >>>>> special load and store instructions to use alternate keys? > >>> > >>>> I wouldn't call it POWER semantics. The way I implemented it on power > >>>> lead to the semantics, given that nothing was explicitly stated > >>>> about how the semantics should work within a signal handler. > >>> > >>> I think that this is further evidence that we should introduce a new > >>> pkey_alloc() mode and deprecate the old. To the extent possible, this > >>> thing should work the same way on x86 and POWER. > > > >> Do you propose to change POWER or to change x86? > > > > Sorry for being slow to reply. I propose to introduce a new > > PKEY_ALLOC_something variant on x86 and POWER and to make the behavior > > match on both. > So basically implement PKEY_ALLOC_SETSIGNAL for POWER, and keep the > existing (different) behavior without the flag? > Ram, would you be okay with that? Could you give me a hand if > necessary? (I assume we have silicon in-house because it's a > long-standing feature of the POWER platform which was simply dormant on > Linux until now.) > > It should at least update the values loaded when a signal > > is delivered and it should probably also update it for new threads. > I think we should keep inheritance for new threads and fork. pkey_alloc > only has a single access rights argument, which makes it hard to reuse > this interface if there are two (three) separate sets of access rights. Hmm. I can get on board with the idea that fork() / clone() / pthread_create() are all just special cases of the idea that the thread that *calls* them should have the right pkey values, and the latter is already busted given our inability to asynchronously propagate the new mode in pkey_alloc(). So let's so PKEY_ALLOC_SETSIGNAL as a starting point. One thing we could do, though: the current initual state on process creation is all access blocked on all keys. We could change it so that half the keys are fully blocked and half are read-only. Then we could add a PKEY_ALLOC_STRICT or similar that allocates a key with the correct initial state *and* does the setsignal thing. If there are no keys left with the correct initial state, then it fails.