2024-03-15 14:05:04

by H.J. Lu

[permalink] [raw]
Subject: [PATCH] x86/shstk: Enable shadow stack 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.

Cc: Rick P Edgecombe <[email protected]>
Tested-by: H.J. Lu <[email protected]>
Signed-off-by: H.J. Lu <[email protected]>
---
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



2024-03-15 14:20:43

by Edgecombe, Rick P

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

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 <[email protected]>
> Tested-by: H.J. Lu <[email protected]>
> Signed-off-by: H.J. Lu <[email protected]>

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?

2024-03-15 14:38:59

by H.J. Lu

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

On Fri, Mar 15, 2024 at 7:20 AM Edgecombe, Rick P
<[email protected]> 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 <[email protected]>
> > Tested-by: H.J. Lu <[email protected]>
> > Signed-off-by: H.J. Lu <[email protected]>
>
> 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.

Subject: [tip: x86/shstk] x86/shstk: Enable shadow stacks for x32

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 <[email protected]>
AuthorDate: Fri, 15 Mar 2024 07:04:33 -07:00
Committer: Ingo Molnar <[email protected]>
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 <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Tested-by: H.J. Lu <[email protected]>
Cc: "Edgecombe, Rick P" <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
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;


2024-03-22 14:07:42

by Edgecombe, Rick P

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

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?

2024-03-22 14:29:19

by Edgecombe, Rick P

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

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?

2024-03-22 15:07:20

by H.J. Lu

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

On Fri, Mar 22, 2024 at 7:07 AM Edgecombe, Rick P
<[email protected]> 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.

2024-03-22 15:58:44

by Edgecombe, Rick P

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

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
> <[email protected]> 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.

2024-03-22 16:08:26

by H.J. Lu

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

On Fri, Mar 22, 2024 at 8:58 AM Edgecombe, Rick P
<[email protected]> 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
> > <[email protected]> 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.

2024-03-22 16:22:37

by Edgecombe, Rick P

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

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.

2024-03-22 16:35:50

by H.J. Lu

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

On Fri, Mar 22, 2024 at 9:21 AM Edgecombe, Rick P
<[email protected]> 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.

2024-03-22 16:49:59

by Borislav Petkov

[permalink] [raw]
Subject: Re: [tip: x86/shstk] x86/shstk: Enable shadow stacks for x32

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 <[email protected]>
> AuthorDate: Fri, 15 Mar 2024 07:04:33 -07:00
> Committer: Ingo Molnar <[email protected]>
> 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

2024-03-22 16:51:04

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

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?

2024-03-22 16:58:25

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

On 3/22/24 09:52, H.J. Lu wrote:
> On Fri, Mar 22, 2024 at 9:49 AM Dave Hansen <[email protected]> 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.

2024-03-22 17:01:06

by H.J. Lu

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

On Fri, Mar 22, 2024 at 9:56 AM Dave Hansen <[email protected]> wrote:
>
> On 3/22/24 09:52, H.J. Lu wrote:
> > On Fri, Mar 22, 2024 at 9:49 AM Dave Hansen <[email protected]> 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.

2024-03-22 17:04:42

by H.J. Lu

[permalink] [raw]
Subject: Re: [PATCH] x86/shstk: Enable shadow stack for x32

On Fri, Mar 22, 2024 at 9:49 AM Dave Hansen <[email protected]> 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.