LASS throws a #GP for any violations except for stack register accesses,
in which case it throws a #SS instead. Handle this similarly to how other
LASS violations are handled.
In case of FRED, before handling #SS as LASS violation, kernel has to
check if there's a fixup for the exception. It can address #SS due to
invalid user context on ERETU. See 5105e7687ad3 ("x86/fred: Fixup
fault on ERETU by jumping to fred_entrypoint_user") for more details.
Co-developed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
arch/x86/kernel/traps.c | 39 +++++++++++++++++++++++++++++++++------
1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index ceb091f17a5b..f9ca5b911141 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -418,12 +418,6 @@ DEFINE_IDTENTRY_ERRORCODE(exc_segment_not_present)
SIGBUS, 0, NULL);
}
-DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
-{
- do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
- 0, NULL);
-}
-
DEFINE_IDTENTRY_ERRORCODE(exc_alignment_check)
{
char *str = "alignment check";
@@ -866,6 +860,39 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
cond_local_irq_disable(regs);
}
+#define SSFSTR "stack segment fault"
+
+DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
+{
+ if (user_mode(regs))
+ goto error_trap;
+
+ if (cpu_feature_enabled(X86_FEATURE_FRED) &&
+ fixup_exception(regs, X86_TRAP_SS, error_code, 0))
+ return;
+
+ if (cpu_feature_enabled(X86_FEATURE_LASS)) {
+ enum kernel_exc_hint hint;
+ unsigned long exc_addr;
+
+ hint = get_kernel_exc_address(regs, &exc_addr);
+ if (hint != EXC_NO_HINT) {
+ printk(SSFSTR ", %s 0x%lx", kernel_exc_hint_help[hint],
+ exc_addr);
+ }
+
+ if (hint != EXC_NON_CANONICAL)
+ exc_addr = 0;
+
+ die_addr(SSFSTR, regs, error_code, exc_addr);
+ return;
+ }
+
+error_trap:
+ do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
+ 0, NULL);
+}
+
static bool do_int3(struct pt_regs *regs)
{
int res;
--
2.47.2On 7/1/2025 2:58 AM, Kirill A. Shutemov wrote:
> LASS throws a #GP for any violations except for stack register accesses,
> in which case it throws a #SS instead. Handle this similarly to how other
> LASS violations are handled.
>
Maybe I've misunderstood something:
Is the underlying assumption here that #SS were previously only
generated by userspace, but now they can also be generated by the
kernel? And we want the kernel generated #SS to behave the same as the #GP?
> In case of FRED, before handling #SS as LASS violation, kernel has to
> check if there's a fixup for the exception. It can address #SS due to
> invalid user context on ERETU. See 5105e7687ad3 ("x86/fred: Fixup
> fault on ERETU by jumping to fred_entrypoint_user") for more details.
>
> Co-developed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
> arch/x86/kernel/traps.c | 39 +++++++++++++++++++++++++++++++++------
> 1 file changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index ceb091f17a5b..f9ca5b911141 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -418,12 +418,6 @@ DEFINE_IDTENTRY_ERRORCODE(exc_segment_not_present)
> SIGBUS, 0, NULL);
> }
>
> -DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
> -{
> - do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
> - 0, NULL);
> -}
> -
> DEFINE_IDTENTRY_ERRORCODE(exc_alignment_check)
> {
> char *str = "alignment check";
> @@ -866,6 +860,39 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
> cond_local_irq_disable(regs);
> }
>
> +#define SSFSTR "stack segment fault"
> +
> +DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
> +{
> + if (user_mode(regs))
> + goto error_trap;
> +
> + if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> + fixup_exception(regs, X86_TRAP_SS, error_code, 0))
> + return;
> +
> + if (cpu_feature_enabled(X86_FEATURE_LASS)) {
> + enum kernel_exc_hint hint;
> + unsigned long exc_addr;
> +
> + hint = get_kernel_exc_address(regs, &exc_addr);
> + if (hint != EXC_NO_HINT) {
The brackets are not needed for singular statements. Also the max line
length is longer now. You can fit this all in a single line.
> + printk(SSFSTR ", %s 0x%lx", kernel_exc_hint_help[hint],
> + exc_addr);
> + }
> +
> + if (hint != EXC_NON_CANONICAL)
> + exc_addr = 0;
> +
> + die_addr(SSFSTR, regs, error_code, exc_addr);
The variable names in die_addr() should be generalized as well. They
seem to assume the caller to be a #GP handler.
> + return;
> + }
> +
> +error_trap:
> + do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
> + 0, NULL);
> +}
> +
> static bool do_int3(struct pt_regs *regs)
> {
> int res;
On Tue, Jul 01, 2025 at 06:35:40PM -0700, Sohil Mehta wrote:
> On 7/1/2025 2:58 AM, Kirill A. Shutemov wrote:
> > LASS throws a #GP for any violations except for stack register accesses,
> > in which case it throws a #SS instead. Handle this similarly to how other
> > LASS violations are handled.
> >
>
> Maybe I've misunderstood something:
>
> Is the underlying assumption here that #SS were previously only
> generated by userspace, but now they can also be generated by the
> kernel? And we want the kernel generated #SS to behave the same as the #GP?
It can be generated by both kernel and userspace if RSP gets corrupted.
So far, do_error_trap() did the trick, handling what has to be handled.
LASS requires a bit more, though.
>
> > In case of FRED, before handling #SS as LASS violation, kernel has to
> > check if there's a fixup for the exception. It can address #SS due to
> > invalid user context on ERETU. See 5105e7687ad3 ("x86/fred: Fixup
> > fault on ERETU by jumping to fred_entrypoint_user") for more details.
> >
> > Co-developed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > ---
> > arch/x86/kernel/traps.c | 39 +++++++++++++++++++++++++++++++++------
> > 1 file changed, 33 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> > index ceb091f17a5b..f9ca5b911141 100644
> > --- a/arch/x86/kernel/traps.c
> > +++ b/arch/x86/kernel/traps.c
> > @@ -418,12 +418,6 @@ DEFINE_IDTENTRY_ERRORCODE(exc_segment_not_present)
> > SIGBUS, 0, NULL);
> > }
> >
> > -DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
> > -{
> > - do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
> > - 0, NULL);
> > -}
> > -
> > DEFINE_IDTENTRY_ERRORCODE(exc_alignment_check)
> > {
> > char *str = "alignment check";
> > @@ -866,6 +860,39 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
> > cond_local_irq_disable(regs);
> > }
> >
> > +#define SSFSTR "stack segment fault"
> > +
> > +DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
> > +{
> > + if (user_mode(regs))
> > + goto error_trap;
> > +
> > + if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> > + fixup_exception(regs, X86_TRAP_SS, error_code, 0))
> > + return;
> > +
> > + if (cpu_feature_enabled(X86_FEATURE_LASS)) {
> > + enum kernel_exc_hint hint;
> > + unsigned long exc_addr;
> > +
> > + hint = get_kernel_exc_address(regs, &exc_addr);
> > + if (hint != EXC_NO_HINT) {
>
> The brackets are not needed for singular statements. Also the max line
> length is longer now. You can fit this all in a single line.
I think line split if justified. It is 120 characters long otherwise.
And with multi-line statement, brackets help readability.
I don't see a reason to change it.
> > + printk(SSFSTR ", %s 0x%lx", kernel_exc_hint_help[hint],
> > + exc_addr);
> > + }
> > +
>
> > + if (hint != EXC_NON_CANONICAL)
> > + exc_addr = 0;
> > +
> > + die_addr(SSFSTR, regs, error_code, exc_addr);
>
> The variable names in die_addr() should be generalized as well. They
> seem to assume the caller to be a #GP handler.
Okay, will fold into "x86/traps: Generalize #GP address decode and hint
code".
> > + return;
> > + }
> > +
> > +error_trap:
> > + do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
> > + 0, NULL);
> > +}
> > +
> > static bool do_int3(struct pt_regs *regs)
> > {
> > int res;
>
--
Kiryl Shutsemau / Kirill A. Shutemov
On 7/2/2025 6:27 AM, Kirill A. Shutemov wrote: >> >> Maybe I've misunderstood something: >> >> Is the underlying assumption here that #SS were previously only >> generated by userspace, but now they can also be generated by the >> kernel? And we want the kernel generated #SS to behave the same as the #GP? > > It can be generated by both kernel and userspace if RSP gets corrupted. > > So far, do_error_trap() did the trick, handling what has to be handled. > LASS requires a bit more, though. > Thank you for the information! The discussion in the other thread helped clarify my confusion about the new FRED specific fixup outside the LASS check. IIUC, for kernel generated #SS, the prior code in do_error_trap() would've done a few things such as notify_die() and cond_local_irq_enable() before calling die(). The new code now directly calls die_addr(). Are we changing the behavior for legacy kernel #SS? Also, why don't we need those calls for the new LASS #SS? I apologize if the questions seem too naive. I am finding the exception handling code a bit convoluted to understand. In general, I would suggest adding some code comments at least for the new code to help ignorant folks like me looking at this in the future.
On Wed, Jul 02, 2025 at 01:05:17PM -0700, Sohil Mehta wrote: > On 7/2/2025 6:27 AM, Kirill A. Shutemov wrote: > > >> > >> Maybe I've misunderstood something: > >> > >> Is the underlying assumption here that #SS were previously only > >> generated by userspace, but now they can also be generated by the > >> kernel? And we want the kernel generated #SS to behave the same as the #GP? > > > > It can be generated by both kernel and userspace if RSP gets corrupted. > > > > So far, do_error_trap() did the trick, handling what has to be handled. > > LASS requires a bit more, though. > > > Thank you for the information! The discussion in the other thread helped > clarify my confusion about the new FRED specific fixup outside the LASS > check. > > IIUC, for kernel generated #SS, the prior code in do_error_trap() > would've done a few things such as notify_die() and > cond_local_irq_enable() before calling die(). cond_local_irq_enable() need to happen if we want to do something sleepable during exception handling. It is not the case here. notify_die() will be called die_addr()->__die_body()->notify_die(). > The new code now directly calls die_addr(). Are we changing the behavior > for legacy kernel #SS? Also, why don't we need those calls for the new > LASS #SS? do_error_trap() provides catch-all handling for unallowed-thing-happened exception handling in either kernel or userspace. We can take simpler path for fatal in-kernel exception. Following #GP logic matches what we need. -- Kiryl Shutsemau / Kirill A. Shutemov
On 7/3/2025 4:31 AM, Kirill A. Shutemov wrote: > > cond_local_irq_enable() need to happen if we want to do something > sleepable during exception handling. It is not the case here. > Makes sense. > notify_die() will be called die_addr()->__die_body()->notify_die(). The notify_die() within __die_body() is slightly different from the one called from the exception handlers. __die_body(): notify_die(DIE_OOPS, str, regs, err, current->thread.trap_nr, SIGSEGV) exc_alignment_check(): notify_die(DIE_TRAP, str, regs, error_code, X86_TRAP_AC, SIGBUS) I believe we should include a #SS specific notify before calling die_addr(). Similar to exc_alignment_check() which dies on kernel exceptions. if (notify_die(DIE_TRAP, str, regs, error_code, X86_TRAP_SS, SIGBUS) == NOTIFY_STOP) return; This way the behavior remains consistent with other exception handlers as well as with/without LASS.
On Thu, Jul 03, 2025 at 01:12:11PM -0700, Sohil Mehta wrote: > I believe we should include a #SS specific notify before calling > die_addr(). Similar to exc_alignment_check() which dies on kernel > exceptions. You are right. Will update this. -- Kiryl Shutsemau / Kirill A. Shutemov
On 7/2/2025 6:27 AM, Kirill A. Shutemov wrote:
>>> +
>>> + if (cpu_feature_enabled(X86_FEATURE_LASS)) {
>>> + enum kernel_exc_hint hint;
>>> + unsigned long exc_addr;
>>> +
>>> + hint = get_kernel_exc_address(regs, &exc_addr);
>>> + if (hint != EXC_NO_HINT) {
>>
>> The brackets are not needed for singular statements. Also the max line
>> length is longer now. You can fit this all in a single line.
>
> I think line split if justified. It is 120 characters long otherwise.
> And with multi-line statement, brackets help readability.
>
Are you sure? Below ends at 90 characters for me, including the three
8-char tabs:
printk(SSFSTR ", %s 0x%lx", kernel_exc_hint_help[hint], exc_addr);
> I don't see a reason to change it.
To reduce indentation, you could also do:
if (!cpu_feature_enabled(X86_FEATURE_LASS))
goto error_trap;
>
>>> + printk(SSFSTR ", %s 0x%lx", kernel_exc_hint_help[hint],
>>> + exc_addr);
>>> + }
>>> +
>>
On Wed, Jul 02, 2025 at 10:56:25AM -0700, Sohil Mehta wrote: > To reduce indentation, you could also do: > > if (!cpu_feature_enabled(X86_FEATURE_LASS)) > goto error_trap; Okay, will do this. -- Kiryl Shutsemau / Kirill A. Shutemov
On July 1, 2025 6:35:40 PM PDT, Sohil Mehta <sohil.mehta@intel.com> wrote:
>On 7/1/2025 2:58 AM, Kirill A. Shutemov wrote:
>> LASS throws a #GP for any violations except for stack register accesses,
>> in which case it throws a #SS instead. Handle this similarly to how other
>> LASS violations are handled.
>>
>
>Maybe I've misunderstood something:
>
>Is the underlying assumption here that #SS were previously only
>generated by userspace, but now they can also be generated by the
>kernel? And we want the kernel generated #SS to behave the same as the #GP?
>
>> In case of FRED, before handling #SS as LASS violation, kernel has to
>> check if there's a fixup for the exception. It can address #SS due to
>> invalid user context on ERETU. See 5105e7687ad3 ("x86/fred: Fixup
>> fault on ERETU by jumping to fred_entrypoint_user") for more details.
>>
>> Co-developed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> ---
>> arch/x86/kernel/traps.c | 39 +++++++++++++++++++++++++++++++++------
>> 1 file changed, 33 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
>> index ceb091f17a5b..f9ca5b911141 100644
>> --- a/arch/x86/kernel/traps.c
>> +++ b/arch/x86/kernel/traps.c
>> @@ -418,12 +418,6 @@ DEFINE_IDTENTRY_ERRORCODE(exc_segment_not_present)
>> SIGBUS, 0, NULL);
>> }
>>
>> -DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
>> -{
>> - do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
>> - 0, NULL);
>> -}
>> -
>> DEFINE_IDTENTRY_ERRORCODE(exc_alignment_check)
>> {
>> char *str = "alignment check";
>> @@ -866,6 +860,39 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
>> cond_local_irq_disable(regs);
>> }
>>
>> +#define SSFSTR "stack segment fault"
>> +
>> +DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
>> +{
>> + if (user_mode(regs))
>> + goto error_trap;
>> +
>> + if (cpu_feature_enabled(X86_FEATURE_FRED) &&
>> + fixup_exception(regs, X86_TRAP_SS, error_code, 0))
>> + return;
>> +
>> + if (cpu_feature_enabled(X86_FEATURE_LASS)) {
>> + enum kernel_exc_hint hint;
>> + unsigned long exc_addr;
>> +
>> + hint = get_kernel_exc_address(regs, &exc_addr);
>> + if (hint != EXC_NO_HINT) {
>
>The brackets are not needed for singular statements. Also the max line
>length is longer now. You can fit this all in a single line.
>
>> + printk(SSFSTR ", %s 0x%lx", kernel_exc_hint_help[hint],
>> + exc_addr);
>> + }
>> +
>
>> + if (hint != EXC_NON_CANONICAL)
>> + exc_addr = 0;
>> +
>> + die_addr(SSFSTR, regs, error_code, exc_addr);
>
>The variable names in die_addr() should be generalized as well. They
>seem to assume the caller to be a #GP handler.
>
>> + return;
>> + }
>> +
>> +error_trap:
>> + do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
>> + 0, NULL);
>> +}
>> +
>> static bool do_int3(struct pt_regs *regs)
>> {
>> int res;
>
Note: for a FRED system, ERETU can generate #SS for a non-canonical user space RSP even in the absence of LASS, so if that is not currently handled that is an active bug.
On Tue, 01 Jul 2025 19:06:10 -0700 "H. Peter Anvin" <hpa@zytor.com> wrote: ... > Note: for a FRED system, ERETU can generate #SS for a non-canonical user space > RSP even in the absence of LASS, so if that is not currently handled that is an active bug. Is that a fault in kernel space, or a fault in user space. Some of the traps for 'iret' happen after the transition to user space, so the kernel doesn't have to handle them as special cases. (I simplified (and fixed) one version of that code.) David
On July 6, 2025 2:22:13 AM PDT, David Laight <david.laight.linux@gmail.com> wrote: >On Tue, 01 Jul 2025 19:06:10 -0700 >"H. Peter Anvin" <hpa@zytor.com> wrote: >... >> Note: for a FRED system, ERETU can generate #SS for a non-canonical user space >> RSP even in the absence of LASS, so if that is not currently handled that is an active bug. > >Is that a fault in kernel space, or a fault in user space. > >Some of the traps for 'iret' happen after the transition to user space, >so the kernel doesn't have to handle them as special cases. >(I simplified (and fixed) one version of that code.) > > David > It's a fault in user space. I had a brain fart and managed to forget that RSP is technically a GPR and as such is not limited to the VA width of the machine.
> Note: for a FRED system, ERETU can generate #SS for a non-canonical user space RSP How? Or to phrase it differently, I hope not. %rsp is a 64bit value and does not have canonical restrictions elsewhere in the architecture, so far as I'm aware. IRET really can restore a non-canonical %rsp, and userspace can run for an indeterminate period of time with a non-canonical %rsp as long as there are no stack accesses. Accesses relative to the the stack using a non-canonical pointer will suffer #SS, but ERETU doesn't modify the userspace stack AFAICT. I can't see anything in the ERETU pseudocode in the FRED spec that mentions a canonical check or memory access using %rsp. ~Andrew
On July 2, 2025 4:42:27 PM PDT, Andrew Cooper <andrew.cooper3@citrix.com> wrote: >> Note: for a FRED system, ERETU can generate #SS for a non-canonical user space RSP > >How? Or to phrase it differently, I hope not. > >%rsp is a 64bit value and does not have canonical restrictions elsewhere >in the architecture, so far as I'm aware. IRET really can restore a >non-canonical %rsp, and userspace can run for an indeterminate period of >time with a non-canonical %rsp as long as there are no stack accesses. > >Accesses relative to the the stack using a non-canonical pointer will >suffer #SS, but ERETU doesn't modify the userspace stack AFAICT. I >can't see anything in the ERETU pseudocode in the FRED spec that >mentions a canonical check or memory access using %rsp. > >~Andrew You are right of course. Brainfart on my part.
On Tue, Jul 01, 2025 at 07:06:10PM -0700, H. Peter Anvin wrote:
> On July 1, 2025 6:35:40 PM PDT, Sohil Mehta <sohil.mehta@intel.com> wrote:
> >On 7/1/2025 2:58 AM, Kirill A. Shutemov wrote:
> >> LASS throws a #GP for any violations except for stack register accesses,
> >> in which case it throws a #SS instead. Handle this similarly to how other
> >> LASS violations are handled.
> >>
> >
> >Maybe I've misunderstood something:
> >
> >Is the underlying assumption here that #SS were previously only
> >generated by userspace, but now they can also be generated by the
> >kernel? And we want the kernel generated #SS to behave the same as the #GP?
> >
> >> In case of FRED, before handling #SS as LASS violation, kernel has to
> >> check if there's a fixup for the exception. It can address #SS due to
> >> invalid user context on ERETU. See 5105e7687ad3 ("x86/fred: Fixup
> >> fault on ERETU by jumping to fred_entrypoint_user") for more details.
> >>
> >> Co-developed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> >> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> >> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> >> ---
> >> arch/x86/kernel/traps.c | 39 +++++++++++++++++++++++++++++++++------
> >> 1 file changed, 33 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> >> index ceb091f17a5b..f9ca5b911141 100644
> >> --- a/arch/x86/kernel/traps.c
> >> +++ b/arch/x86/kernel/traps.c
> >> @@ -418,12 +418,6 @@ DEFINE_IDTENTRY_ERRORCODE(exc_segment_not_present)
> >> SIGBUS, 0, NULL);
> >> }
> >>
> >> -DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
> >> -{
> >> - do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
> >> - 0, NULL);
> >> -}
> >> -
> >> DEFINE_IDTENTRY_ERRORCODE(exc_alignment_check)
> >> {
> >> char *str = "alignment check";
> >> @@ -866,6 +860,39 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
> >> cond_local_irq_disable(regs);
> >> }
> >>
> >> +#define SSFSTR "stack segment fault"
> >> +
> >> +DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
> >> +{
> >> + if (user_mode(regs))
> >> + goto error_trap;
> >> +
> >> + if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> >> + fixup_exception(regs, X86_TRAP_SS, error_code, 0))
> >> + return;
> >> +
> >> + if (cpu_feature_enabled(X86_FEATURE_LASS)) {
> >> + enum kernel_exc_hint hint;
> >> + unsigned long exc_addr;
> >> +
> >> + hint = get_kernel_exc_address(regs, &exc_addr);
> >> + if (hint != EXC_NO_HINT) {
> >
> >The brackets are not needed for singular statements. Also the max line
> >length is longer now. You can fit this all in a single line.
> >
> >> + printk(SSFSTR ", %s 0x%lx", kernel_exc_hint_help[hint],
> >> + exc_addr);
> >> + }
> >> +
> >
> >> + if (hint != EXC_NON_CANONICAL)
> >> + exc_addr = 0;
> >> +
> >> + die_addr(SSFSTR, regs, error_code, exc_addr);
> >
> >The variable names in die_addr() should be generalized as well. They
> >seem to assume the caller to be a #GP handler.
> >
> >> + return;
> >> + }
> >> +
> >> +error_trap:
> >> + do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
> >> + 0, NULL);
> >> +}
> >> +
> >> static bool do_int3(struct pt_regs *regs)
> >> {
> >> int res;
> >
>
> Note: for a FRED system, ERETU can generate #SS for a non-canonical user space RSP even in the absence of LASS, so if that is not currently handled that is an active bug.
It is handled by fixup code inside do_error_trap(). We need to add
explicit fixup before LASS handling to avoid treating bad userspace RSP as
kernel LASS violation.
--
Kiryl Shutsemau / Kirill A. Shutemov
On July 2, 2025 3:17:10 AM PDT, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote:
>On Tue, Jul 01, 2025 at 07:06:10PM -0700, H. Peter Anvin wrote:
>> On July 1, 2025 6:35:40 PM PDT, Sohil Mehta <sohil.mehta@intel.com> wrote:
>> >On 7/1/2025 2:58 AM, Kirill A. Shutemov wrote:
>> >> LASS throws a #GP for any violations except for stack register accesses,
>> >> in which case it throws a #SS instead. Handle this similarly to how other
>> >> LASS violations are handled.
>> >>
>> >
>> >Maybe I've misunderstood something:
>> >
>> >Is the underlying assumption here that #SS were previously only
>> >generated by userspace, but now they can also be generated by the
>> >kernel? And we want the kernel generated #SS to behave the same as the #GP?
>> >
>> >> In case of FRED, before handling #SS as LASS violation, kernel has to
>> >> check if there's a fixup for the exception. It can address #SS due to
>> >> invalid user context on ERETU. See 5105e7687ad3 ("x86/fred: Fixup
>> >> fault on ERETU by jumping to fred_entrypoint_user") for more details.
>> >>
>> >> Co-developed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> >> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> >> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> >> ---
>> >> arch/x86/kernel/traps.c | 39 +++++++++++++++++++++++++++++++++------
>> >> 1 file changed, 33 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
>> >> index ceb091f17a5b..f9ca5b911141 100644
>> >> --- a/arch/x86/kernel/traps.c
>> >> +++ b/arch/x86/kernel/traps.c
>> >> @@ -418,12 +418,6 @@ DEFINE_IDTENTRY_ERRORCODE(exc_segment_not_present)
>> >> SIGBUS, 0, NULL);
>> >> }
>> >>
>> >> -DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
>> >> -{
>> >> - do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
>> >> - 0, NULL);
>> >> -}
>> >> -
>> >> DEFINE_IDTENTRY_ERRORCODE(exc_alignment_check)
>> >> {
>> >> char *str = "alignment check";
>> >> @@ -866,6 +860,39 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
>> >> cond_local_irq_disable(regs);
>> >> }
>> >>
>> >> +#define SSFSTR "stack segment fault"
>> >> +
>> >> +DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
>> >> +{
>> >> + if (user_mode(regs))
>> >> + goto error_trap;
>> >> +
>> >> + if (cpu_feature_enabled(X86_FEATURE_FRED) &&
>> >> + fixup_exception(regs, X86_TRAP_SS, error_code, 0))
>> >> + return;
>> >> +
>> >> + if (cpu_feature_enabled(X86_FEATURE_LASS)) {
>> >> + enum kernel_exc_hint hint;
>> >> + unsigned long exc_addr;
>> >> +
>> >> + hint = get_kernel_exc_address(regs, &exc_addr);
>> >> + if (hint != EXC_NO_HINT) {
>> >
>> >The brackets are not needed for singular statements. Also the max line
>> >length is longer now. You can fit this all in a single line.
>> >
>> >> + printk(SSFSTR ", %s 0x%lx", kernel_exc_hint_help[hint],
>> >> + exc_addr);
>> >> + }
>> >> +
>> >
>> >> + if (hint != EXC_NON_CANONICAL)
>> >> + exc_addr = 0;
>> >> +
>> >> + die_addr(SSFSTR, regs, error_code, exc_addr);
>> >
>> >The variable names in die_addr() should be generalized as well. They
>> >seem to assume the caller to be a #GP handler.
>> >
>> >> + return;
>> >> + }
>> >> +
>> >> +error_trap:
>> >> + do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
>> >> + 0, NULL);
>> >> +}
>> >> +
>> >> static bool do_int3(struct pt_regs *regs)
>> >> {
>> >> int res;
>> >
>>
>> Note: for a FRED system, ERETU can generate #SS for a non-canonical user space RSP even in the absence of LASS, so if that is not currently handled that is an active bug.
>
>It is handled by fixup code inside do_error_trap(). We need to add
>explicit fixup before LASS handling to avoid treating bad userspace RSP as
>kernel LASS violation.
>
Great. I was pretty sure, but I wanted to address Sohil's question directly. Thanks for verifying.
A LASS violation of any kind in the kernel (unless handled by fixup, including user access fixup) ought to be fatal, correct?
On Wed, Jul 02, 2025 at 07:37:12AM -0700, H. Peter Anvin wrote: > A LASS violation of any kind in the kernel (unless handled by fixup, > including user access fixup) ought to be fatal, correct? Yes, LASS violation is fatal for !user_mode(regs), unless addressed by fixups. For user_mode(regs), emulate_vsyscall_gp() is the notable exception. -- Kiryl Shutsemau / Kirill A. Shutemov
On July 2, 2025 7:47:30 AM PDT, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote: >On Wed, Jul 02, 2025 at 07:37:12AM -0700, H. Peter Anvin wrote: >> A LASS violation of any kind in the kernel (unless handled by fixup, >> including user access fixup) ought to be fatal, correct? > >Yes, LASS violation is fatal for !user_mode(regs), unless addressed by >fixups. > >For user_mode(regs), emulate_vsyscall_gp() is the notable exception. > Note also that for FRED we can have separate kernel and user space paths basically "for free". I'm not sure if we do that yet (if the infrastructure is there), but we could. Not that it matters for this case. This is a slow path.
On July 1, 2025 6:35:40 PM PDT, Sohil Mehta <sohil.mehta@intel.com> wrote:
>On 7/1/2025 2:58 AM, Kirill A. Shutemov wrote:
>> LASS throws a #GP for any violations except for stack register accesses,
>> in which case it throws a #SS instead. Handle this similarly to how other
>> LASS violations are handled.
>>
>
>Maybe I've misunderstood something:
>
>Is the underlying assumption here that #SS were previously only
>generated by userspace, but now they can also be generated by the
>kernel? And we want the kernel generated #SS to behave the same as the #GP?
>
>> In case of FRED, before handling #SS as LASS violation, kernel has to
>> check if there's a fixup for the exception. It can address #SS due to
>> invalid user context on ERETU. See 5105e7687ad3 ("x86/fred: Fixup
>> fault on ERETU by jumping to fred_entrypoint_user") for more details.
>>
>> Co-developed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> ---
>> arch/x86/kernel/traps.c | 39 +++++++++++++++++++++++++++++++++------
>> 1 file changed, 33 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
>> index ceb091f17a5b..f9ca5b911141 100644
>> --- a/arch/x86/kernel/traps.c
>> +++ b/arch/x86/kernel/traps.c
>> @@ -418,12 +418,6 @@ DEFINE_IDTENTRY_ERRORCODE(exc_segment_not_present)
>> SIGBUS, 0, NULL);
>> }
>>
>> -DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
>> -{
>> - do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
>> - 0, NULL);
>> -}
>> -
>> DEFINE_IDTENTRY_ERRORCODE(exc_alignment_check)
>> {
>> char *str = "alignment check";
>> @@ -866,6 +860,39 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
>> cond_local_irq_disable(regs);
>> }
>>
>> +#define SSFSTR "stack segment fault"
>> +
>> +DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
>> +{
>> + if (user_mode(regs))
>> + goto error_trap;
>> +
>> + if (cpu_feature_enabled(X86_FEATURE_FRED) &&
>> + fixup_exception(regs, X86_TRAP_SS, error_code, 0))
>> + return;
>> +
>> + if (cpu_feature_enabled(X86_FEATURE_LASS)) {
>> + enum kernel_exc_hint hint;
>> + unsigned long exc_addr;
>> +
>> + hint = get_kernel_exc_address(regs, &exc_addr);
>> + if (hint != EXC_NO_HINT) {
>
>The brackets are not needed for singular statements. Also the max line
>length is longer now. You can fit this all in a single line.
>
>> + printk(SSFSTR ", %s 0x%lx", kernel_exc_hint_help[hint],
>> + exc_addr);
>> + }
>> +
>
>> + if (hint != EXC_NON_CANONICAL)
>> + exc_addr = 0;
>> +
>> + die_addr(SSFSTR, regs, error_code, exc_addr);
>
>The variable names in die_addr() should be generalized as well. They
>seem to assume the caller to be a #GP handler.
>
>> + return;
>> + }
>> +
>> +error_trap:
>> + do_error_trap(regs, error_code, "stack segment", X86_TRAP_SS, SIGBUS,
>> + 0, NULL);
>> +}
>> +
>> static bool do_int3(struct pt_regs *regs)
>> {
>> int res;
>
An #SS can be generated by the kernel if RSP is corrupted. This is fatal, but as always we want to get a message out.
© 2016 - 2025 Red Hat, Inc.