From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.zytor.com (terminus.zytor.com [198.137.202.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 43A0B30F81B for ; Sat, 20 Dec 2025 20:12:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.137.202.136 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766261568; cv=none; b=GaBSTKfEmXxrXhZuuRHNDQ19ARCu6YufIxhiKoSD3yXlPfWd1zsig+EqkbwuJeThD1Jlhgut0V0A/+bh4hsMi4TbUYk3jaQhHMUK1SLEWWcfzh6Q4YAbsbk3Wyow04uSOwOthaGpLQsq+5l7hIFDZ8s8zAKiyISZ2w7QKO1aCrI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766261568; c=relaxed/simple; bh=ZjM8bvJNOztbwLS2Jvx9sYnEKcNk2RxpOpn2CQieao4=; h=Date:From:To:CC:Subject:In-Reply-To:References:Message-ID: MIME-Version:Content-Type; b=kAp4Ii4RH7EYTmWzF7Wjw8yp43MmvgiTJAMaVcw0bkFXeNO3KcIKZXotqlAJriJHPcgUOerBdMkJHsNLax/TiZNOScHSsBlAI4XKt+zQS6aeFbHnex9jWQGO60BwPtFP/9Gaxb0hwlvKTrW2gZYliVU7iBUZ2YZl4G0PKcm2WZ0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=zytor.com; spf=pass smtp.mailfrom=zytor.com; dkim=pass (2048-bit key) header.d=zytor.com header.i=@zytor.com header.b=tnZ0VQZw; arc=none smtp.client-ip=198.137.202.136 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=zytor.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=zytor.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=zytor.com header.i=@zytor.com header.b="tnZ0VQZw" Received: from ehlo.thunderbird.net (c-76-133-66-138.hsd1.ca.comcast.net [76.133.66.138]) (authenticated bits=0) by mail.zytor.com (8.18.1/8.17.1) with ESMTPSA id 5BKKBmh21929526 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NO); Sat, 20 Dec 2025 12:11:48 -0800 DKIM-Filter: OpenDKIM Filter v2.11.0 mail.zytor.com 5BKKBmh21929526 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=zytor.com; s=2025112201; t=1766261509; bh=r9mnsrjpgh751SOlpNxHbfoLbgrMlnreSUECb2dK4Pw=; h=Date:From:To:CC:Subject:In-Reply-To:References:From; b=tnZ0VQZwFHop18A6GR+LukVc3DP8/BmfDwZh7FnC8jHXbLm2aQUj3OnkoqwUqFj9u Z5adNNSY6SvJNfmhRw6USrn1vAbyiwSSV54jsX2tCSNkYSmT3kDWIDVZmKnDu0bOil cnUC7eUyUqJ4bnrP3miaaQpqs+HwGjJliBi9maam3lj3Etgzea0t9zRcXG5PzHr07+ d3ZXqzd3zQb7f3SzNOXGi2OF88syGs4cwFnwzP92Q9Oa2VH9+opJadO8eZEHo8qRPv rR+zST6JdxPDqs0UZc7XKDBzL4oyG5HUJ88bPn3Du1ZXJ09uqE5B9g27k1ooCwSvaY 4dIbTqMb1mXYA== Date: Sat, 20 Dec 2025 12:11:46 -0800 From: "H. Peter Anvin" To: Brian Gerst , ellyndra CC: Peter Zijlstra , linux-kernel@vger.kernel.org, luto@kernel.org, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org Subject: Re: [PATCH v2] x86/syscall: make sys_call helpers static User-Agent: K-9 Mail for Android In-Reply-To: References: <20251218145714.70220-1-ellyesparza8@gmail.com> <20251220141037.GT3707891@noisy.programming.kicks-ass.net> Message-ID: <77216C33-487F-459D-8BAB-5DB0D7097386@zytor.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On December 20, 2025 10:21:49 AM PST, Brian Gerst wro= te: >On Sat, Dec 20, 2025 at 12:45=E2=80=AFPM ellyndra wrote: >> >> On Sat, Dec 20, 2025 at 2:10=E2=80=AFPM Brian Gerst wrote: >> > >> > On Sat, Dec 20, 2025 at 11:52=E2=80=AFAM ellyndra wrote: >> > > >> > > On Sat, Dec 20, 2025 at 11:32=E2=80=AFAM Brian Gerst wrote: >> > > > >> > > > On Sat, Dec 20, 2025 at 9:10=E2=80=AFAM Peter Zijlstra wrote: >> > > > > >> > > > > On Sat, Dec 20, 2025 at 08:50:31AM -0500, Brian Gerst wrote: >> > > > > > On Thu, Dec 18, 2025 at 10:01=E2=80=AFAM ellyndra wrote: >> > > > > > > >> > > > > > > Make 'x64_sys_call()' 'x32_sys_call()' and 'ia32_sys_call()= ' static to >> > > > > > > prevent them from being resolved via kallsyms and by extens= ion kprobes=2E >> > > > > > > >> > > > > > > These functions contain the final calls to the syscall hand= lers, having >> > > > > > > access to their address can be used to hook this calls alte= ring their >> > > > > > > behavior=2E Preventing symbol visibility limits the area of= such attacks=2E >> > > > > > > >> > > > > > > Suggested-by: Peter Zijlstra >> > > > > > > Signed-off-by: Elly I Esparza >> > > > > > > --- >> > > > > > > arch/x86/entry/syscall_32=2Ec | 6 ++++-- >> > > > > > > arch/x86/entry/syscall_64=2Ec | 12 ++++++++---- >> > > > > > > arch/x86/include/asm/syscall=2Eh | 8 -------- >> > > > > > > 3 files changed, 12 insertions(+), 14 deletions(-) >> > > > > > > >> > > > > > > diff --git a/arch/x86/entry/syscall_32=2Ec b/arch/x86/entry= /syscall_32=2Ec >> > > > > > > index 2b15ea17bb7c=2E=2E28c9a99fb2de 100644 >> > > > > > > --- a/arch/x86/entry/syscall_32=2Ec >> > > > > > > +++ b/arch/x86/entry/syscall_32=2Ec >> > > > > > > @@ -41,7 +41,7 @@ const sys_call_ptr_t sys_call_table[] =3D= { >> > > > > > > #endif >> > > > > > > >> > > > > > > #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(re= gs); >> > > > > > > -long ia32_sys_call(const struct pt_regs *regs, unsigned in= t nr) >> > > > > > > +static __always_inline long ia32_sys_call(const struct pt_= regs *regs, unsigned int nr) >> > > > > > > { >> > > > > > > switch (nr) { >> > > > > > > #include >> > > > > > > @@ -70,7 +70,7 @@ early_param("ia32_emulation", ia32_emulat= ion_override_cmdline); >> > > > > > > /* >> > > > > > > * Invoke a 32-bit syscall=2E Called with IRQs on in CT_S= TATE_KERNEL=2E >> > > > > > > */ >> > > > > > > -static __always_inline void do_syscall_32_irqs_on(struct p= t_regs *regs, int nr) >> > > > > > > +static noinstr void do_syscall_32_irqs_on(struct pt_regs *= regs, int nr) >> > > > > > > { >> > > > > > > /* >> > > > > > > * Convert negative numbers to very high and thus o= ut of range >> > > > > > > @@ -78,12 +78,14 @@ static __always_inline void do_syscall_= 32_irqs_on(struct pt_regs *regs, int nr) >> > > > > > > */ >> > > > > > > unsigned int unr =3D nr; >> > > > > > > >> > > > > > > + instrumentation_begin(); >> > > > > > > if (likely(unr < IA32_NR_syscalls)) { >> > > > > > > unr =3D array_index_nospec(unr, IA32_NR_sys= calls); >> > > > > > > regs->ax =3D ia32_sys_call(regs, unr); >> > > > > > > } else if (nr !=3D -1) { >> > > > > > > regs->ax =3D __ia32_sys_ni_syscall(regs); >> > > > > > > } >> > > > > > > + instrumentation_end(); >> > > > > > > } >> > > > > > > >> > > > > > > #ifdef CONFIG_IA32_EMULATION >> > > > > > > diff --git a/arch/x86/entry/syscall_64=2Ec b/arch/x86/entry= /syscall_64=2Ec >> > > > > > > index b6e68ea98b83=2E=2E55ad274f57f5 100644 >> > > > > > > --- a/arch/x86/entry/syscall_64=2Ec >> > > > > > > +++ b/arch/x86/entry/syscall_64=2Ec >> > > > > > > @@ -32,7 +32,7 @@ const sys_call_ptr_t sys_call_table[] =3D= { >> > > > > > > #undef __SYSCALL >> > > > > > > >> > > > > > > #define __SYSCALL(nr, sym) case nr: return __x64_##sym(reg= s); >> > > > > > > -long x64_sys_call(const struct pt_regs *regs, unsigned int= nr) >> > > > > > > +static __always_inline long x64_sys_call(const struct pt_r= egs *regs, unsigned int nr) >> > > > > > > { >> > > > > > > switch (nr) { >> > > > > > > #include >> > > > > > > @@ -40,15 +40,17 @@ long x64_sys_call(const struct pt_regs = *regs, unsigned int nr) >> > > > > > > } >> > > > > > > } >> > > > > > > >> > > > > > > -#ifdef CONFIG_X86_X32_ABI >> > > > > > > -long x32_sys_call(const struct pt_regs *regs, unsigned int= nr) >> > > > > > > +static __always_inline long x32_sys_call(const struct pt_r= egs *regs, unsigned int nr) >> > > > > > > { >> > > > > > > +#ifdef CONFIG_X86_X32_ABI >> > > > > > > switch (nr) { >> > > > > > > #include >> > > > > > > default: return __x64_sys_ni_syscall(regs); >> > > > > > > } >> > > > > > > -} >> > > > > > > +#else >> > > > > > > + return __x64_sys_ni_syscall(regs); >> > > > > > > #endif >> > > > > > > +} >> > > > > > > >> > > > > > > static __always_inline bool do_syscall_x64(struct pt_regs = *regs, int nr) >> > > > > > > { >> > > > > > > @@ -58,11 +60,13 @@ static __always_inline bool do_syscall_= x64(struct pt_regs *regs, int nr) >> > > > > > > */ >> > > > > > > unsigned int unr =3D nr; >> > > > > > > >> > > > > > > + instrumentation_begin(); >> > > > > > > if (likely(unr < NR_syscalls)) { >> > > > > > > unr =3D array_index_nospec(unr, NR_syscalls= ); >> > > > > > > regs->ax =3D x64_sys_call(regs, unr); >> > > > > > > return true; >> > > > > > > } >> > > > > > > + instrumentation_end(); >> > > > > > > return false; >> > > > > > > } >> > > > > > > >> > > > > > > diff --git a/arch/x86/include/asm/syscall=2Eh b/arch/x86/in= clude/asm/syscall=2Eh >> > > > > > > index c10dbb74cd00=2E=2E59a406074dc0 100644 >> > > > > > > --- a/arch/x86/include/asm/syscall=2Eh >> > > > > > > +++ b/arch/x86/include/asm/syscall=2Eh >> > > > > > > @@ -20,14 +20,6 @@ >> > > > > > > typedef long (*sys_call_ptr_t)(const struct pt_regs *); >> > > > > > > extern const sys_call_ptr_t sys_call_table[]; >> > > > > > > >> > > > > > > -/* >> > > > > > > - * These may not exist, but still put the prototypes in so= we >> > > > > > > - * can use IS_ENABLED()=2E >> > > > > > > - */ >> > > > > > > -extern long ia32_sys_call(const struct pt_regs *, unsigned= int nr); >> > > > > > > -extern long x32_sys_call(const struct pt_regs *, unsigned = int nr); >> > > > > > > -extern long x64_sys_call(const struct pt_regs *, unsigned = int nr); >> > > > > > > - >> > > > > > > /* >> > > > > > > * Only the low 32 bits of orig_ax are meaningful, so we r= eturn int=2E >> > > > > > > * This importantly ignores the high bits on 64-bit, so co= mparisons >> > > > > > > -- >> > > > > > > 2=2E43=2E0 >> > > > > > > >> > > > > > > >> > > > > > >> > > > > > The main benefit of having the switch in a discrete function = is that >> > > > > > it allows the compiler to do tail-call optimizations, reducin= g text >> > > > > > size=2E Isn't it possible to blacklist functions from kprobe= s? >> > > > > >> > > > > Ofcourse=2E But I'm not sure I understand your concern=2E Both >> > > > > x{64,32}_sys_call() only have a single caller, inlining them av= oids the >> > > > > whole call, tail or otherwise, how is that a bad thing? >> > > > > >> > > > >> > > > text data bss dec hex filename >> > > > 13194 128 0 13322 340a >> > > > arch/x86/entry/syscall_64=2Eo >> > > > 10239 136 0 10375 2887 >> > > > arch/x86/entry/syscall_64=2Eo=2Eorig >> > > > >> > > > That's a 3k increase in text size when it's inlined=2E >> > > > >> > > > >> > > > Brian Gerst >> > > >> > > I just check size too and agree with Brian=2E Regarding >> > > blacklisting, that was the original idea, it has the added problem >> > > of kallsyms_lookup_name() being available to bypass the blacklist= =2E >> > > >> > > Is this fine for next version then? >> > > >> > > diff --git a/arch/x86/entry/syscall_32=2Ec b/arch/x86/entry/syscall= _32=2Ec >> > > index 2b15ea17bb7c=2E=2E75d34d2c8067 100644 >> > > --- a/arch/x86/entry/syscall_32=2Ec >> > > +++ b/arch/x86/entry/syscall_32=2Ec >> > > @@ -1,6 +1,7 @@ >> > > // SPDX-License-Identifier: GPL-2=2E0-only >> > > /* 32-bit system call dispatch */ >> > > >> > > +#include >> > > #include >> > > #include >> > > #include >> > > @@ -48,6 +49,7 @@ long ia32_sys_call(const struct pt_regs *regs, >> > > unsigned int nr) >> > > default: return __ia32_sys_ni_syscall(regs); >> > > } >> > > } >> > > +NOKPROBE_SYMBOL(ia32_sys_call) >> > > >> > > static __always_inline int syscall_32_enter(struct pt_regs *regs) >> > > { >> > > diff --git a/arch/x86/entry/syscall_64=2Ec b/arch/x86/entry/syscall= _64=2Ec >> > > index b6e68ea98b83=2E=2E1e8045a499d4 100644 >> > > --- a/arch/x86/entry/syscall_64=2Ec >> > > +++ b/arch/x86/entry/syscall_64=2Ec >> > > @@ -1,6 +1,7 @@ >> > > // SPDX-License-Identifier: GPL-2=2E0-only >> > > /* 64-bit system call dispatch */ >> > > >> > > +#include >> > > #include >> > > #include >> > > #include >> > > @@ -39,6 +40,7 @@ long x64_sys_call(const struct pt_regs *regs, uns= igned int nr) >> > > default: return __x64_sys_ni_syscall(regs); >> > > } >> > > } >> > > +NOKPROBE_SYMBOL(x64_sys_call) >> > > >> > > #ifdef CONFIG_X86_X32_ABI >> > > long x32_sys_call(const struct pt_regs *regs, unsigned int nr) >> > > @@ -48,6 +50,7 @@ long x32_sys_call(const struct pt_regs *regs, uns= igned int nr) >> > > default: return __x64_sys_ni_syscall(regs); >> > > } >> > > } >> > > +NOKPROBE_SYMBOL(x32_sys_call) >> > > #endif >> > > >> > > static __always_inline bool do_syscall_x64(struct pt_regs *regs, i= nt nr) >> > > diff --git a/kernel/kallsyms=2Ec b/kernel/kallsyms=2Ec >> > > index 1e7635864124=2E=2E13a7c0fdb5da 100644 >> > > --- a/kernel/kallsyms=2Ec >> > > +++ b/kernel/kallsyms=2Ec >> > > @@ -234,6 +234,7 @@ unsigned long kallsyms_lookup_name(const char *= name) >> > > >> > > return module_kallsyms_lookup_name(name); >> > > } >> > > +NOKPROBE_SYMBOL(kallsyms_lookup_name) >> > > >> > > /* >> > > * Iterate over all symbols in vmlinux=2E For symbols from module= s use >> > >> > Hardening isn't my area of expertise, but what prevents hooking >> > individual syscalls in the same manner? >> > >> > >> > Brian Gerst >> >> You can probably still alter syscall handlers if that's what you mean >> but that would require more effort and it's more prone to failure=2E >> >> Hooking is generally done by changing the address of a branch since >> it's more reliable and easier to clean up=2E The aim was to eliminate >> that low-effort popular attack vector=2E > >Inlining doesn't really fix that=2E It just moves the target branch to >another function=2E > 3K for inlining in one of the hottest paths in the system doesn't seem unr= easonable, though, if the performance is neutral or better=2E