diff mbox series

[v2,1/3] MIPS: signal: Protect against sigaltstack wraparound

Message ID 1639974460-3278-2-git-send-email-yangtiezhu@loongson.cn (mailing list archive)
State Accepted
Headers show
Series MIPS: signal: Modify some code | expand

Commit Message

Tiezhu Yang Dec. 20, 2021, 4:27 a.m. UTC
If a process uses alternative signal stack by using sigaltstack(),
then that stack overflows and stack wraparound occurs.

Simple Explanation:
The accurate sp order is A,B,C,D,...
But now the sp points to A,B,C and A,B,C again.

This problem can reproduce by the following code:

  $ cat test_sigaltstack.c
  #include <stdio.h>
  #include <signal.h>
  #include <stdlib.h>
  #include <string.h>

  volatile int counter = 0;

  void print_sp()
  {
      unsigned long sp;

      __asm__ __volatile__("move %0, $sp" : "=r" (sp));
      printf("sp = 0x%08lx\n", sp);
  }

  void segv_handler()
  {
      int *c = NULL;

      print_sp();
      counter++;
      printf("%d\n", counter);

      if (counter == 23)
          abort();

      *c = 1;	// SEGV
  }

  int main()
  {
      int *c = NULL;
      char *s = malloc(SIGSTKSZ);
      stack_t stack;
      struct sigaction action;

      memset(s, 0, SIGSTKSZ);
      stack.ss_sp = s;
      stack.ss_flags = 0;
      stack.ss_size = SIGSTKSZ;
      if (sigaltstack(&stack, NULL)) {
          printf("Failed to use sigaltstack!\n");
          return -1;
      }

      memset(&action, 0, sizeof(action));
      action.sa_handler = segv_handler;
      action.sa_flags = SA_ONSTACK | SA_NODEFER;
      sigemptyset(&action.sa_mask);
      sigaction(SIGSEGV, &action, NULL);

      *c = 0;	//SEGV

      if (!s)
          free(s);

      return 0;
  }
  $ gcc test_sigaltstack.c -o test_sigaltstack
  $ ./test_sigaltstack
  sp = 0x120015c80
  1
  sp = 0x120015900
  2
  sp = 0x120015580
  3
  sp = 0x120015200
  4
  sp = 0x120014e80
  5
  sp = 0x120014b00
  6
  sp = 0x120014780
  7
  sp = 0x120014400
  8
  sp = 0x120014080
  9
  sp = 0x120013d00
  10
  sp = 0x120015c80
  11               # wraparound occurs! the 11nd output is same as 1st.
  sp = 0x120015900
  12
  sp = 0x120015580
  13
  sp = 0x120015200
  14
  sp = 0x120014e80
  15
  sp = 0x120014b00
  16
  sp = 0x120014780
  17
  sp = 0x120014400
  18
  sp = 0x120014080
  19
  sp = 0x120013d00
  20
  sp = 0x120015c80
  21                # wraparound occurs! the 21nd output is same as 1st.
  sp = 0x120015900
  22
  sp = 0x120015580
  23
  Aborted

With this patch:

  $ ./test_sigaltstack
  sp = 0x120015c80
  1
  sp = 0x120015900
  2
  sp = 0x120015580
  3
  sp = 0x120015200
  4
  sp = 0x120014e80
  5
  sp = 0x120014b00
  6
  sp = 0x120014780
  7
  sp = 0x120014400
  8
  sp = 0x120014080
  9
  Segmentation fault

If we are on the alternate signal stack and would overflow it, don't.
Return an always-bogus address instead so we will die with SIGSEGV.

