uml: fix uml-use-sysemu-for-tt.patch

PATCH In review Bodo Stroesser 2 participants 6 messages Nov 12, 2004 – Nov 26, 2004 User-mode Linux (UML)

lore

Share & tools

lore

Bodo Stroesser

author

2004-11-12 14:10 UTC #0 lore raw

PATCH 1/1 uml: fix uml-use-sysemu-for-tt.patch

From: Bodo Stroesser <[email protected]>

The patch needs some small corrections:
1) local_using_sysemu must be sampled *before* the next ptrace(PTRACE_SYSEMU/SYSCALL) and must stay the same until do_syscall() has been done. Currently it is sampled before do_syscall() and is used after this for ptrace(PTRACE_SYSEMU/SYSCALL). Even if no problem is visible to the UML user, a single syscall could be executed on the host when switching on sysemu. The result of this then is overwritten by the syscall execution in UML.
Since the first event the tracer has to handle is not a syscall, it's enough to initialize local_using_sysemu to 0;
2) Even if the host never *does* a syscall in SYSEMU, we have to write the syscall number with -1, to not have the host doing syscall restarting. This would happen only with an invalid syscall number equal to one of the -ERESTART values. But to be perfect ...
Additionally I changed do_syscall() to be void instead of int.

Signed-off-by Bodo Stroesser <[email protected]>

---

arch/um/kernel/tt/include/tt.h +1 -1

@@ -26,7 +26,7 @@ extern void set_tracing(void *t, int tra
 extern int is_tracing(void *task);
 extern void syscall_handler(int sig, union uml_pt_regs *regs);
 extern void exit_kernel(int pid, void *task);
-extern int do_syscall(void *task, int pid, int local_using_sysemu);
+extern void do_syscall(void *task, int pid, int local_using_sysemu);
 extern void do_sigtrap(void *task);
 extern int is_valid_pid(int pid);
 extern void remap_data(void *segment_start, void *segment_end, int w);

arch/um/kernel/tt/tracer.c +4 -4

@@ -186,7 +186,7 @@ int tracer(int (*init_proc)(void *), voi
 	unsigned long eip = 0;
 	int status, pid = 0, sig = 0, cont_type, tracing = 0, op = 0;
 	int last_index, proc_id = 0, n, err, old_tracing = 0, strace = 0;
-	int pt_syscall_parm, local_using_sysemu;
+	int pt_syscall_parm, local_using_sysemu = 0;
 
 	signal(SIGPIPE, SIG_IGN);
 	setup_tracer_winch();

@@ -305,9 +305,6 @@ int tracer(int (*init_proc)(void *), voi
 			if ( tracing ) /* Assume: no syscall, when coming from user */
 				do_sigtrap(task);
 
-			local_using_sysemu = get_using_sysemu();
-			pt_syscall_parm = local_using_sysemu ? PTRACE_SYSEMU : PTRACE_SYSCALL;
-
 			switch(sig){
 			case SIGUSR1:
 				sig = 0;

@@ -385,6 +382,9 @@ int tracer(int (*init_proc)(void *), voi
 				continue;
 			}
 
+			local_using_sysemu = get_using_sysemu();
+			pt_syscall_parm = local_using_sysemu ? PTRACE_SYSEMU : PTRACE_SYSCALL;
+
 			if(tracing){
 				if(singlestepping(task))
 					cont_type = PTRACE_SINGLESTEP;

arch/um/kernel/tt/syscall_user.c +3 -6

@@ -48,7 +48,7 @@ void do_sigtrap(void *task)
 	UPT_SYSCALL_NR(TASK_REGS(task)) = -1;
 }
 
-int do_syscall(void *task, int pid, int local_using_sysemu)
+void do_syscall(void *task, int pid, int local_using_sysemu)
 {
 	unsigned long proc_regs[FRAME_SIZE];

@@ -61,14 +61,11 @@ int do_syscall(void *task, int pid, int
 	   ((unsigned long *) PT_IP(proc_regs) <= &_etext))
 		tracer_panic("I'm tracing myself and I can't get out");
 
-	if(local_using_sysemu)
-		return(1);
-
+	/* syscall number -1 in sysemu skips syscall restarting in host */
 	if(ptrace(PTRACE_POKEUSER, pid, PT_SYSCALL_NR_OFFSET,
-		  __NR_getpid) < 0)
+		  local_using_sysemu ? -1 : __NR_getpid) < 0)
 		tracer_panic("do_syscall : Nullifying syscall failed, "
 			     "errno = %d", errno);
-	return(1);
 }
 
 /*

-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Blaisorblade

2004-11-13 07:53 UTC #1 lore raw

Quoted 11 lines · Bodo Stroesser

From: Bodo Stroesser <[email protected]>
The patch needs some small corrections:
1) local_using_sysemu must be sampled *before* the next ptrace(PTRACE_SYSEMU/SYSCALL) and must stay the same until do_syscall() has been done. Currently it is sampled before do_syscall() and is used after this for ptrace(PTRACE_SYSEMU/SYSCALL). Even if no problem is visible to the UML user, a single syscall could be executed on the host when switching on sysemu. The result of this then is overwritten by the syscall execution in UML.

Quoted 2 lines

Since the first event the tracer has to handle is not a syscall, it's enough to initialize local_using_sysemu to 0;

Sorry, what happens if the first signal it gets is a SIGTRAP, and so local_using_sysemu is not yet set? If this is impossible, please add a comment in the code for this. However, it seems that it can get to the SIGTRAP case with tracing == 1. When beginning the procedure, it is 0, but it can be changed with the value from is_tracing(task). I've not checked if that is zeroed on process creation (i.e. by do_fork() calling copy_thread()), but just note that in the code.

Quoted 6 lines

2) Even if the host never *does* a syscall in SYSEMU, we have to write the syscall number with -1, to not have the host doing syscall restarting. This would happen only with an invalid syscall number equal to one of the -ERESTART values.
But to be perfect ...

Yes, but shouldn't this be handled on the host? Restarting a syscall which has never been done does not seem something that SYSEMU should allow... I don't want anybody to need going through the code and checking that this is safe.

Signature · 13 lines

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729


-------------------------------------------------------
This SF.Net email is sponsored by: InterSystems CACHE
FREE OODBMS DOWNLOAD - A multidimensional database that combines
robust object and relational technologies, making it a perfect match
for Java, C++,COM, XML, ODBC and JDBC. www.intersystems.com/match8
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Bodo Stroesser

author

2004-11-15 19:05 UTC #2 lore raw

Quoted 14 lines · Blaisorblade

Quoted 10 lines · Bodo Stroesser
From: Bodo Stroesser <[email protected]>
The patch needs some small corrections:
1) local_using_sysemu must be sampled *before* the next ptrace(PTRACE_SYSEMU/SYSCALL) and must stay the same until do_syscall() has been done. Currently it is sampled before do_syscall() and is used after this for ptrace(PTRACE_SYSEMU/SYSCALL). Even if no problem is visible to the UML user, a single syscall could be executed on the host when switching on sysemu. The result of this then is overwritten by the syscall execution in UML.

Quoted 5 lines

Quoted 2 lines
Since the first event the tracer has to handle is not a syscall, it's enough to initialize local_using_sysemu to 0;
Sorry, what happens if the first signal it gets is a SIGTRAP, and so local_using_sysemu is not yet set? If this is impossible, please add a comment in the code for this. However, it seems that it can get to the SIGTRAP case with tracing == 1. When beginning the procedure, it is 0, but it can be changed with the value from is_tracing(task). I've not checked if that is zeroed on process creation (i.e. by do_fork() calling copy_thread()), but just note that in the code.

OK: Let's summarize:
1) tracer() is started exactly once.
2) The first this it does, is starting the first ptraced-process via clone().
3) Then it waits until the new process stops.
4) Since the process will run start_kernel() in kernel space, it is resumed with PTRACE_CONT.
Thus, before having any syscall interception, the process has to stop itself with a SIGUSR1, giving the tracer an OP_TRACE_OP request. After this local_using_sysemu will be set and the process will be resumed with PTRACE_SYSCALL or PTRACE_SYSEMU.

Quoted 8 lines

Quoted 6 lines
2) Even if the host never *does* a syscall in SYSEMU, we have to write the syscall number with -1, to not have the host doing syscall restarting. This would happen only with an invalid syscall number equal to one of the -ERESTART values.

Quoted 4 lines

Quoted 1 line
But to be perfect ...
Yes, but shouldn't this be handled on the host? Restarting a syscall which has never been done does not seem something that SYSEMU should allow... I don't want anybody to need going through the code and checking that this is safe.

Yes. It should. So I exactly added this to the "advanced sysemu". But despite this UML should work on an older sysemu host, too. So I add this now, and with the SYSEMU_SINGLESTEP-patches a skip of writing -1 is inserted, if the new sysemu is in use.

-------------------------------------------------------
This SF.Net email is sponsored by: InterSystems CACHE FREE OODBMS DOWNLOAD - A multidimensional database that combines robust object and relational technologies, making it a perfect match for Java, C++,COM, XML, ODBC and JDBC. www.intersystems.com/match8
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Blaisorblade

2004-11-15 20:09 UTC #3 lore raw

Quoted 48 lines · Bodo Stroesser

Quoted 15 lines · Blaisorblade
Quoted 11 lines · Bodo Stroesser
From: Bodo Stroesser <[email protected]>
The patch needs some small corrections:
1) local_using_sysemu must be sampled *before* the next ptrace(PTRACE_SYSEMU/SYSCALL) and must stay the same until
do_syscall() has been done. Currently it is sampled before do_syscall() and is used after this for ptrace(PTRACE_SYSEMU/SYSCALL). Even if no problem is visible to the UML user, a single syscall could be executed on the host when switching on sysemu. The result of this then is overwritten by the syscall execution in UML.
Quoted 2 lines
Since the first event the tracer has to handle is not a syscall, it's enough to initialize local_using_sysemu to 0;
Quoted 7 lines
Sorry, what happens if the first signal it gets is a SIGTRAP, and so local_using_sysemu is not yet set? If this is impossible, please add a comment in the code for this. However, it seems that it can get to the SIGTRAP case with tracing == 1. When beginning the procedure, it is 0, but it can be changed with the value from is_tracing(task). I've not checked if that is zeroed on process creation (i.e. by do_fork() calling copy_thread()), but just note that in the code.
OK: Let's summarize:
1) tracer() is started exactly once.
2) The first this it does, is starting the first ptraced-process via clone(). 3) Then it waits until the new process stops.
4) Since the process will run start_kernel() in kernel space, it is resumed with PTRACE_CONT.
Thus, before having any syscall interception, the process has to stop itself with a SIGUSR1, giving the tracer an OP_TRACE_OP request. After this local_using_sysemu will be set and the process will be resumed with PTRACE_SYSCALL or PTRACE_SYSEMU.
Quoted 6 lines
Quoted 6 lines
2) Even if the host never *does* a syscall in SYSEMU, we have to write the syscall number with -1, to not have the host doing syscall restarting. This would happen only with an invalid syscall number equal to one of the -ERESTART values.
Quoted 6 lines
Quoted 1 line
But to be perfect ...
Yes, but shouldn't this be handled on the host? Restarting a syscall which has never been done does not seem something that SYSEMU should allow... I don't want anybody to need going through the code and checking that this is safe.

Quoted 4 lines

Yes. It should. So I exactly added this to the "advanced sysemu". But despite this UML should work on an older sysemu host, too. So I add this now, and with the SYSEMU_SINGLESTEP-patches a skip of writing -1 is inserted, if the new sysemu is in use.

Hmm - not yet had time to get near that (I'm busy), however if SYSEMU_SINGLESTEP fixes this API inconsistency, the old SYSEMU API will probably not be accepted by mainline developers, not it should be sent by us
- remember that we want to try merging SYSEMU sooner than SKAS4.

No single UML (except some ones using the sysemu incremental version, which we don't want to support) is unable to run without SYSEMU, so dropping that support should be ok.

Also, this new mode must wait for now - get some time to test it, I'll do the same when I've time. Currently I have too few time.

Signature · 13 lines

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729


-------------------------------------------------------
This SF.Net email is sponsored by: InterSystems CACHE
FREE OODBMS DOWNLOAD - A multidimensional database that combines
robust object and relational technologies, making it a perfect match
for Java, C++,COM, XML, ODBC and JDBC. www.intersystems.com/match8
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Bodo Stroesser

author

2004-11-16 09:19 UTC #4 lore raw

Quoted 63 lines · Blaisorblade

Quoted 56 lines · Bodo Stroesser
Quoted 18 lines · Blaisorblade
Quoted 12 lines · Bodo Stroesser
From: Bodo Stroesser <[email protected]>
The patch needs some small corrections:
1) local_using_sysemu must be sampled *before* the next ptrace(PTRACE_SYSEMU/SYSCALL) and must stay the same until
do_syscall() has been done. Currently it is sampled before do_syscall() and is used after this for ptrace(PTRACE_SYSEMU/SYSCALL). Even if no problem is visible to the UML user, a single syscall could be executed on the host when switching on sysemu. The result of this then is overwritten by the syscall execution in UML.
Quoted 2 lines
Since the first event the tracer has to handle is not a syscall, it's enough to initialize local_using_sysemu to 0;
Quoted 7 lines
Sorry, what happens if the first signal it gets is a SIGTRAP, and so local_using_sysemu is not yet set? If this is impossible, please add a comment in the code for this. However, it seems that it can get to the SIGTRAP case with tracing == 1. When beginning the procedure, it is 0, but it can be changed with the value from is_tracing(task). I've not checked if that is zeroed on process creation (i.e. by do_fork() calling copy_thread()), but just note that in the code.
OK: Let's summarize:
1) tracer() is started exactly once.
2) The first this it does, is starting the first ptraced-process via clone(). 3) Then it waits until the new process stops.
4) Since the process will run start_kernel() in kernel space, it is resumed with PTRACE_CONT.
Thus, before having any syscall interception, the process has to stop itself with a SIGUSR1, giving the tracer an OP_TRACE_OP request. After this local_using_sysemu will be set and the process will be resumed with PTRACE_SYSCALL or PTRACE_SYSEMU.
Quoted 6 lines
Quoted 6 lines
2) Even if the host never *does* a syscall in SYSEMU, we have to write the syscall number with -1, to not have the host doing syscall restarting. This would happen only with an invalid syscall number equal to one of the -ERESTART values.
Quoted 7 lines
Quoted 1 line
But to be perfect ...
Yes, but shouldn't this be handled on the host? Restarting a syscall which has never been done does not seem something that SYSEMU should allow... I don't want anybody to need going through the code and checking that this is safe.
Quoted 4 lines
Yes. It should. So I exactly added this to the "advanced sysemu". But despite this UML should work on an older sysemu host, too. So I add this now, and with the SYSEMU_SINGLESTEP-patches a skip of writing -1 is inserted, if the new sysemu is in use.

Quoted 6 lines

Hmm - not yet had time to get near that (I'm busy), however if SYSEMU_SINGLESTEP fixes this API inconsistency, the old SYSEMU API will probably not be accepted by mainline developers, not it should be sent by us
- remember that we want to try merging SYSEMU sooner than SKAS4.
No single UML (except some ones using the sysemu incremental version, which we don't want to support) is unable to run without SYSEMU, so dropping that support should be ok.
Also, this new mode must wait for now - get some time to test it, I'll do the same when I've time. Currently I have too few time.

No problem. I already tested a lot. But it should be tested *very* well, before creating a new SKAS3 version from it.
I'm planning this to be the first version that is supported by s390-host!

Blaisorblade

2004-11-26 02:37 UTC #5 lore raw

In reply to Bodo Stroesser

OK, now finally I went studying the patch.

Quoted 9 lines · Bodo Stroesser

Quoted 8 lines · Blaisorblade
Quoted 7 lines · Bodo Stroesser
From: Bodo Stroesser <[email protected]>
The patch needs some small corrections:
1) local_using_sysemu must be sampled *before* the next ptrace(PTRACE_SYSEMU/SYSCALL) and must stay the same until
do_syscall() has been done. Currently it is sampled before do_syscall() and is used after this for ptrace(PTRACE_SYSEMU/SYSCALL).

Ok, now I went analizing the code and what you say is correct. In fact, when merging this I just threw in Jeff Dike's incremental. However, to be correct, I must say that in his tree /proc/sysemu was never introduced, so his tree was correct.

Quoted 16 lines

Quoted 8 lines
Quoted 7 lines
Even if no
problem is visible to the UML user, a single syscall could be executed on the host when switching on sysemu. The result of this then is overwritten by the syscall execution in UML.
Since the first event the tracer has to handle is not a syscall, it's enough to initialize local_using_sysemu to 0;
Quoted 7 lines
Sorry, what happens if the first signal it gets is a SIGTRAP, and so local_using_sysemu is not yet set? If this is impossible, please add a comment in the code for this. However, it seems that it can get to the SIGTRAP case with tracing == 1. When beginning the procedure, it is 0, but it can be changed with the value from is_tracing(task). I've not checked if that is zeroed on process creation (i.e. by do_fork() calling copy_thread()), but just note that in the code.

Quoted 6 lines

OK: Let's summarize:
1) tracer() is started exactly once.
2) The first this it does, is starting the first ptraced-process via clone(). 3) Then it waits until the new process stops.
4) Since the process will run start_kernel() in kernel space, it is resumed with PTRACE_CONT.

And it has its is_tracing() set to 0 when it is in kernel mode.

Quoted 1 line

Thus,

(I've filled in the actual reasoning below).

Quoted 1 line

before having any syscall interception,

it needs to get is_tracing() set to 1, so

Quoted 2 lines

the process has to stop
itself with a SIGUSR1, giving the tracer an OP_TRACE_OP request.

Ok.

Quoted 3 lines

After this
local_using_sysemu will be set and the process will be resumed with PTRACE_SYSCALL or PTRACE_SYSEMU.

So, agreed. Yet, for now I'm still disabling TT SYSEMU in the -bb tree, just in case anything else comes up (it does not give a big advantage anyway).

I'm also going to merge this in 2.4-bs

Signature · 14 lines

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel