[uml-devel] [PATCH] um: pass siginfo to guest process

Martin Pärtel

author

2012-06-07 21:00 UTC #0 lore raw

PATCH 1/1 um: pass siginfo to guest process

Signal handlers in UML guest processes now get correct siginfo_t fields for SIGTRAP, SIGFPE, SIGILL and SIGBUS. Specifically, si_addr and si_code are now correct where previously they were si_addr = NULL and si_code = 128.

Signed-off-by Martin Pärtel <[email protected]>

---

arch/um/include/shared/as-layout.h +2 -1

diff -uprN -X linux-3.4.1/Documentation/dontdiff -x config.c -x config.tmp linux-3.4.1/arch/um/include/shared/as-layout.h linux-3.4.1-mod/arch/um/include/shared/as-layout.h
@@ -60,7 +60,8 @@ extern unsigned long host_task_size;
 
 extern int linux_main(int argc, char **argv);
 
-extern void (*sig_info[])(int, struct uml_pt_regs *);
+struct siginfo;
+extern void (*sig_info[])(int, struct siginfo *si, struct uml_pt_regs *);
 
 #endif

arch/um/include/shared/kern_util.h +4 -3

diff -uprN -X linux-3.4.1/Documentation/dontdiff -x config.c -x config.tmp linux-3.4.1/arch/um/include/shared/kern_util.h linux-3.4.1-mod/arch/um/include/shared/kern_util.h
@@ -9,6 +9,8 @@
 #include "sysdep/ptrace.h"
 #include "sysdep/faultinfo.h"
 
+struct siginfo;
+
 extern int uml_exitcode;
 
 extern int ncpus;

