[SRU][Xenial][PATCH 0/2] arm64: sigaltstack fails with MINSIGSTKSZ for 32-bit processes (LP: #1844155)

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH 0/2] arm64: sigaltstack fails with MINSIGSTKSZ for 32-bit processes (LP: #1844155)

Juerg Haefliger
[Impact]

The arm64 kernel allows one to run aarch32 processes on an aarch64 processor, using the standard 32/64-bit syscall compatibility. However this compat layer does not correctly validate the arguments of the sigaltstack syscall which can result in process failures.

[Test Case]

The simple reproducer from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=904385 triggers a memory allocation error with the current Xenial 4.4 kernel.

[Fix]

Backport the following two upstream commits:
24951465cbd2 arm64: compat: Provide definition for COMPAT_SIGMINSTKSZ
22839869f21a signal: Introduce COMPAT_SIGMINSTKSZ for use in compat_sys_sigaltstack

With these two commits, the reproducer no longer fails.

[Regression Potential]

Low. The modifications are trivial and the two patches have been in upstream for quite a while.

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.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH 1/2] signal: Introduce COMPAT_SIGMINSTKSZ for use in compat_sys_sigaltstack

Juerg Haefliger
From: Will Deacon <[hidden email]>

https://bugs.launchpad.net/bugs/1844155

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 <[hidden email]>
Cc: Dominik Brodowski <[hidden email]>
Cc: "Eric W. Biederman" <[hidden email]>
Cc: Andrew Morton <[hidden email]>
Cc: Al Viro <[hidden email]>
Cc: Oleg Nesterov <[hidden email]>
Reported-by: Steve McIntyre <[hidden email]>
Tested-by: Steve McIntyre <[hidden email]>
Signed-off-by: Will Deacon <[hidden email]>
Signed-off-by: Catalin Marinas <[hidden email]>

(backported from commit 22839869f21ab3850fbbac9b425ccc4c0023926f)
[juergh: Adjusted context.]
Signed-off-by: Juerg Haefliger <[hidden email]>
---
 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 a76c9172b2eb..1e1348b64d5c 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -65,6 +65,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 072fd152ab01..b4fa1d864a79 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3170,7 +3170,8 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 }
 
 static int
-do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long sp)
+do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long sp,
+ size_t min_ss_size)
 {
  stack_t oss;
  int error;
@@ -3213,7 +3214,7 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
  ss_sp = NULL;
  } else {
  error = -ENOMEM;
- if (ss_size < MINSIGSTKSZ)
+ if (ss_size < min_ss_size)
  goto out;
  }
 
@@ -3236,12 +3237,14 @@ out:
 }
 SYSCALL_DEFINE2(sigaltstack,const stack_t __user *,uss, stack_t __user *,uoss)
 {
- return do_sigaltstack(uss, uoss, current_user_stack_pointer());
+ return do_sigaltstack(uss, uoss, current_user_stack_pointer(),
+      MINSIGSTKSZ);
 }
 
 int restore_altstack(const stack_t __user *uss)
 {
- int err = do_sigaltstack(uss, NULL, current_user_stack_pointer());
+ int err = do_sigaltstack(uss, NULL, current_user_stack_pointer(),
+ MINSIGSTKSZ);
  /* squash all but EFAULT for now */
  return err == -EFAULT ? err : 0;
 }
