lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4b97c5c8-73ac-417d-8b1c-61ccd0768bda@paulmck-laptop>
Date: Tue, 25 Jun 2024 19:21:34 -0700
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Daniel Sneddon <daniel.sneddon@...ux.intel.com>,
	Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Alexandre Chartre <alexandre.chartre@...cle.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Sean Christopherson <seanjc@...gle.com>,
	Andrew Cooper <andrew.cooper3@...rix.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Nikolay Borisov <nik.borisov@...e.com>,
	KP Singh <kpsingh@...nel.org>, Waiman Long <longman@...hat.com>,
	Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH v5 1/3] x86/syscall: Mark exit[_group] syscall handlers
 __noreturn

On Tue, May 07, 2024 at 07:38:32AM -0700, Paul E. McKenney wrote:
> On Mon, May 06, 2024 at 10:30:04PM -0700, Josh Poimboeuf wrote:
> > The direct-call syscall dispatch function doesn't know that the exit()
> > and exit_group() syscall handlers don't return, so the call sites aren't
> > optimized accordingly.
> > 
> > Fix that by marking those exit syscall declarations __noreturn.
> > 
> > Fixes the following warnings:
> > 
> >   vmlinux.o: warning: objtool: x64_sys_call+0x2804: __x64_sys_exit() is missing a __noreturn annotation
> >   vmlinux.o: warning: objtool: ia32_sys_call+0x29b6: __ia32_sys_exit_group() is missing a __noreturn annotation
> > 
> > Fixes: 7390db8aea0d ("x86/bhi: Add support for clearing branch history at syscall entry")
> > Reported-by: "Paul E. McKenney" <paulmck@...nel.org>
> > Closes: https://lkml.kernel.org/lkml/6dba9b32-db2c-4e6d-9500-7a08852f17a3@paulmck-laptop
> > Tested-by: Paul E. McKenney <paulmck@...nel.org>
> 
> Just reaffirming my Tested-by, and thank you!

And just following up, given that I do not yet see this in -next.  Any
chance of this making the upcoming merge window?

							Thanx, Paul

