* [PATCH] x86/shstk: Enable shadow stack for x32
@ 2024-03-15 14:04 H.J. Lu
2024-03-15 14:20 ` Edgecombe, Rick P
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: H.J. Lu @ 2024-03-15 14:04 UTC (permalink / raw
To: linux-kernel; +Cc: x86, Rick P Edgecombe
1. Add shadow stack support to x32 signal.
2. Use the 64-bit map_shadow_stack syscall for x32.
3. Set up shadow stack for x32.
Tested with shadow stack enabled x32 glibc on Intel Tiger Lake.
Cc: Rick P Edgecombe <rick.p.edgecombe@intel.com>
Tested-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
arch/x86/entry/syscalls/syscall_64.tbl | 2 +-
arch/x86/kernel/shstk.c | 4 ++--
arch/x86/kernel/signal_64.c | 6 ++++++
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 7e8d46f4147f..cc78226ffc35 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -374,7 +374,7 @@
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
451 common cachestat sys_cachestat
452 common fchmodat2 sys_fchmodat2
-453 64 map_shadow_stack sys_map_shadow_stack
+453 common map_shadow_stack sys_map_shadow_stack
454 common futex_wake sys_futex_wake
455 common futex_wait sys_futex_wait
456 common futex_requeue sys_futex_requeue
diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c
index 59e15dd8d0f8..6f1e9883f074 100644
--- a/arch/x86/kernel/shstk.c
+++ b/arch/x86/kernel/shstk.c
@@ -163,8 +163,8 @@ static int shstk_setup(void)
if (features_enabled(ARCH_SHSTK_SHSTK))
return 0;
- /* Also not supported for 32 bit and x32 */
- if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK) || in_32bit_syscall())
+ /* Also not supported for 32 bit */
+ if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK) || in_ia32_syscall())
return -EOPNOTSUPP;
size = adjust_shstk_size(0);
diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
index 23d8aaf8d9fd..8a94053c5444 100644
--- a/arch/x86/kernel/signal_64.c
+++ b/arch/x86/kernel/signal_64.c
@@ -315,6 +315,9 @@ int x32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs)
uc_flags = frame_uc_flags(regs);
+ if (setup_signal_shadow_stack(ksig))
+ return -EFAULT;
+
if (!user_access_begin(frame, sizeof(*frame)))
return -EFAULT;
@@ -377,6 +380,9 @@ COMPAT_SYSCALL_DEFINE0(x32_rt_sigreturn)
if (!restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
goto badframe;
+ if (restore_signal_shadow_stack())
+ goto badframe;
+
if (compat_restore_altstack(&frame->uc.uc_stack))
goto badframe;
--
2.44.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-15 14:04 [PATCH] x86/shstk: Enable shadow stack for x32 H.J. Lu
@ 2024-03-15 14:20 ` Edgecombe, Rick P
2024-03-15 14:34 ` H.J. Lu
2024-03-22 10:59 ` [tip: x86/shstk] x86/shstk: Enable shadow stacks " tip-bot2 for H.J. Lu
2024-03-22 16:49 ` [PATCH] x86/shstk: Enable shadow stack " Dave Hansen
2 siblings, 1 reply; 16+ messages in thread
From: Edgecombe, Rick P @ 2024-03-15 14:20 UTC (permalink / raw
To: linux-kernel@vger.kernel.org, hjl.tools@gmail.com; +Cc: x86@kernel.org
On Fri, 2024-03-15 at 07:04 -0700, H.J. Lu wrote:
> 1. Add shadow stack support to x32 signal.
> 2. Use the 64-bit map_shadow_stack syscall for x32.
> 3. Set up shadow stack for x32.
>
> Tested with shadow stack enabled x32 glibc on Intel Tiger Lake.
>
> Cc: Rick P Edgecombe <rick.p.edgecombe@intel.com>
> Tested-by: H.J. Lu <hjl.tools@gmail.com>
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
How many people do you think will use this?
I would have thought it would require more changes for basic x32
operation. What was the testing exactly?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-15 14:20 ` Edgecombe, Rick P
@ 2024-03-15 14:34 ` H.J. Lu
2024-03-22 14:07 ` Edgecombe, Rick P
0 siblings, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2024-03-15 14:34 UTC (permalink / raw
To: Edgecombe, Rick P; +Cc: linux-kernel@vger.kernel.org, x86@kernel.org
On Fri, Mar 15, 2024 at 7:20 AM Edgecombe, Rick P
<rick.p.edgecombe@intel.com> wrote:
>
> On Fri, 2024-03-15 at 07:04 -0700, H.J. Lu wrote:
> > 1. Add shadow stack support to x32 signal.
> > 2. Use the 64-bit map_shadow_stack syscall for x32.
> > 3. Set up shadow stack for x32.
> >
> > Tested with shadow stack enabled x32 glibc on Intel Tiger Lake.
> >
> > Cc: Rick P Edgecombe <rick.p.edgecombe@intel.com>
> > Tested-by: H.J. Lu <hjl.tools@gmail.com>
> > Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
>
> How many people do you think will use this?
>
> I would have thought it would require more changes for basic x32
This is all needed.
> operation. What was the testing exactly?
I configured x32 glibc with --enable-cet, build glibc and
run all glibc tests with shadow stack enabled. There are
no regressions. I verified that shadow stack is enabled
via /proc/pid/status.
--
H.J.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [tip: x86/shstk] x86/shstk: Enable shadow stacks for x32
2024-03-15 14:04 [PATCH] x86/shstk: Enable shadow stack for x32 H.J. Lu
2024-03-15 14:20 ` Edgecombe, Rick P
@ 2024-03-22 10:59 ` tip-bot2 for H.J. Lu
2024-03-22 16:49 ` Borislav Petkov
2024-03-22 16:49 ` [PATCH] x86/shstk: Enable shadow stack " Dave Hansen
2 siblings, 1 reply; 16+ messages in thread
From: tip-bot2 for H.J. Lu @ 2024-03-22 10:59 UTC (permalink / raw
To: linux-tip-commits
Cc: H.J. Lu, Ingo Molnar, Edgecombe, Rick P, H. Peter Anvin,
Linus Torvalds, Andy Lutomirski, x86, linux-kernel
The following commit has been merged into the x86/shstk branch of tip:
Commit-ID: 2883f01ec37dd8668e7222dfdb5980c86fdfe277
Gitweb: https://git.kernel.org/tip/2883f01ec37dd8668e7222dfdb5980c86fdfe277
Author: H.J. Lu <hjl.tools@gmail.com>
AuthorDate: Fri, 15 Mar 2024 07:04:33 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Fri, 22 Mar 2024 10:17:11 +01:00
x86/shstk: Enable shadow stacks for x32
1. Add shadow stack support to x32 signal.
2. Use the 64-bit map_shadow_stack syscall for x32.
3. Set up shadow stack for x32.
Tested with shadow stack enabled x32 glibc on Intel Tiger Lake:
I configured x32 glibc with --enable-cet, build glibc and
run all glibc tests with shadow stack enabled. There are
no regressions. I verified that shadow stack is enabled
via /proc/pid/status.
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Tested-by: H.J. Lu <hjl.tools@gmail.com>
Cc: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Link: https://lore.kernel.org/r/20240315140433.1966543-1-hjl.tools@gmail.com
---
arch/x86/entry/syscalls/syscall_64.tbl | 2 +-
arch/x86/kernel/shstk.c | 4 ++--
arch/x86/kernel/signal_64.c | 6 ++++++
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 7e8d46f..cc78226 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -374,7 +374,7 @@
450 common set_mempolicy_home_node sys_set_mempolicy_home_node
451 common cachestat sys_cachestat
452 common fchmodat2 sys_fchmodat2
-453 64 map_shadow_stack sys_map_shadow_stack
+453 common map_shadow_stack sys_map_shadow_stack
454 common futex_wake sys_futex_wake
455 common futex_wait sys_futex_wait
456 common futex_requeue sys_futex_requeue
diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c
index 59e15dd..6f1e988 100644
--- a/arch/x86/kernel/shstk.c
+++ b/arch/x86/kernel/shstk.c
@@ -163,8 +163,8 @@ static int shstk_setup(void)
if (features_enabled(ARCH_SHSTK_SHSTK))
return 0;
- /* Also not supported for 32 bit and x32 */
- if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK) || in_32bit_syscall())
+ /* Also not supported for 32 bit */
+ if (!cpu_feature_enabled(X86_FEATURE_USER_SHSTK) || in_ia32_syscall())
return -EOPNOTSUPP;
size = adjust_shstk_size(0);
diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
index 23d8aaf..8a94053 100644
--- a/arch/x86/kernel/signal_64.c
+++ b/arch/x86/kernel/signal_64.c
@@ -315,6 +315,9 @@ int x32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs)
uc_flags = frame_uc_flags(regs);
+ if (setup_signal_shadow_stack(ksig))
+ return -EFAULT;
+
if (!user_access_begin(frame, sizeof(*frame)))
return -EFAULT;
@@ -377,6 +380,9 @@ COMPAT_SYSCALL_DEFINE0(x32_rt_sigreturn)
if (!restore_sigcontext(regs, &frame->uc.uc_mcontext, uc_flags))
goto badframe;
+ if (restore_signal_shadow_stack())
+ goto badframe;
+
if (compat_restore_altstack(&frame->uc.uc_stack))
goto badframe;
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-15 14:34 ` H.J. Lu
@ 2024-03-22 14:07 ` Edgecombe, Rick P
2024-03-22 14:28 ` Edgecombe, Rick P
2024-03-22 15:06 ` H.J. Lu
0 siblings, 2 replies; 16+ messages in thread
From: Edgecombe, Rick P @ 2024-03-22 14:07 UTC (permalink / raw
To: hjl.tools@gmail.com; +Cc: linux-kernel@vger.kernel.org, x86@kernel.org
On Fri, 2024-03-15 at 07:34 -0700, H.J. Lu wrote:
> > How many people do you think will use this?
I'm concerned that the only use of this will ever be exercise via the
glibc unit tests, but will still require work to support.
> >
> > I would have thought it would require more changes for basic x32
>
> This is all needed.
>
> > operation. What was the testing exactly?
>
> I configured x32 glibc with --enable-cet, build glibc and
> run all glibc tests with shadow stack enabled. There are
> no regressions. I verified that shadow stack is enabled
> via /proc/pid/status.
The shadow stack is supposed to be mapped above 4G, so how is this
supposed to work for x32?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-22 14:07 ` Edgecombe, Rick P
@ 2024-03-22 14:28 ` Edgecombe, Rick P
2024-03-22 15:06 ` H.J. Lu
1 sibling, 0 replies; 16+ messages in thread
From: Edgecombe, Rick P @ 2024-03-22 14:28 UTC (permalink / raw
To: hjl.tools@gmail.com; +Cc: linux-kernel@vger.kernel.org, x86@kernel.org
On Fri, 2024-03-22 at 07:07 -0700, Rick Edgecombe wrote:
> On Fri, 2024-03-15 at 07:34 -0700, H.J. Lu wrote:
> > > How many people do you think will use this?
>
> I'm concerned that the only use of this will ever be exercise via the
> glibc unit tests, but will still require work to support.
To elaborate more on this... The main usage of shadow stack is
security, and comes with some overhead. IIUC the main usage of x32 is
performance benchmarking type stuff. Why would someone want to use
shadow stack and x32 together?
>
> > >
> > > I would have thought it would require more changes for basic x32
> >
> > This is all needed.
> >
> > > operation. What was the testing exactly?
> >
> > I configured x32 glibc with --enable-cet, build glibc and
> > run all glibc tests with shadow stack enabled. There are
> > no regressions. I verified that shadow stack is enabled
> > via /proc/pid/status.
>
> The shadow stack is supposed to be mapped above 4G, so how is this
> supposed to work for x32?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-22 14:07 ` Edgecombe, Rick P
2024-03-22 14:28 ` Edgecombe, Rick P
@ 2024-03-22 15:06 ` H.J. Lu
2024-03-22 15:58 ` Edgecombe, Rick P
1 sibling, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2024-03-22 15:06 UTC (permalink / raw
To: Edgecombe, Rick P; +Cc: linux-kernel@vger.kernel.org, x86@kernel.org
On Fri, Mar 22, 2024 at 7:07 AM Edgecombe, Rick P
<rick.p.edgecombe@intel.com> wrote:
>
> On Fri, 2024-03-15 at 07:34 -0700, H.J. Lu wrote:
> > > How many people do you think will use this?
>
> I'm concerned that the only use of this will ever be exercise via the
> glibc unit tests, but will still require work to support.
Correct. A small glibc change is needed. Will post it after
my kernel change is merged.
> > >
> > > I would have thought it would require more changes for basic x32
> >
> > This is all needed.
> >
> > > operation. What was the testing exactly?
> >
> > I configured x32 glibc with --enable-cet, build glibc and
> > run all glibc tests with shadow stack enabled. There are
> > no regressions. I verified that shadow stack is enabled
> > via /proc/pid/status.
>
> The shadow stack is supposed to be mapped above 4G, so how is this
> supposed to work for x32?
This is not what I see:
(gdb) info reg
...
pl3_ssp 0xf7dcbfe8 0xf7dcbfe8
--
H.J.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-22 15:06 ` H.J. Lu
@ 2024-03-22 15:58 ` Edgecombe, Rick P
2024-03-22 16:07 ` H.J. Lu
0 siblings, 1 reply; 16+ messages in thread
From: Edgecombe, Rick P @ 2024-03-22 15:58 UTC (permalink / raw
To: hjl.tools@gmail.com; +Cc: linux-kernel@vger.kernel.org, x86@kernel.org
On Fri, 2024-03-22 at 08:06 -0700, H.J. Lu wrote:
> On Fri, Mar 22, 2024 at 7:07 AM Edgecombe, Rick P
> <rick.p.edgecombe@intel.com> wrote:
> >
> > On Fri, 2024-03-15 at 07:34 -0700, H.J. Lu wrote:
> > > > How many people do you think will use this?
> >
> > I'm concerned that the only use of this will ever be exercise via
> > the
> > glibc unit tests, but will still require work to support.
>
> Correct. A small glibc change is needed. Will post it after
> my kernel change is merged.
I mean it will require kernel work in the future to maintain support.
That we will have to think about x32 effects when making other shadow
stack changes.
I'll paste my other comment in this thread:
The main usage of shadow stack is security, and comes with some
overhead. IIUC the main usage of x32 is performance benchmarking type
stuff. Why would someone want to use shadow stack and x32 together?
>
>
> > > >
> > > > I would have thought it would require more changes for basic
> > > > x32
> > >
> > > This is all needed.
> > >
> > > > operation. What was the testing exactly?
> > >
> > > I configured x32 glibc with --enable-cet, build glibc and
> > > run all glibc tests with shadow stack enabled. There are
> > > no regressions. I verified that shadow stack is enabled
> > > via /proc/pid/status.
> >
> > The shadow stack is supposed to be mapped above 4G, so how is this
> > supposed to work for x32?
>
> This is not what I see:
>
> (gdb) info reg
> ...
> pl3_ssp 0xf7dcbfe8 0xf7dcbfe8
The mapping above 4G was because Peterz raised the possibility that a
64 bit process could far call into a 32 bit segment and start doing
signal stuff that would encounter undefined behavior. He wanted it
cleanly blocked. So by keeping the shadow stack above 4GB, existing
processes that turned on shadow stack would be preventing from
transitioning to 32 bit and encountering the missing 32 bit signal
support (because the CPU would #GP during the 32 bit transition if SSP
is above 4GB).
Probably there is some interplay between the x32 mmap logic and shadow
stacks mapping, where it then becomes possible to get below 4GB. Since
x32 needs the shadow stack to be below 4GB, it's incompatible with that
solution. So this patch is not sufficient to enable x32 without side
effects that were previously considered bad.
I see this is in tip now. I don't think it's a good idea to support
upstream. The implications need more discussion, and there doesn't seem
to be any real end user value.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-22 15:58 ` Edgecombe, Rick P
@ 2024-03-22 16:07 ` H.J. Lu
2024-03-22 16:21 ` Edgecombe, Rick P
0 siblings, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2024-03-22 16:07 UTC (permalink / raw
To: Edgecombe, Rick P; +Cc: linux-kernel@vger.kernel.org, x86@kernel.org
On Fri, Mar 22, 2024 at 8:58 AM Edgecombe, Rick P
<rick.p.edgecombe@intel.com> wrote:
>
> On Fri, 2024-03-22 at 08:06 -0700, H.J. Lu wrote:
> > On Fri, Mar 22, 2024 at 7:07 AM Edgecombe, Rick P
> > <rick.p.edgecombe@intel.com> wrote:
> > >
> > > On Fri, 2024-03-15 at 07:34 -0700, H.J. Lu wrote:
> > > > > How many people do you think will use this?
> > >
> > > I'm concerned that the only use of this will ever be exercise via
> > > the
> > > glibc unit tests, but will still require work to support.
> >
> > Correct. A small glibc change is needed. Will post it after
> > my kernel change is merged.
>
> I mean it will require kernel work in the future to maintain support.
> That we will have to think about x32 effects when making other shadow
> stack changes.
It is way more than kernel SHSTK self tests.
> I'll paste my other comment in this thread:
>
> The main usage of shadow stack is security, and comes with some
> overhead. IIUC the main usage of x32 is performance benchmarking type
> stuff. Why would someone want to use shadow stack and x32 together?
Improve x32 security and user space IBT will add more improvement.
> >
> >
> > > > >
> > > > > I would have thought it would require more changes for basic
> > > > > x32
> > > >
> > > > This is all needed.
> > > >
> > > > > operation. What was the testing exactly?
> > > >
> > > > I configured x32 glibc with --enable-cet, build glibc and
> > > > run all glibc tests with shadow stack enabled. There are
> > > > no regressions. I verified that shadow stack is enabled
> > > > via /proc/pid/status.
> > >
> > > The shadow stack is supposed to be mapped above 4G, so how is this
> > > supposed to work for x32?
> >
> > This is not what I see:
> >
> > (gdb) info reg
> > ...
> > pl3_ssp 0xf7dcbfe8 0xf7dcbfe8
>
> The mapping above 4G was because Peterz raised the possibility that a
> 64 bit process could far call into a 32 bit segment and start doing
> signal stuff that would encounter undefined behavior. He wanted it
> cleanly blocked. So by keeping the shadow stack above 4GB, existing
> processes that turned on shadow stack would be preventing from
> transitioning to 32 bit and encountering the missing 32 bit signal
> support (because the CPU would #GP during the 32 bit transition if SSP
> is above 4GB).
>
> Probably there is some interplay between the x32 mmap logic and shadow
> stacks mapping, where it then becomes possible to get below 4GB. Since
> x32 needs the shadow stack to be below 4GB, it's incompatible with that
> solution. So this patch is not sufficient to enable x32 without side
> effects that were previously considered bad.
Mapping shadow stack below 4GB on x32 still provides security improvements
over no show stack.
> I see this is in tip now. I don't think it's a good idea to support
> upstream. The implications need more discussion, and there doesn't seem
> to be any real end user value.
--
H.J.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-22 16:07 ` H.J. Lu
@ 2024-03-22 16:21 ` Edgecombe, Rick P
2024-03-22 16:35 ` H.J. Lu
0 siblings, 1 reply; 16+ messages in thread
From: Edgecombe, Rick P @ 2024-03-22 16:21 UTC (permalink / raw
To: hjl.tools@gmail.com; +Cc: linux-kernel@vger.kernel.org, x86@kernel.org
On Fri, 2024-03-22 at 09:07 -0700, H.J. Lu wrote:
> > I mean it will require kernel work in the future to maintain
> > support.
> > That we will have to think about x32 effects when making other
> > shadow
> > stack changes.
>
> It is way more than kernel SHSTK self tests.
>
> > I'll paste my other comment in this thread:
> >
> > The main usage of shadow stack is security, and comes with some
> > overhead. IIUC the main usage of x32 is performance benchmarking
> > type
> > stuff. Why would someone want to use shadow stack and x32 together?
>
> Improve x32 security and user space IBT will add more improvement.
Please elaborate on the users that will use x32 and shadow stack
together. How many people are we talking about? They care enough about
performance to use x32, but also don't mind overhead to increase
security? Perhaps I'm missing something on what x32 is used for today.
[snip]
> >
> > The mapping above 4G was because Peterz raised the possibility that
> > a
> > 64 bit process could far call into a 32 bit segment and start doing
> > signal stuff that would encounter undefined behavior. He wanted it
> > cleanly blocked. So by keeping the shadow stack above 4GB, existing
> > processes that turned on shadow stack would be preventing from
> > transitioning to 32 bit and encountering the missing 32 bit signal
> > support (because the CPU would #GP during the 32 bit transition if
> > SSP
> > is above 4GB).
> >
> > Probably there is some interplay between the x32 mmap logic and
> > shadow
> > stacks mapping, where it then becomes possible to get below 4GB.
> > Since
> > x32 needs the shadow stack to be below 4GB, it's incompatible with
> > that
> > solution. So this patch is not sufficient to enable x32 without
> > side
> > effects that were previously considered bad.
>
> Mapping shadow stack below 4GB on x32 still provides security
> improvements
> over no show stack.
Agreed on this point. I don't think x32 shadow stack has to be perfect
to improve security of the x32 apps.
But Peterz's concern (I think it could probably be re-opened) was that
the ia32 signal stuff should not be just declared unsupported with
shadow stack, but blocked from being used somehow. So it was really
about being able to not have to think about the implications of the
undefined behavior. (was my understanding at least)
This patch is just turning things on and finding that nothing crashes.
It needs more analysis.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-22 16:21 ` Edgecombe, Rick P
@ 2024-03-22 16:35 ` H.J. Lu
0 siblings, 0 replies; 16+ messages in thread
From: H.J. Lu @ 2024-03-22 16:35 UTC (permalink / raw
To: Edgecombe, Rick P; +Cc: linux-kernel@vger.kernel.org, x86@kernel.org
On Fri, Mar 22, 2024 at 9:21 AM Edgecombe, Rick P
<rick.p.edgecombe@intel.com> wrote:
>
> On Fri, 2024-03-22 at 09:07 -0700, H.J. Lu wrote:
> > > I mean it will require kernel work in the future to maintain
> > > support.
> > > That we will have to think about x32 effects when making other
> > > shadow
> > > stack changes.
> >
> > It is way more than kernel SHSTK self tests.
> >
> > > I'll paste my other comment in this thread:
> > >
> > > The main usage of shadow stack is security, and comes with some
> > > overhead. IIUC the main usage of x32 is performance benchmarking
> > > type
> > > stuff. Why would someone want to use shadow stack and x32 together?
> >
> > Improve x32 security and user space IBT will add more improvement.
>
> Please elaborate on the users that will use x32 and shadow stack
> together. How many people are we talking about? They care enough about
> performance to use x32, but also don't mind overhead to increase
> security? Perhaps I'm missing something on what x32 is used for today.
SHSTK is enabled by -fcf-protection which is the default for some OSes
where x32 binaries are SHSTK enabled already. Enable SHSTK should
have minimum performance overhead.
>
> [snip]
>
> > >
> > > The mapping above 4G was because Peterz raised the possibility that
> > > a
> > > 64 bit process could far call into a 32 bit segment and start doing
> > > signal stuff that would encounter undefined behavior. He wanted it
> > > cleanly blocked. So by keeping the shadow stack above 4GB, existing
> > > processes that turned on shadow stack would be preventing from
> > > transitioning to 32 bit and encountering the missing 32 bit signal
> > > support (because the CPU would #GP during the 32 bit transition if
> > > SSP
> > > is above 4GB).
> > >
> > > Probably there is some interplay between the x32 mmap logic and
> > > shadow
> > > stacks mapping, where it then becomes possible to get below 4GB.
> > > Since
> > > x32 needs the shadow stack to be below 4GB, it's incompatible with
> > > that
> > > solution. So this patch is not sufficient to enable x32 without
> > > side
> > > effects that were previously considered bad.
> >
> > Mapping shadow stack below 4GB on x32 still provides security
> > improvements
> > over no show stack.
>
> Agreed on this point. I don't think x32 shadow stack has to be perfect
> to improve security of the x32 apps.
>
> But Peterz's concern (I think it could probably be re-opened) was that
> the ia32 signal stuff should not be just declared unsupported with
> shadow stack, but blocked from being used somehow. So it was really
> about being able to not have to think about the implications of the
> undefined behavior. (was my understanding at least)
>
> This patch is just turning things on and finding that nothing crashes.
> It needs more analysis.
>
It is more than that. There are 3 glibc tests to check if SHSTK triggers
show stack violation when there is an intentional show stack mismatch:
[hjl@gnu-tgl-1 build-x86_64-linux]$ ./elf/tst-shstk-legacy-1b
[hjl@gnu-tgl-1 build-x86_64-linux]$ ./elf/tst-shstk-legacy-1b --direct
Segmentation fault (core dumped)
[hjl@gnu-tgl-1 build-x86_64-linux]$
GLIBC_TUNABLES=glibc.cpu.hwcaps=-SHSTK ./elf/tst-shstk-legacy-1b
--direct
[hjl@gnu-tgl-1 build-x86_64-linux]$
They pass on x32.
--
H.J.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [tip: x86/shstk] x86/shstk: Enable shadow stacks for x32
2024-03-22 10:59 ` [tip: x86/shstk] x86/shstk: Enable shadow stacks " tip-bot2 for H.J. Lu
@ 2024-03-22 16:49 ` Borislav Petkov
0 siblings, 0 replies; 16+ messages in thread
From: Borislav Petkov @ 2024-03-22 16:49 UTC (permalink / raw
To: linux-kernel
Cc: linux-tip-commits, H.J. Lu, Ingo Molnar, Edgecombe, Rick P,
H. Peter Anvin, Linus Torvalds, Andy Lutomirski, x86
On Fri, Mar 22, 2024 at 10:59:58AM -0000, tip-bot2 for H.J. Lu wrote:
> The following commit has been merged into the x86/shstk branch of tip:
>
> Commit-ID: 2883f01ec37dd8668e7222dfdb5980c86fdfe277
> Gitweb: https://git.kernel.org/tip/2883f01ec37dd8668e7222dfdb5980c86fdfe277
> Author: H.J. Lu <hjl.tools@gmail.com>
> AuthorDate: Fri, 15 Mar 2024 07:04:33 -07:00
> Committer: Ingo Molnar <mingo@kernel.org>
> CommitterDate: Fri, 22 Mar 2024 10:17:11 +01:00
>
> x86/shstk: Enable shadow stacks for x32
>
> 1. Add shadow stack support to x32 signal.
> 2. Use the 64-bit map_shadow_stack syscall for x32.
> 3. Set up shadow stack for x32.
Why are we still diddling with x32 and not patching it out instead?
https://lore.kernel.org/all/CALCETrXoRAibsbWa9nfbDrt0iEuebMnCMhSFg-d9W-J2g8mDjw@mail.gmail.com/
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-15 14:04 [PATCH] x86/shstk: Enable shadow stack for x32 H.J. Lu
2024-03-15 14:20 ` Edgecombe, Rick P
2024-03-22 10:59 ` [tip: x86/shstk] x86/shstk: Enable shadow stacks " tip-bot2 for H.J. Lu
@ 2024-03-22 16:49 ` Dave Hansen
2024-03-22 16:52 ` H.J. Lu
2 siblings, 1 reply; 16+ messages in thread
From: Dave Hansen @ 2024-03-22 16:49 UTC (permalink / raw
To: H.J. Lu, linux-kernel; +Cc: x86, Rick P Edgecombe
On 3/15/24 07:04, H.J. Lu wrote:
> 1. Add shadow stack support to x32 signal.
> 2. Use the 64-bit map_shadow_stack syscall for x32.
> 3. Set up shadow stack for x32.
>
> Tested with shadow stack enabled x32 glibc on Intel Tiger Lake.
I don't think we should wire up code that's never going to get used in
practice. I think I'd want to be sure of the existence of some
specific, real-world, long-term users of this before we add a new ABI
that we need to support forever.
Are there real users?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-22 16:49 ` [PATCH] x86/shstk: Enable shadow stack " Dave Hansen
@ 2024-03-22 16:52 ` H.J. Lu
2024-03-22 16:56 ` Dave Hansen
0 siblings, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2024-03-22 16:52 UTC (permalink / raw
To: Dave Hansen; +Cc: linux-kernel, x86, Rick P Edgecombe
On Fri, Mar 22, 2024 at 9:49 AM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 3/15/24 07:04, H.J. Lu wrote:
> > 1. Add shadow stack support to x32 signal.
> > 2. Use the 64-bit map_shadow_stack syscall for x32.
> > 3. Set up shadow stack for x32.
> >
> > Tested with shadow stack enabled x32 glibc on Intel Tiger Lake.
>
> I don't think we should wire up code that's never going to get used in
> practice. I think I'd want to be sure of the existence of some
> specific, real-world, long-term users of this before we add a new ABI
> that we need to support forever.
>
> Are there real users?
Were you talking about x32 users or x32 users with shadow stack?
--
H.J.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-22 16:52 ` H.J. Lu
@ 2024-03-22 16:56 ` Dave Hansen
2024-03-22 17:00 ` H.J. Lu
0 siblings, 1 reply; 16+ messages in thread
From: Dave Hansen @ 2024-03-22 16:56 UTC (permalink / raw
To: H.J. Lu; +Cc: linux-kernel, x86, Rick P Edgecombe
On 3/22/24 09:52, H.J. Lu wrote:
> On Fri, Mar 22, 2024 at 9:49 AM Dave Hansen <dave.hansen@intel.com> wrote:
>> On 3/15/24 07:04, H.J. Lu wrote:
>>> 1. Add shadow stack support to x32 signal.
>>> 2. Use the 64-bit map_shadow_stack syscall for x32.
>>> 3. Set up shadow stack for x32.
>>>
>>> Tested with shadow stack enabled x32 glibc on Intel Tiger Lake.
>> I don't think we should wire up code that's never going to get used in
>> practice. I think I'd want to be sure of the existence of some
>> specific, real-world, long-term users of this before we add a new ABI
>> that we need to support forever.
>>
>> Are there real users?
> Were you talking about x32 users or x32 users with shadow stack?
x32 users who both want and will use shadow stacks.
It's been a long time since I've seen an x32 bug report.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] x86/shstk: Enable shadow stack for x32
2024-03-22 16:56 ` Dave Hansen
@ 2024-03-22 17:00 ` H.J. Lu
0 siblings, 0 replies; 16+ messages in thread
From: H.J. Lu @ 2024-03-22 17:00 UTC (permalink / raw
To: Dave Hansen; +Cc: linux-kernel, x86, Rick P Edgecombe
On Fri, Mar 22, 2024 at 9:56 AM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 3/22/24 09:52, H.J. Lu wrote:
> > On Fri, Mar 22, 2024 at 9:49 AM Dave Hansen <dave.hansen@intel.com> wrote:
> >> On 3/15/24 07:04, H.J. Lu wrote:
> >>> 1. Add shadow stack support to x32 signal.
> >>> 2. Use the 64-bit map_shadow_stack syscall for x32.
> >>> 3. Set up shadow stack for x32.
> >>>
> >>> Tested with shadow stack enabled x32 glibc on Intel Tiger Lake.
> >> I don't think we should wire up code that's never going to get used in
> >> practice. I think I'd want to be sure of the existence of some
> >> specific, real-world, long-term users of this before we add a new ABI
> >> that we need to support forever.
> >>
> >> Are there real users?
> > Were you talking about x32 users or x32 users with shadow stack?
>
> x32 users who both want and will use shadow stacks.
Someone can do a survey among x32 users.
> It's been a long time since I've seen an x32 bug report.
Because it just works?
--
H.J.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-03-22 17:00 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-15 14:04 [PATCH] x86/shstk: Enable shadow stack for x32 H.J. Lu
2024-03-15 14:20 ` Edgecombe, Rick P
2024-03-15 14:34 ` H.J. Lu
2024-03-22 14:07 ` Edgecombe, Rick P
2024-03-22 14:28 ` Edgecombe, Rick P
2024-03-22 15:06 ` H.J. Lu
2024-03-22 15:58 ` Edgecombe, Rick P
2024-03-22 16:07 ` H.J. Lu
2024-03-22 16:21 ` Edgecombe, Rick P
2024-03-22 16:35 ` H.J. Lu
2024-03-22 10:59 ` [tip: x86/shstk] x86/shstk: Enable shadow stacks " tip-bot2 for H.J. Lu
2024-03-22 16:49 ` Borislav Petkov
2024-03-22 16:49 ` [PATCH] x86/shstk: Enable shadow stack " Dave Hansen
2024-03-22 16:52 ` H.J. Lu
2024-03-22 16:56 ` Dave Hansen
2024-03-22 17:00 ` H.J. Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).