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@xxxxxxxxxxxx wrote:
From: Deepak Gupta <debug@xxxxxxxxxxxx>
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@xxxxxxxxxxxx>
---
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