> > Signed-off-by: Josh Poimboeuf <jpoimboe@...nel.org>
> > ---
> >  arch/x86/entry/syscall_32.c            | 10 ++++++----
> >  arch/x86/entry/syscall_64.c            |  9 ++++++---
> >  arch/x86/entry/syscall_x32.c           |  7 +++++--
> >  arch/x86/entry/syscalls/syscall_32.tbl |  6 +++---
> >  arch/x86/entry/syscalls/syscall_64.tbl |  6 +++---
> >  arch/x86/um/sys_call_table_32.c        | 10 ++++++----
> >  arch/x86/um/sys_call_table_64.c        | 11 +++++++----
> >  scripts/syscalltbl.sh                  | 18 ++++++++++++++++--
> >  tools/objtool/noreturns.h              |  4 ++++
> >  9 files changed, 56 insertions(+), 25 deletions(-)
> > 
> > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c
> > index c2235bae17ef..8cc9950d7104 100644
> > --- a/arch/x86/entry/syscall_32.c
> > +++ b/arch/x86/entry/syscall_32.c
> > @@ -14,9 +14,12 @@
> >  #endif
> >  
> >  #define __SYSCALL(nr, sym) extern long __ia32_##sym(const struct pt_regs *);
> > -
> > +#define __SYSCALL_NORETURN(nr, sym) extern long __noreturn __ia32_##sym(const struct pt_regs *);
> >  #include <asm/syscalls_32.h>
> > -#undef __SYSCALL
> > +#undef  __SYSCALL
> > +
> > +#undef  __SYSCALL_NORETURN
> > +#define __SYSCALL_NORETURN __SYSCALL
> >  
> >  /*
> >   * The sys_call_table[] is no longer used for system calls, but
> > @@ -28,11 +31,10 @@
> >  const sys_call_ptr_t sys_call_table[] = {
> >  #include <asm/syscalls_32.h>
> >  };
> > -#undef __SYSCALL
> > +#undef  __SYSCALL
> >  #endif
> >  
> >  #define __SYSCALL(nr, sym) case nr: return __ia32_##sym(regs);
> > -
> >  long ia32_sys_call(const struct pt_regs *regs, unsigned int nr)
> >  {
> >  	switch (nr) {
> > diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c
> > index 33b3f09e6f15..ba8354424860 100644
> > --- a/arch/x86/entry/syscall_64.c
> > +++ b/arch/x86/entry/syscall_64.c
> > @@ -8,8 +8,12 @@
> >  #include <asm/syscall.h>
> >  
> >  #define __SYSCALL(nr, sym) extern long __x64_##sym(const struct pt_regs *);
> > +#define __SYSCALL_NORETURN(nr, sym) extern long __noreturn __x64_##sym(const struct pt_regs *);
> >  #include <asm/syscalls_64.h>
> > -#undef __SYSCALL
> > +#undef  __SYSCALL
> > +
> > +#undef  __SYSCALL_NORETURN
> > +#define __SYSCALL_NORETURN __SYSCALL
> >  
> >  /*
> >   * The sys_call_table[] is no longer used for system calls, but
> > @@ -20,10 +24,9 @@
> >  const sys_call_ptr_t sys_call_table[] = {
> >  #include <asm/syscalls_64.h>
> >  };
> > -#undef __SYSCALL
> > +#undef  __SYSCALL
> >  
> >  #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs);
> > -
> >  long x64_sys_call(const struct pt_regs *regs, unsigned int nr)
> >  {
> >  	switch (nr) {
> > diff --git a/arch/x86/entry/syscall_x32.c b/arch/x86/entry/syscall_x32.c
> > index 03de4a932131..fb77908f44f3 100644
> > --- a/arch/x86/entry/syscall_x32.c
> > +++ b/arch/x86/entry/syscall_x32.c
> > @@ -8,11 +8,14 @@
> >  #include <asm/syscall.h>
> >  
> >  #define __SYSCALL(nr, sym) extern long __x64_##sym(const struct pt_regs *);
> > +#define __SYSCALL_NORETURN(nr, sym) extern long __noreturn __x64_##sym(const struct pt_regs *);
> >  #include <asm/syscalls_x32.h>
> > -#undef __SYSCALL
> > +#undef  __SYSCALL
> > +
> > +#undef  __SYSCALL_NORETURN
> > +#define __SYSCALL_NORETURN __SYSCALL
> >  
> >  #define __SYSCALL(nr, sym) case nr: return __x64_##sym(regs);
> > -
> >  long x32_sys_call(const struct pt_regs *regs, unsigned int nr)
> >  {
> >  	switch (nr) {
> > diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> > index 5f8591ce7f25..9e9a908cd50d 100644
> > --- a/arch/x86/entry/syscalls/syscall_32.tbl
> > +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> > @@ -2,7 +2,7 @@
> >  # 32-bit system call numbers and entry vectors
> >  #
> >  # The format is:
> > -# <number> <abi> <name> <entry point> <compat entry point>
> > +# <number> <abi> <name> <entry point> [<compat entry point> [noreturn]]
> >  #
> >  # The __ia32_sys and __ia32_compat_sys stubs are created on-the-fly for
> >  # sys_*() system calls and compat_sys_*() compat system calls if
> > @@ -12,7 +12,7 @@
> >  # The abi is always "i386" for this file.
> >  #
> >  0	i386	restart_syscall		sys_restart_syscall
> > -1	i386	exit			sys_exit
> > +1	i386	exit			sys_exit			-			noreturn
> >  2	i386	fork			sys_fork
> >  3	i386	read			sys_read
> >  4	i386	write			sys_write
> > @@ -263,7 +263,7 @@
> >  249	i386	io_cancel		sys_io_cancel
> >  250	i386	fadvise64		sys_ia32_fadvise64
> >  # 251 is available for reuse (was briefly sys_set_zone_reclaim)
> > -252	i386	exit_group		sys_exit_group
> > +252	i386	exit_group		sys_exit_group			-			noreturn
> >  253	i386	lookup_dcookie
> >  254	i386	epoll_create		sys_epoll_create
> >  255	i386	epoll_ctl		sys_epoll_ctl
> > diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> > index 7e8d46f4147f..5ea7387c1aa1 100644
> > --- a/arch/x86/entry/syscalls/syscall_64.tbl
> > +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> > @@ -2,7 +2,7 @@
> >  # 64-bit system call numbers and entry vectors
> >  #
> >  # The format is:
> > -# <number> <abi> <name> <entry point>
> > +# <number> <abi> <name> <entry point> [<compat entry point> [noreturn]]
> >  #
> >  # The __x64_sys_*() stubs are created on-the-fly for sys_*() system calls
> >  #
> > @@ -68,7 +68,7 @@
> >  57	common	fork			sys_fork
> >  58	common	vfork			sys_vfork
> >  59	64	execve			sys_execve
> > -60	common	exit			sys_exit
> > +60	common	exit			sys_exit			-			noreturn
> >  61	common	wait4			sys_wait4
> >  62	common	kill			sys_kill
> >  63	common	uname			sys_newuname
> > @@ -239,7 +239,7 @@
> >  228	common	clock_gettime		sys_clock_gettime
> >  229	common	clock_getres		sys_clock_getres
> >  230	common	clock_nanosleep		sys_clock_nanosleep
> > -231	common	exit_group		sys_exit_group
> > +231	common	exit_group		sys_exit_group			-			noreturn
> >  232	common	epoll_wait		sys_epoll_wait
> >  233	common	epoll_ctl		sys_epoll_ctl
> >  234	common	tgkill			sys_tgkill
> > diff --git a/arch/x86/um/sys_call_table_32.c b/arch/x86/um/sys_call_table_32.c
> > index 89df5d89d664..51655133eee3 100644
> > --- a/arch/x86/um/sys_call_table_32.c
> > +++ b/arch/x86/um/sys_call_table_32.c
> > @@ -9,6 +9,10 @@
> >  #include <linux/cache.h>
> >  #include <asm/syscall.h>
> >  
> > +extern asmlinkage long sys_ni_syscall(unsigned long, unsigned long,
> > +				      unsigned long, unsigned long,
> > +				      unsigned long, unsigned long);
> > +
> >  /*
> >   * Below you can see, in terms of #define's, the differences between the x86-64
> >   * and the UML syscall table.
> > @@ -22,15 +26,13 @@
> >  #define sys_vm86 sys_ni_syscall
> >  
> >  #define __SYSCALL_WITH_COMPAT(nr, native, compat)	__SYSCALL(nr, native)
> > +#define __SYSCALL_NORETURN __SYSCALL
> >  
> >  #define __SYSCALL(nr, sym) extern asmlinkage long sym(unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long);
> >  #include <asm/syscalls_32.h>
> > +#undef  __SYSCALL
> >  
> > -#undef __SYSCALL
> >  #define __SYSCALL(nr, sym) sym,
> > -
> > -extern asmlinkage long sys_ni_syscall(unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long);
> > -
> >  const sys_call_ptr_t sys_call_table[] ____cacheline_aligned = {
> >  #include <asm/syscalls_32.h>
> >  };
> > diff --git a/arch/x86/um/sys_call_table_64.c b/arch/x86/um/sys_call_table_64.c
> > index b0b4cfd2308c..943d414f2109 100644
> > --- a/arch/x86/um/sys_call_table_64.c
> > +++ b/arch/x86/um/sys_call_table_64.c
> > @@ -9,6 +9,10 @@
> >  #include <linux/cache.h>
> >  #include <asm/syscall.h>
> >  
> > +extern asmlinkage long sys_ni_syscall(unsigned long, unsigned long,
> > +				      unsigned long, unsigned long,
> > +				      unsigned long, unsigned long);
> > +
> >  /*
> >   * Below you can see, in terms of #define's, the differences between the x86-64
> >   * and the UML syscall table.
> > @@ -18,14 +22,13 @@
> >  #define sys_iopl sys_ni_syscall
> >  #define sys_ioperm sys_ni_syscall
> >  
> > +#define __SYSCALL_NORETURN __SYSCALL
> > +
> >  #define __SYSCALL(nr, sym) extern asmlinkage long sym(unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long);
> >  #include <asm/syscalls_64.h>
> > +#undef  __SYSCALL
> >  
> > -#undef __SYSCALL
> >  #define __SYSCALL(nr, sym) sym,
> > -
> > -extern asmlinkage long sys_ni_syscall(unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long);
> > -
> >  const sys_call_ptr_t sys_call_table[] ____cacheline_aligned = {
> >  #include <asm/syscalls_64.h>
> >  };
> > diff --git a/scripts/syscalltbl.sh b/scripts/syscalltbl.sh
> > index 6abe143889ef..6a903b87a7c2 100755
> > --- a/scripts/syscalltbl.sh
> > +++ b/scripts/syscalltbl.sh
> > @@ -54,7 +54,7 @@ nxt=0
> >  
> >  grep -E "^[0-9]+[[:space:]]+$abis" "$infile" | {
> >  
> > -	while read nr abi name native compat ; do
> > +	while read nr abi name native compat noreturn; do
> >  
> >  		if [ $nxt -gt $nr ]; then
> >  			echo "error: $infile: syscall table is not sorted or duplicates the same syscall number" >&2
> > @@ -66,7 +66,21 @@ grep -E "^[0-9]+[[:space:]]+$abis" "$infile" | {
> >  			nxt=$((nxt + 1))
> >  		done
> >  
> > -		if [ -n "$compat" ]; then
> > +		if [ "$compat" = "-" ]; then
> > +			unset compat
> > +		fi
> > +
> > +		if [ -n "$noreturn" ]; then
> > +			if [ "$noreturn" != "noreturn" ]; then
> > +				echo "error: $infile: invalid string \"$noreturn\" in 'noreturn' column"
> > +				exit 1
> > +			fi
> > +			if [ -n "$compat" ]; then
> > +				echo "__SYSCALL_COMPAT_NORETURN($nr, $native, $compat)"
> > +			else
> > +				echo "__SYSCALL_NORETURN($nr, $native)"
> > +			fi
> > +		elif [ -n "$compat" ]; then
> >  			echo "__SYSCALL_WITH_COMPAT($nr, $native, $compat)"
> >  		elif [ -n "$native" ]; then
> >  			echo "__SYSCALL($nr, $native)"
> > diff --git a/tools/objtool/noreturns.h b/tools/objtool/noreturns.h
> > index 7ebf29c91184..1e8141ef1b15 100644
> > --- a/tools/objtool/noreturns.h
> > +++ b/tools/objtool/noreturns.h
> > @@ -7,12 +7,16 @@
> >   * Yes, this is unfortunate.  A better solution is in the works.
> >   */
> >  NORETURN(__fortify_panic)
> > +NORETURN(__ia32_sys_exit)
> > +NORETURN(__ia32_sys_exit_group)
> >  NORETURN(__kunit_abort)
> >  NORETURN(__module_put_and_kthread_exit)
> >  NORETURN(__reiserfs_panic)
> >  NORETURN(__stack_chk_fail)
> >  NORETURN(__tdx_hypercall_failed)
> >  NORETURN(__ubsan_handle_builtin_unreachable)
> > +NORETURN(__x64_sys_exit)
> > +NORETURN(__x64_sys_exit_group)
> >  NORETURN(arch_cpu_idle_dead)
> >  NORETURN(bch2_trans_in_restart_error)
> >  NORETURN(bch2_trans_restart_error)
> > -- 
> > 2.44.0
> > 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