* [RESEND PATCH 1/2] signal: Introduce COMPAT_SIGMINSTKSZ for use in compat_sys_sigaltstack
2018-09-05 14:34 [RESEND PATCH 0/2] Don't use SIGMINSTKSZ when enforcing alternative signal stack size for compat tasks Will Deacon
@ 2018-09-05 14:34 ` Will Deacon
2018-09-05 14:34 ` [RESEND PATCH 2/2] arm64: compat: Provide definition for COMPAT_SIGMINSTKSZ Will Deacon
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2018-09-05 14:34 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-kernel, arnd, linux, ebiederm, akpm, viro, oleg,
steve.mcintyre, dave.martin, Will Deacon
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 1a3c4f37e908..de0c13bdcd2c 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 5843c541fda9..e4aad0e90882 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3460,7 +3460,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;
@@ -3490,7 +3491,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;
}
@@ -3508,7 +3509,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;
@@ -3519,7 +3521,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);
/* squash all but EFAULT for now */
return 0;
}
@@ -3553,7 +3556,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 (ret >= 0 && uoss_ptr) {
compat_stack_t old;
memset(&old, 0, sizeof(old));
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [RESEND PATCH 2/2] arm64: compat: Provide definition for COMPAT_SIGMINSTKSZ
2018-09-05 14:34 [RESEND PATCH 0/2] Don't use SIGMINSTKSZ when enforcing alternative signal stack size for compat tasks Will Deacon
2018-09-05 14:34 ` [RESEND PATCH 1/2] signal: Introduce COMPAT_SIGMINSTKSZ for use in compat_sys_sigaltstack Will Deacon
@ 2018-09-05 14:34 ` Will Deacon
2018-09-12 13:11 ` [RESEND PATCH 0/2] Don't use SIGMINSTKSZ when enforcing alternative signal stack size for compat tasks Will Deacon
2018-09-26 10:36 ` Steve McIntyre
3 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2018-09-05 14:34 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-kernel, arnd, linux, ebiederm, akpm, viro, oleg,
steve.mcintyre, dave.martin, Will Deacon
arch/arm/ defines a SIGMINSTKSZ of 2k, so we should use the same value
for compat tasks.
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>
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
Reported-by: Steve McIntyre <steve.mcintyre@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
arch/arm64/include/asm/compat.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 1a037b94eba1..cee28a05ee98 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -159,6 +159,7 @@ static inline compat_uptr_t ptr_to_compat(void __user *uptr)
}
#define compat_user_stack_pointer() (user_stack_pointer(task_pt_regs(current)))
+#define COMPAT_MINSIGSTKSZ 2048
static inline void __user *arch_compat_alloc_user_space(long len)
{
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [RESEND PATCH 0/2] Don't use SIGMINSTKSZ when enforcing alternative signal stack size for compat tasks
2018-09-05 14:34 [RESEND PATCH 0/2] Don't use SIGMINSTKSZ when enforcing alternative signal stack size for compat tasks Will Deacon
2018-09-05 14:34 ` [RESEND PATCH 1/2] signal: Introduce COMPAT_SIGMINSTKSZ for use in compat_sys_sigaltstack Will Deacon
2018-09-05 14:34 ` [RESEND PATCH 2/2] arm64: compat: Provide definition for COMPAT_SIGMINSTKSZ Will Deacon
@ 2018-09-12 13:11 ` Will Deacon
2018-09-26 10:36 ` Steve McIntyre
3 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2018-09-12 13:11 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-kernel, arnd, linux, ebiederm, akpm, viro, oleg,
steve.mcintyre, dave.martin, christian, ldv
Hi all,
[+ more people from get_maintainers.pl]
On Wed, Sep 05, 2018 at 03:34:41PM +0100, Will Deacon wrote:
> This is a resend of:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2018-July/593559.html
>
> now based on 4.19-rc2.
>
> The Debian folks have observed a failure in the 32-bit arm glibc testsuite
> when running under a 64-bit kernel. They tracked this down to sigaltstack(2)
> enforcing the alternative signal stack to be at least SIGMINSTKSZ bytes,
> which is higher for native arm64 tasks than compat 32-bit tasks.
>
> These patches resolve the issue by allowing an architecture to define
> COMPAT_SIGMINSTKSZ for compat tasks, which is then used by the sigaltstack
> checking code.
>
> Feedback welcome,
Any comments on this, please? I'd really like to get this fixed, since
Debian have an outstanding issue without these patches merged. I'm happy
to take them via arm64, but would really like an Ack on the changes to
signal.c first...
Thanks,
Will
> Will Deacon (2):
> signal: Introduce COMPAT_SIGMINSTKSZ for use in compat_sys_sigaltstack
> arm64: compat: Provide definition for COMPAT_SIGMINSTKSZ
>
> arch/arm64/include/asm/compat.h | 1 +
> include/linux/compat.h | 3 +++
> kernel/signal.c | 14 +++++++++-----
> 3 files changed, 13 insertions(+), 5 deletions(-)
>
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RESEND PATCH 0/2] Don't use SIGMINSTKSZ when enforcing alternative signal stack size for compat tasks
2018-09-05 14:34 [RESEND PATCH 0/2] Don't use SIGMINSTKSZ when enforcing alternative signal stack size for compat tasks Will Deacon
` (2 preceding siblings ...)
2018-09-12 13:11 ` [RESEND PATCH 0/2] Don't use SIGMINSTKSZ when enforcing alternative signal stack size for compat tasks Will Deacon
@ 2018-09-26 10:36 ` Steve McIntyre
2018-09-28 13:18 ` Steve McIntyre
3 siblings, 1 reply; 7+ messages in thread
From: Steve McIntyre @ 2018-09-26 10:36 UTC (permalink / raw)
To: Will Deacon
Cc: linux-kernel, linux-arm-kernel, arnd, linux, ebiederm, akpm,
viro, oleg, dave.martin
On Wed, Sep 05, 2018 at 03:34:41PM +0100, Will Deacon wrote:
>Hi all,
>
>This is a resend of:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2018-July/593559.html
>
>now based on 4.19-rc2.
>
>The Debian folks have observed a failure in the 32-bit arm glibc testsuite
>when running under a 64-bit kernel. They tracked this down to sigaltstack(2)
>enforcing the alternative signal stack to be at least SIGMINSTKSZ bytes,
>which is higher for native arm64 tasks than compat 32-bit tasks.
>
>These patches resolve the issue by allowing an architecture to define
>COMPAT_SIGMINSTKSZ for compat tasks, which is then used by the sigaltstack
>checking code.
>
>Feedback welcome,
Apologies for the delayed response here - conference travel etc . got
in the way... I've just tested and I can confirm that this patchset
fixes our reported bug (as in https://bugs.debian.org/904385). Thanks
Will!
Tested-by: Steve McIntyre <93sam@debian.org>
--
Steve McIntyre
steve.mcintyre@arm.com
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 7+ messages in thread