linux-kernel - Re: [RFC PATCH v1 09/28] mm: abstract shadow stack vma behind `arch_is_shadow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZdakSM7KcXGgOoU6@debug.ba.rivosinc.com>
Date: Wed, 21 Feb 2024 17:32:56 -0800
From: Deepak Gupta <debug@...osinc.com>
To: David Hildenbrand <david@...hat.com>
Cc: rick.p.edgecombe@...el.com, broonie@...nel.org, Szabolcs.Nagy@....com,
	kito.cheng@...ive.com, keescook@...omium.org,
	ajones@...tanamicro.com, paul.walmsley@...ive.com,
	palmer@...belt.com, conor.dooley@...rochip.com, cleger@...osinc.com,
	atishp@...shpatra.org, alex@...ti.fr, bjorn@...osinc.com,
	alexghiti@...osinc.com, corbet@....net, aou@...s.berkeley.edu,
	oleg@...hat.com, akpm@...ux-foundation.org, arnd@...db.de,
	ebiederm@...ssion.com, shuah@...nel.org, brauner@...nel.org,
	guoren@...nel.org, samitolvanen@...gle.com, evan@...osinc.com,
	xiao.w.wang@...el.com, apatel@...tanamicro.com,
	mchitale@...tanamicro.com, waylingii@...il.com,
	greentime.hu@...ive.com, heiko@...ech.de, jszhang@...nel.org,
	shikemeng@...weicloud.com, charlie@...osinc.com,
	panqinglin2020@...as.ac.cn, willy@...radead.org,
	vincent.chen@...ive.com, andy.chiu@...ive.com, gerg@...nel.org,
	jeeheng.sia@...rfivetech.com, mason.huo@...rfivetech.com,
	ancientmodern4@...il.com, mathis.salmen@...sal.de,
	cuiyunhui@...edance.com, bhe@...hat.com, ruscur@...sell.cc,
	bgray@...ux.ibm.com, alx@...nel.org, baruch@...s.co.il,
	zhangqing@...ngson.cn, catalin.marinas@....com, revest@...omium.org,
	josh@...htriplett.org, joey.gouly@....com, shr@...kernel.io,
	omosnace@...hat.com, ojeda@...nel.org, jhubbard@...dia.com,
	linux-doc@...r.kernel.org, linux-riscv@...ts.infradead.org,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	linux-arch@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [RFC PATCH v1 09/28] mm: abstract shadow stack vma behind
 `arch_is_shadow_stack`

On Tue, Feb 13, 2024 at 11:34:59AM +0100, David Hildenbrand wrote:
>On 25.01.24 18:07, Deepak Gupta wrote:
>>On Thu, Jan 25, 2024 at 09:18:07AM +0100, David Hildenbrand wrote:
>>>On 25.01.24 07:21, debug@...osinc.com wrote:
>>>>From: Deepak Gupta <debug@...osinc.com>
>>>>
>>>>x86 has used VM_SHADOW_STACK (alias to VM_HIGH_ARCH_5) to encode shadow
>>>>stack VMA. VM_SHADOW_STACK is thus not possible on 32bit. Some arches may
>>>>need a way to encode shadow stack on 32bit and 64bit both and they may
>>>>encode this information differently in VMAs.
>>>>
>>>>This patch changes checks of VM_SHADOW_STACK flag in generic code to call
>>>>to a function `arch_is_shadow_stack` which will return true if arch
>>>>supports shadow stack and vma is shadow stack else stub returns false.
>>>>
>>>>There was a suggestion to name it as `vma_is_shadow_stack`. I preferred to
>>>>keep `arch` prefix in there because it's each arch specific.
>>>>
>>>>Signed-off-by: Deepak Gupta <debug@...osinc.com>
>>>>---
>>>>  include/linux/mm.h | 18 +++++++++++++++++-
>>>>  mm/gup.c           |  5 +++--
>>>>  mm/internal.h      |  2 +-
>>>>  3 files changed, 21 insertions(+), 4 deletions(-)
>>>>
>>>>diff --git a/include/linux/mm.h b/include/linux/mm.h
>>>>index dfe0e8118669..15c70fc677a3 100644
>>>>--- a/include/linux/mm.h
>>>>+++ b/include/linux/mm.h
>>>>@@ -352,6 +352,10 @@ extern unsigned int kobjsize(const void *objp);
>>>>   * for more details on the guard size.
>>>>   */
>>>>  # define VM_SHADOW_STACK	VM_HIGH_ARCH_5
>>>>+static inline bool arch_is_shadow_stack(vm_flags_t vm_flags)
>>>>+{
>>>>+	return (vm_flags & VM_SHADOW_STACK);
>>>>+}
>>>>  #endif
>>>>  #ifdef CONFIG_RISCV_USER_CFI
>>>>@@ -362,10 +366,22 @@ extern unsigned int kobjsize(const void *objp);
>>>>   * with VM_SHARED.
>>>>   */
>>>>  #define VM_SHADOW_STACK	VM_WRITE
>>>>+
>>>>+static inline bool arch_is_shadow_stack(vm_flags_t vm_flags)
>>>>+{
>>>>+	return ((vm_flags & (VM_WRITE | VM_READ | VM_EXEC)) == VM_WRITE);
>>>>+}
>>>>+
>>>
>>>Please no such hacks just to work around the 32bit vmflags limitation.
>>
>>As I said in another response. Noted.
>>And if there're no takers for 32bit on riscv (which highly likely is the case)
>>This will go away in next version of patchsets.
>
>Sorry for the (unusually for me) late reply. Simplifying to riscv64 
>sounds great.
>
>Alternatively, maybe VM_SHADOW_STACK is not even required at all on 
>riscv if we can teach all code to only stare at arch_is_shadow_stack() 
>instead.

Sorry for late reply.

I think for risc-v this can be done, if done in following way

static inline bool arch_is_shadow_stack(struct vm_flags_t vm_flags)
{
		return (vm_get_page_prot(vm_flags) == PAGE_SHADOWSTACK);
}

But doing above will require following

- Inventing a new PROT_XXX type (let's call it PROT_SHADOWSTACK) that
   is only exposed to kernel. PROT_SHADOWSTACK protection flag would allow
   `do_mmap` to do right thing and setup appropriate protection perms.
   We wouldn't want to expose this protection type to user mode (because
   `map_shadow_stack` already exists for that).
   Current patch series uses PROT_SHADOWSTACK because VM_SHADOW_STACK was
   aliased to VM_WRITE.

- As you said teach all the generic code as well to use arch_is_shadow_stack
   which might become complicated (can't say for sure)


>
>... but, just using the same VM_SHADOW_STACK will it all much cleaner. 
>Eventually, we can just stop playing arch-specific games with 
>arch_is_shadow_stack and VM_SHADOW_STACK.

Yeah for now, looks like easier thing to do.

>
>-- 
>Cheers,
>
>David / dhildenb
>