* [PATCH 0/3] MIPS: signal: Modify some code
@ 2021-12-18 3:23 Tiezhu Yang
2021-12-18 3:23 ` [PATCH 1/3] MIPS: signal: Protect against sigaltstack wraparound Tiezhu Yang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tiezhu Yang @ 2021-12-18 3:23 UTC (permalink / raw)
To: Thomas Bogendoerfer; +Cc: Xuefeng Li, linux-mips, linux-kernel
Tiezhu Yang (3):
MIPS: signal: Protect against sigaltstack wraparound
MIPS: signal: Return immediately if call fails
MIPS: signal: Remove unnecessary DEBUG_SIG related code
arch/mips/kernel/signal-common.h | 8 --------
arch/mips/kernel/signal.c | 39 ++++++++++++++++++++++++++-------------
arch/mips/kernel/signal_n32.c | 4 ----
arch/mips/kernel/signal_o32.c | 8 --------
4 files changed, 26 insertions(+), 33 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] MIPS: signal: Protect against sigaltstack wraparound
2021-12-18 3:23 [PATCH 0/3] MIPS: signal: Modify some code Tiezhu Yang
@ 2021-12-18 3:23 ` Tiezhu Yang
2021-12-18 3:23 ` [PATCH 2/3] MIPS: signal: Return immediately if call fails Tiezhu Yang
2021-12-18 3:23 ` [PATCH 3/3] MIPS: signal: Remove unnecessary DEBUG_SIG related code Tiezhu Yang
2 siblings, 0 replies; 6+ messages in thread
From: Tiezhu Yang @ 2021-12-18 3:23 UTC (permalink / raw)
To: Thomas Bogendoerfer; +Cc: Xuefeng Li, linux-mips, linux-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] MIPS: signal: Return immediately if call fails
2021-12-18 3:23 [PATCH 0/3] MIPS: signal: Modify some code Tiezhu Yang
2021-12-18 3:23 ` [PATCH 1/3] MIPS: signal: Protect against sigaltstack wraparound Tiezhu Yang
@ 2021-12-18 3:23 ` Tiezhu Yang
2021-12-18 14:08 ` Jiaxun Yang
2021-12-18 3:23 ` [PATCH 3/3] MIPS: signal: Remove unnecessary DEBUG_SIG related code Tiezhu Yang
2 siblings, 1 reply; 6+ messages in thread
From: Tiezhu Yang @ 2021-12-18 3:23 UTC (permalink / raw)
To: Thomas Bogendoerfer; +Cc: Xuefeng Li, linux-mips, linux-kernel
When debug sigaltstack(), copy_siginfo_to_user() fails first in
setup_rt_frame() if the alternate signal stack is too small, so
it should return immediately if call fails, no need to call the
following functions.
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
arch/mips/kernel/signal.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index c1632e8..4cd3969 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -761,15 +761,28 @@ static int setup_rt_frame(void *sig_return, struct ksignal *ksig,
return -EFAULT;
/* Create siginfo. */
- err |= copy_siginfo_to_user(&frame->rs_info, &ksig->info);
+ err = copy_siginfo_to_user(&frame->rs_info, &ksig->info);
+ if (err)
+ return -EFAULT;
/* Create the ucontext. */
- err |= __put_user(0, &frame->rs_uc.uc_flags);
- err |= __put_user(NULL, &frame->rs_uc.uc_link);
- err |= __save_altstack(&frame->rs_uc.uc_stack, regs->regs[29]);
- err |= setup_sigcontext(regs, &frame->rs_uc.uc_mcontext);
- err |= __copy_to_user(&frame->rs_uc.uc_sigmask, set, sizeof(*set));
+ err = __put_user(0, &frame->rs_uc.uc_flags);
+ if (err)
+ return -EFAULT;
+
+ err = __put_user(NULL, &frame->rs_uc.uc_link);
+ if (err)
+ return -EFAULT;
+
+ err = __save_altstack(&frame->rs_uc.uc_stack, regs->regs[29]);
+ if (err)
+ return -EFAULT;
+
+ err = setup_sigcontext(regs, &frame->rs_uc.uc_mcontext);
+ if (err)
+ return -EFAULT;
+ err = __copy_to_user(&frame->rs_uc.uc_sigmask, set, sizeof(*set));
if (err)
return -EFAULT;
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] MIPS: signal: Remove unnecessary DEBUG_SIG related code
2021-12-18 3:23 [PATCH 0/3] MIPS: signal: Modify some code Tiezhu Yang
2021-12-18 3:23 ` [PATCH 1/3] MIPS: signal: Protect against sigaltstack wraparound Tiezhu Yang
2021-12-18 3:23 ` [PATCH 2/3] MIPS: signal: Return immediately if call fails Tiezhu Yang
@ 2021-12-18 3:23 ` Tiezhu Yang
2 siblings, 0 replies; 6+ messages in thread
From: Tiezhu Yang @ 2021-12-18 3:23 UTC (permalink / raw)
To: Thomas Bogendoerfer; +Cc: Xuefeng Li, linux-mips, linux-kernel
Since DEBUG_SIG is not defined on MIPS, so DEBUGP() is an empty function.
Additionally, it is unacceptable to printk message in the normal path of
signal handling, the system can not work well if DEBUG_SIG is defined, so
just remove the related code.
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
arch/mips/kernel/signal-common.h | 8 --------
arch/mips/kernel/signal.c | 7 -------
arch/mips/kernel/signal_n32.c | 4 ----
arch/mips/kernel/signal_o32.c | 8 --------
4 files changed, 27 deletions(-)
diff --git a/arch/mips/kernel/signal-common.h b/arch/mips/kernel/signal-common.h
index f50d484..f70135f 100644
--- a/arch/mips/kernel/signal-common.h
+++ b/arch/mips/kernel/signal-common.h
@@ -11,14 +11,6 @@
#ifndef __SIGNAL_COMMON_H
#define __SIGNAL_COMMON_H
-/* #define DEBUG_SIG */
-
-#ifdef DEBUG_SIG
-# define DEBUGP(fmt, args...) printk("%s: " fmt, __func__, ##args)
-#else
-# define DEBUGP(fmt, args...)
-#endif
-
/*
* Determine which stack to use..
*/
diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index 4cd3969..feb0cba 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -743,9 +743,6 @@ static int setup_frame(void *sig_return, struct ksignal *ksig,
regs->regs[31] = (unsigned long) sig_return;
regs->cp0_epc = regs->regs[25] = (unsigned long) ksig->ka.sa.sa_handler;
- DEBUGP("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%lx\n",
- current->comm, current->pid,
- frame, regs->cp0_epc, regs->regs[31]);
return 0;
}
#endif
@@ -803,10 +800,6 @@ static int setup_rt_frame(void *sig_return, struct ksignal *ksig,
regs->regs[31] = (unsigned long) sig_return;
regs->cp0_epc = regs->regs[25] = (unsigned long) ksig->ka.sa.sa_handler;
- DEBUGP("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%lx\n",
- current->comm, current->pid,
- frame, regs->cp0_epc, regs->regs[31]);
-
return 0;
}
diff --git a/arch/mips/kernel/signal_n32.c b/arch/mips/kernel/signal_n32.c
index 7bd00fa..d0e3f74 100644
--- a/arch/mips/kernel/signal_n32.c
+++ b/arch/mips/kernel/signal_n32.c
@@ -130,10 +130,6 @@ static int setup_rt_frame_n32(void *sig_return, struct ksignal *ksig,
regs->regs[31] = (unsigned long) sig_return;
regs->cp0_epc = regs->regs[25] = (unsigned long) ksig->ka.sa.sa_handler;
- DEBUGP("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%lx\n",
- current->comm, current->pid,
- frame, regs->cp0_epc, regs->regs[31]);
-
return 0;
}
diff --git a/arch/mips/kernel/signal_o32.c b/arch/mips/kernel/signal_o32.c
index 299a7a2..3691f74 100644
--- a/arch/mips/kernel/signal_o32.c
+++ b/arch/mips/kernel/signal_o32.c
@@ -144,10 +144,6 @@ static int setup_frame_32(void *sig_return, struct ksignal *ksig,
regs->regs[31] = (unsigned long) sig_return;
regs->cp0_epc = regs->regs[25] = (unsigned long) ksig->ka.sa.sa_handler;
- DEBUGP("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%lx\n",
- current->comm, current->pid,
- frame, regs->cp0_epc, regs->regs[31]);
-
return 0;
}
@@ -230,10 +226,6 @@ static int setup_rt_frame_32(void *sig_return, struct ksignal *ksig,
regs->regs[31] = (unsigned long) sig_return;
regs->cp0_epc = regs->regs[25] = (unsigned long) ksig->ka.sa.sa_handler;
- DEBUGP("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%lx\n",
- current->comm, current->pid,
- frame, regs->cp0_epc, regs->regs[31]);
-
return 0;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] MIPS: signal: Return immediately if call fails
2021-12-18 3:23 ` [PATCH 2/3] MIPS: signal: Return immediately if call fails Tiezhu Yang
@ 2021-12-18 14:08 ` Jiaxun Yang
2021-12-20 2:25 ` Tiezhu Yang
0 siblings, 1 reply; 6+ messages in thread
From: Jiaxun Yang @ 2021-12-18 14:08 UTC (permalink / raw)
To: Tiezhu Yang, Thomas Bogendoerfer; +Cc: Xuefeng Li, linux-mips, linux-kernel
在 2021/12/18 3:23, Tiezhu Yang 写道:
> When debug sigaltstack(), copy_siginfo_to_user() fails first in
> setup_rt_frame() if the alternate signal stack is too small, so
> it should return immediately if call fails, no need to call the
> following functions.
Hi Tiezhu,
Thanks for your patch.
If we are doing so I see no reason for keeping the err variable.
Just
if (copy_siginfo_to_user(&frame->rs_info, &ksig->info))
return -EFAULT;
seems much more clear.
Thanks.
- Jiaxun
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
> arch/mips/kernel/signal.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
> index c1632e8..4cd3969 100644
> --- a/arch/mips/kernel/signal.c
> +++ b/arch/mips/kernel/signal.c
> @@ -761,15 +761,28 @@ static int setup_rt_frame(void *sig_return, struct ksignal *ksig,
> return -EFAULT;
>
> /* Create siginfo. */
> - err |= copy_siginfo_to_user(&frame->rs_info, &ksig->info);
> + err = copy_siginfo_to_user(&frame->rs_info, &ksig->info);
> + if (err)
> + return -EFAULT;
>
> /* Create the ucontext. */
> - err |= __put_user(0, &frame->rs_uc.uc_flags);
> - err |= __put_user(NULL, &frame->rs_uc.uc_link);
> - err |= __save_altstack(&frame->rs_uc.uc_stack, regs->regs[29]);
> - err |= setup_sigcontext(regs, &frame->rs_uc.uc_mcontext);
> - err |= __copy_to_user(&frame->rs_uc.uc_sigmask, set, sizeof(*set));
> + err = __put_user(0, &frame->rs_uc.uc_flags);
> + if (err)
> + return -EFAULT;
> +
> + err = __put_user(NULL, &frame->rs_uc.uc_link);
> + if (err)
> + return -EFAULT;
> +
> + err = __save_altstack(&frame->rs_uc.uc_stack, regs->regs[29]);
> + if (err)
> + return -EFAULT;
> +
> + err = setup_sigcontext(regs, &frame->rs_uc.uc_mcontext);
> + if (err)
> + return -EFAULT;
>
> + err = __copy_to_user(&frame->rs_uc.uc_sigmask, set, sizeof(*set));
> if (err)
> return -EFAULT;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] MIPS: signal: Return immediately if call fails
2021-12-18 14:08 ` Jiaxun Yang
@ 2021-12-20 2:25 ` Tiezhu Yang
0 siblings, 0 replies; 6+ messages in thread
From: Tiezhu Yang @ 2021-12-20 2:25 UTC (permalink / raw)
To: Jiaxun Yang, Thomas Bogendoerfer; +Cc: Xuefeng Li, linux-mips, linux-kernel
On 12/18/2021 10:08 PM, Jiaxun Yang wrote:
>
>
> 在 2021/12/18 3:23, Tiezhu Yang 写道:
>> When debug sigaltstack(), copy_siginfo_to_user() fails first in
>> setup_rt_frame() if the alternate signal stack is too small, so
>> it should return immediately if call fails, no need to call the
>> following functions.
>
> Hi Tiezhu,
>
> Thanks for your patch.
> If we are doing so I see no reason for keeping the err variable.
> Just
>
> if (copy_siginfo_to_user(&frame->rs_info, &ksig->info))
> return -EFAULT;
>
> seems much more clear.
OK, thank you, I will send v2 later.
Thanks,
Tiezhu
>
> Thanks.
>
> - Jiaxun
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-20 2:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-18 3:23 [PATCH 0/3] MIPS: signal: Modify some code Tiezhu Yang
2021-12-18 3:23 ` [PATCH 1/3] MIPS: signal: Protect against sigaltstack wraparound Tiezhu Yang
2021-12-18 3:23 ` [PATCH 2/3] MIPS: signal: Return immediately if call fails Tiezhu Yang
2021-12-18 14:08 ` Jiaxun Yang
2021-12-20 2:25 ` Tiezhu Yang
2021-12-18 3:23 ` [PATCH 3/3] MIPS: signal: Remove unnecessary DEBUG_SIG related code Tiezhu Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).