* [PATCH v2] LoongArch: Add safer signal handler for TLS access
@ 2022-08-31 7:48 Mao Bibo
2022-09-01 2:30 ` WANG Xuerui
0 siblings, 1 reply; 3+ messages in thread
From: Mao Bibo @ 2022-08-31 7:48 UTC (permalink / raw)
To: Huacai Chen, Arnd Bergmann, Christian Brauner
Cc: WANG Xuerui, Jiaxun Yang, loongarch, linux-kernel
LoongArch uses general purpose register R2 as thread pointer(TP)
register, signal hanlder also uses TP register to access variables
in TLS area, such as errno and variable in TLS.
If GPR R2 is modified with wrong value, signal handler still uses
the wrong TP register, so signal hanlder is insafe to access TLS
variable.
This patch adds one arch specific syscall set_thread_area, and
restore previoud TP value before signal handler, so that signal
handler is safe to access TLS variable.
It passes to run with the following test case.
=======8<======
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <signal.h>
#include <pthread.h>
#include <asm/ucontext.h>
#include <asm/sigcontext.h>
#define ILL_INSN ".word 0x000001f0"
static inline long test_sigill(unsigned long fid)
{
register long ret __asm__("$r4");
register unsigned long fun __asm__("$r4") = fid;
__asm__ __volatile__("move $r2, $r0 \r\n");
__asm__ __volatile__(
ILL_INSN
: "=r" (ret)
: "r" (fun)
: "memory"
);
return ret;
}
static void set_sigill_handler(void (*fn) (int, siginfo_t *, void *))
{
struct sigaction sa;
memset(&sa, 0, sizeof(struct sigaction));
sa.sa_sigaction = fn;
sa.sa_flags = SA_SIGINFO;
sigemptyset(&sa.sa_mask);
if (sigaction(SIGILL, &sa, 0) != 0) {
perror("sigaction");
}
}
void catch_sig(int sig, siginfo_t *si, void *vuc)
{
struct ucontext *uc = vuc;
register unsigned long tls __asm__("$r2");
uc->uc_mcontext.sc_pc +=4;
uc->uc_mcontext.sc_regs[2] = tls;
printf("catched signal %d\n", sig);
}
void *print_message_function( void *ptr )
{
char *message;
message = (char *) ptr;
printf("%s \n", message);
test_sigill(1);
}
void pthread_test(void)
{
pthread_t thread1, thread2;
char *message1 = "Thread 1";
char *message2 = "Thread 2";
int iret1, iret2;
iret1 = pthread_create( &thread1, NULL, print_message_function,
(void*) message1);
iret2 = pthread_create( &thread2, NULL, print_message_function,
(void*) message2);
pthread_join( thread1, NULL);
pthread_join( thread2, NULL);
printf("Thread 1 returns: %d\n",iret1);
printf("Thread 2 returns: %d\n",iret2);
exit(0);
}
void exec_test(void) {
test_sigill(1);
}
void main() {
register unsigned long tls __asm__("$r2");
int val;
val = syscall(244, tls);
set_sigill_handler(&catch_sig);
pthread_test();
//exec_test();
return;
}
=======8<======
Signed-off-by: Mao Bibo <maobibo@loongson.cn>
---
v1->v2:
- Clear TP value in clone function if CLONE_SETTLS is not set
---
arch/loongarch/include/asm/unistd.h | 1 +
arch/loongarch/include/uapi/asm/unistd.h | 2 ++
arch/loongarch/kernel/process.c | 10 +++++++++-
arch/loongarch/kernel/signal.c | 5 +++++
arch/loongarch/kernel/syscall.c | 10 ++++++++++
5 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/arch/loongarch/include/asm/unistd.h b/arch/loongarch/include/asm/unistd.h
index cfddb0116a8c..1581624f0115 100644
--- a/arch/loongarch/include/asm/unistd.h
+++ b/arch/loongarch/include/asm/unistd.h
@@ -9,3 +9,4 @@
#include <uapi/asm/unistd.h>
#define NR_syscalls (__NR_syscalls)
+__SYSCALL(__NR_set_thread_area, sys_set_thread_area)
diff --git a/arch/loongarch/include/uapi/asm/unistd.h b/arch/loongarch/include/uapi/asm/unistd.h
index fcb668984f03..b47f26b5307b 100644
--- a/arch/loongarch/include/uapi/asm/unistd.h
+++ b/arch/loongarch/include/uapi/asm/unistd.h
@@ -3,3 +3,5 @@
#define __ARCH_WANT_SYS_CLONE3
#include <asm-generic/unistd.h>
+
+#define __NR_set_thread_area (__NR_arch_specific_syscall + 0)
diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c
index 660492f064e7..f513b3d845b4 100644
--- a/arch/loongarch/kernel/process.c
+++ b/arch/loongarch/kernel/process.c
@@ -88,6 +88,7 @@ void start_thread(struct pt_regs *regs, unsigned long pc, unsigned long sp)
clear_used_math();
regs->csr_era = pc;
regs->regs[3] = sp;
+ task_thread_info(current)->tp_value = 0;
}
void exit_thread(struct task_struct *tsk)
@@ -176,8 +177,15 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
clear_tsk_thread_flag(p, TIF_LSX_CTX_LIVE);
clear_tsk_thread_flag(p, TIF_LASX_CTX_LIVE);
- if (clone_flags & CLONE_SETTLS)
+ /*
+ * record tls val for cloned threads
+ * clear it for forked process
+ */
+ if (clone_flags & CLONE_SETTLS) {
childregs->regs[2] = tls;
+ task_thread_info(p)->tp_value = tls;
+ } else
+ task_thread_info(p)->tp_value = 0;
return 0;
}
diff --git a/arch/loongarch/kernel/signal.c b/arch/loongarch/kernel/signal.c
index 7f4889df4a17..9e0c9755e548 100644
--- a/arch/loongarch/kernel/signal.c
+++ b/arch/loongarch/kernel/signal.c
@@ -480,6 +480,9 @@ static int setup_rt_frame(void *sig_return, struct ksignal *ksig,
*
* c0_era point to the signal handler, $r3 (sp) points to
* the struct rt_sigframe.
+ *
+ * Use recorded TP to signal handler if exists since $r2 may be
+ * corrupted already.
*/
regs->regs[4] = ksig->sig;
regs->regs[5] = (unsigned long) &frame->rs_info;
@@ -487,6 +490,8 @@ static int setup_rt_frame(void *sig_return, struct ksignal *ksig,
regs->regs[3] = (unsigned long) frame;
regs->regs[1] = (unsigned long) sig_return;
regs->csr_era = (unsigned long) ksig->ka.sa.sa_handler;
+ if (task_thread_info(current)->tp_value)
+ regs->regs[2] = task_thread_info(current)->tp_value;
DEBUGP("SIG deliver (%s:%d): sp=0x%p pc=0x%lx ra=0x%lx\n",
current->comm, current->pid,
diff --git a/arch/loongarch/kernel/syscall.c b/arch/loongarch/kernel/syscall.c
index 3fc4211db989..b62560d8fe24 100644
--- a/arch/loongarch/kernel/syscall.c
+++ b/arch/loongarch/kernel/syscall.c
@@ -29,6 +29,16 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len, unsigned long,
return ksys_mmap_pgoff(addr, len, prot, flags, fd, offset >> PAGE_SHIFT);
}
+SYSCALL_DEFINE1(set_thread_area, unsigned long, addr)
+{
+ struct thread_info *ti = task_thread_info(current);
+ struct pt_regs *regs = current_pt_regs();
+
+ regs->regs[2] = addr;
+ ti->tp_value = addr;
+ return 0;
+}
+
void *sys_call_table[__NR_syscalls] = {
[0 ... __NR_syscalls - 1] = sys_ni_syscall,
#include <asm/unistd.h>
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] LoongArch: Add safer signal handler for TLS access
2022-08-31 7:48 [PATCH v2] LoongArch: Add safer signal handler for TLS access Mao Bibo
@ 2022-09-01 2:30 ` WANG Xuerui
2022-09-01 3:02 ` maobibo
0 siblings, 1 reply; 3+ messages in thread
From: WANG Xuerui @ 2022-09-01 2:30 UTC (permalink / raw)
To: Mao Bibo, Huacai Chen, Arnd Bergmann, Christian Brauner
Cc: Jiaxun Yang, loongarch, linux-kernel
On 2022/8/31 15:48, Mao Bibo wrote:
> LoongArch uses general purpose register R2 as thread pointer(TP)
> register, signal hanlder also uses TP register to access variables
> in TLS area, such as errno and variable in TLS.
>
> If GPR R2 is modified with wrong value, signal handler still uses
> the wrong TP register, so signal hanlder is insafe to access TLS
> variable.
>
> This patch adds one arch specific syscall set_thread_area, and
> restore previoud TP value before signal handler, so that signal
> handler is safe to access TLS variable.
>
> It passes to run with the following test case.
> =======8<======
> #define _GNU_SOURCE
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <string.h>
> #include <sys/syscall.h>
> #include <sys/types.h>
> #include <signal.h>
> #include <pthread.h>
> #include <asm/ucontext.h>
> #include <asm/sigcontext.h>
>
> #define ILL_INSN ".word 0x000001f0"
> static inline long test_sigill(unsigned long fid)
> {
> register long ret __asm__("$r4");
> register unsigned long fun __asm__("$r4") = fid;
>
> __asm__ __volatile__("move $r2, $r0 \r\n");
> __asm__ __volatile__(
> ILL_INSN
> : "=r" (ret)
> : "r" (fun)
> : "memory"
> );
>
> return ret;
> }
>
> static void set_sigill_handler(void (*fn) (int, siginfo_t *, void *))
> {
> struct sigaction sa;
> memset(&sa, 0, sizeof(struct sigaction));
>
> sa.sa_sigaction = fn;
> sa.sa_flags = SA_SIGINFO;
> sigemptyset(&sa.sa_mask);
> if (sigaction(SIGILL, &sa, 0) != 0) {
> perror("sigaction");
> }
> }
>
> void catch_sig(int sig, siginfo_t *si, void *vuc)
> {
> struct ucontext *uc = vuc;
> register unsigned long tls __asm__("$r2");
>
> uc->uc_mcontext.sc_pc +=4;
> uc->uc_mcontext.sc_regs[2] = tls;
> printf("catched signal %d\n", sig);
> }
>
> void *print_message_function( void *ptr )
> {
> char *message;
> message = (char *) ptr;
> printf("%s \n", message);
> test_sigill(1);
> }
>
> void pthread_test(void)
> {
> pthread_t thread1, thread2;
> char *message1 = "Thread 1";
> char *message2 = "Thread 2";
> int iret1, iret2;
>
> iret1 = pthread_create( &thread1, NULL, print_message_function,
> (void*) message1);
> iret2 = pthread_create( &thread2, NULL, print_message_function,
> (void*) message2);
> pthread_join( thread1, NULL);
> pthread_join( thread2, NULL);
> printf("Thread 1 returns: %d\n",iret1);
> printf("Thread 2 returns: %d\n",iret2);
> exit(0);
> }
>
> void exec_test(void) {
> test_sigill(1);
> }
>
> void main() {
> register unsigned long tls __asm__("$r2");
> int val;
>
> val = syscall(244, tls);
> set_sigill_handler(&catch_sig);
> pthread_test();
> //exec_test();
> return;
> }
> =======8<======
Sorry, but what is the concrete use case you have in mind? arch/riscv
for example doesn't have set_thread_area but is apparently doing fine. I
haven't studied their code in great detail to find out myself though.
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] LoongArch: Add safer signal handler for TLS access
2022-09-01 2:30 ` WANG Xuerui
@ 2022-09-01 3:02 ` maobibo
0 siblings, 0 replies; 3+ messages in thread
From: maobibo @ 2022-09-01 3:02 UTC (permalink / raw)
To: WANG Xuerui, Huacai Chen, Arnd Bergmann, Christian Brauner
Cc: Jiaxun Yang, loongarch, linux-kernel
在 2022/9/1 10:30, WANG Xuerui 写道:
> On 2022/8/31 15:48, Mao Bibo wrote:
>> LoongArch uses general purpose register R2 as thread pointer(TP)
>> register, signal hanlder also uses TP register to access variables
>> in TLS area, such as errno and variable in TLS.
>>
>> If GPR R2 is modified with wrong value, signal handler still uses
>> the wrong TP register, so signal hanlder is insafe to access TLS
>> variable.
>>
>> This patch adds one arch specific syscall set_thread_area, and
>> restore previoud TP value before signal handler, so that signal
>> handler is safe to access TLS variable.
>>
>> It passes to run with the following test case.
>> =======8<======
>> #define _GNU_SOURCE
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <unistd.h>
>> #include <string.h>
>> #include <sys/syscall.h>
>> #include <sys/types.h>
>> #include <signal.h>
>> #include <pthread.h>
>> #include <asm/ucontext.h>
>> #include <asm/sigcontext.h>
>>
>> #define ILL_INSN ".word 0x000001f0"
>> static inline long test_sigill(unsigned long fid)
>> {
>> register long ret __asm__("$r4");
>> register unsigned long fun __asm__("$r4") = fid;
>>
>> __asm__ __volatile__("move $r2, $r0 \r\n");
>> __asm__ __volatile__(
>> ILL_INSN
>> : "=r" (ret)
>> : "r" (fun)
>> : "memory"
>> );
>>
>> return ret;
>> }
>>
>> static void set_sigill_handler(void (*fn) (int, siginfo_t *, void *))
>> {
>> struct sigaction sa;
>> memset(&sa, 0, sizeof(struct sigaction));
>>
>> sa.sa_sigaction = fn;
>> sa.sa_flags = SA_SIGINFO;
>> sigemptyset(&sa.sa_mask);
>> if (sigaction(SIGILL, &sa, 0) != 0) {
>> perror("sigaction");
>> }
>> }
>>
>> void catch_sig(int sig, siginfo_t *si, void *vuc)
>> {
>> struct ucontext *uc = vuc;
>> register unsigned long tls __asm__("$r2");
>>
>> uc->uc_mcontext.sc_pc +=4;
>> uc->uc_mcontext.sc_regs[2] = tls;
>> printf("catched signal %d\n", sig);
>> }
>>
>> void *print_message_function( void *ptr )
>> {
>> char *message;
>> message = (char *) ptr;
>> printf("%s \n", message);
>> test_sigill(1);
>> }
>>
>> void pthread_test(void)
>> {
>> pthread_t thread1, thread2;
>> char *message1 = "Thread 1";
>> char *message2 = "Thread 2";
>> int iret1, iret2;
>>
>> iret1 = pthread_create( &thread1, NULL, print_message_function,
>> (void*) message1);
>> iret2 = pthread_create( &thread2, NULL, print_message_function,
>> (void*) message2);
>> pthread_join( thread1, NULL);
>> pthread_join( thread2, NULL);
>> printf("Thread 1 returns: %d\n",iret1);
>> printf("Thread 2 returns: %d\n",iret2);
>> exit(0);
>> }
>>
>> void exec_test(void) {
>> test_sigill(1);
>> }
>>
>> void main() {
>> register unsigned long tls __asm__("$r2");
>> int val;
>>
>> val = syscall(244, tls);
>> set_sigill_handler(&catch_sig);
>> pthread_test();
>> //exec_test();
>> return;
>> }
>> =======8<======
>
> Sorry, but what is the concrete use case you have in mind? arch/riscv for example doesn't have set_thread_area but is apparently doing fine. I haven't studied their code in great detail to find out myself though.
We prepare to add loongarch support to risu project, which injects random instructions in user space. The website is:
https://git.linaro.org/people/peter.maydell/risu.git
After injected instruction is executed, there will be special illegal instruction where signal handler is used to control to capture the whole registers and continue to run. However if random injected instruction modify TP register, signal handler fails to run.
I double that riscv has the same issue.
regards
bibo,mao
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-01 3:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-31 7:48 [PATCH v2] LoongArch: Add safer signal handler for TLS access Mao Bibo
2022-09-01 2:30 ` WANG Xuerui
2022-09-01 3:02 ` maobibo
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).