Re: [RFC PATCH v5 00/29] TDX KVM selftests

2024-06-05 Thread Edgecombe, Rick P
On Wed, 2024-06-05 at 16:34 -0500, Sagi Shahar wrote: > > We don't currently have plans to post a whole ~130 patch series. Instead we > > plan > > to post subsections out of the series as they slowly move into a maintainer > > branch. > > So this means that we won't be able to post an updated

Re: [RFC PATCH v5 00/29] TDX KVM selftests

2024-06-05 Thread Edgecombe, Rick P
On Wed, 2024-06-05 at 15:42 -0500, Sagi Shahar wrote: > > > Hm you're right, I was looking more narrowly because of the kvm-coco- > > > queue conflicts, for some of which even v19 might be too old. The MMU > > > prep series uses a much more recent kvm-coco-queue baseline. > > > > > > Rick, can we

Re: [PATCHv6 bpf-next 1/9] x86/shstk: Make return uprobe work with shadow stack

2024-05-30 Thread Edgecombe, Rick P
On Tue, 2024-05-21 at 12:48 +0200, Jiri Olsa wrote: > Currently the application with enabled shadow stack will crash > if it sets up return uprobe. The reason is the uretprobe kernel > code changes the user space task's stack, but does not update > shadow stack accordingly. > > Adding new

Re: [PATCHv5 bpf-next 6/8] x86/shstk: Add return uprobe support

2024-05-20 Thread Edgecombe, Rick P
On Mon, 2024-05-20 at 00:18 +0200, Jiri Olsa wrote: > anyway I think we can fix that in another way by using the optimized > trampoline, > but returning to the user space through iret when shadow stack is detected > (as I did in the first version, before you adjusted it to the sysret path). > >

Re: [PATCHv5 bpf-next 6/8] x86/shstk: Add return uprobe support

2024-05-15 Thread Edgecombe, Rick P
On Wed, 2024-05-15 at 17:26 +0200, Oleg Nesterov wrote: > > I think it will crash, there's explanation in the comment in > > tools/testing/selftests/x86/test_shadow_stack.c test > > OK, thanks... > > But test_shadow_stack.c doesn't do ARCH_PRCTL(ARCH_SHSTK_DISABLE) if > all the tests succeed ?

Re: [PATCHv5 bpf-next 6/8] x86/shstk: Add return uprobe support

2024-05-15 Thread Edgecombe, Rick P
On Wed, 2024-05-15 at 08:36 -0600, Jiri Olsa wrote: > > > > Let me ask a couple of really stupid questions. What if the shadow stack > > is "shorter" than the normal stack? I mean, The shadow stack could overflow if it is not big enough. However since the normal stack has return addresses and

Re: [PATCHv5 bpf-next 6/8] x86/shstk: Add return uprobe support

2024-05-15 Thread Edgecombe, Rick P
On Wed, 2024-05-15 at 13:35 +0200, Oleg Nesterov wrote: > Let me repeat I know nothing about shadow stacks, only tried to > read Documentation/arch/x86/shstk.rst few minutes ago ;) > > On 05/13, Jiri Olsa wrote: > > > > 1) current uretprobe which are not working at the moment and we change > >   

Re: [PATCHv5 bpf-next 6/8] x86/shstk: Add return uprobe support

2024-05-14 Thread Edgecombe, Rick P
On Mon, 2024-05-13 at 15:23 -0600, Jiri Olsa wrote: > so at the moment the patch 6 changes shadow stack for > > 1) current uretprobe which are not working at the moment and we change >    the top value of shadow stack with shstk_push_frame > 2) optimized uretprobe which needs to push new frame on

Re: [PATCHv5 bpf-next 6/8] x86/shstk: Add return uprobe support

2024-05-13 Thread Edgecombe, Rick P
On Mon, 2024-05-13 at 18:50 +0900, Masami Hiramatsu wrote: > > I guess it's doable, we'd need to keep both trampolines around, because > > shadow stack is enabled by app dynamically and use one based on the > > state of shadow stack when uretprobe is installed > > > > so you're worried the

Re: [PATCHv5 bpf-next 6/8] x86/shstk: Add return uprobe support

2024-05-09 Thread Edgecombe, Rick P
On Thu, 2024-05-09 at 10:30 +0200, Jiri Olsa wrote: > > Per the earlier discussion, this cannot be reached unless uretprobes are in > > use, > > which cannot happen without something with privileges taking an action. But > > are > > uretprobes ever used for monitoring applications where security

Re: [PATCHv5 bpf-next 6/8] x86/shstk: Add return uprobe support

2024-05-07 Thread Edgecombe, Rick P
On Tue, 2024-05-07 at 12:53 +0200, Jiri Olsa wrote: > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c > index 81e6ee95784d..ae6c3458a675 100644 > --- a/arch/x86/kernel/uprobes.c > +++ b/arch/x86/kernel/uprobes.c > @@ -406,6 +406,11 @@ SYSCALL_DEFINE0(uretprobe) > *

Re: [PATCH] mm: Remove mm argument from mm_get_unmapped_area()

