* [PATCH v2] x86/syscall: make sys_call helpers static @ 2025-12-18 14:57 ellyndra 2025-12-20 13:50 ` Brian Gerst 0 siblings, 1 reply; 10+ messages in thread From: ellyndra @ 2025-12-18 14:57 UTC (permalink / raw) To: linux-kernel Cc: luto, tglx, mingo, bp, dave.hansen, x86, hpa, ellyndra, Peter Zijlstra Make 'x64_sys_call()' 'x32_sys_call()' and 'ia32_sys_call()' static to prevent them from being resolved via kallsyms and by extension kprobes. These functions contain the final calls to the syscall handlers, having access to their address can be used to hook this calls altering their behavior. Preventing symbol visibility limits the area of such attacks. Suggested-by: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Elly I Esparza <ellyesparza8@gmail.com> --- arch/x86/entry/syscall_32.c | 6 ++++-- arch/x86/entry/syscall_64.c | 12 ++++++++---- arch/x86/include/asm/syscall.h | 8 -------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c index 2b15ea17bb7c..28c9a99fb2de 100644 --- a/arch/x86/entry/syscall_32.c +++ b/arch/x86/entry/syscall_32.c @@ -41,7 +41,7 @@ const sys_call_ptr_t sys_call_table[] = { #endif #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(regs); -long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) +static __always_inline long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) { switch (nr) { #include <asm/syscalls_32.h> @@ -70,7 +70,7 @@ early_param("ia32_emulation", ia32_emulation_override_cmdline); /* * Invoke a 32-bit syscall. Called with IRQs on in CT_STATE_KERNEL. */ -static __always_inline void do_syscall_32_irqs_on(struct pt_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 out of range @@ -78,12 +78,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs, int nr) */ unsigned int unr = nr; + instrumentation_begin(); if (likely(unr < IA32_NR_syscalls)) { unr = array_index_nospec(unr, IA32_NR_syscalls); regs->ax = ia32_sys_call(regs, unr); } else if (nr != -1) { regs->ax = __ia32_sys_ni_syscall(regs); } + instrumentation_end(); } #ifdef CONFIG_IA32_EMULATION diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c index b6e68ea98b83..55ad274f57f5 100644 --- a/arch/x86/entry/syscall_64.c +++ b/arch/x86/entry/syscall_64.c @@ -32,7 +32,7 @@ const sys_call_ptr_t sys_call_table[] = { #undef __SYSCALL #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs); -long x64_sys_call(const struct pt_regs *regs, unsigned int nr) +static __always_inline long x64_sys_call(const struct pt_regs *regs, unsigned int nr) { switch (nr) { #include <asm/syscalls_64.h> @@ -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_regs *regs, unsigned int nr) { +#ifdef CONFIG_X86_X32_ABI switch (nr) { #include <asm/syscalls_x32.h> 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 = nr; + instrumentation_begin(); if (likely(unr < NR_syscalls)) { unr = array_index_nospec(unr, NR_syscalls); regs->ax = x64_sys_call(regs, unr); return true; } + instrumentation_end(); return false; } diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h index c10dbb74cd00..59a406074dc0 100644 --- a/arch/x86/include/asm/syscall.h +++ b/arch/x86/include/asm/syscall.h @@ -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(). - */ -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 return int. * This importantly ignores the high bits on 64-bit, so comparisons -- 2.43.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] x86/syscall: make sys_call helpers static 2025-12-18 14:57 [PATCH v2] x86/syscall: make sys_call helpers static ellyndra @ 2025-12-20 13:50 ` Brian Gerst 2025-12-20 14:10 ` Peter Zijlstra 0 siblings, 1 reply; 10+ messages in thread From: Brian Gerst @ 2025-12-20 13:50 UTC (permalink / raw) To: ellyndra Cc: linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, hpa, Peter Zijlstra On Thu, Dec 18, 2025 at 10:01 AM ellyndra <ellyesparza8@gmail.com> wrote: > > Make 'x64_sys_call()' 'x32_sys_call()' and 'ia32_sys_call()' static to > prevent them from being resolved via kallsyms and by extension kprobes. > > These functions contain the final calls to the syscall handlers, having > access to their address can be used to hook this calls altering their > behavior. Preventing symbol visibility limits the area of such attacks. > > Suggested-by: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: Elly I Esparza <ellyesparza8@gmail.com> > --- > arch/x86/entry/syscall_32.c | 6 ++++-- > arch/x86/entry/syscall_64.c | 12 ++++++++---- > arch/x86/include/asm/syscall.h | 8 -------- > 3 files changed, 12 insertions(+), 14 deletions(-) > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c > index 2b15ea17bb7c..28c9a99fb2de 100644 > --- a/arch/x86/entry/syscall_32.c > +++ b/arch/x86/entry/syscall_32.c > @@ -41,7 +41,7 @@ const sys_call_ptr_t sys_call_table[] = { > #endif > > #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(regs); > -long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > +static __always_inline long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > { > switch (nr) { > #include <asm/syscalls_32.h> > @@ -70,7 +70,7 @@ early_param("ia32_emulation", ia32_emulation_override_cmdline); > /* > * Invoke a 32-bit syscall. Called with IRQs on in CT_STATE_KERNEL. > */ > -static __always_inline void do_syscall_32_irqs_on(struct pt_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 out of range > @@ -78,12 +78,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs, int nr) > */ > unsigned int unr = nr; > > + instrumentation_begin(); > if (likely(unr < IA32_NR_syscalls)) { > unr = array_index_nospec(unr, IA32_NR_syscalls); > regs->ax = ia32_sys_call(regs, unr); > } else if (nr != -1) { > regs->ax = __ia32_sys_ni_syscall(regs); > } > + instrumentation_end(); > } > > #ifdef CONFIG_IA32_EMULATION > diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c > index b6e68ea98b83..55ad274f57f5 100644 > --- a/arch/x86/entry/syscall_64.c > +++ b/arch/x86/entry/syscall_64.c > @@ -32,7 +32,7 @@ const sys_call_ptr_t sys_call_table[] = { > #undef __SYSCALL > > #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs); > -long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > +static __always_inline long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > { > switch (nr) { > #include <asm/syscalls_64.h> > @@ -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_regs *regs, unsigned int nr) > { > +#ifdef CONFIG_X86_X32_ABI > switch (nr) { > #include <asm/syscalls_x32.h> > 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 = nr; > > + instrumentation_begin(); > if (likely(unr < NR_syscalls)) { > unr = array_index_nospec(unr, NR_syscalls); > regs->ax = x64_sys_call(regs, unr); > return true; > } > + instrumentation_end(); > return false; > } > > diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h > index c10dbb74cd00..59a406074dc0 100644 > --- a/arch/x86/include/asm/syscall.h > +++ b/arch/x86/include/asm/syscall.h > @@ -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(). > - */ > -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 return int. > * This importantly ignores the high bits on 64-bit, so comparisons > -- > 2.43.0 > > The main benefit of having the switch in a discrete function is that it allows the compiler to do tail-call optimizations, reducing text size. Isn't it possible to blacklist functions from kprobes? Brian Gerst ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] x86/syscall: make sys_call helpers static 2025-12-20 13:50 ` Brian Gerst @ 2025-12-20 14:10 ` Peter Zijlstra 2025-12-20 14:32 ` Brian Gerst 0 siblings, 1 reply; 10+ messages in thread From: Peter Zijlstra @ 2025-12-20 14:10 UTC (permalink / raw) To: Brian Gerst Cc: ellyndra, linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, hpa On Sat, Dec 20, 2025 at 08:50:31AM -0500, Brian Gerst wrote: > On Thu, Dec 18, 2025 at 10:01 AM ellyndra <ellyesparza8@gmail.com> wrote: > > > > Make 'x64_sys_call()' 'x32_sys_call()' and 'ia32_sys_call()' static to > > prevent them from being resolved via kallsyms and by extension kprobes. > > > > These functions contain the final calls to the syscall handlers, having > > access to their address can be used to hook this calls altering their > > behavior. Preventing symbol visibility limits the area of such attacks. > > > > Suggested-by: Peter Zijlstra <peterz@infradead.org> > > Signed-off-by: Elly I Esparza <ellyesparza8@gmail.com> > > --- > > arch/x86/entry/syscall_32.c | 6 ++++-- > > arch/x86/entry/syscall_64.c | 12 ++++++++---- > > arch/x86/include/asm/syscall.h | 8 -------- > > 3 files changed, 12 insertions(+), 14 deletions(-) > > > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c > > index 2b15ea17bb7c..28c9a99fb2de 100644 > > --- a/arch/x86/entry/syscall_32.c > > +++ b/arch/x86/entry/syscall_32.c > > @@ -41,7 +41,7 @@ const sys_call_ptr_t sys_call_table[] = { > > #endif > > > > #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(regs); > > -long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > +static __always_inline long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > { > > switch (nr) { > > #include <asm/syscalls_32.h> > > @@ -70,7 +70,7 @@ early_param("ia32_emulation", ia32_emulation_override_cmdline); > > /* > > * Invoke a 32-bit syscall. Called with IRQs on in CT_STATE_KERNEL. > > */ > > -static __always_inline void do_syscall_32_irqs_on(struct pt_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 out of range > > @@ -78,12 +78,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs, int nr) > > */ > > unsigned int unr = nr; > > > > + instrumentation_begin(); > > if (likely(unr < IA32_NR_syscalls)) { > > unr = array_index_nospec(unr, IA32_NR_syscalls); > > regs->ax = ia32_sys_call(regs, unr); > > } else if (nr != -1) { > > regs->ax = __ia32_sys_ni_syscall(regs); > > } > > + instrumentation_end(); > > } > > > > #ifdef CONFIG_IA32_EMULATION > > diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c > > index b6e68ea98b83..55ad274f57f5 100644 > > --- a/arch/x86/entry/syscall_64.c > > +++ b/arch/x86/entry/syscall_64.c > > @@ -32,7 +32,7 @@ const sys_call_ptr_t sys_call_table[] = { > > #undef __SYSCALL > > > > #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs); > > -long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > +static __always_inline long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > { > > switch (nr) { > > #include <asm/syscalls_64.h> > > @@ -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_regs *regs, unsigned int nr) > > { > > +#ifdef CONFIG_X86_X32_ABI > > switch (nr) { > > #include <asm/syscalls_x32.h> > > 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 = nr; > > > > + instrumentation_begin(); > > if (likely(unr < NR_syscalls)) { > > unr = array_index_nospec(unr, NR_syscalls); > > regs->ax = x64_sys_call(regs, unr); > > return true; > > } > > + instrumentation_end(); > > return false; > > } > > > > diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h > > index c10dbb74cd00..59a406074dc0 100644 > > --- a/arch/x86/include/asm/syscall.h > > +++ b/arch/x86/include/asm/syscall.h > > @@ -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(). > > - */ > > -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 return int. > > * This importantly ignores the high bits on 64-bit, so comparisons > > -- > > 2.43.0 > > > > > > The main benefit of having the switch in a discrete function is that > it allows the compiler to do tail-call optimizations, reducing text > size. Isn't it possible to blacklist functions from kprobes? Ofcourse. But I'm not sure I understand your concern. Both x{64,32}_sys_call() only have a single caller, inlining them avoids the whole call, tail or otherwise, how is that a bad thing? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] x86/syscall: make sys_call helpers static 2025-12-20 14:10 ` Peter Zijlstra @ 2025-12-20 14:32 ` Brian Gerst 2025-12-20 16:52 ` ellyndra 2025-12-20 20:40 ` Peter Zijlstra 0 siblings, 2 replies; 10+ messages in thread From: Brian Gerst @ 2025-12-20 14:32 UTC (permalink / raw) To: Peter Zijlstra Cc: ellyndra, linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, hpa On Sat, Dec 20, 2025 at 9:10 AM Peter Zijlstra <peterz@infradead.org> wrote: > > On Sat, Dec 20, 2025 at 08:50:31AM -0500, Brian Gerst wrote: > > On Thu, Dec 18, 2025 at 10:01 AM ellyndra <ellyesparza8@gmail.com> wrote: > > > > > > Make 'x64_sys_call()' 'x32_sys_call()' and 'ia32_sys_call()' static to > > > prevent them from being resolved via kallsyms and by extension kprobes. > > > > > > These functions contain the final calls to the syscall handlers, having > > > access to their address can be used to hook this calls altering their > > > behavior. Preventing symbol visibility limits the area of such attacks. > > > > > > Suggested-by: Peter Zijlstra <peterz@infradead.org> > > > Signed-off-by: Elly I Esparza <ellyesparza8@gmail.com> > > > --- > > > arch/x86/entry/syscall_32.c | 6 ++++-- > > > arch/x86/entry/syscall_64.c | 12 ++++++++---- > > > arch/x86/include/asm/syscall.h | 8 -------- > > > 3 files changed, 12 insertions(+), 14 deletions(-) > > > > > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c > > > index 2b15ea17bb7c..28c9a99fb2de 100644 > > > --- a/arch/x86/entry/syscall_32.c > > > +++ b/arch/x86/entry/syscall_32.c > > > @@ -41,7 +41,7 @@ const sys_call_ptr_t sys_call_table[] = { > > > #endif > > > > > > #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(regs); > > > -long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > > +static __always_inline long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > > { > > > switch (nr) { > > > #include <asm/syscalls_32.h> > > > @@ -70,7 +70,7 @@ early_param("ia32_emulation", ia32_emulation_override_cmdline); > > > /* > > > * Invoke a 32-bit syscall. Called with IRQs on in CT_STATE_KERNEL. > > > */ > > > -static __always_inline void do_syscall_32_irqs_on(struct pt_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 out of range > > > @@ -78,12 +78,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs, int nr) > > > */ > > > unsigned int unr = nr; > > > > > > + instrumentation_begin(); > > > if (likely(unr < IA32_NR_syscalls)) { > > > unr = array_index_nospec(unr, IA32_NR_syscalls); > > > regs->ax = ia32_sys_call(regs, unr); > > > } else if (nr != -1) { > > > regs->ax = __ia32_sys_ni_syscall(regs); > > > } > > > + instrumentation_end(); > > > } > > > > > > #ifdef CONFIG_IA32_EMULATION > > > diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c > > > index b6e68ea98b83..55ad274f57f5 100644 > > > --- a/arch/x86/entry/syscall_64.c > > > +++ b/arch/x86/entry/syscall_64.c > > > @@ -32,7 +32,7 @@ const sys_call_ptr_t sys_call_table[] = { > > > #undef __SYSCALL > > > > > > #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs); > > > -long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > > +static __always_inline long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > > { > > > switch (nr) { > > > #include <asm/syscalls_64.h> > > > @@ -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_regs *regs, unsigned int nr) > > > { > > > +#ifdef CONFIG_X86_X32_ABI > > > switch (nr) { > > > #include <asm/syscalls_x32.h> > > > 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 = nr; > > > > > > + instrumentation_begin(); > > > if (likely(unr < NR_syscalls)) { > > > unr = array_index_nospec(unr, NR_syscalls); > > > regs->ax = x64_sys_call(regs, unr); > > > return true; > > > } > > > + instrumentation_end(); > > > return false; > > > } > > > > > > diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h > > > index c10dbb74cd00..59a406074dc0 100644 > > > --- a/arch/x86/include/asm/syscall.h > > > +++ b/arch/x86/include/asm/syscall.h > > > @@ -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(). > > > - */ > > > -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 return int. > > > * This importantly ignores the high bits on 64-bit, so comparisons > > > -- > > > 2.43.0 > > > > > > > > > > The main benefit of having the switch in a discrete function is that > > it allows the compiler to do tail-call optimizations, reducing text > > size. Isn't it possible to blacklist functions from kprobes? > > Ofcourse. But I'm not sure I understand your concern. Both > x{64,32}_sys_call() only have a single caller, inlining them avoids 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.o 10239 136 0 10375 2887 arch/x86/entry/syscall_64.o.orig That's a 3k increase in text size when it's inlined. Brian Gerst ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] x86/syscall: make sys_call helpers static 2025-12-20 14:32 ` Brian Gerst @ 2025-12-20 16:52 ` ellyndra 2025-12-20 17:10 ` Brian Gerst 2025-12-20 20:40 ` Peter Zijlstra 1 sibling, 1 reply; 10+ messages in thread From: ellyndra @ 2025-12-20 16:52 UTC (permalink / raw) To: Brian Gerst Cc: Peter Zijlstra, linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, hpa On Sat, Dec 20, 2025 at 11:32 AM Brian Gerst <brgerst@gmail.com> wrote: > > On Sat, Dec 20, 2025 at 9:10 AM Peter Zijlstra <peterz@infradead.org> wrote: > > > > On Sat, Dec 20, 2025 at 08:50:31AM -0500, Brian Gerst wrote: > > > On Thu, Dec 18, 2025 at 10:01 AM ellyndra <ellyesparza8@gmail.com> wrote: > > > > > > > > Make 'x64_sys_call()' 'x32_sys_call()' and 'ia32_sys_call()' static to > > > > prevent them from being resolved via kallsyms and by extension kprobes. > > > > > > > > These functions contain the final calls to the syscall handlers, having > > > > access to their address can be used to hook this calls altering their > > > > behavior. Preventing symbol visibility limits the area of such attacks. > > > > > > > > Suggested-by: Peter Zijlstra <peterz@infradead.org> > > > > Signed-off-by: Elly I Esparza <ellyesparza8@gmail.com> > > > > --- > > > > arch/x86/entry/syscall_32.c | 6 ++++-- > > > > arch/x86/entry/syscall_64.c | 12 ++++++++---- > > > > arch/x86/include/asm/syscall.h | 8 -------- > > > > 3 files changed, 12 insertions(+), 14 deletions(-) > > > > > > > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c > > > > index 2b15ea17bb7c..28c9a99fb2de 100644 > > > > --- a/arch/x86/entry/syscall_32.c > > > > +++ b/arch/x86/entry/syscall_32.c > > > > @@ -41,7 +41,7 @@ const sys_call_ptr_t sys_call_table[] = { > > > > #endif > > > > > > > > #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(regs); > > > > -long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > +static __always_inline long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > { > > > > switch (nr) { > > > > #include <asm/syscalls_32.h> > > > > @@ -70,7 +70,7 @@ early_param("ia32_emulation", ia32_emulation_override_cmdline); > > > > /* > > > > * Invoke a 32-bit syscall. Called with IRQs on in CT_STATE_KERNEL. > > > > */ > > > > -static __always_inline void do_syscall_32_irqs_on(struct pt_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 out of range > > > > @@ -78,12 +78,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs, int nr) > > > > */ > > > > unsigned int unr = nr; > > > > > > > > + instrumentation_begin(); > > > > if (likely(unr < IA32_NR_syscalls)) { > > > > unr = array_index_nospec(unr, IA32_NR_syscalls); > > > > regs->ax = ia32_sys_call(regs, unr); > > > > } else if (nr != -1) { > > > > regs->ax = __ia32_sys_ni_syscall(regs); > > > > } > > > > + instrumentation_end(); > > > > } > > > > > > > > #ifdef CONFIG_IA32_EMULATION > > > > diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c > > > > index b6e68ea98b83..55ad274f57f5 100644 > > > > --- a/arch/x86/entry/syscall_64.c > > > > +++ b/arch/x86/entry/syscall_64.c > > > > @@ -32,7 +32,7 @@ const sys_call_ptr_t sys_call_table[] = { > > > > #undef __SYSCALL > > > > > > > > #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs); > > > > -long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > +static __always_inline long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > { > > > > switch (nr) { > > > > #include <asm/syscalls_64.h> > > > > @@ -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_regs *regs, unsigned int nr) > > > > { > > > > +#ifdef CONFIG_X86_X32_ABI > > > > switch (nr) { > > > > #include <asm/syscalls_x32.h> > > > > 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 = nr; > > > > > > > > + instrumentation_begin(); > > > > if (likely(unr < NR_syscalls)) { > > > > unr = array_index_nospec(unr, NR_syscalls); > > > > regs->ax = x64_sys_call(regs, unr); > > > > return true; > > > > } > > > > + instrumentation_end(); > > > > return false; > > > > } > > > > > > > > diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h > > > > index c10dbb74cd00..59a406074dc0 100644 > > > > --- a/arch/x86/include/asm/syscall.h > > > > +++ b/arch/x86/include/asm/syscall.h > > > > @@ -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(). > > > > - */ > > > > -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 return int. > > > > * This importantly ignores the high bits on 64-bit, so comparisons > > > > -- > > > > 2.43.0 > > > > > > > > > > > > > > The main benefit of having the switch in a discrete function is that > > > it allows the compiler to do tail-call optimizations, reducing text > > > size. Isn't it possible to blacklist functions from kprobes? > > > > Ofcourse. But I'm not sure I understand your concern. Both > > x{64,32}_sys_call() only have a single caller, inlining them avoids 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.o > 10239 136 0 10375 2887 > arch/x86/entry/syscall_64.o.orig > > That's a 3k increase in text size when it's inlined. > > > Brian Gerst I just check size too and agree with Brian. Regarding blacklisting, that was the original idea, it has the added problem of kallsyms_lookup_name() being available to bypass the blacklist. Is this fine for next version then? diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c index 2b15ea17bb7c..75d34d2c8067 100644 --- a/arch/x86/entry/syscall_32.c +++ b/arch/x86/entry/syscall_32.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* 32-bit system call dispatch */ +#include <linux/kprobes.h> #include <linux/linkage.h> #include <linux/sys.h> #include <linux/cache.h> @@ -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.c b/arch/x86/entry/syscall_64.c index b6e68ea98b83..1e8045a499d4 100644 --- a/arch/x86/entry/syscall_64.c +++ b/arch/x86/entry/syscall_64.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* 64-bit system call dispatch */ +#include <linux/kprobes.h> #include <linux/linkage.h> #include <linux/sys.h> #include <linux/cache.h> @@ -39,6 +40,7 @@ long x64_sys_call(const struct pt_regs *regs, unsigned 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, unsigned 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, int nr) diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c index 1e7635864124..13a7c0fdb5da 100644 --- a/kernel/kallsyms.c +++ b/kernel/kallsyms.c @@ -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. For symbols from modules use ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] x86/syscall: make sys_call helpers static 2025-12-20 16:52 ` ellyndra @ 2025-12-20 17:10 ` Brian Gerst 2025-12-20 17:45 ` ellyndra 0 siblings, 1 reply; 10+ messages in thread From: Brian Gerst @ 2025-12-20 17:10 UTC (permalink / raw) To: ellyndra Cc: Peter Zijlstra, linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, hpa On Sat, Dec 20, 2025 at 11:52 AM ellyndra <ellyesparza8@gmail.com> wrote: > > On Sat, Dec 20, 2025 at 11:32 AM Brian Gerst <brgerst@gmail.com> wrote: > > > > On Sat, Dec 20, 2025 at 9:10 AM Peter Zijlstra <peterz@infradead.org> wrote: > > > > > > On Sat, Dec 20, 2025 at 08:50:31AM -0500, Brian Gerst wrote: > > > > On Thu, Dec 18, 2025 at 10:01 AM ellyndra <ellyesparza8@gmail.com> wrote: > > > > > > > > > > Make 'x64_sys_call()' 'x32_sys_call()' and 'ia32_sys_call()' static to > > > > > prevent them from being resolved via kallsyms and by extension kprobes. > > > > > > > > > > These functions contain the final calls to the syscall handlers, having > > > > > access to their address can be used to hook this calls altering their > > > > > behavior. Preventing symbol visibility limits the area of such attacks. > > > > > > > > > > Suggested-by: Peter Zijlstra <peterz@infradead.org> > > > > > Signed-off-by: Elly I Esparza <ellyesparza8@gmail.com> > > > > > --- > > > > > arch/x86/entry/syscall_32.c | 6 ++++-- > > > > > arch/x86/entry/syscall_64.c | 12 ++++++++---- > > > > > arch/x86/include/asm/syscall.h | 8 -------- > > > > > 3 files changed, 12 insertions(+), 14 deletions(-) > > > > > > > > > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c > > > > > index 2b15ea17bb7c..28c9a99fb2de 100644 > > > > > --- a/arch/x86/entry/syscall_32.c > > > > > +++ b/arch/x86/entry/syscall_32.c > > > > > @@ -41,7 +41,7 @@ const sys_call_ptr_t sys_call_table[] = { > > > > > #endif > > > > > > > > > > #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(regs); > > > > > -long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > +static __always_inline long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > { > > > > > switch (nr) { > > > > > #include <asm/syscalls_32.h> > > > > > @@ -70,7 +70,7 @@ early_param("ia32_emulation", ia32_emulation_override_cmdline); > > > > > /* > > > > > * Invoke a 32-bit syscall. Called with IRQs on in CT_STATE_KERNEL. > > > > > */ > > > > > -static __always_inline void do_syscall_32_irqs_on(struct pt_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 out of range > > > > > @@ -78,12 +78,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs, int nr) > > > > > */ > > > > > unsigned int unr = nr; > > > > > > > > > > + instrumentation_begin(); > > > > > if (likely(unr < IA32_NR_syscalls)) { > > > > > unr = array_index_nospec(unr, IA32_NR_syscalls); > > > > > regs->ax = ia32_sys_call(regs, unr); > > > > > } else if (nr != -1) { > > > > > regs->ax = __ia32_sys_ni_syscall(regs); > > > > > } > > > > > + instrumentation_end(); > > > > > } > > > > > > > > > > #ifdef CONFIG_IA32_EMULATION > > > > > diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c > > > > > index b6e68ea98b83..55ad274f57f5 100644 > > > > > --- a/arch/x86/entry/syscall_64.c > > > > > +++ b/arch/x86/entry/syscall_64.c > > > > > @@ -32,7 +32,7 @@ const sys_call_ptr_t sys_call_table[] = { > > > > > #undef __SYSCALL > > > > > > > > > > #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs); > > > > > -long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > +static __always_inline long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > { > > > > > switch (nr) { > > > > > #include <asm/syscalls_64.h> > > > > > @@ -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_regs *regs, unsigned int nr) > > > > > { > > > > > +#ifdef CONFIG_X86_X32_ABI > > > > > switch (nr) { > > > > > #include <asm/syscalls_x32.h> > > > > > 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 = nr; > > > > > > > > > > + instrumentation_begin(); > > > > > if (likely(unr < NR_syscalls)) { > > > > > unr = array_index_nospec(unr, NR_syscalls); > > > > > regs->ax = x64_sys_call(regs, unr); > > > > > return true; > > > > > } > > > > > + instrumentation_end(); > > > > > return false; > > > > > } > > > > > > > > > > diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h > > > > > index c10dbb74cd00..59a406074dc0 100644 > > > > > --- a/arch/x86/include/asm/syscall.h > > > > > +++ b/arch/x86/include/asm/syscall.h > > > > > @@ -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(). > > > > > - */ > > > > > -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 return int. > > > > > * This importantly ignores the high bits on 64-bit, so comparisons > > > > > -- > > > > > 2.43.0 > > > > > > > > > > > > > > > > > > The main benefit of having the switch in a discrete function is that > > > > it allows the compiler to do tail-call optimizations, reducing text > > > > size. Isn't it possible to blacklist functions from kprobes? > > > > > > Ofcourse. But I'm not sure I understand your concern. Both > > > x{64,32}_sys_call() only have a single caller, inlining them avoids 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.o > > 10239 136 0 10375 2887 > > arch/x86/entry/syscall_64.o.orig > > > > That's a 3k increase in text size when it's inlined. > > > > > > Brian Gerst > > I just check size too and agree with Brian. Regarding > blacklisting, that was the original idea, it has the added problem > of kallsyms_lookup_name() being available to bypass the blacklist. > > Is this fine for next version then? > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c > index 2b15ea17bb7c..75d34d2c8067 100644 > --- a/arch/x86/entry/syscall_32.c > +++ b/arch/x86/entry/syscall_32.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* 32-bit system call dispatch */ > > +#include <linux/kprobes.h> > #include <linux/linkage.h> > #include <linux/sys.h> > #include <linux/cache.h> > @@ -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.c b/arch/x86/entry/syscall_64.c > index b6e68ea98b83..1e8045a499d4 100644 > --- a/arch/x86/entry/syscall_64.c > +++ b/arch/x86/entry/syscall_64.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* 64-bit system call dispatch */ > > +#include <linux/kprobes.h> > #include <linux/linkage.h> > #include <linux/sys.h> > #include <linux/cache.h> > @@ -39,6 +40,7 @@ long x64_sys_call(const struct pt_regs *regs, unsigned 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, unsigned 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, int nr) > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c > index 1e7635864124..13a7c0fdb5da 100644 > --- a/kernel/kallsyms.c > +++ b/kernel/kallsyms.c > @@ -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. For symbols from modules use Hardening isn't my area of expertise, but what prevents hooking individual syscalls in the same manner? Brian Gerst ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] x86/syscall: make sys_call helpers static 2025-12-20 17:10 ` Brian Gerst @ 2025-12-20 17:45 ` ellyndra 2025-12-20 18:21 ` Brian Gerst 0 siblings, 1 reply; 10+ messages in thread From: ellyndra @ 2025-12-20 17:45 UTC (permalink / raw) To: Brian Gerst Cc: Peter Zijlstra, linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, hpa On Sat, Dec 20, 2025 at 2:10 PM Brian Gerst <brgerst@gmail.com> wrote: > > On Sat, Dec 20, 2025 at 11:52 AM ellyndra <ellyesparza8@gmail.com> wrote: > > > > On Sat, Dec 20, 2025 at 11:32 AM Brian Gerst <brgerst@gmail.com> wrote: > > > > > > On Sat, Dec 20, 2025 at 9:10 AM Peter Zijlstra <peterz@infradead.org> wrote: > > > > > > > > On Sat, Dec 20, 2025 at 08:50:31AM -0500, Brian Gerst wrote: > > > > > On Thu, Dec 18, 2025 at 10:01 AM ellyndra <ellyesparza8@gmail.com> wrote: > > > > > > > > > > > > Make 'x64_sys_call()' 'x32_sys_call()' and 'ia32_sys_call()' static to > > > > > > prevent them from being resolved via kallsyms and by extension kprobes. > > > > > > > > > > > > These functions contain the final calls to the syscall handlers, having > > > > > > access to their address can be used to hook this calls altering their > > > > > > behavior. Preventing symbol visibility limits the area of such attacks. > > > > > > > > > > > > Suggested-by: Peter Zijlstra <peterz@infradead.org> > > > > > > Signed-off-by: Elly I Esparza <ellyesparza8@gmail.com> > > > > > > --- > > > > > > arch/x86/entry/syscall_32.c | 6 ++++-- > > > > > > arch/x86/entry/syscall_64.c | 12 ++++++++---- > > > > > > arch/x86/include/asm/syscall.h | 8 -------- > > > > > > 3 files changed, 12 insertions(+), 14 deletions(-) > > > > > > > > > > > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c > > > > > > index 2b15ea17bb7c..28c9a99fb2de 100644 > > > > > > --- a/arch/x86/entry/syscall_32.c > > > > > > +++ b/arch/x86/entry/syscall_32.c > > > > > > @@ -41,7 +41,7 @@ const sys_call_ptr_t sys_call_table[] = { > > > > > > #endif > > > > > > > > > > > > #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(regs); > > > > > > -long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > > +static __always_inline long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > > { > > > > > > switch (nr) { > > > > > > #include <asm/syscalls_32.h> > > > > > > @@ -70,7 +70,7 @@ early_param("ia32_emulation", ia32_emulation_override_cmdline); > > > > > > /* > > > > > > * Invoke a 32-bit syscall. Called with IRQs on in CT_STATE_KERNEL. > > > > > > */ > > > > > > -static __always_inline void do_syscall_32_irqs_on(struct pt_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 out of range > > > > > > @@ -78,12 +78,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs, int nr) > > > > > > */ > > > > > > unsigned int unr = nr; > > > > > > > > > > > > + instrumentation_begin(); > > > > > > if (likely(unr < IA32_NR_syscalls)) { > > > > > > unr = array_index_nospec(unr, IA32_NR_syscalls); > > > > > > regs->ax = ia32_sys_call(regs, unr); > > > > > > } else if (nr != -1) { > > > > > > regs->ax = __ia32_sys_ni_syscall(regs); > > > > > > } > > > > > > + instrumentation_end(); > > > > > > } > > > > > > > > > > > > #ifdef CONFIG_IA32_EMULATION > > > > > > diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c > > > > > > index b6e68ea98b83..55ad274f57f5 100644 > > > > > > --- a/arch/x86/entry/syscall_64.c > > > > > > +++ b/arch/x86/entry/syscall_64.c > > > > > > @@ -32,7 +32,7 @@ const sys_call_ptr_t sys_call_table[] = { > > > > > > #undef __SYSCALL > > > > > > > > > > > > #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs); > > > > > > -long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > > +static __always_inline long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > > { > > > > > > switch (nr) { > > > > > > #include <asm/syscalls_64.h> > > > > > > @@ -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_regs *regs, unsigned int nr) > > > > > > { > > > > > > +#ifdef CONFIG_X86_X32_ABI > > > > > > switch (nr) { > > > > > > #include <asm/syscalls_x32.h> > > > > > > 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 = nr; > > > > > > > > > > > > + instrumentation_begin(); > > > > > > if (likely(unr < NR_syscalls)) { > > > > > > unr = array_index_nospec(unr, NR_syscalls); > > > > > > regs->ax = x64_sys_call(regs, unr); > > > > > > return true; > > > > > > } > > > > > > + instrumentation_end(); > > > > > > return false; > > > > > > } > > > > > > > > > > > > diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h > > > > > > index c10dbb74cd00..59a406074dc0 100644 > > > > > > --- a/arch/x86/include/asm/syscall.h > > > > > > +++ b/arch/x86/include/asm/syscall.h > > > > > > @@ -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(). > > > > > > - */ > > > > > > -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 return int. > > > > > > * This importantly ignores the high bits on 64-bit, so comparisons > > > > > > -- > > > > > > 2.43.0 > > > > > > > > > > > > > > > > > > > > > > The main benefit of having the switch in a discrete function is that > > > > > it allows the compiler to do tail-call optimizations, reducing text > > > > > size. Isn't it possible to blacklist functions from kprobes? > > > > > > > > Ofcourse. But I'm not sure I understand your concern. Both > > > > x{64,32}_sys_call() only have a single caller, inlining them avoids 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.o > > > 10239 136 0 10375 2887 > > > arch/x86/entry/syscall_64.o.orig > > > > > > That's a 3k increase in text size when it's inlined. > > > > > > > > > Brian Gerst > > > > I just check size too and agree with Brian. Regarding > > blacklisting, that was the original idea, it has the added problem > > of kallsyms_lookup_name() being available to bypass the blacklist. > > > > Is this fine for next version then? > > > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c > > index 2b15ea17bb7c..75d34d2c8067 100644 > > --- a/arch/x86/entry/syscall_32.c > > +++ b/arch/x86/entry/syscall_32.c > > @@ -1,6 +1,7 @@ > > // SPDX-License-Identifier: GPL-2.0-only > > /* 32-bit system call dispatch */ > > > > +#include <linux/kprobes.h> > > #include <linux/linkage.h> > > #include <linux/sys.h> > > #include <linux/cache.h> > > @@ -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.c b/arch/x86/entry/syscall_64.c > > index b6e68ea98b83..1e8045a499d4 100644 > > --- a/arch/x86/entry/syscall_64.c > > +++ b/arch/x86/entry/syscall_64.c > > @@ -1,6 +1,7 @@ > > // SPDX-License-Identifier: GPL-2.0-only > > /* 64-bit system call dispatch */ > > > > +#include <linux/kprobes.h> > > #include <linux/linkage.h> > > #include <linux/sys.h> > > #include <linux/cache.h> > > @@ -39,6 +40,7 @@ long x64_sys_call(const struct pt_regs *regs, unsigned 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, unsigned 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, int nr) > > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c > > index 1e7635864124..13a7c0fdb5da 100644 > > --- a/kernel/kallsyms.c > > +++ b/kernel/kallsyms.c > > @@ -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. For symbols from modules 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. Hooking is generally done by changing the address of a branch since it's more reliable and easier to clean up. The aim was to eliminate that low-effort popular attack vector. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] x86/syscall: make sys_call helpers static 2025-12-20 17:45 ` ellyndra @ 2025-12-20 18:21 ` Brian Gerst 2025-12-20 20:11 ` H. Peter Anvin 0 siblings, 1 reply; 10+ messages in thread From: Brian Gerst @ 2025-12-20 18:21 UTC (permalink / raw) To: ellyndra Cc: Peter Zijlstra, linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, hpa On Sat, Dec 20, 2025 at 12:45 PM ellyndra <ellyesparza8@gmail.com> wrote: > > On Sat, Dec 20, 2025 at 2:10 PM Brian Gerst <brgerst@gmail.com> wrote: > > > > On Sat, Dec 20, 2025 at 11:52 AM ellyndra <ellyesparza8@gmail.com> wrote: > > > > > > On Sat, Dec 20, 2025 at 11:32 AM Brian Gerst <brgerst@gmail.com> wrote: > > > > > > > > On Sat, Dec 20, 2025 at 9:10 AM Peter Zijlstra <peterz@infradead.org> wrote: > > > > > > > > > > On Sat, Dec 20, 2025 at 08:50:31AM -0500, Brian Gerst wrote: > > > > > > On Thu, Dec 18, 2025 at 10:01 AM ellyndra <ellyesparza8@gmail.com> wrote: > > > > > > > > > > > > > > Make 'x64_sys_call()' 'x32_sys_call()' and 'ia32_sys_call()' static to > > > > > > > prevent them from being resolved via kallsyms and by extension kprobes. > > > > > > > > > > > > > > These functions contain the final calls to the syscall handlers, having > > > > > > > access to their address can be used to hook this calls altering their > > > > > > > behavior. Preventing symbol visibility limits the area of such attacks. > > > > > > > > > > > > > > Suggested-by: Peter Zijlstra <peterz@infradead.org> > > > > > > > Signed-off-by: Elly I Esparza <ellyesparza8@gmail.com> > > > > > > > --- > > > > > > > arch/x86/entry/syscall_32.c | 6 ++++-- > > > > > > > arch/x86/entry/syscall_64.c | 12 ++++++++---- > > > > > > > arch/x86/include/asm/syscall.h | 8 -------- > > > > > > > 3 files changed, 12 insertions(+), 14 deletions(-) > > > > > > > > > > > > > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c > > > > > > > index 2b15ea17bb7c..28c9a99fb2de 100644 > > > > > > > --- a/arch/x86/entry/syscall_32.c > > > > > > > +++ b/arch/x86/entry/syscall_32.c > > > > > > > @@ -41,7 +41,7 @@ const sys_call_ptr_t sys_call_table[] = { > > > > > > > #endif > > > > > > > > > > > > > > #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(regs); > > > > > > > -long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > > > +static __always_inline long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > > > { > > > > > > > switch (nr) { > > > > > > > #include <asm/syscalls_32.h> > > > > > > > @@ -70,7 +70,7 @@ early_param("ia32_emulation", ia32_emulation_override_cmdline); > > > > > > > /* > > > > > > > * Invoke a 32-bit syscall. Called with IRQs on in CT_STATE_KERNEL. > > > > > > > */ > > > > > > > -static __always_inline void do_syscall_32_irqs_on(struct pt_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 out of range > > > > > > > @@ -78,12 +78,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs, int nr) > > > > > > > */ > > > > > > > unsigned int unr = nr; > > > > > > > > > > > > > > + instrumentation_begin(); > > > > > > > if (likely(unr < IA32_NR_syscalls)) { > > > > > > > unr = array_index_nospec(unr, IA32_NR_syscalls); > > > > > > > regs->ax = ia32_sys_call(regs, unr); > > > > > > > } else if (nr != -1) { > > > > > > > regs->ax = __ia32_sys_ni_syscall(regs); > > > > > > > } > > > > > > > + instrumentation_end(); > > > > > > > } > > > > > > > > > > > > > > #ifdef CONFIG_IA32_EMULATION > > > > > > > diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c > > > > > > > index b6e68ea98b83..55ad274f57f5 100644 > > > > > > > --- a/arch/x86/entry/syscall_64.c > > > > > > > +++ b/arch/x86/entry/syscall_64.c > > > > > > > @@ -32,7 +32,7 @@ const sys_call_ptr_t sys_call_table[] = { > > > > > > > #undef __SYSCALL > > > > > > > > > > > > > > #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs); > > > > > > > -long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > > > +static __always_inline long x64_sys_call(const struct pt_regs *regs, unsigned int nr) > > > > > > > { > > > > > > > switch (nr) { > > > > > > > #include <asm/syscalls_64.h> > > > > > > > @@ -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_regs *regs, unsigned int nr) > > > > > > > { > > > > > > > +#ifdef CONFIG_X86_X32_ABI > > > > > > > switch (nr) { > > > > > > > #include <asm/syscalls_x32.h> > > > > > > > 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 = nr; > > > > > > > > > > > > > > + instrumentation_begin(); > > > > > > > if (likely(unr < NR_syscalls)) { > > > > > > > unr = array_index_nospec(unr, NR_syscalls); > > > > > > > regs->ax = x64_sys_call(regs, unr); > > > > > > > return true; > > > > > > > } > > > > > > > + instrumentation_end(); > > > > > > > return false; > > > > > > > } > > > > > > > > > > > > > > diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h > > > > > > > index c10dbb74cd00..59a406074dc0 100644 > > > > > > > --- a/arch/x86/include/asm/syscall.h > > > > > > > +++ b/arch/x86/include/asm/syscall.h > > > > > > > @@ -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(). > > > > > > > - */ > > > > > > > -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 return int. > > > > > > > * This importantly ignores the high bits on 64-bit, so comparisons > > > > > > > -- > > > > > > > 2.43.0 > > > > > > > > > > > > > > > > > > > > > > > > > > The main benefit of having the switch in a discrete function is that > > > > > > it allows the compiler to do tail-call optimizations, reducing text > > > > > > size. Isn't it possible to blacklist functions from kprobes? > > > > > > > > > > Ofcourse. But I'm not sure I understand your concern. Both > > > > > x{64,32}_sys_call() only have a single caller, inlining them avoids 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.o > > > > 10239 136 0 10375 2887 > > > > arch/x86/entry/syscall_64.o.orig > > > > > > > > That's a 3k increase in text size when it's inlined. > > > > > > > > > > > > Brian Gerst > > > > > > I just check size too and agree with Brian. Regarding > > > blacklisting, that was the original idea, it has the added problem > > > of kallsyms_lookup_name() being available to bypass the blacklist. > > > > > > Is this fine for next version then? > > > > > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c > > > index 2b15ea17bb7c..75d34d2c8067 100644 > > > --- a/arch/x86/entry/syscall_32.c > > > +++ b/arch/x86/entry/syscall_32.c > > > @@ -1,6 +1,7 @@ > > > // SPDX-License-Identifier: GPL-2.0-only > > > /* 32-bit system call dispatch */ > > > > > > +#include <linux/kprobes.h> > > > #include <linux/linkage.h> > > > #include <linux/sys.h> > > > #include <linux/cache.h> > > > @@ -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.c b/arch/x86/entry/syscall_64.c > > > index b6e68ea98b83..1e8045a499d4 100644 > > > --- a/arch/x86/entry/syscall_64.c > > > +++ b/arch/x86/entry/syscall_64.c > > > @@ -1,6 +1,7 @@ > > > // SPDX-License-Identifier: GPL-2.0-only > > > /* 64-bit system call dispatch */ > > > > > > +#include <linux/kprobes.h> > > > #include <linux/linkage.h> > > > #include <linux/sys.h> > > > #include <linux/cache.h> > > > @@ -39,6 +40,7 @@ long x64_sys_call(const struct pt_regs *regs, unsigned 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, unsigned 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, int nr) > > > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c > > > index 1e7635864124..13a7c0fdb5da 100644 > > > --- a/kernel/kallsyms.c > > > +++ b/kernel/kallsyms.c > > > @@ -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. For symbols from modules 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. > > Hooking is generally done by changing the address of a branch since > it's more reliable and easier to clean up. The aim was to eliminate > that low-effort popular attack vector. Inlining doesn't really fix that. It just moves the target branch to another function. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] x86/syscall: make sys_call helpers static 2025-12-20 18:21 ` Brian Gerst @ 2025-12-20 20:11 ` H. Peter Anvin 0 siblings, 0 replies; 10+ messages in thread From: H. Peter Anvin @ 2025-12-20 20:11 UTC (permalink / raw) To: Brian Gerst, ellyndra Cc: Peter Zijlstra, linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86 On December 20, 2025 10:21:49 AM PST, Brian Gerst <brgerst@gmail.com> wrote: >On Sat, Dec 20, 2025 at 12:45 PM ellyndra <ellyesparza8@gmail.com> wrote: >> >> On Sat, Dec 20, 2025 at 2:10 PM Brian Gerst <brgerst@gmail.com> wrote: >> > >> > On Sat, Dec 20, 2025 at 11:52 AM ellyndra <ellyesparza8@gmail.com> wrote: >> > > >> > > On Sat, Dec 20, 2025 at 11:32 AM Brian Gerst <brgerst@gmail.com> wrote: >> > > > >> > > > On Sat, Dec 20, 2025 at 9:10 AM Peter Zijlstra <peterz@infradead.org> wrote: >> > > > > >> > > > > On Sat, Dec 20, 2025 at 08:50:31AM -0500, Brian Gerst wrote: >> > > > > > On Thu, Dec 18, 2025 at 10:01 AM ellyndra <ellyesparza8@gmail.com> wrote: >> > > > > > > >> > > > > > > Make 'x64_sys_call()' 'x32_sys_call()' and 'ia32_sys_call()' static to >> > > > > > > prevent them from being resolved via kallsyms and by extension kprobes. >> > > > > > > >> > > > > > > These functions contain the final calls to the syscall handlers, having >> > > > > > > access to their address can be used to hook this calls altering their >> > > > > > > behavior. Preventing symbol visibility limits the area of such attacks. >> > > > > > > >> > > > > > > Suggested-by: Peter Zijlstra <peterz@infradead.org> >> > > > > > > Signed-off-by: Elly I Esparza <ellyesparza8@gmail.com> >> > > > > > > --- >> > > > > > > arch/x86/entry/syscall_32.c | 6 ++++-- >> > > > > > > arch/x86/entry/syscall_64.c | 12 ++++++++---- >> > > > > > > arch/x86/include/asm/syscall.h | 8 -------- >> > > > > > > 3 files changed, 12 insertions(+), 14 deletions(-) >> > > > > > > >> > > > > > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c >> > > > > > > index 2b15ea17bb7c..28c9a99fb2de 100644 >> > > > > > > --- a/arch/x86/entry/syscall_32.c >> > > > > > > +++ b/arch/x86/entry/syscall_32.c >> > > > > > > @@ -41,7 +41,7 @@ const sys_call_ptr_t sys_call_table[] = { >> > > > > > > #endif >> > > > > > > >> > > > > > > #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(regs); >> > > > > > > -long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) >> > > > > > > +static __always_inline long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) >> > > > > > > { >> > > > > > > switch (nr) { >> > > > > > > #include <asm/syscalls_32.h> >> > > > > > > @@ -70,7 +70,7 @@ early_param("ia32_emulation", ia32_emulation_override_cmdline); >> > > > > > > /* >> > > > > > > * Invoke a 32-bit syscall. Called with IRQs on in CT_STATE_KERNEL. >> > > > > > > */ >> > > > > > > -static __always_inline void do_syscall_32_irqs_on(struct pt_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 out of range >> > > > > > > @@ -78,12 +78,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs, int nr) >> > > > > > > */ >> > > > > > > unsigned int unr = nr; >> > > > > > > >> > > > > > > + instrumentation_begin(); >> > > > > > > if (likely(unr < IA32_NR_syscalls)) { >> > > > > > > unr = array_index_nospec(unr, IA32_NR_syscalls); >> > > > > > > regs->ax = ia32_sys_call(regs, unr); >> > > > > > > } else if (nr != -1) { >> > > > > > > regs->ax = __ia32_sys_ni_syscall(regs); >> > > > > > > } >> > > > > > > + instrumentation_end(); >> > > > > > > } >> > > > > > > >> > > > > > > #ifdef CONFIG_IA32_EMULATION >> > > > > > > diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c >> > > > > > > index b6e68ea98b83..55ad274f57f5 100644 >> > > > > > > --- a/arch/x86/entry/syscall_64.c >> > > > > > > +++ b/arch/x86/entry/syscall_64.c >> > > > > > > @@ -32,7 +32,7 @@ const sys_call_ptr_t sys_call_table[] = { >> > > > > > > #undef __SYSCALL >> > > > > > > >> > > > > > > #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs); >> > > > > > > -long x64_sys_call(const struct pt_regs *regs, unsigned int nr) >> > > > > > > +static __always_inline long x64_sys_call(const struct pt_regs *regs, unsigned int nr) >> > > > > > > { >> > > > > > > switch (nr) { >> > > > > > > #include <asm/syscalls_64.h> >> > > > > > > @@ -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_regs *regs, unsigned int nr) >> > > > > > > { >> > > > > > > +#ifdef CONFIG_X86_X32_ABI >> > > > > > > switch (nr) { >> > > > > > > #include <asm/syscalls_x32.h> >> > > > > > > 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 = nr; >> > > > > > > >> > > > > > > + instrumentation_begin(); >> > > > > > > if (likely(unr < NR_syscalls)) { >> > > > > > > unr = array_index_nospec(unr, NR_syscalls); >> > > > > > > regs->ax = x64_sys_call(regs, unr); >> > > > > > > return true; >> > > > > > > } >> > > > > > > + instrumentation_end(); >> > > > > > > return false; >> > > > > > > } >> > > > > > > >> > > > > > > diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h >> > > > > > > index c10dbb74cd00..59a406074dc0 100644 >> > > > > > > --- a/arch/x86/include/asm/syscall.h >> > > > > > > +++ b/arch/x86/include/asm/syscall.h >> > > > > > > @@ -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(). >> > > > > > > - */ >> > > > > > > -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 return int. >> > > > > > > * This importantly ignores the high bits on 64-bit, so comparisons >> > > > > > > -- >> > > > > > > 2.43.0 >> > > > > > > >> > > > > > > >> > > > > > >> > > > > > The main benefit of having the switch in a discrete function is that >> > > > > > it allows the compiler to do tail-call optimizations, reducing text >> > > > > > size. Isn't it possible to blacklist functions from kprobes? >> > > > > >> > > > > Ofcourse. But I'm not sure I understand your concern. Both >> > > > > x{64,32}_sys_call() only have a single caller, inlining them avoids 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.o >> > > > 10239 136 0 10375 2887 >> > > > arch/x86/entry/syscall_64.o.orig >> > > > >> > > > That's a 3k increase in text size when it's inlined. >> > > > >> > > > >> > > > Brian Gerst >> > > >> > > I just check size too and agree with Brian. Regarding >> > > blacklisting, that was the original idea, it has the added problem >> > > of kallsyms_lookup_name() being available to bypass the blacklist. >> > > >> > > Is this fine for next version then? >> > > >> > > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c >> > > index 2b15ea17bb7c..75d34d2c8067 100644 >> > > --- a/arch/x86/entry/syscall_32.c >> > > +++ b/arch/x86/entry/syscall_32.c >> > > @@ -1,6 +1,7 @@ >> > > // SPDX-License-Identifier: GPL-2.0-only >> > > /* 32-bit system call dispatch */ >> > > >> > > +#include <linux/kprobes.h> >> > > #include <linux/linkage.h> >> > > #include <linux/sys.h> >> > > #include <linux/cache.h> >> > > @@ -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.c b/arch/x86/entry/syscall_64.c >> > > index b6e68ea98b83..1e8045a499d4 100644 >> > > --- a/arch/x86/entry/syscall_64.c >> > > +++ b/arch/x86/entry/syscall_64.c >> > > @@ -1,6 +1,7 @@ >> > > // SPDX-License-Identifier: GPL-2.0-only >> > > /* 64-bit system call dispatch */ >> > > >> > > +#include <linux/kprobes.h> >> > > #include <linux/linkage.h> >> > > #include <linux/sys.h> >> > > #include <linux/cache.h> >> > > @@ -39,6 +40,7 @@ long x64_sys_call(const struct pt_regs *regs, unsigned 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, unsigned 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, int nr) >> > > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c >> > > index 1e7635864124..13a7c0fdb5da 100644 >> > > --- a/kernel/kallsyms.c >> > > +++ b/kernel/kallsyms.c >> > > @@ -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. For symbols from modules 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. >> >> Hooking is generally done by changing the address of a branch since >> it's more reliable and easier to clean up. The aim was to eliminate >> that low-effort popular attack vector. > >Inlining doesn't really fix that. It just moves the target branch to >another function. > 3K for inlining in one of the hottest paths in the system doesn't seem unreasonable, though, if the performance is neutral or better. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] x86/syscall: make sys_call helpers static 2025-12-20 14:32 ` Brian Gerst 2025-12-20 16:52 ` ellyndra @ 2025-12-20 20:40 ` Peter Zijlstra 1 sibling, 0 replies; 10+ messages in thread From: Peter Zijlstra @ 2025-12-20 20:40 UTC (permalink / raw) To: Brian Gerst Cc: ellyndra, linux-kernel, luto, tglx, mingo, bp, dave.hansen, x86, hpa On Sat, Dec 20, 2025 at 09:32:27AM -0500, Brian Gerst wrote: > > Ofcourse. But I'm not sure I understand your concern. Both > > x{64,32}_sys_call() only have a single caller, inlining them avoids 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.o > 10239 136 0 10375 2887 > arch/x86/entry/syscall_64.o.orig > > That's a 3k increase in text size when it's inlined. Oh, you're talking about the calls in the switch; when that's inlined it ends up generating a pile of doo. The below should do. --- diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c index a67a644d0cfe..47ef0a10e42e 100644 --- a/arch/x86/entry/syscall_32.c +++ b/arch/x86/entry/syscall_32.c @@ -41,12 +41,14 @@ const sys_call_ptr_t sys_call_table[] = { #endif #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(regs); -long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) +static noinstr long ia32_sys_call(const struct pt_regs *regs, unsigned int nr) { + instrumentation_begin(); switch (nr) { #include <asm/syscalls_32.h> default: return __ia32_sys_ni_syscall(regs); } + instrumentation_end(); } static __always_inline int syscall_32_enter(struct pt_regs *regs) diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c index b6e68ea98b83..0f26f3f60b4a 100644 --- a/arch/x86/entry/syscall_64.c +++ b/arch/x86/entry/syscall_64.c @@ -32,23 +32,33 @@ const sys_call_ptr_t sys_call_table[] = { #undef __SYSCALL #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs); -long x64_sys_call(const struct pt_regs *regs, unsigned int nr) +/* + * Explicitly noinline such that the whole switch table ends up generating + * tail-calls. When inlined into do_syscall_x64() it will + */ +static noinstr long x64_sys_call(const struct pt_regs *regs, unsigned int nr) { + instrumentation_begin(); switch (nr) { #include <asm/syscalls_64.h> default: return __x64_sys_ni_syscall(regs); } + instrumentation_end(); } -#ifdef CONFIG_X86_X32_ABI -long x32_sys_call(const struct pt_regs *regs, unsigned int nr) +static noinstr long x32_sys_call(const struct pt_regs *regs, unsigned int nr) { + instrumentation_begin(); +#ifdef CONFIG_X86_X32_ABI switch (nr) { #include <asm/syscalls_x32.h> default: return __x64_sys_ni_syscall(regs); } -} +#else + return __x64_sys_ni_syscall(regs); #endif + instrumentation_end(); +} static __always_inline bool do_syscall_x64(struct pt_regs *regs, int nr) { diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h index c10dbb74cd00..59a406074dc0 100644 --- a/arch/x86/include/asm/syscall.h +++ b/arch/x86/include/asm/syscall.h @@ -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(). - */ -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 return int. * This importantly ignores the high bits on 64-bit, so comparisons ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-12-20 20:41 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-12-18 14:57 [PATCH v2] x86/syscall: make sys_call helpers static ellyndra 2025-12-20 13:50 ` Brian Gerst 2025-12-20 14:10 ` Peter Zijlstra 2025-12-20 14:32 ` Brian Gerst 2025-12-20 16:52 ` ellyndra 2025-12-20 17:10 ` Brian Gerst 2025-12-20 17:45 ` ellyndra 2025-12-20 18:21 ` Brian Gerst 2025-12-20 20:11 ` H. Peter Anvin 2025-12-20 20:40 ` Peter Zijlstra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®