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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx> > >> Signed-off-by: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx> > >> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx> > >> --- > >> 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