@@ -22,7 +24,7 @@ extern void free_stack(unsigned long sta
 
 extern int do_signal(void);
 extern void interrupt_end(void);
-extern void relay_signal(int sig, struct uml_pt_regs *regs);
+extern void relay_signal(int sig, struct siginfo *si, struct uml_pt_regs *regs);
 
 extern unsigned long segv(struct faultinfo fi, unsigned long ip,
 			  int is_user, struct uml_pt_regs *regs);

@@ -33,7 +35,6 @@ extern unsigned int do_IRQ(int irq, stru
 extern int smp_sigio_handler(void);
 extern void initial_thread_cb(void (*proc)(void *), void *arg);
 extern int is_syscall(unsigned long addr);
-extern void timer_handler(int sig, struct uml_pt_regs *regs);
 
 extern void timer_handler(int sig, struct uml_pt_regs *regs);

@@ -60,7 +61,7 @@ extern void syscall_trace(struct uml_pt_
 extern int singlestepping(void *t);
 
 extern void segv_handler(int sig, struct uml_pt_regs *regs);
-extern void bus_handler(int sig, struct uml_pt_regs *regs);
+extern void bus_handler(int sig, struct siginfo *si, struct uml_pt_regs *regs);
 extern void winch(int sig, struct uml_pt_regs *regs);
 extern void fatal_sigsegv(void) __attribute__ ((noreturn));

arch/um/kernel/trap.c +5 -4

diff -uprN -X linux-3.4.1/Documentation/dontdiff -x config.c -x config.tmp linux-3.4.1/arch/um/kernel/trap.c linux-3.4.1-mod/arch/um/kernel/trap.c
@@ -244,7 +244,7 @@ unsigned long segv(struct faultinfo fi,
 	return 0;
 }
 
-void relay_signal(int sig, struct uml_pt_regs *regs)
+void relay_signal(int sig, struct siginfo *si, struct uml_pt_regs *regs)
 {
 	if (!UPT_IS_USER(regs)) {
 		if (sig == SIGBUS)

@@ -256,14 +256,15 @@ void relay_signal(int sig, struct uml_pt
 	arch_examine_signal(sig, regs);
 
 	current->thread.arch.faultinfo = *UPT_FAULTINFO(regs);
-	force_sig(sig, current);
+	force_sig_info(sig, si, current);
 }
 
-void bus_handler(int sig, struct uml_pt_regs *regs)
+void bus_handler(int sig, struct siginfo *si, struct uml_pt_regs *regs)
 {
 	if (current->thread.fault_catcher != NULL)
 		UML_LONGJMP(current->thread.fault_catcher, 1);
-	else relay_signal(sig, regs);
+	else
+		relay_signal(sig, si, regs);
 }
 
 void winch(int sig, struct uml_pt_regs *regs)

arch/um/os-Linux/signal.c +54 ~~-14~~

127 lines not on this page · raw message

arch/um/os-Linux/skas/process.c +9 -4

diff -uprN -X linux-3.4.1/Documentation/dontdiff -x config.c -x config.tmp linux-3.4.1/arch/um/os-Linux/skas/process.c linux-3.4.1-mod/arch/um/os-Linux/skas/process.c
@@ -346,6 +346,7 @@ void userspace(struct uml_pt_regs *regs)
 	int err, status, op, pid = userspace_pid[0];
 	/* To prevent races if using_sysemu changes under us.*/
 	int local_using_sysemu;
+	siginfo_t si;
 
 	if (getitimer(ITIMER_VIRTUAL, &timer))
 		printk(UM_KERN_ERR "Failed to get itimer, errno = %d\n", errno);

@@ -404,13 +405,17 @@ void userspace(struct uml_pt_regs *regs)
 
 		if (WIFSTOPPED(status)) {
 			int sig = WSTOPSIG(status);
+
+			ptrace(PTRACE_GETSIGINFO, pid, 0, &si);
+
 			switch (sig) {
 			case SIGSEGV:
 				if (PTRACE_FULL_FAULTINFO ||
 				    !ptrace_faultinfo) {
 					get_skas_faultinfo(pid,
 							   &regs->faultinfo);
-					(*sig_info[SIGSEGV])(SIGSEGV, regs);
+					(*sig_info[SIGSEGV])(SIGSEGV, &si,
+							     regs);
 				}
 				else handle_segv(pid, regs);
 				break;

@@ -418,14 +423,14 @@ void userspace(struct uml_pt_regs *regs)
 			        handle_trap(pid, regs, local_using_sysemu);
 				break;
 			case SIGTRAP:
-				relay_signal(SIGTRAP, regs);
+				relay_signal(SIGTRAP, &si, regs);
 				break;
 			case SIGVTALRM:
 				now = os_nsecs();
 				if (now < nsecs)
 					break;
 				block_signals();
-				(*sig_info[sig])(sig, regs);
+				(*sig_info[sig])(sig, &si, regs);
 				unblock_signals();
 				nsecs = timer.it_value.tv_sec *
 					UM_NSEC_PER_SEC +

@@ -439,7 +444,7 @@ void userspace(struct uml_pt_regs *regs)
 			case SIGFPE:
 			case SIGWINCH:
 				block_signals();
-				(*sig_info[sig])(sig, regs);
+				(*sig_info[sig])(sig, &si, regs);
 				unblock_signals();
 				break;
 			default:

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Richard Weinberger

maintainer

2012-06-07 21:26 UTC #1 lore raw

Am 07.06.2012 22:59, schrieb Martin Pärtel:

Quoted 3 lines

Signal handlers in UML guest processes now get correct siginfo_t fields for SIGTRAP, SIGFPE, SIGILL and SIGBUS. Specifically, si_addr and si_code are now correct where previously they were si_addr = NULL and si_code = 128.

What exactly is broken?
In my SIGSEGV test case si_addr is not NULL, it contains the correct faulting address.

arch/um/os-Linux/skas/process.c void userspace(struct uml_pt_regs *regs) · line 410

Quoted 3 lines

arch/um/os-Linux/skas/process.c +3 -0
+
+			ptrace(PTRACE_GETSIGINFO, pid, 0, &si);
+

Doesn't this leak the host siginfo_t into the guest?

Thanks,
//richard

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Martin Pärtel

author

2012-06-07 21:41 UTC #2 lore raw

(ack, forgot to reply to list)

Quoted 8 lines · Richard Weinberger

Am 07.06.2012 22:59, schrieb Martin Pärtel:
Quoted 3 lines
Signal handlers in UML guest processes now get correct siginfo_t fields for SIGTRAP, SIGFPE, SIGILL and SIGBUS. Specifically, si_addr and si_code are now correct where previously they were si_addr = NULL and si_code = 128.
What exactly is broken?
In my SIGSEGV test case si_addr is not NULL, it contains the correct faulting address.

SIGSEGV is probably fine. At least SIGFPE is not. Test program below.

Quoted 6 lines

arch/um/os-Linux/signal.c line 31
Quoted 3 lines
arch/um/os-Linux/signal.c +3 -0
+
+			ptrace(PTRACE_GETSIGINFO, pid, 0,&si);
+
Doesn't this leak the host siginfo_t into the guest?

Docs for PTRACE_GETSIGINFO say si' gets a copy. After that, si' is not used for anything other than giving it to the guest. But I really can't say I understand the surrounding code too well so please review carefully :)

---
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <signal.h>

void handler(int num, siginfo_t *info, void *data)
{
printf("CAUGHT SIGNAL %d - si_addr = %p - si_code = %d\n", num,

info->si_addr, info->si_code);
     _exit(1);
}

int main()

{
     struct sigaction act;
     act.sa_sigaction = &handler;
     sigemptyset(&act.sa_mask);
     act.sa_flags = SA_SIGINFO;

     sigaction(SIGFPE, &act, NULL);

     printf("%d\n", 3 / 0);

     return 0;
}

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Richard Weinberger

maintainer

2012-06-07 22:08 UTC #4 lore raw

Am 07.06.2012 23:39, schrieb Martin Pärtel:

Quoted 22 lines · Richard Weinberger

Quoted 9 lines
Am 07.06.2012 22:59, schrieb Martin Pärtel:
Quoted 3 lines
Signal handlers in UML guest processes now get correct siginfo_t fields for SIGTRAP, SIGFPE, SIGILL and SIGBUS. Specifically, si_addr and si_code are now correct where previously they were si_addr = NULL and si_code = 128.
What exactly is broken?
In my SIGSEGV test case si_addr is not NULL, it contains the correct faulting address.
SIGSEGV is probably fine. At least SIGFPE is not. Test program below.
Quoted 6 lines
arch/um/os-Linux/signal.c line 31
Quoted 3 lines
arch/um/os-Linux/signal.c +3 -0
+
+            ptrace(PTRACE_GETSIGINFO, pid, 0,&si);
+
Doesn't this leak the host siginfo_t into the guest?

Quoted 2 lines

Docs for PTRACE_GETSIGINFO say si' gets a copy. After that, si' is not used for anything other than giving it to the guest. But I really can't say I understand the surrounding code too well so please review carefully :)

I was not talking about a memory leak.
What I meant was a information leak.
Using the host siginfo_t a guest process may get it's UID, PID, memory location, etc... on the host side.

Anyway, thanks for the test case!
This seems to be really broken.
I had only a few minutes to look at the issue but I think the correct way to fix is changing arch/um/kernel/trap.c:relay_signal() to use force_sig_info() instead of force_sig(). Create siginfo_t and fill by hand like segv() does.

Thanks,
//richard

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Martin Pärtel

author

2012-06-07 22:42 UTC #5 lore raw

Quoted 23 lines · Richard Weinberger

Am 07.06.2012 23:39, schrieb Martin Pärtel:
Quoted 22 lines · Richard Weinberger
Quoted 9 lines
Am 07.06.2012 22:59, schrieb Martin Pärtel:
Quoted 3 lines
Signal handlers in UML guest processes now get correct siginfo_t fields for SIGTRAP, SIGFPE, SIGILL and SIGBUS. Specifically, si_addr and si_code are now correct where previously they were si_addr = NULL and si_code = 128.
What exactly is broken?
In my SIGSEGV test case si_addr is not NULL, it contains the correct faulting address.
SIGSEGV is probably fine. At least SIGFPE is not. Test program below.
Quoted 6 lines
arch/um/os-Linux/signal.c line 31
Quoted 3 lines
arch/um/os-Linux/signal.c +3 -0
+
+            ptrace(PTRACE_GETSIGINFO, pid, 0,&si);
+
Doesn't this leak the host siginfo_t into the guest?

Quoted 7 lines

Quoted 2 lines
Docs for PTRACE_GETSIGINFO say si' gets a copy. After that, si' is not used for anything other than giving it to the guest. But I really can't say I understand the surrounding code too well so please review carefully :)
I was not talking about a memory leak.
What I meant was a information leak.
Using the host siginfo_t a guest process may get it's UID, PID, memory location, etc... on the host side.

Oh, darn, indeed. Well, getting si_code right fixed my immediate problem, but I might look at this again some time next week unless you've fixed it yourself by then. Thanks!

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Richard Weinberger

maintainer

2012-06-07 22:47 UTC #6 lore raw

Am 08.06.2012 00:42, schrieb Martin Pärtel:

Quoted 3 lines

Oh, darn, indeed. Well, getting si_code right fixed my immediate problem, but I might look at this again some time next week unless you've fixed it yourself by then. Thanks!

I can wait for your patch.
No need to hurry. :)

Thanks,
//richard

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel