[PATCH] Linux: Inhibit tail calls in cancellable system calls

[Mark Wielaard]

Hi Florian,

On Fri, 2025-03-21 at 14:01 +0100, Florian Weimer wrote:
> Without this change, the system call wrapper function is not visible
> on the stack at the time of the system call, which causes problems
> for interception tools such as valgrind.
> 
> Enhances commit 89b53077d2a58f00e7debdfe58afabe953dac60d ("nptl: Fix
> Race conditions in pthread cancellation [BZ#12683]").
> 
> Tested on i686-linux-gnu, powerpc64le-linux-gnu, x86_64-linux-gnu.
> (We're still discussing if valgrind needs this, but if it does, here's a
> patch.)

I implemented the valgrind part of skipping the syscall_cancel frames
here: https://bugs.kde.org/show_bug.cgi?id=502126#c2
And there is a valgrind package build for fedora rawhide:
https://koji.fedoraproject.org/koji/buildinfo?buildID=2687393

For ppc64le, s390x and x86_64 that patch seems enough.

For i686 and aarch64 there does seem to be an issue with missing the
glibc calling function because of a tail call.

Also on i686 there is another extra frame on top __libc_do_syscall.

I haven't yet tried on a system with a glibc with this patch applied. 

Cheers,

Mark

> ---
>  sysdeps/unix/sysdep.h | 57 ++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 36 insertions(+), 21 deletions(-)
> 
> diff --git a/sysdeps/unix/sysdep.h b/sysdeps/unix/sysdep.h
> index 2cc98725c3..8f1e4fc4f9 100644
> --- a/sysdeps/unix/sysdep.h
> +++ b/sysdeps/unix/sysdep.h
> @@ -166,30 +166,45 @@ long int __syscall_cancel (__syscall_arg_t arg1, __syscall_arg_t arg2,
>  			   __SYSCALL_CANCEL7_ARG_DEF
>  			   __syscall_arg_t nr) attribute_hidden;
>  
> +/* Inhibit tail call optimization, so that the stack frame of the
> +   system-call-implementing function is visible at the time of the
> +   system call.  */
> +#define __syscall_cancel_barrier(...)					\
> +  ({									\
> +     long int __syscall_cancel_result = __syscall_cancel (__VA_ARGS__); \
> +     asm volatile ("" ::: "memory");					\
> +     __syscall_cancel_result;						\
> +  })
> +
>  #define __SYSCALL_CANCEL0(name)						\
> -  __syscall_cancel (0, 0, 0, 0, 0, 0, __SYSCALL_CANCEL7_ARG __NR_##name)
> +  __syscall_cancel_barrier (0, 0, 0, 0, 0, 0,				\
> +			    __SYSCALL_CANCEL7_ARG __NR_##name)
>  #define __SYSCALL_CANCEL1(name, a1)					\
> -  __syscall_cancel (__SSC (a1), 0, 0, 0, 0, 0,				\
> -		    __SYSCALL_CANCEL7_ARG __NR_##name)
> -#define __SYSCALL_CANCEL2(name, a1, a2) \
> -  __syscall_cancel (__SSC (a1), __SSC (a2), 0, 0, 0, 0,			\
> -		    __SYSCALL_CANCEL7_ARG __NR_##name)
> -#define __SYSCALL_CANCEL3(name, a1, a2, a3) \
> -  __syscall_cancel (__SSC (a1), __SSC (a2), __SSC (a3), 0, 0, 0,	\
> -		    __SYSCALL_CANCEL7_ARG __NR_##name)
> -#define __SYSCALL_CANCEL4(name, a1, a2, a3, a4) \
> -  __syscall_cancel (__SSC (a1), __SSC (a2), __SSC (a3),			\
> -		    __SSC(a4), 0, 0, __SYSCALL_CANCEL7_ARG __NR_##name)
> -#define __SYSCALL_CANCEL5(name, a1, a2, a3, a4, a5) \
> -  __syscall_cancel (__SSC (a1), __SSC (a2), __SSC (a3), __SSC(a4),	\
> -		    __SSC (a5), 0, __SYSCALL_CANCEL7_ARG __NR_##name)
> -#define __SYSCALL_CANCEL6(name, a1, a2, a3, a4, a5, a6) \
> -  __syscall_cancel (__SSC (a1), __SSC (a2), __SSC (a3), __SSC (a4),	\
> -		    __SSC (a5), __SSC (a6), __SYSCALL_CANCEL7_ARG	\
> -		    __NR_##name)
> +  __syscall_cancel_barrier (__SSC (a1), 0, 0, 0, 0, 0,			\
> +			    __SYSCALL_CANCEL7_ARG __NR_##name)
> +#define __SYSCALL_CANCEL2(name, a1, a2)					\
> +  __syscall_cancel_barrier (__SSC (a1), __SSC (a2), 0, 0, 0, 0,		\
> +			    __SYSCALL_CANCEL7_ARG __NR_##name)
> +#define __SYSCALL_CANCEL3(name, a1, a2, a3)				\
> +  __syscall_cancel_barrier (__SSC (a1), __SSC (a2), __SSC (a3),		\
> +			    0, 0, 0, __SYSCALL_CANCEL7_ARG __NR_##name)
> +#define __SYSCALL_CANCEL4(name, a1, a2, a3, a4)				\
> +  __syscall_cancel_barrier (__SSC (a1), __SSC (a2), __SSC (a3),		\
> +			    __SSC(a4), 0, 0,				\
> +			    __SYSCALL_CANCEL7_ARG __NR_##name)
> +#define __SYSCALL_CANCEL5(name, a1, a2, a3, a4, a5)			\
> +  __syscall_cancel_barrier (__SSC (a1), __SSC (a2), __SSC (a3),		\
> +			    __SSC(a4), __SSC (a5), 0,			\
> +			    __SYSCALL_CANCEL7_ARG __NR_##name)
> +#define __SYSCALL_CANCEL6(name, a1, a2, a3, a4, a5, a6)			\
> +  __syscall_cancel_barrier (__SSC (a1), __SSC (a2), __SSC (a3),		\
> +			    __SSC (a4), __SSC (a5), __SSC (a6),		\
> +			    __SYSCALL_CANCEL7_ARG			\
> +			    __NR_##name)
>  #define __SYSCALL_CANCEL7(name, a1, a2, a3, a4, a5, a6, a7)		\
> -  __syscall_cancel (__SSC (a1), __SSC (a2), __SSC (a3), __SSC (a4),	\
> -		    __SSC (a5), __SSC (a6), __SSC (a7), __NR_##name)
> +  __syscall_cancel_barrier (__SSC (a1), __SSC (a2), __SSC (a3),		\
> +			    __SSC (a4), __SSC (a5), __SSC (a6),		\
> +			    __SSC (a7), __NR_##name)
>  
>  #define __SYSCALL_CANCEL_NARGS_X(a,b,c,d,e,f,g,h,n,...) n
>  #define __SYSCALL_CANCEL_NARGS(...) \
> 
> base-commit: 3e2be87832781a29ed67f38f87c1ce3dd4c1b866
>