[uml-devel] [Patch 1/1] uml: fix uml-hang-on-2.6.9-host.patch

[uml-devel] [Patch 1/1] uml: fix uml-hang-on-2.6.9-host.patch

[Bodo Stroesser]

From: Bodo Stroesser <bstroesser@fujitsu-siemens.com>

The patch needs to be corrected. The use of
os_kill_ptraced_process() is restricted to the tracer-thread!
No other process is allowed to do ptrace(PTRACE_KILL).
So I had "dead" processes hanging in my host system until UML
was shutdown. And I had panic()s, because processes were not
stopped.
Thus, I implemented request_kill_ptraced_process(), which lets
the tracer do the ptrace(PTRACE_KILL). Sorry, this method is
slow, but I didn't see an other solution.
Also, I modified some other places to use the new call instead
of os_kill_process(). But this didn't work on the first step.
kill_off_processes_tt() couldn't kill the remaining processes
on shutdown. This happens, because ptrace(PTRACE_KILL) doesn't
really send a SIGKILL to the process, but writes SIGKILL to the
exit_code. That does results in send_sig(SIGKILL) for processes
being stopped on a ptrace event only, but not even for all of
these events.
So I added an option to request_kill_ptraced_process() that
allows to force an additional os_kill_process() to be executed
before the os_kill_ptraced_process().
Summary: The behavior of 2.6.9 host is very uggly. You need to
do kill() and ptrace(PTRACE_KILL) to be shure, a process will
really exit. If the host is changed to be more consistent, we
should change all this again.

Signed-off-by: Bodo Stroesser <bstroesser@fujitsu-siemens.com>
---