@@ -3277,7 +3280,8 @@ COMPAT_SYSCALL_DEFINE2(sigaltstack,
  set_fs(KERNEL_DS);
  ret = do_sigaltstack((stack_t __force __user *) (uss_ptr ? &uss : NULL),
      (stack_t __force __user *) &uoss,
-     compat_user_stack_pointer());
+     compat_user_stack_pointer(),
+     COMPAT_MINSIGSTKSZ);
  set_fs(seg);
  if (ret >= 0 && uoss_ptr)  {
  if (!access_ok(VERIFY_WRITE, uoss_ptr, sizeof(compat_stack_t)) ||
--
2.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH 2/2] arm64: compat: Provide definition for COMPAT_SIGMINSTKSZ

Juerg Haefliger
In reply to this post by Juerg Haefliger
From: Will Deacon <[hidden email]>

https://bugs.launchpad.net/bugs/1844155

arch/arm/ defines a SIGMINSTKSZ of 2k, so we should use the same value
for compat tasks.

Cc: Arnd Bergmann <[hidden email]>
Cc: Dominik Brodowski <[hidden email]>
Cc: "Eric W. Biederman" <[hidden email]>
Cc: Andrew Morton <[hidden email]>
Cc: Al Viro <[hidden email]>
Cc: Oleg Nesterov <[hidden email]>
Reviewed-by: Dave Martin <[hidden email]>
Reported-by: Steve McIntyre <[hidden email]>
Tested-by: Steve McIntyre <[hidden email]>
Signed-off-by: Will Deacon <[hidden email]>
Signed-off-by: Catalin Marinas <[hidden email]>

(cherry picked from commit 24951465cbd279f60b1fdc2421b3694405bcff42)
Signed-off-by: Juerg Haefliger <[hidden email]>
---
 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 eb8432bb82b8..b69e27152ea5 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -234,6 +234,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.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Xenial][PATCH 0/2] arm64: sigaltstack fails with MINSIGSTKSZ for 32-bit processes (LP: #1844155)

Stefan Bader-2
In reply to this post by Juerg Haefliger
On 01.10.19 19:56, Juerg Haefliger wrote:

> [Impact]
>
> The arm64 kernel allows one to run aarch32 processes on an aarch64 processor, using the standard 32/64-bit syscall compatibility. However this compat layer does not correctly validate the arguments of the sigaltstack syscall which can result in process failures.
>
> [Test Case]
>
> The simple reproducer from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=904385 triggers a memory allocation error with the current Xenial 4.4 kernel.
>
> [Fix]
>
> Backport the following two upstream commits:
> 24951465cbd2 arm64: compat: Provide definition for COMPAT_SIGMINSTKSZ
> 22839869f21a signal: Introduce COMPAT_SIGMINSTKSZ for use in compat_sys_sigaltstack
>
> With these two commits, the reproducer no longer fails.
>
> [Regression Potential]
>
> Low. The modifications are trivial and the two patches have been in upstream for quite a while.
>
> 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(-)
>
This looks right from what I remember (one additional function argument and
three call sites to adapt) for the larger patch. And is testable.

Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Xenial][PATCH 0/2] arm64: sigaltstack fails with MINSIGSTKSZ for 32-bit processes (LP: #1844155)

Connor Kuehl
In reply to this post by Juerg Haefliger
On 10/1/19 10:56 AM, Juerg Haefliger wrote:

> [Impact]
>
> The arm64 kernel allows one to run aarch32 processes on an aarch64 processor, using the standard 32/64-bit syscall compatibility. However this compat layer does not correctly validate the arguments of the sigaltstack syscall which can result in process failures.
>
> [Test Case]
>
> The simple reproducer from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=904385 triggers a memory allocation error with the current Xenial 4.4 kernel.
>
> [Fix]
>
> Backport the following two upstream commits:
> 24951465cbd2 arm64: compat: Provide definition for COMPAT_SIGMINSTKSZ
> 22839869f21a signal: Introduce COMPAT_SIGMINSTKSZ for use in compat_sys_sigaltstack
>
> With these two commits, the reproducer no longer fails.
>
> [Regression Potential]
>
> Low. The modifications are trivial and the two patches have been in upstream for quite a while.
>
> 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(-)
>

Acked-by: Connor Kuehl <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [SRU][Xenial][PATCH 0/2] arm64: sigaltstack fails with MINSIGSTKSZ for 32-bit processes (LP: #1844155)

Juerg Haefliger
In reply to this post by Juerg Haefliger
Applied to xenial/master-next.

...Juerg


> [Impact]
>
> The arm64 kernel allows one to run aarch32 processes on an aarch64 processor, using the standard 32/64-bit syscall compatibility. However this compat layer does not correctly validate the arguments of the sigaltstack syscall which can result in process failures.
>
> [Test Case]
>
> The simple reproducer from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=904385 triggers a memory allocation error with the current Xenial 4.4 kernel.
>
> [Fix]
>
> Backport the following two upstream commits:
> 24951465cbd2 arm64: compat: Provide definition for COMPAT_SIGMINSTKSZ
> 22839869f21a signal: Introduce COMPAT_SIGMINSTKSZ for use in compat_sys_sigaltstack
>
> With these two commits, the reproducer no longer fails.
>
> [Regression Potential]
>
> Low. The modifications are trivial and the two patches have been in upstream for quite a while.
>
> 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(-)
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

attachment0 (849 bytes) Download Attachment