lkml.org 
[lkml]   [2018]   [Jul]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] signal: Introduce COMPAT_SIGMINSTKSZ for use in compat_sys_sigaltstack
On Wed, Jul 25, 2018 at 02:45:11PM +0100, Will Deacon wrote:
> The sigaltstack(2) system call fails with -ENOMEM if the new alternative
> signal stack is found to be smaller than SIGMINSTKSZ. On architectures
> such as arm64, where the native value for SIGMINSTKSZ is larger than
> the compat value, this can result in an unexpected error being reported
> to a compat task. See, for example:
>
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=904385
>
> This patch fixes the problem by extending do_sigaltstack to take the
> minimum signal stack size as an additional parameter, allowing the
> native and compat system call entry code to pass in their respective
> values. COMPAT_SIGMINSTKSZ is just defined as SIGMINSTKSZ if it has not
> been defined by the architecture.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Dominik Brodowski <linux@dominikbrodowski.net>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Reported-by: Steve McIntyre <steve.mcintyre@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
> include/linux/compat.h | 3 +++
> kernel/signal.c | 14 +++++++++-----
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index c68acc47da57..47041c7fed28 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -103,6 +103,9 @@ typedef struct compat_sigaltstack {
> compat_size_t ss_size;
> } compat_stack_t;
> #endif
> +#ifndef COMPAT_MINSIGSTKSZ
> +#define COMPAT_MINSIGSTKSZ MINSIGSTKSZ
> +#endif
>
> #define compat_jiffies_to_clock_t(x) \
> (((unsigned long)(x) * COMPAT_USER_HZ) / HZ)
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 8d8a940422a8..41a5dd2df27d 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -3417,7 +3417,8 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
> }
>
> static int
> -do_sigaltstack (const stack_t *ss, stack_t *oss, unsigned long sp)
> +do_sigaltstack (const stack_t *ss, stack_t *oss, unsigned long sp,
> + size_t min_ss_size)
> {
> struct task_struct *t = current;
>
> @@ -3447,7 +3448,7 @@ do_sigaltstack (const stack_t *ss, stack_t *oss, unsigned long sp)
> ss_size = 0;
> ss_sp = NULL;
> } else {
> - if (unlikely(ss_size < MINSIGSTKSZ))
> + if (unlikely(ss_size < min_ss_size))
> return -ENOMEM;
> }
>
> @@ -3465,7 +3466,8 @@ SYSCALL_DEFINE2(sigaltstack,const stack_t __user *,uss, stack_t __user *,uoss)
> if (uss && copy_from_user(&new, uss, sizeof(stack_t)))
> return -EFAULT;
> err = do_sigaltstack(uss ? &new : NULL, uoss ? &old : NULL,
> - current_user_stack_pointer());
> + current_user_stack_pointer(),
> + MINSIGSTKSZ);
> if (!err && uoss && copy_to_user(uoss, &old, sizeof(stack_t)))
> err = -EFAULT;
> return err;
> @@ -3476,7 +3478,8 @@ int restore_altstack(const stack_t __user *uss)
> stack_t new;
> if (copy_from_user(&new, uss, sizeof(stack_t)))
> return -EFAULT;
> - (void)do_sigaltstack(&new, NULL, current_user_stack_pointer());
> + (void)do_sigaltstack(&new, NULL, current_user_stack_pointer(),
> + MINSIGSTKSZ);

Why can't this fail?

If this fails here we silently go wrong, but...

> /* squash all but EFAULT for now */
> return 0;
> }
> @@ -3510,7 +3513,8 @@ static int do_compat_sigaltstack(const compat_stack_t __user *uss_ptr,
> uss.ss_size = uss32.ss_size;
> }
> ret = do_sigaltstack(uss_ptr ? &uss : NULL, &uoss,
> - compat_user_stack_pointer());
> + compat_user_stack_pointer(),
> + COMPAT_MINSIGSTKSZ);

If this fails on arm64, we seem to SEGV (see compat_sys_rt_sigreturn()).

This patch doesn't introduce this inconsistency, this might be a good
opportunity to clean it up.

Cheers
---Dave

\
 
 \ /
  Last update: 2018-07-25 17:54    [W:0.089 / U:0.552 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site