--- a/include/asm-um/processor-generic.h	2004-11-11 20:08:28.382687160 +0100
+++ b/include/asm-um/processor-generic.h	2004-11-11 20:09:09.167486928 +0100
@@ -54,7 +54,8 @@ struct thread_struct {
  		union {
  			struct {
  				int pid;
-			} fork, exec;
+				int mode;
+			} fork, exec, kill;
  			struct {
  				int (*proc)(void *);
  				void *arg;
--- a/arch/um/kernel/tt/include/mode-tt.h	2004-11-11 20:05:55.383946504 +0100
+++ b/arch/um/kernel/tt/include/mode-tt.h	2004-11-11 20:06:44.263515680 +0100
@@ -8,7 +8,7 @@

  #include "sysdep/ptrace.h"

-enum { OP_NONE, OP_EXEC, OP_FORK, OP_TRACE_ON, OP_REBOOT, OP_HALT, OP_CB };
+enum { OP_NONE, OP_EXEC, OP_FORK, OP_KILL, OP_TRACE_ON, OP_REBOOT, OP_HALT, OP_CB };

  extern int tracing_pid;

--- a/arch/um/kernel/tt/exec_user.c	2004-11-11 19:43:38.511182056 +0100
+++ b/arch/um/kernel/tt/exec_user.c	2004-11-11 19:50:41.288910072 +0100
@@ -16,6 +16,7 @@
  #include "kern_util.h"
  #include "user.h"
  #include "ptrace_user.h"
+#include "os.h"

  void do_exec(int old_pid, int new_pid)
  {
@@ -36,7 +37,7 @@ void do_exec(int old_pid, int new_pid)
  		tracer_panic("do_exec failed to get registers - errno = %d",
  			     errno);

-	kill(old_pid, SIGKILL);
+	os_kill_ptraced_process(old_pid, 0);

  	if (ptrace(PTRACE_SETOPTIONS, new_pid, 0, (void *)PTRACE_O_TRACESYSGOOD) < 0)
  		tracer_panic("do_exec: PTRACE_SETOPTIONS failed, errno = %d", errno);
--- a/arch/um/kernel/tt/process_kern.c	2004-11-11 20:03:15.852199016 +0100
+++ b/arch/um/kernel/tt/process_kern.c	2004-11-11 21:03:59.624261464 +0100
@@ -82,7 +82,7 @@ void *switch_to_tt(void *prev, void *nex
  	prev_sched = current->thread.prev_sched;
  	if((prev_sched->state == TASK_ZOMBIE) ||
  	   (prev_sched->state == TASK_DEAD))
-		os_kill_ptraced_process(prev_sched->thread.mode.tt.extern_pid, 1);
+		request_kill_ptraced_process( prev_sched->thread.mode.tt.extern_pid, 1);

  	/* This works around a nasty race with 'jail'.  If we are switching
  	 * between two threads of a threaded app and the incoming process
@@ -119,7 +119,7 @@ void release_thread_tt(struct task_struc
  	int pid = task->thread.mode.tt.extern_pid;

  	if(os_getpid() != pid)
-		os_kill_process(pid, 0);
+		request_kill_ptraced_process(pid, 0);
  }

  void exit_thread_tt(void)
@@ -330,10 +330,10 @@ void kill_off_processes_tt(void)
  	me = os_getpid();
          for_each_process(p){
  		if(p->thread.mode.tt.extern_pid != me)
-			os_kill_process(p->thread.mode.tt.extern_pid, 0);
+			request_kill_ptraced_process(p->thread.mode.tt.extern_pid, 2);
  	}
  	if(init_task.thread.mode.tt.extern_pid != me)
-		os_kill_process(init_task.thread.mode.tt.extern_pid, 0);
+		request_kill_ptraced_process(init_task.thread.mode.tt.extern_pid, 2);
  }

  void initial_thread_cb_tt(void (*proc)(void *), void *arg)
@@ -352,6 +352,20 @@ void initial_thread_cb_tt(void (*proc)(v
  	}
  }

+void request_kill_ptraced_process(int pid, int mode)
+{
+	/* The tracer has to do the kill, since killing must be
+	 * done with ptrace(PTRACE_KILL, pid), which is possible
+	 * from the father only! */
+	current->thread.request.op = OP_KILL;
+	current->thread.request.u.exec.pid = pid;
+	current->thread.request.u.exec.mode = mode;
+	os_usr1_process(os_getpid());
+	change_sig(SIGUSR1, 1);
+
+	change_sig(SIGUSR1, 0);
+}
+
  int do_proc_op(void *t, int proc_id)
  {
  	struct task_struct *task;
@@ -374,6 +388,12 @@ int do_proc_op(void *t, int proc_id)
  	case OP_FORK:
  		attach_process(thread->request.u.fork.pid);
  		break;
+	case OP_KILL:
+		if ( thread->request.u.kill.mode > 1 )
+			os_kill_process(thread->request.u.kill.pid, 0);
+		os_kill_ptraced_process(thread->request.u.kill.pid,
+		                        thread->request.u.kill.mode);
+		break;
  	case OP_CB:
  		(*thread->request.u.cb.proc)(thread->request.u.cb.arg);
  		break;
--- a/arch/um/kernel/tt/include/mode-tt.h	2004-11-12 12:14:24.239057776 +0100
+++ b/arch/um/kernel/tt/include/mode-tt.h	2004-11-12 12:15:17.496961344 +0100
@@ -20,6 +20,7 @@ extern void reboot_tt(void);
  extern void halt_tt(void);
  extern int is_tracer_winch(int pid, int fd, void *data);
  extern void kill_off_processes_tt(void);
+extern void request_kill_ptraced_process(int pid, int mode);

  #endif

--- a/arch/um/kernel/reboot.c	2004-11-12 12:18:29.356794216 +0100
+++ b/arch/um/kernel/reboot.c	2004-11-12 12:19:20.996943720 +0100
@@ -22,7 +22,7 @@ static void kill_idlers(int me)
  	for(i = 0; i < sizeof(idle_threads)/sizeof(idle_threads[0]); i++){
  		p = idle_threads[i];
  		if((p != NULL) && (p->thread.mode.tt.extern_pid != me))
-			os_kill_process(p->thread.mode.tt.extern_pid, 0);
+			request_kill_ptraced_process(p->thread.mode.tt.extern_pid, 2);
  	}
  #endif
  }


-------------------------------------------------------
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
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [Patch 1/1] uml: fix uml-hang-on-2.6.9-host.patch

[Blaisorblade]

On Friday 12 November 2004 15:46, Bodo Stroesser wrote:
> From: Bodo Stroesser <bstroesser@fujitsu-siemens.com>
>
> The patch needs to be corrected. The use of
> os_kill_ptraced_process() is restricted to the tracer-thread!
> No other process is allowed to do ptrace(PTRACE_KILL).
> So I had "dead" processes hanging in my host system until UML
> was shutdown. And I had panic()s, because processes were not
> stopped.

> Thus, I implemented request_kill_ptraced_process(), which lets
> the tracer do the ptrace(PTRACE_KILL). Sorry, this method is
> slow, but I didn't see an other solution.
Hmm, given how slow TT mode is, I think it may not be noticeable. Especially 
since the signal handling is asynchronous, the exited thread gets "scheduled 
to be killed when the signal is delivered". However, also look at Jeff Dike 
patch he's pushing for testing.

A doubt: IIRC, the queue keeps only one instance of each signal, right? Should 
not we switch all uses of SIGUSR* to real-time signals (which could also 
reduce latency, maybe)?

> Also, I modified some other places to use the new call instead
> of os_kill_process(). But this didn't work on the first step.
> kill_off_processes_tt() couldn't kill the remaining processes
> on shutdown. This happens, because ptrace(PTRACE_KILL) doesn't
> really send a SIGKILL to the process, but writes SIGKILL to the
> exit_code.

> That does results in send_sig(SIGKILL) for processes 
> being stopped on a ptrace event only, but not even for all of
> these events.

> So I added an option to request_kill_ptraced_process() that
> allows to force an additional os_kill_process() to be executed
> before the os_kill_ptraced_process().

About this: it's better to use symbolic constants for such usages, never to 
use numbers. And if you meet code snippets in UML were this is not applied, 
feel free to fix them.

> Summary: The behavior of 2.6.9 host is very uggly. You need to
> do kill() and ptrace(PTRACE_KILL) to be shure, a process will
> really exit.
Hmm.... could you supply a testcase for this?

No, I've modified the Gerd Knorr testcase, replacing kill(SIGKILL) with 
ptrace(PTRACE_KILL), and the result was that PTRACE_KILL alone works, in the 
simplest scenario at least.

> If the host is changed to be more consistent, we 
> should change all this again.

PTRACE_KILL should always work, on every kernel. If not, then we must fix that 
in mainline, and also 

PTRACE_KILL not working would be a real bug, but I would rather check if the 
childs are stopped, and I would happily ask Roland McGrath to fix it and I'd 
include the fix in the SKAS patch (I'm worried it's getting too big, but so 
far, so good). We are facing, instead, a correct API change.

The problem is that we used SIGKILL, which should never have worked. I first 
thought Roland McGrath didn't realize this fact. Now I understand, instead, 
that the kernel should always disallow a ptraced process to be killed by 
someone which is not his parent, when it's stopped (when you reboot, you 
killall everything).

So, the behaviour needed by older UMLs will never be restored - I thought this 
was possible until now, but no more.
-- 
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
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel