From: Bodo S. <bst...@fu...> - 2004-11-12 14:10:15
|
From: Bodo Stroesser <bst...@fu...> 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 <bst...@fu...> --- --- a/arch/um/kernel/tt/include/tt.h 2004-11-11 21:27:08.405134680 +0100 +++ b/arch/um/kernel/tt/include/tt.h 2004-11-11 21:27:23.807793120 +0100 @@ -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); --- a/arch/um/kernel/tt/tracer.c 2004-11-12 10:28:46.776498528 +0100 +++ a/arch/um/kernel/tt/tracer.c 2004-11-12 10:34:41.381590360 +0100 @@ -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; --- a/arch/um/kernel/tt/syscall_user.c 2004-11-12 10:30:32.181474536 +0100 +++ b/arch/um/kernel/tt/syscall_user.c 2004-11-12 10:41:04.146401264 +0100 @@ -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); } /* |
From: Blaisorblade <bla...@ya...> - 2004-11-13 07:53:35
|
On Friday 12 November 2004 15:10, Bodo Stroesser wrote: > From: Bodo Stroesser <bst...@fu...> > > 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; 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. > 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. -- Paolo Giarrusso, aka Blaisorblade Linux registered user n. 292729 |
From: Bodo S. <bst...@fu...> - 2004-11-15 19:05:09
|
Blaisorblade wrote: > On Friday 12 November 2004 15:10, Bodo Stroesser wrote: > >>From: Bodo Stroesser <bst...@fu...> >> >>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; > > > 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. > > >>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. 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. |
From: Blaisorblade <bla...@ya...> - 2004-11-15 20:09:10
|
On Monday 15 November 2004 20:04, Bodo Stroesser wrote: > Blaisorblade wrote: > > On Friday 12 November 2004 15:10, Bodo Stroesser wrote: > >>From: Bodo Stroesser <bst...@fu...> > >> > >>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; > > 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. > >>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. > 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. -- Paolo Giarrusso, aka Blaisorblade Linux registered user n. 292729 |
From: Bodo S. <bst...@fu...> - 2004-11-16 09:19:15
|
Blaisorblade wrote: > On Monday 15 November 2004 20:04, Bodo Stroesser wrote: > >>Blaisorblade wrote: >> >>>On Friday 12 November 2004 15:10, Bodo Stroesser wrote: >>> >>>>From: Bodo Stroesser <bst...@fu...> >>>> >>>>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; > > >>>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. > > >>>>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. > > >>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. 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! |
From: Blaisorblade <bla...@ya...> - 2004-11-26 02:37:07
|
OK, now finally I went studying the patch. On Monday 15 November 2004 20:04, Bodo Stroesser wrote: > Blaisorblade wrote: > > On Friday 12 November 2004 15:10, Bodo Stroesser wrote: > >>From: Bodo Stroesser <bst...@fu...> > >> > >>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. > >> 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; > > > > 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. And it has its is_tracing() set to 0 when it is in kernel mode. > Thus, (I've filled in the actual reasoning below). > before having any syscall interception, it needs to get is_tracing() set to 1, so > the process has to stop > itself with a SIGUSR1, giving the tracer an OP_TRACE_OP request. Ok. > 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 -- Paolo Giarrusso, aka Blaisorblade Linux registered user n. 292729 http://www.user-mode-linux.org/~blaisorblade |