2024-05-07 Thread Edgecombe, Rick P
On Mon, 2024-05-06 at 09:18 -0700, Christoph Hellwig wrote: > > On Mon, May 06, 2024 at 09:07:47AM -0700, Rick Edgecombe wrote: > > > > if (flags & MAP_FIXED) { > > > > /* Ok, don't mess with it. */ > > > > -   return mm_get_unmapped_area(current->mm, NULL, > >

Re: [PATCH] mm: Remove mm argument from mm_get_unmapped_area()

2024-05-07 Thread Edgecombe, Rick P
On Mon, 2024-05-06 at 12:32 -0400, Liam R. Howlett wrote: > > I like this patch. Thanks for taking a look. > > I think the context of current->mm is implied. IOW, could we call it > get_unmapped_area() instead?  There are other functions today that use > current->mm that don't start with

Re: [PATCHv4 bpf-next 2/7] uprobe: Add uretprobe syscall to speed up return probe

2024-05-03 Thread Edgecombe, Rick P
On Fri, 2024-05-03 at 22:17 +0200, Jiri Olsa wrote: > when uretprobe is created, kernel overwrites the return address on user > stack to point to user space trampoline, so the setup is in kernel hands I mean for uprobes in general. I'm didn't have any specific ideas in mind, but in general when

Re: [PATCHv4 bpf-next 2/7] uprobe: Add uretprobe syscall to speed up return probe

2024-05-03 Thread Edgecombe, Rick P
+Some more shadow stack folks from other archs. We are discussing how uretprobes work with shadow stack. Context: https://lore.kernel.org/lkml/ZjU4ganRF1Cbiug6@krava/ On Fri, 2024-05-03 at 21:18 +0200, Jiri Olsa wrote: > > hack below seems to fix it for the current uprobe setup, > we need

Re: [PATCHv4 bpf-next 2/7] uprobe: Add uretprobe syscall to speed up return probe

2024-05-03 Thread Edgecombe, Rick P
On Fri, 2024-05-03 at 15:04 +0200, Jiri Olsa wrote: > On Fri, May 03, 2024 at 01:34:53PM +0200, Peter Zijlstra wrote: > > On Thu, May 02, 2024 at 02:23:08PM +0200, Jiri Olsa wrote: > > > Adding uretprobe syscall instead of trap to speed up return probe. > > > > > > At the moment the uretprobe

Re: [PATCH 0/5] Handle set_memory_XXcrypted() errors in Hyper-V

2024-04-11 Thread Edgecombe, Rick P
On Wed, 2024-04-10 at 21:34 +, Wei Liu wrote: > > Applied to hyperv-fixes. Thanks. Thanks, and thanks to Michael for getting it across the finish line.

Re: [PATCH v4 02/14] mm: Switch mm->get_unmapped_area() to a flag

2024-03-27 Thread Edgecombe, Rick P
On Wed, 2024-03-27 at 15:15 +0200, Jarkko Sakkinen wrote: > I mean I believe the change itself makes sense, it is just not > fully documented in the commit message. Ah, I see. Yes, there could be more background on arch_pick_mmap_layout().

Re: [PATCH v4 02/14] mm: Switch mm->get_unmapped_area() to a flag

2024-03-27 Thread Edgecombe, Rick P
On Tue, 2024-03-26 at 23:38 -0700, Dan Williams wrote: > > +unsigned long > > +mm_get_unmapped_area(struct mm_struct *mm, struct file *file, > > +    unsigned long addr, unsigned long len, > > +    unsigned long pgoff, unsigned long flags) > > +{ > > Seems like a

Re: [PATCH v4 02/14] mm: Switch mm->get_unmapped_area() to a flag

2024-03-26 Thread Edgecombe, Rick P
On Tue, 2024-03-26 at 13:57 +0200, Jarkko Sakkinen wrote: > In which conditions which path is used during the initialization of mm > and why is this the case? It is an open claim in the current form. There is an arch_pick_mmap_layout() that arch's can have their own rules for. There is also a

Re: [RFC PATCH v2 00/19] PKS write protected page tables

2024-03-14 Thread Edgecombe, Rick P
On Thu, 2024-03-14 at 09:27 -0700, Kees Cook wrote: > On Mon, Aug 30, 2021 at 04:59:08PM -0700, Rick Edgecombe wrote: > > This is a second RFC for the PKS write protected tables concept. > > I'm sharing to > > show the progress to interested people. I'd also appreciate any > > comments, > >

Re: [PATCH v3 08/12] treewide: Use initializer for struct vm_unmapped_area_info

2024-03-13 Thread Edgecombe, Rick P
On Tue, 2024-03-12 at 20:18 -0700, Kees Cook wrote: > > Thanks! This looks to do exactly what it describes. :) > > Reviewed-by: Kees Cook Thanks! ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org

Re: [PATCH v3 08/12] treewide: Use initializer for struct vm_unmapped_area_info

2024-03-13 Thread Edgecombe, Rick P
On Tue, 2024-03-12 at 20:18 -0700, Kees Cook wrote: > > Thanks! This looks to do exactly what it describes. :) > > Reviewed-by: Kees Cook Thanks!

Re: [PATCH v3 07/12] powerpc: Use initializer for struct vm_unmapped_area_info

2024-03-13 Thread Edgecombe, Rick P
On Wed, 2024-03-13 at 06:44 +, Christophe Leroy wrote: > I understand from this text that, as agreed, this patch removes the > pointless/redundant zero-init of individual members. But it is not > what > is done, see below ? Err, right. I think I decided to leave it because it was already

Re: [RFC RFT PATCH 0/4] Handle set_memory_XXcrypted() errors in hyperv

2024-03-07 Thread Edgecombe, Rick P
On Thu, 2024-03-07 at 17:11 +, Michael Kelley wrote: > Using your patches plus the changes in my comments, I've > done most of the testing described above. The normal > paths work, and when I hack set_memory_encrypted() > to fail, the error paths correctly did not free the memory. > I checked

Re: [RFC v2.1 03/12] csky: Use initializer for struct vm_unmapped_area_info

2024-03-05 Thread Edgecombe, Rick P
On Sun, 2024-03-03 at 11:09 +0800, Guo Ren wrote: > LGTM, that's equivalent. > > Reviewed-by: Guo Ren Thanks!

Re: [RFC v2.1 07/12] powerpc: Use initializer for struct vm_unmapped_area_info

2024-03-05 Thread Edgecombe, Rick P
On Tue, 2024-03-05 at 11:51 +1100, Michael Ellerman wrote: > I gave it a quick boot test, all good. > > Acked-by: Michael Ellerman (powerpc) Thanks! Christophe was advocating for slight spin on this (not doing the member initializing in the declaration, but dropping the assignments that set 0):

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-03-04 Thread Edgecombe, Rick P
On Mon, 2024-03-04 at 18:00 +, Christophe Leroy wrote: > > Personally, I think a single patch that sets "= {}" for all of them > > and > > drop the all the "= 0" or "= NULL" assignments would be the > > cleanest way > > to go. > > I agree with Kees, set = {} and drop all the "something = 0;"

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-03-04 Thread Edgecombe, Rick P
On Mon, 2024-03-04 at 18:00 +, Christophe Leroy wrote: > > Personally, I think a single patch that sets "= {}" for all of them > > and > > drop the all the "= 0" or "= NULL" assignments would be the > > cleanest way > > to go. > > I agree with Kees, set = {} and drop all the "something = 0;"

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-03-04 Thread Edgecombe, Rick P
On Mon, 2024-03-04 at 18:00 +, Christophe Leroy wrote: > > Personally, I think a single patch that sets "= {}" for all of them > > and > > drop the all the "= 0" or "= NULL" assignments would be the > > cleanest way > > to go. > > I agree with Kees, set = {} and drop all the "something = 0;"

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-03-01 Thread Edgecombe, Rick P
On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote: > I totally understand. If the "uninitialized" warnings were actually > reliable, I would agree. I look at it this way: > > - initializations can be missed either in static initializers or via >   run time initializers. (So the risk of mistake

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-03-01 Thread Edgecombe, Rick P
On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote: > I totally understand. If the "uninitialized" warnings were actually > reliable, I would agree. I look at it this way: > > - initializations can be missed either in static initializers or via >   run time initializers. (So the risk of mistake

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-03-01 Thread Edgecombe, Rick P
On Wed, 2024-02-28 at 09:21 -0800, Kees Cook wrote: > I totally understand. If the "uninitialized" warnings were actually > reliable, I would agree. I look at it this way: > > - initializations can be missed either in static initializers or via >   run time initializers. (So the risk of mistake

Re: [RFC RFT PATCH 1/4] hv: Leak pages if set_memory_encrypted() fails

2024-03-01 Thread Edgecombe, Rick P
On Fri, 2024-03-01 at 20:21 +, Michael Kelley wrote: > > The Hyper-V case can actually be a third path when a paravisor > is being used.  In that case, for both TDX and SEV-SNP, the > hypervisor callbacks in __set_memory_enc_pgtable() go > to Hyper-V specific functions that talk to the

Re: [RFC RFT PATCH 1/4] hv: Leak pages if set_memory_encrypted() fails

2024-03-01 Thread Edgecombe, Rick P
On Fri, 2024-03-01 at 19:00 +, Michael Kelley wrote: > From: Rick Edgecombe Sent: Wednesday, > February 21, 2024 6:10 PM > > > > Historically, the preferred Subject prefix for changes to > connection.c has > been "Drivers: hv: vmbus:", not just "hv:".  Sometimes that > preference > isn't

Re: [RFC RFT PATCH 1/4] hv: Leak pages if set_memory_encrypted() fails

2024-03-01 Thread Edgecombe, Rick P
On Fri, 2024-03-01 at 09:26 +, Wei Liu wrote: > > Hyperv could free decrypted/shared pages if set_memory_encrypted() > > fails. > > "Hyper-V" throughout. Ok. > > > Leak the pages if this happens. > > > > Only compile tested. > > > > Cc: "K. Y. Srinivasan" > > Cc: Haiyang Zhang > > Cc:

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-28 Thread Edgecombe, Rick P
On Wed, 2024-02-28 at 13:22 +, Christophe Leroy wrote: > > Any preference? Or maybe am I missing your point and talking > > nonsense? > > > > So my preference would go to the addition of: > > info.new_field = 0; > > But that's very minor and if you think it is easier to manage and

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-28 Thread Edgecombe, Rick P
On Wed, 2024-02-28 at 13:22 +, Christophe Leroy wrote: > > Any preference? Or maybe am I missing your point and talking > > nonsense? > > > > So my preference would go to the addition of: > > info.new_field = 0; > > But that's very minor and if you think it is easier to manage and

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-28 Thread Edgecombe, Rick P
On Wed, 2024-02-28 at 13:22 +, Christophe Leroy wrote: > > Any preference? Or maybe am I missing your point and talking > > nonsense? > > > > So my preference would go to the addition of: > > info.new_field = 0; > > But that's very minor and if you think it is easier to manage and

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-27 Thread Edgecombe, Rick P
On Tue, 2024-02-27 at 18:16 +, Christophe Leroy wrote: > > > Why doing a full init of the struct when all fields are re- > > > written a few > > > lines after ? > > > > It's a nice change for robustness and makes future changes easier. > > It's > > not actually wasteful since the compiler

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-27 Thread Edgecombe, Rick P
On Tue, 2024-02-27 at 18:16 +, Christophe Leroy wrote: > > > Why doing a full init of the struct when all fields are re- > > > written a few > > > lines after ? > > > > It's a nice change for robustness and makes future changes easier. > > It's > > not actually wasteful since the compiler

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-27 Thread Edgecombe, Rick P
On Tue, 2024-02-27 at 18:16 +, Christophe Leroy wrote: > > > Why doing a full init of the struct when all fields are re- > > > written a few > > > lines after ? > > > > It's a nice change for robustness and makes future changes easier. > > It's > > not actually wasteful since the compiler

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-27 Thread Edgecombe, Rick P
On Tue, 2024-02-27 at 07:02 +, Christophe Leroy wrote: > > It could be possible to initialize the new field for each arch to > > 0, but > > instead simply inialize the field with a C99 struct inializing > > syntax. > > Why doing a full init of the struct when all fields are re-written a > few

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-27 Thread Edgecombe, Rick P
On Tue, 2024-02-27 at 07:02 +, Christophe Leroy wrote: > > It could be possible to initialize the new field for each arch to > > 0, but > > instead simply inialize the field with a C99 struct inializing > > syntax. > > Why doing a full init of the struct when all fields are re-written a > few

Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info

2024-02-27 Thread Edgecombe, Rick P
On Tue, 2024-02-27 at 07:02 +, Christophe Leroy wrote: > > It could be possible to initialize the new field for each arch to > > 0, but > > instead simply inialize the field with a C99 struct inializing > > syntax. > > Why doing a full init of the struct when all fields are re-written a > few

Re: [musl] Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace

2024-02-21 Thread Edgecombe, Rick P
On Wed, 2024-02-21 at 14:06 -0500, dal...@libc.org wrote: > Due to arbitrarily nestable signal frames, no, this does not suffice. > An interrupted operation using the lock could be arbitrarily delayed, > even never execute again, making any call to dlopen deadlock. Doh! Yep, it is not robust to

Re: [musl] Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace

2024-02-21 Thread Edgecombe, Rick P
On Wed, 2024-02-21 at 13:30 -0500, dal...@libc.org wrote: > > 3 is the cleanest and safest I think, and it was thought it might > > not > > need kernel help, due to a scheme Florian had to direct signals to > > specific threads. It's my preference at this point. > > The operations where the

Re: [musl] Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace

2024-02-21 Thread Edgecombe, Rick P
On Wed, 2024-02-21 at 12:57 -0500, dal...@libc.org wrote: > > This feels like it's getting complicated and I fear it may be an > > uphill > > struggle to get such code merged, at least for arm64.  My instinct > > is > > that it's going to be much more robust and generally tractable to > > let > >

Re: [musl] Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace

2024-02-20 Thread Edgecombe, Rick P
On Tue, 2024-02-20 at 18:59 -0500, Stefan O'Rear wrote: > > Ideally for riscv only writes would cause conversion, an incssp > underflow > which performs shadow stack reads would be able to fault early. Why can't makecontext() just clobber part of the low address side of the passed in stack with

Re: [musl] Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace

2024-02-20 Thread Edgecombe, Rick P
On Tue, 2024-02-20 at 18:11 -0800, Rick Edgecombe wrote: > Some specific cases that were still open were longjmp()ing off of a > custom userspace threading library stack, which may not have left a > token behind when it jumped to a new stack. And also, potentially off > of an alt shadow stack in

Re: [musl] Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace

2024-02-20 Thread Edgecombe, Rick P
On Tue, 2024-02-20 at 20:27 -0500, dal...@libc.org wrote: > > Then I think WRSS might fit your requirements better than what > > glibc > > did. It was considered a reduced security mode that made libc's job > > much easier and had better compatibility, but the last discussion > > was > > to try to

Re: [musl] Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace

2024-02-20 Thread Edgecombe, Rick P
On Tue, 2024-02-20 at 18:54 -0500, dal...@libc.org wrote: > On Tue, Feb 20, 2024 at 11:30:22PM +0000, Edgecombe, Rick P wrote: > > On Tue, 2024-02-20 at 13:57 -0500, Rich Felker wrote: > > > On Tue, Feb 20, 2024 at 06:41:05PM +0000, Edgecombe, Rick P > > > wrote: &g

Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace

2024-02-20 Thread Edgecombe, Rick P
On Tue, 2024-02-20 at 20:14 +, Mark Brown wrote: > > Hmm, could the shadow stack underflow onto the real stack then? Not > > sure how bad that is. INCSSP (incrementing the SSP register on x86) > > loops are not rare so it seems like something that could happen. > > Yes, they'd trash any pages

Re: [musl] Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace

2024-02-20 Thread Edgecombe, Rick P
On Tue, 2024-02-20 at 13:57 -0500, Rich Felker wrote: > On Tue, Feb 20, 2024 at 06:41:05PM +0000, Edgecombe, Rick P wrote: > > Hmm, could the shadow stack underflow onto the real stack then? Not > > sure how bad that is. INCSSP (incrementing the SSP register on x86) > >

Re: [PATCH v8 00/38] arm64/gcs: Provide support for GCS in userspace

2024-02-20 Thread Edgecombe, Rick P
Hi, I worked on the x86 kernel shadow stack support. I think it is an interesting suggestion. Some questions below, and I will think more on it. On Tue, 2024-02-20 at 11:36 -0500, Stefan O'Rear wrote: > While discussing the ABI implications of shadow stacks in the context > of > Zicfiss and musl

Re: [PATCH RFT v5 4/7] fork: Add shadow stack support to clone3()

2024-02-09 Thread Edgecombe, Rick P
On Sat, 2024-02-03 at 00:05 +, Mark Brown wrote: > +   if (args->shadow_stack) { > +   addr = args->shadow_stack; > +   size = args->shadow_stack_size; >   > -   size = adjust_shstk_size(stack_size); > -   addr = alloc_shstk(0, size, 0, false); > -   if

Re: [PATCH RFT v5 4/7] fork: Add shadow stack support to clone3()

2024-02-09 Thread Edgecombe, Rick P
On Fri, 2024-02-09 at 12:18 -0800, Rick Edgecombe wrote: > > So, don't we want to consume the token on the *new* task's MM, which > was already duplicated but still unmapped? In which case I think the > other arch's would need to GUP regardless of the existence of shadow > stack atomic ops. I

Re: [RFC PATCH v1 15/28] riscv/mm: Implement map_shadow_stack() syscall

2024-02-09 Thread Edgecombe, Rick P
On Wed, 2024-01-24 at 22:21 -0800, de...@rivosinc.com wrote: > From: Deepak Gupta > > As discussed extensively in the changelog for the addition of this > syscall on x86 ("x86/shstk: Introduce map_shadow_stack syscall") the > existing mmap() and madvise() syscalls do not map entirely well onto >

Re: [RFC PATCH v1 11/28] riscv: Implementing "PROT_SHADOWSTACK" on riscv

2024-02-09 Thread Edgecombe, Rick P
On Wed, 2024-01-24 at 22:21 -0800, de...@rivosinc.com wrote: > +   /* > +    * PROT_SHADOWSTACK is a kernel only protection flag on risc- > v. > +    * mmap doesn't expect PROT_SHADOWSTACK to be set by user > space. > +    * User space can rely on `map_shadow_stack` syscall to >

Re: [PATCH RFT v5 2/7] selftests: Provide helper header for shadow stack testing

2024-02-09 Thread Edgecombe, Rick P
On Sat, 2024-02-03 at 00:04 +, Mark Brown wrote: > While almost all users of shadow stacks should be relying on the > dynamic > linker and libc to enable the feature there are several low level > test > programs where it is useful to enable without any libc support, > allowing > testing

Re: [PATCH RFT v5 3/7] mm: Introduce ARCH_HAS_USER_SHADOW_STACK

2024-02-09 Thread Edgecombe, Rick P
On Sat, 2024-02-03 at 00:04 +, Mark Brown wrote: > Since multiple architectures have support for shadow stacks and we > need to > select support for this feature in several places in the generic code > provide a generic config option that the architectures can select. > > Suggested-by: David

Re: [PATCH RFT v5 0/7] fork: Support shadow stacks in clone3()

2024-02-09 Thread Edgecombe, Rick P
On Sat, 2024-02-03 at 00:04 +, Mark Brown wrote: > Please note that the x86 portions of this code are build tested only, > I > don't appear to have a system that can run CET avaible to me, I have > done testing with an integration into my pending work for GCS.  There > is > some possibility

Re: [PATCH RFT v5 4/7] fork: Add shadow stack support to clone3()

2024-02-09 Thread Edgecombe, Rick P
On Sat, 2024-02-03 at 00:05 +, Mark Brown wrote: > +static bool shstk_consume_token(struct task_struct *tsk, > +   unsigned long addr) > +{ > +   /* > +    * SSP is aligned, so reserved bits and mode bit are a zero, > just mark > +    * the token 64-bit.

Re: [PATCH v4 0/3] x86/hyperv: Mark CoCo VM pages not present when changing encrypted state

2024-01-16 Thread Edgecombe, Rick P

Re: [PATCH v4 1/3] x86/hyperv: Use slow_virt_to_phys() in page transition hypervisor callback

2024-01-16 Thread Edgecombe, Rick P

Re: [PATCH v3 1/3] x86/hyperv: Use slow_virt_to_phys() in page transition hypervisor callback

2024-01-12 Thread Edgecombe, Rick P
On Fri, 2024-01-12 at 15:07 +, Michael Kelley wrote: > The comment is Kirill Shutemov's suggestion based on comments in > an earlier version of the patch series.  See [1].   The intent is to > prevent > some later revision to slow_virt_to_phys() from adding a check for > the > present bit and

Re: [PATCH v3 1/3] x86/hyperv: Use slow_virt_to_phys() in page transition hypervisor callback

2024-01-11 Thread Edgecombe, Rick P
On Fri, 2024-01-05 at 10:30 -0800, mhkelle...@gmail.com wrote: > + * It is also used in callbacks for CoCo VM page transitions between > private > + * and shared because it works when the PRESENT bit is not set in > the leaf > + * PTE. In such cases, the state of the PTEs, including the PFN, is >

Re: [PATCH v3 2/3] x86/mm: Regularize set_memory_p() parameters and make non-static

2024-01-11 Thread Edgecombe, Rick P
On Fri, 2024-01-05 at 10:30 -0800, mhkelle...@gmail.com wrote: > From: Michael Kelley > > set_memory_p() is currently static.  It has parameters that don't > match set_memory_p() under arch/powerpc and that aren't congruent > with the other set_memory_* functions. There's no good reason for >

Re: [PATCH v3 3/3] x86/hyperv: Make encrypted/decrypted changes safe for load_unaligned_zeropad()

2024-01-11 Thread Edgecombe, Rick P
On Fri, 2024-01-05 at 10:30 -0800, mhkelle...@gmail.com wrote: >   * hv_vtom_set_host_visibility - Set specified memory visible to > host. >   * > @@ -521,7 +547,7 @@ static bool hv_vtom_set_host_visibility(unsigned > long kbuffer, int pagecount, bo >   > pfn_array =

Re: [PATCHv5 10/16] x86/tdx: Convert shared memory back to private on kexec

2024-01-05 Thread Edgecombe, Rick P
On Sat, 2024-01-06 at 03:59 +0300, kirill.shute...@linux.intel.com wrote: > But why? There's no concurrency at this point. Interrupts are > disabled and > only one CPU is active. Nobody can touch the memory relevant for the > PTE. Oh, right, sorry. I had thought there could be other CPUs active

Re: [PATCHv5 10/16] x86/tdx: Convert shared memory back to private on kexec

2024-01-05 Thread Edgecombe, Rick P
The break apart looks good. On Sat, 2023-12-23 at 02:52 +0300, Kirill A. Shutemov wrote: > +   while (addr < end) { > +   unsigned long size; > +   unsigned int level; > +   pte_t *pte; > + > +   pte = lookup_address(addr, ); > +   

Re: [PATCH v7 02/39] prctl: arch-agnostic prctl for shadow stack

2023-12-12 Thread Edgecombe, Rick P
+Mike, who did the CRIU work Re: https://lore.kernel.org/lkml/e1362732ba86990b7707d3f5b785358b77c5f896.ca...@intel.com/ On Tue, 2023-12-12 at 20:26 +, Mark Brown wrote: > The set of locked features is read/write via ptrace in my arm64 > series, > that's architecture specific unfortunately

Re: [PATCH v7 02/39] prctl: arch-agnostic prctl for shadow stack

2023-12-12 Thread Edgecombe, Rick P
On Wed, 2023-11-22 at 09:42 +, Mark Brown wrote: > These features are expected to be inherited by new threads and > cleared > on exec(), unknown features should be rejected for enable but > accepted > for locking (in order to allow for future proofing). The reason why I stuck with arch_prctl

Re: [PATCHv4 10/14] x86/tdx: Convert shared memory back to private on kexec

2023-12-06 Thread Edgecombe, Rick P
On Wed, 2023-12-06 at 18:07 +0300, kirill.shute...@linux.intel.com wrote: >  I can't think of any non-ridiculous way to handle this case. Maybe > we > > need VMM help. > > Do you see a specific way how VMM can help here? I didn't have a specific idea. I was just thinking that the problem is that

Re: [PATCHv4 10/14] x86/tdx: Convert shared memory back to private on kexec

2023-12-05 Thread Edgecombe, Rick P
On Tue, 2023-12-05 at 03:45 +0300, Kirill A. Shutemov wrote:  > +static void tdx_kexec_unshare_mem(bool crash) > +{ > +   unsigned long addr, end; > +   long found = 0, shared; > + > +   /* Stop new private<->shared conversions */ > +   conversion_allowed = false; I wonder if this

Re: [PATCH RFT v4 5/5] kselftest/clone3: Test shadow stack support

2023-12-05 Thread Edgecombe, Rick P
On Tue, 2023-12-05 at 16:43 +, Mark Brown wrote: > Right, it's a small and fairly easily auditable list - it's more > about > the app than the double enable which was what I thought your concern > was.  It's a bit annoying definitely and not something we want to do > in > general but for

Re: [PATCH RFT v4 2/5] fork: Add shadow stack support to clone3()

2023-12-05 Thread Edgecombe, Rick P
On Tue, 2023-12-05 at 15:51 +, Mark Brown wrote: > On Tue, Dec 05, 2023 at 12:26:57AM +0000, Edgecombe, Rick P wrote: > > On Tue, 2023-11-28 at 18:22 +, Mark Brown wrote: > > > > -   size = adjust_shstk_size(stack_size); > > > +   size = adjust_shstk_

Re: [PATCH RFT v4 5/5] kselftest/clone3: Test shadow stack support

2023-12-05 Thread Edgecombe, Rick P
On Tue, 2023-12-05 at 15:05 +, Mark Brown wrote: > > But I wonder if the clone3 test should get its shadow stack enabled > > the > > conventional elf bit way. So if it's all there (HW, kernel, glibc) > > then > > the test will run with shadow stack. Otherwise the test will run > > without

Re: [PATCH RFT v4 2/5] fork: Add shadow stack support to clone3()

2023-12-04 Thread Edgecombe, Rick P
On Tue, 2023-11-28 at 18:22 +, Mark Brown wrote: > -unsigned long shstk_alloc_thread_stack(struct task_struct *tsk, > unsigned long clone_flags, > -  unsigned long stack_size) > +unsigned long shstk_alloc_thread_stack(struct task_struct *tsk, > + 

Re: [PATCH RFT v4 5/5] kselftest/clone3: Test shadow stack support

2023-12-04 Thread Edgecombe, Rick P
On Tue, 2023-11-28 at 18:22 +, Mark Brown wrote: > + > +#define ENABLE_SHADOW_STACK > +static inline void enable_shadow_stack(void) > +{ > +   int ret = ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK); > +   if (ret == 0) > +   shadow_stack_enabled = true; > +} > + >

Re: [RFC PATCH v2 17/19] heki: x86: Update permissions counters during text patching

2023-11-30 Thread Edgecombe, Rick P
On Wed, 2023-11-29 at 15:07 -0600, Madhavan T. Venkataraman wrote: > Threat Model > > > In the threat model in Heki, the attacker is a user space attacker > who exploits > a kernel vulnerability to gain more privileges or bypass the kernel's > access > control and self-protection

Re: [RFC PATCH v2 17/19] heki: x86: Update permissions counters during text patching

2023-11-30 Thread Edgecombe, Rick P
On Wed, 2023-11-29 at 15:07 -0600, Madhavan T. Venkataraman wrote: > Threat Model > > > In the threat model in Heki, the attacker is a user space attacker > who exploits > a kernel vulnerability to gain more privileges or bypass the kernel's > access > control and self-protection

Re: [RFC PATCH v2 17/19] heki: x86: Update permissions counters during text patching

2023-11-30 Thread Edgecombe, Rick P
On Wed, 2023-11-29 at 15:07 -0600, Madhavan T. Venkataraman wrote: > Threat Model > > > In the threat model in Heki, the attacker is a user space attacker > who exploits > a kernel vulnerability to gain more privileges or bypass the kernel's > access > control and self-protection

Re: [PATCH RFT v4 0/5] fork: Support shadow stacks in clone3()

2023-11-30 Thread Edgecombe, Rick P
On Thu, 2023-11-30 at 21:51 +, Mark Brown wrote: > On Thu, Nov 30, 2023 at 07:00:58PM +, Catalin Marinas wrote: > > > My hope when looking at the arm64 patches was that we can > > completely > > avoid the kernel allocation/deallocation of the shadow stack since > > it > > doesn't need to

Re: [PATCH v2 4/8] x86/sev: Enable PVALIDATE for PFNs without a valid virtual address

2023-11-28 Thread Edgecombe, Rick P
On Tue, 2023-11-28 at 18:08 +, Michael Kelley wrote: > > > > Sort of separately, if those vmalloc objections can't be worked > > through, did you consider doing something like text_poke() does > > (create > > the temporary mapping in a temporary MM) for pvalidate purposes? I > > don't know

Re: [PATCH v2 2/8] x86/mm: Don't do a TLB flush if changing a PTE that isn't marked present

2023-11-27 Thread Edgecombe, Rick P
On Tue, 2023-11-21 at 13:20 -0800, mhkelle...@gmail.com wrote: > --- a/arch/x86/mm/pat/set_memory.c > +++ b/arch/x86/mm/pat/set_memory.c > @@ -1636,7 +1636,10 @@ static int __change_page_attr(struct cpa_data > *cpa, int primary) > */ > if (pte_val(old_pte) !=

Re: [PATCH v2 4/8] x86/sev: Enable PVALIDATE for PFNs without a valid virtual address

2023-11-27 Thread Edgecombe, Rick P
On Tue, 2023-11-21 at 13:20 -0800, mhkelle...@gmail.com wrote: > +static int pvalidate_pfn(unsigned long vaddr, unsigned int size, > +    unsigned long pfn, bool validate, int *rc2) > +{ > +   int rc; > +   struct page *page = pfn_to_page(pfn); > + > +   *rc2 =

Re: [PATCH RFC RFT v2 5/5] kselftest/clone3: Test shadow stack support

2023-11-17 Thread Edgecombe, Rick P
On Tue, 2023-11-14 at 15:11 -0800, Rick Edgecombe wrote: > The other tests passed in both cases. I'm going to dig into the other > parts now but can circle back if it's not obvious what's going on > there. Finally got back to this. I think it's just a problem with the shadow stack detection logic

Re: [PATCH RFC RFT v2 2/5] fork: Add shadow stack support to clone3()

2023-11-17 Thread Edgecombe, Rick P
On Thu, 2023-11-16 at 18:41 +, Mark Brown wrote: > > What about a CLONE_NEW_SHSTK for clone3 that forces a new shadow > > stack? > > So keep the existing logic, but the new flag can override the logic > > for > > !CLONE_VM and CLONE_VFORK if the caller wants. The behavior of > >

Re: [PATCH RFC RFT v2 2/5] fork: Add shadow stack support to clone3()

2023-11-16 Thread Edgecombe, Rick P
On Thu, 2023-11-16 at 18:14 +, Mark Brown wrote: > On Thu, Nov 16, 2023 at 12:52:09AM +0000, Edgecombe, Rick P wrote: > > On Wed, 2023-11-15 at 18:43 +, Mark Brown wrote: > > > > end marker token (0) needs it i guess. > > > > x86 doesn't currently ha

Re: [PATCH RFC RFT v2 2/5] fork: Add shadow stack support to clone3()

2023-11-16 Thread Edgecombe, Rick P
On Thu, 2023-11-16 at 15:35 +, Mark Brown wrote: > On Thu, Nov 16, 2023 at 01:55:07PM +, > szabolcs.n...@arm.com wrote: > > The 11/16/2023 12:33, Mark Brown wrote: > > > On Thu, Nov 16, 2023 at 10:32:06AM +, > > > szabolcs.n...@arm.com wrote: > > > > > i guess the tricky case is

Re: [PATCH RFC RFT v2 2/5] fork: Add shadow stack support to clone3()

2023-11-15 Thread Edgecombe, Rick P
On Wed, 2023-11-15 at 18:43 +, Mark Brown wrote: > > end marker token (0) needs it i guess. > > x86 doesn't currently have end markers.  Actually, that's a point - > should we add a flag for specifying the use of end markers here? > There's code in my map_shadow_stack() implementation for

Re: [PATCH RFC RFT v2 2/5] fork: Add shadow stack support to clone3()

2023-11-14 Thread Edgecombe, Rick P
On Tue, 2023-11-14 at 20:05 +, Mark Brown wrote: > diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c > index 59e15dd8d0f8..7ffe90010587 100644 > --- a/arch/x86/kernel/shstk.c > +++ b/arch/x86/kernel/shstk.c > @@ -191,18 +191,38 @@ void reset_thread_features(void) >

Re: [PATCH RFC RFT v2 1/5] mm: Introduce ARCH_HAS_USER_SHADOW_STACK

2023-11-14 Thread Edgecombe, Rick P
On Tue, 2023-11-14 at 20:05 +, Mark Brown wrote: > +config ARCH_HAS_USER_SHADOW_STACK > +   bool > +   help > + The architecture has hardware support for userspace shadow > call > +  stacks (eg, x86 CET, arm64 GCS, RISC-V Zisslpcfi). I feel like normally a patch like

Re: [PATCH RFC RFT v2 5/5] kselftest/clone3: Test shadow stack support

2023-11-14 Thread Edgecombe, Rick P
On Tue, 2023-11-14 at 20:05 +, Mark Brown wrote: > +static void test_shadow_stack_supported(void) > +{ > +    long shadow_stack; > + > +   shadow_stack = syscall(__NR_map_shadow_stack, 0, > getpagesize(), 0); Hmm, x86 fails this call if user shadow stack is not supported in the HW or

Re: [PATCH RFC RFT 2/5] fork: Add shadow stack support to clone3()

2023-10-27 Thread Edgecombe, Rick P
On Fri, 2023-10-27 at 12:49 +0100, szabolcs.n...@arm.com wrote: > no. the lifetime is the issue: a stack in principle can outlive > a thread and resumed even after the original thread exited. > for that to work the shadow stack has to outlive the thread too. Hmm, this makes me think about the

Re: [PATCH RFC RFT 2/5] fork: Add shadow stack support to clone3()

2023-10-26 Thread Edgecombe, Rick P
On Thu, 2023-10-26 at 13:40 -0700, Deepak Gupta wrote: > > FWIW, from arch specific perspective, RISC-V shadow stack extension > has > `ssamoswap` to perform this token exchange. But I understand x86 has > this > limitation (not sure about arm GCS). > >  From security perspective:-- > Someone

Re: [PATCH RFC RFT 2/5] fork: Add shadow stack support to clone3()

2023-10-26 Thread Edgecombe, Rick P
On Thu, 2023-10-26 at 18:53 +0100, Mark Brown wrote: > Particularly given my inability to test x86 I'm certainly way > more happy pushing this series forward implementing size only than I > am > doing token validation. I can help with testing/development once we get the plan settled on.

Re: [PATCH RFC RFT 2/5] fork: Add shadow stack support to clone3()

2023-10-26 Thread Edgecombe, Rick P
On Mon, 2023-10-23 at 19:32 +0100, Mark Brown wrote: > Right.  We're already adding the cost of the extra map_shadow_stack() > so > it doesn't seem that out of scope.  We could also allow clone3() to > be > used for allocation, potentially removing the ability to specify the > address entirely and

Re: [PATCH RFC RFT 2/5] fork: Add shadow stack support to clone3()

2023-10-23 Thread Edgecombe, Rick P
+Some security folks On Mon, 2023-10-23 at 14:20 +0100, Mark Brown wrote: > Unlike with the normal stack there is no API for configuring the the > shadow > stack for a new thread, instead the kernel will dynamically allocate > a new > shadow stack with the same size as the normal stack. This

  1   2   3   4   >