[uml-devel] [PATCH] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes

[uml-devel] [PATCH] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes

[Chris Wedgwood]

kill(..., SIGKILL) doesn't work to kill host-OS processes created in
the exec path in TT mode --- for this we need PTRACE_KILL (it did work
in previous kernels, but not by design).  Without this process will
accumulate on the host-OS (although the won't be visible inside UML).

Signed-off-by: Chris Wedgwood <cw@f00f.org>
---

Yes, there are other fixes along these lines which are needed but one
at a time as we test these...

Index: cw-current/arch/um/kernel/tt/exec_user.c
===================================================================
--- cw-current.orig/arch/um/kernel/tt/exec_user.c	2004-11-03 02:10:18.064830204 -0800
+++ cw-current/arch/um/kernel/tt/exec_user.c	2004-11-03 02:12:10.447716745 -0800
@@ -35,7 +35,8 @@
 		tracer_panic("do_exec failed to get registers - errno = %d",
 			     errno);
 
-	kill(old_pid, SIGKILL);
+	if (ptrace(PTRACE_KILL, old_pid, NULL, NULL))
+		printk("Warning: ptrace(PTRACE_KILL, %d, ...) saw %d\n", errno);
 
 	if(ptrace_setregs(new_pid, regs) < 0)
 		tracer_panic("do_exec failed to start new proc - errno = %d",


-------------------------------------------------------
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

[uml-devel] Re: [PATCH] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes

[Anton Altaparmakov]

On Wed, 2004-11-03 at 11:37, Chris Wedgwood wrote:
> kill(..., SIGKILL) doesn't work to kill host-OS processes created in
> the exec path in TT mode --- for this we need PTRACE_KILL (it did work
> in previous kernels, but not by design).  Without this process will
> accumulate on the host-OS (although the won't be visible inside UML).
> 
> Signed-off-by: Chris Wedgwood <cw@f00f.org>
> ---
> 
> Yes, there are other fixes along these lines which are needed but one
> at a time as we test these...
> 
> Index: cw-current/arch/um/kernel/tt/exec_user.c
> ===================================================================
> --- cw-current.orig/arch/um/kernel/tt/exec_user.c	2004-11-03 02:10:18.064830204 -0800
> +++ cw-current/arch/um/kernel/tt/exec_user.c	2004-11-03 02:12:10.447716745 -0800
> @@ -35,7 +35,8 @@
>  		tracer_panic("do_exec failed to get registers - errno = %d",
>  			     errno);
>  
> -	kill(old_pid, SIGKILL);
> +	if (ptrace(PTRACE_KILL, old_pid, NULL, NULL))
> +		printk("Warning: ptrace(PTRACE_KILL, %d, ...) saw %d\n", errno);

You have two %d but only one argument.  You seem to have forgotten an
"old_pid, " in there.

>  
>  	if(ptrace_setregs(new_pid, regs) < 0)
>  		tracer_panic("do_exec failed to start new proc - errno = %d",

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/, http://www-stu.christs.cam.ac.uk/~aia21/



-------------------------------------------------------
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

[uml-devel] [PATCH] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes (take #2)

[Chris Wedgwood]

On Wed, Nov 03, 2004 at 11:47:38AM +0000, Anton Altaparmakov wrote:

> You have two %d but only one argument.  You seem to have forgotten
> an "old_pid, " in there.

doh!  it's a warning if something iffy happens (which so far for me it
hasn't) which explains why i missed it...  not sure why i didn't get a
build warning though...  thanks!

---
kill(..., SIGKILL) doesn't work to kill host-OS processes created in
the exec path in TT mode --- for this we need PTRACE_KILL (it did work
in previous kernels, but not by design).  Without this process will
accumulate on the host-OS (although the won't be visible inside UML).

Signed-off-by: Chris Wedgwood <cw@f00f.org>
---

Index: cw-current/arch/um/kernel/tt/exec_user.c
===================================================================
--- cw-current.orig/arch/um/kernel/tt/exec_user.c	2004-11-03 02:10:18.064830204 -0800
+++ cw-current/arch/um/kernel/tt/exec_user.c	2004-11-03 04:05:00.435843464 -0800
@@ -35,7 +35,8 @@
 		tracer_panic("do_exec failed to get registers - errno = %d",
 			     errno);
 
-	kill(old_pid, SIGKILL);
+	if (ptrace(PTRACE_KILL, old_pid, NULL, NULL))
+		printk("Warning: ptrace(PTRACE_KILL, %d, ...) saw %d\n", old_pid, errno);
 
 	if(ptrace_setregs(new_pid, regs) < 0)
 		tracer_panic("do_exec failed to start new proc - errno = %d",


-------------------------------------------------------
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] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes (take #2)

[Blaisorblade]

On Wednesday 03 November 2004 13:08, Chris Wedgwood wrote:
> On Wed, Nov 03, 2004 at 11:47:38AM +0000, Anton Altaparmakov wrote:
> > You have two %d but only one argument.  You seem to have forgotten
> > an "old_pid, " in there.

> kill(..., SIGKILL) doesn't work to kill host-OS processes created in
> the exec path in TT mode --- for this we need PTRACE_KILL (it did work
> in previous kernels, but not by design).  Without this process will
> accumulate on the host-OS (although the won't be visible inside UML).

> Signed-off-by: Chris Wedgwood <cw@f00f.org>
> ---
>
> Index: cw-current/arch/um/kernel/tt/exec_user.c
> ===================================================================
> --- cw-current.orig/arch/um/kernel/tt/exec_user.c 2004-11-03
> 02:10:18.064830204 -0800 +++
> cw-current/arch/um/kernel/tt/exec_user.c 2004-11-03 04:05:00.435843464
> -0800 @@ -35,7 +35,8 @@
>    tracer_panic("do_exec failed to get registers - errno = %d",
>          errno);
>
> - kill(old_pid, SIGKILL);
> + if (ptrace(PTRACE_KILL, old_pid, NULL, NULL))
> +  printk("Warning: ptrace(PTRACE_KILL, %d, ...) saw %d\n", old_pid,
> errno);
>
>   if(ptrace_setregs(new_pid, regs) < 0)
>    tracer_panic("do_exec failed to start new proc - errno = %d",

I'm going to test this. I thought that Gerd Knorr patch (which I sent cc'ing 
LKML and most of you) already solved this (I actually modified that one, 
replacing his SIGCONT kill()ing with a PTRACE_KILL, but I did this in the 
places he identified). I guess that old_pid should either already be dead 
there or going to die after a little, but I'm going to check (after I get UML 
to run in the current snapshot...)

For now, please hold on this.
-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729



-------------------------------------------------------
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] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes (take #2)

[Chris Wedgwood]

On Wed, Nov 03, 2004 at 08:28:44PM +0100, Blaisorblade wrote:

> I'm going to test this.

please do

> I thought that Gerd Knorr patch (which I sent cc'ing LKML and most
> of you) already solved this (I actually modified that one, replacing
> his SIGCONT kill()ing with a PTRACE_KILL, but I did this in the
> places he identified).

it might, i'm going to check soon

what worries me is that two very different code paths might be fixing
the same problem which makes me think the flow of execution here is
very vague and needs cleaninng up

also, check your return values for errors --- i bet you will see
some.  os_kill_process has this problem too --- many invocations of it
are pointless and fail, especially those from relase_thread_tt (i need
to check the details here, this is all from memory and im getting old)

> I guess that old_pid should either already be dead there or going to
> die after a little, but I'm going to check (after I get UML to run
> in the current snapshot...)

it should build pretty close to as-is, if not let me know and i'll
sent you what i have


-------------------------------------------------------
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] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes (take #2)

[Blaisorblade]

On Wednesday 03 November 2004 21:09, Chris Wedgwood wrote:
> On Wed, Nov 03, 2004 at 08:28:44PM +0100, Blaisorblade wrote:
> > I'm going to test this.
>
> please do
The Gerd / mine patch seem to work nicely. No thread is left over.

> > I thought that Gerd Knorr patch (which I sent cc'ing LKML and most
> > of you) already solved this (I actually modified that one, replacing
> > his SIGCONT kill()ing with a PTRACE_KILL, but I did this in the
> > places he identified).

> it might, i'm going to check soon

> what worries me is that two very different code paths might be fixing
> the same problem which makes me think the flow of execution here is
> very vague and needs cleaninng up

Yes, but the cleanup should first be revierwed by Jeff Dike and put in his 
tree. I too don't understand the code. Don't CC Andrew on the first release 
of the cleanup at least.

To go to the actual fact, the code path I'm fixing didn't exist till a few 
releases ago.

Follows is the patch containing this fix against 2.4.27, with the changelog, 
from Jeff Dike.

This fixes a use-after-free bug in the context switching.  A process going out
of context after exiting wakes up the next process and then kills itself.  The
problem is that when it gets around to killing itself is up to the host and
can happen a long time later, including after the incoming process has freed
its stack, and that memory is possibly being used for something else.

The fix is to have the incoming process kill the exiting process just to
make sure it can't be running at the point that its stack is freed.

 um-linux-2.4.27-paolo/arch/um/kernel/tt/process_kern.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletion(-)

diff -puN arch/um/kernel/tt/process_kern.c~scheduler 
arch/um/kernel/tt/process_kern.c
--- um-linux-2.4.27/arch/um/kernel/tt/process_kern.c~scheduler  2004-11-02 
11:12:28.805815264 +0100
+++ um-linux-2.4.27-paolo/arch/um/kernel/tt/process_kern.c      2004-11-02 
11:12:28.807814960 +0100
@@ -25,7 +25,7 @@

 void *_switch_to_tt(void *prev, void *next)
 {
-       struct task_struct *from, *to;
+       struct task_struct *from, *to, *prev_sched;
        unsigned long flags;
        int err, vtalrm, alrm, prof, cpu;
        char c;
@@ -67,6 +67,17 @@ void *_switch_to_tt(void *prev, void *ne
        if(err != sizeof(c))
                panic("read of switch_pipe failed, errno = %d", -err);

+       /* If the process that we have just scheduled away from has exited,
+        * then it needs to be killed here.  The reason is that, even though
+        * it will kill itself when it next runs, that may be too late.  Its
+        * stack will be freed, possibly before then, and if that happens,
+        * we have a use-after-free situation.  So, it gets killed here
+        * in case it has not already killed itself.
+        */
+       prev_sched = current->thread.prev_sched;
+       if(prev_sched->state == TASK_ZOMBIE)
+               os_kill_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
         * runs before the outgoing process reaches the read, and it makes

> also, check your return values for errors --- i bet you will see
> some. 

That is a separate, on-top of this patch.

> os_kill_process has this problem too --- many invocations of it 
> are pointless and fail, especially those from relase_thread_tt (i need
> to check the details here, this is all from memory and im getting old)

> > I guess that old_pid should either already be dead there or going to
> > die after a little, but I'm going to check (after I get UML to run
> > in the current snapshot...)

> it should build pretty close to as-is, if not let me know and i'll
> sent you what i have
No problem actually in UML, the build environment instead is very complicate. 
It refuses to work with NPTL, so I must build it in my old Mandrake chroot 
(it was a distro, now I keep it just for compilation).

Now, I must also run it on the Gentoo, or it gives problems... however, don't 
worry.

I'm also going to merge some patches (21 it should be).
Bye
-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729


-------------------------------------------------------
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] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes (take #2)

[Gerd Knorr]

> I'm going to test this. I thought that Gerd Knorr patch (which I sent cc'ing 
> LKML and most of you) already solved this (I actually modified that one, 

Not sure whenever tt is fixed with my patch, I've tested skas only (I'm
building skas-only dynamically linked kernels these days because due to
working on x11 framebuffer stuff which needs dynamically linked libX11).
So if Chris actually tested TT then his patch probably is ok and needed
as well ...

  Gerd

-- 
#define printk(args...) fprintf(stderr, ## args)


-------------------------------------------------------
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] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes (take #2)

[Chris Wedgwood]

On Wed, Nov 03, 2004 at 09:18:36PM +0100, Gerd Knorr wrote:

> Not sure whenever tt is fixed with my patch, I've tested skas only
> (I'm building skas-only dynamically linked kernels these days
> because due to working on x11 framebuffer stuff which needs
> dynamically linked libX11).

it would be nice to find a way to make this work TT mode for you

> So if Chris actually tested TT then his patch probably is ok and
> needed as well ...

i'm only using TT mode at present, i don't check esoteric modes that
require host-OS patches at present


-------------------------------------------------------
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] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes (take #2)

[Blaisorblade]

On Wednesday 03 November 2004 21:48, Chris Wedgwood wrote:
> On Wed, Nov 03, 2004 at 09:18:36PM +0100, Gerd Knorr wrote:
> > Not sure whenever tt is fixed with my patch, I've tested skas only
> > (I'm building skas-only dynamically linked kernels these days
> > because due to working on x11 framebuffer stuff which needs
> > dynamically linked libX11).

> it would be nice to find a way to make this work TT mode for you

> > So if Chris actually tested TT then his patch probably is ok and
> > needed as well ...

> i'm only using TT mode at present, i don't check esoteric modes that
> require host-OS patches at present
SKAS mode is currently more used and tested than TT mode. And the SKAS patch 
is included at least in WOLK, the SuSE kernel, and the -cko patchset (Con 
Kolivas Overloaded).

That said, feel free not to use it, but at least test that it compiles in SKAS 
mode.

Bye
-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729


-------------------------------------------------------
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] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes (take #2)

[Blaisorblade]

On Wednesday 03 November 2004 21:18, Gerd Knorr wrote:
> > I'm going to test this. I thought that Gerd Knorr patch (which I sent
> > cc'ing LKML and most of you) already solved this (I actually modified
> > that one,

> Not sure whenever tt is fixed with my patch, I've tested skas only

I've tested it, and it works in fact.

> (I'm 
> building skas-only dynamically linked kernels these days because due to
> working on x11 framebuffer stuff which needs dynamically linked libX11).
> So if Chris actually tested TT then his patch probably is ok and needed
> as well ...

Actually, from looking at Jeff Dike comment, it seem that the new kill (the 
one you've changed) is executed earlier and more reliably than the 1st one, 
the one Chris identified.

So we could ask Jeff to see if he can remove the earlier kill, the one 
affected by the Chris Wedgwood.


-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729


-------------------------------------------------------
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] UML: Use PTRACE_KILL instead of SIGKILL to kill host-OS processes

[Bodo Stroesser]

[-- Attachment #1: Type: text/plain, Size: 3491 bytes --]

Chris Wedgwood wrote:
> kill(..., SIGKILL) doesn't work to kill host-OS processes created in
> the exec path in TT mode --- for this we need PTRACE_KILL (it did work
> in previous kernels, but not by design).  Without this process will
> accumulate on the host-OS (although the won't be visible inside UML).
> 
> Signed-off-by: Chris Wedgwood <cw@f00f.org>
> ---
> 
> Yes, there are other fixes along these lines which are needed but one
> at a time as we test these...
> 
> Index: cw-current/arch/um/kernel/tt/exec_user.c
> ===================================================================
> --- cw-current.orig/arch/um/kernel/tt/exec_user.c	2004-11-03 02:10:18.064830204 -0800
> +++ cw-current/arch/um/kernel/tt/exec_user.c	2004-11-03 02:12:10.447716745 -0800
> @@ -35,7 +35,8 @@
>  		tracer_panic("do_exec failed to get registers - errno = %d",
>  			     errno);
>  
> -	kill(old_pid, SIGKILL);
> +	if (ptrace(PTRACE_KILL, old_pid, NULL, NULL))
> +		printk("Warning: ptrace(PTRACE_KILL, %d, ...) saw %d\n", errno);
>  
>  	if(ptrace_setregs(new_pid, regs) < 0)
>  		tracer_panic("do_exec failed to start new proc - errno = %d",
> 
> 
> -------------------------------------------------------
> 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
Now, there are some patches out there, to fix the problem:
- this one here
- uml-hang-on-2.6.9-host from BlaisorBlade
- patch-fix-uml-hang-on-2.6.9-host to combine the two and add some new
   stuff

After a discussion on #uml yesterday, I tried to find out, which parts of the
patches are really neccessary. Now here are the results:

1) Chris' patch is neccessary to avoid the amassing threads in tt-mode.
2) All parts from BlaisorBlade's patch regarding SKAS are needed to not have
    SKAS hanging on shutdown. I used the os_kill_ptraced_process() inserted here
    for Chris' part.
3) The changes done in switch_to_tt() from BlaisorBlades patch are not needed.
    Thus, my rework of this isn't needed, too.
4) In release_thread_tt() my change has to inserted, else UML doesn't boot on
    my system.
5) The changes, I did for kill_of_processes_tt and kill_idlers are not needed.
6) I inserted some small changes in kill_of_processes_tt and kill_idlers to
    have the killing process waiting for the killed one to exit. Not really needed,
    but cleaner.
7) I saw some zombies not being reaped immediately before the exec() in case of
    reboot. Thus, I added a small waitpid-loop in tracer(). Also, not needed,
    but cleaner.

The resulting new patch is attached in a tar-ball. It applies to 2.6.9-bb2, but
you have to remove the patch "uml-hang-on-2.6.9-host and add my small series of
patches, which also is in the tar-ball. The fix some small problems, let SKAS
reboot without problems and add the new SYSEMU_SINGLESTEP-mode (but runs on
older hosts also).
Series:
    patch-bb1-fix-1
    patch-fix-elf_aux
    patch-fix-sysemu-tt
    patch-fix-reboot-skas
    patch-close-all-fds which is the unnamed patch from Jeff, adapted to 2.6.9-bb2
    patch-SYSEMU_SINGLESTEP-1
    patch-SYSEMU_SINGLESTEP-2
    patch-rework-uml-hang-on-2.6.9-host

Regards Bodo

[-- Attachment #2: bb2-addon-bodo.tar.bz2 --]
[-- Type: application/x-bzip2, Size: 7062 bytes --]