This patch is similar with commit 83bd01024b1f ("x86: protect against
sigaltstack wraparound").

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/mips/kernel/signal.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Thomas Bogendoerfer Jan. 2, 2022, 1:31 p.m. UTC | #1
On Mon, Dec 20, 2021 at 12:27:38PM +0800, Tiezhu Yang wrote:
> If a process uses alternative signal stack by using sigaltstack(),
> then that stack overflows and stack wraparound occurs.
> 
> Simple Explanation:
> The accurate sp order is A,B,C,D,...
> But now the sp points to A,B,C and A,B,C again.
> 
> This problem can reproduce by the following code:
> 
>   $ cat test_sigaltstack.c
>   #include <stdio.h>
>   #include <signal.h>
>   #include <stdlib.h>
>   #include <string.h>
> 
>   volatile int counter = 0;
> 
>   void print_sp()
>   {
>       unsigned long sp;
> 
>       __asm__ __volatile__("move %0, $sp" : "=r" (sp));
>       printf("sp = 0x%08lx\n", sp);
>   }
> 
>   void segv_handler()
>   {
>       int *c = NULL;
> 
>       print_sp();
>       counter++;
>       printf("%d\n", counter);
> 
>       if (counter == 23)
>           abort();
> 
>       *c = 1;	// SEGV
>   }
> 
>   int main()
>   {
>       int *c = NULL;
>       char *s = malloc(SIGSTKSZ);
>       stack_t stack;
>       struct sigaction action;
> 
>       memset(s, 0, SIGSTKSZ);
>       stack.ss_sp = s;
>       stack.ss_flags = 0;
>       stack.ss_size = SIGSTKSZ;
>       if (sigaltstack(&stack, NULL)) {
>           printf("Failed to use sigaltstack!\n");
>           return -1;
>       }
> 
>       memset(&action, 0, sizeof(action));
>       action.sa_handler = segv_handler;
>       action.sa_flags = SA_ONSTACK | SA_NODEFER;
>       sigemptyset(&action.sa_mask);
>       sigaction(SIGSEGV, &action, NULL);
> 
>       *c = 0;	//SEGV
> 
>       if (!s)
>           free(s);
> 
>       return 0;
>   }
>   $ gcc test_sigaltstack.c -o test_sigaltstack
>   $ ./test_sigaltstack
>   sp = 0x120015c80
>   1
>   sp = 0x120015900
>   2
>   sp = 0x120015580
>   3
>   sp = 0x120015200
>   4
>   sp = 0x120014e80
>   5
>   sp = 0x120014b00
>   6
>   sp = 0x120014780
>   7
>   sp = 0x120014400
>   8
>   sp = 0x120014080
>   9
>   sp = 0x120013d00
>   10
>   sp = 0x120015c80
>   11               # wraparound occurs! the 11nd output is same as 1st.
>   sp = 0x120015900
>   12
>   sp = 0x120015580
>   13
>   sp = 0x120015200
>   14
>   sp = 0x120014e80
>   15
>   sp = 0x120014b00
>   16
>   sp = 0x120014780
>   17
>   sp = 0x120014400
>   18
>   sp = 0x120014080
>   19
>   sp = 0x120013d00
>   20
>   sp = 0x120015c80
>   21                # wraparound occurs! the 21nd output is same as 1st.
>   sp = 0x120015900
>   22
>   sp = 0x120015580
>   23
>   Aborted
> 
> With this patch:
> 
>   $ ./test_sigaltstack
>   sp = 0x120015c80
>   1
>   sp = 0x120015900
>   2
>   sp = 0x120015580
>   3
>   sp = 0x120015200
>   4
>   sp = 0x120014e80
>   5
>   sp = 0x120014b00
>   6
>   sp = 0x120014780
>   7
>   sp = 0x120014400
>   8
>   sp = 0x120014080
>   9
>   Segmentation fault
> 
> If we are on the alternate signal stack and would overflow it, don't.
> Return an always-bogus address instead so we will die with SIGSEGV.
> 
> This patch is similar with commit 83bd01024b1f ("x86: protect against
> sigaltstack wraparound").
> 
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  arch/mips/kernel/signal.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
> index c9b2a75..c1632e8 100644
> --- a/arch/mips/kernel/signal.c
> +++ b/arch/mips/kernel/signal.c
> @@ -563,6 +563,13 @@ void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs,
>  	sp = regs->regs[29];
>  
>  	/*
> +	 * If we are on the alternate signal stack and would overflow it, don't.
> +	 * Return an always-bogus address instead so we will die with SIGSEGV.
> +	 */
> +	if (on_sig_stack(sp) && !likely(on_sig_stack(sp - frame_size)))
> +		return (void __user __force *)(-1UL);
> +
> +	/*
>  	 * FPU emulator may have it's own trampoline active just
>  	 * above the user stack, 16-bytes before the next lowest
>  	 * 16 byte boundary.  Try to avoid trashing it.
> -- 
> 2.1.0

applied to mips-next.

Thomas.
diff mbox series

Patch

diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index c9b2a75..c1632e8 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -563,6 +563,13 @@  void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs,
 	sp = regs->regs[29];
 
 	/*
+	 * If we are on the alternate signal stack and would overflow it, don't.
+	 * Return an always-bogus address instead so we will die with SIGSEGV.
+	 */
+	if (on_sig_stack(sp) && !likely(on_sig_stack(sp - frame_size)))
+		return (void __user __force *)(-1UL);
+
+	/*
 	 * FPU emulator may have it's own trampoline active just
 	 * above the user stack, 16-bytes before the next lowest
 	 * 16 byte boundary.  Try to avoid trashing it.