Re: [NEEDS-REVIEW] Re: [PATCH v11 25/25] x86/cet/shstk: Add arch_prctl functions for shadow stack

From: Borislav Petkov <bp@alien8.de>
To: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
Cc: "Lutomirski, Andy" <luto@kernel.org>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	"bsingharora@gmail.com" <bsingharora@gmail.com>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"esyr@redhat.com" <esyr@redhat.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"rdunlap@infradead.org" <rdunlap@infradead.org>,
	"keescook@chromium.org" <keescook@chromium.org>,
	"Yu, Yu-cheng" <yu-cheng.yu@intel.com>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"fweimer@redhat.com" <fweimer@redhat.com>,
	"nadav.amit@gmail.com" <nadav.amit@gmail.com>,
	"jannh@google.com" <jannh@google.com>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"kcc@google.com" <kcc@google.com>,
	"oleg@redhat.com" <oleg@redhat.com>,
	"hjl.tools@gmail.com" <hjl.tools@gmail.com>,
	"pavel@ucw.cz" <pavel@ucw.cz>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"Yang, Weijiang" <weijiang.yang@intel.com>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"Moreira, Joao" <joao.moreira@intel.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"mike.kravetz@oracle.com" <mike.kravetz@oracle.com>,
	"x86@kernel.org" <x86@kernel.org>,
	"tarasmadan@google.com" <tarasmadan@google.com>,
	"Dave.Martin@arm.com" <Dave.Martin@arm.com>,
	"vedvyas.shanbhogue@intel.com" <vedvyas.shanbhogue@intel.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
	"gorcunov@gmail.com" <gorcunov@gmail.com>
Subject: Re: [NEEDS-REVIEW] Re: [PATCH v11 25/25] x86/cet/shstk: Add arch_prctl functions for shadow stack
Date: Tue, 14 Sep 2021 11:53:06 +0200	[thread overview]
Message-ID: <YUBxAut2PtGzX/6k@zn.tnic> (raw)
In-Reply-To: <45c62101c065ed7e728fadac7207866bf8c36ec4.camel@intel.com>

On Tue, Sep 14, 2021 at 01:33:02AM +0000, Edgecombe, Rick P wrote:
> The original prctl solution prevents this case since the kernel did the
> allocation and restore token setup, but of course it had other issues.
> The other ideas discussed previously were a new syscall, or some sort
> of new madvise() operation that could be involved in setting up shadow
> stack, such that it is never writable in userspace.

If I had to choose - and this is only my 2¢ anyway - I'd opt for this
until there's a really good reason for allowing shstk programs to fiddle
with their own shstk. Maybe there is but allowing them to do that sounds
to me like: "ew, why do we go to all this trouble to have shadow stacks
if programs would be allowed to fumble with it themselves? Might as well
not do shadow stacks at all."

And if/when there is a good reason, the API should be defined and
discussed properly at first, before we expose it to luserspace, ofc.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette