Control: tag -1 patch On Mon, Jun 24, 2013 at 03:30:22PM +0100, Colin Watson wrote: > For some time I've noticed that, when an Ubuntu build times out (150 > minutes with no output), sbuild tries to terminate it, and I see a > "Session terminated, terminating shell... ...terminated." message in the > log (which is from su), but the build does not actually terminate > properly. Now, in both Debian and Ubuntu, sbuild invokes builds using > something like this simplified command: > > sudo chroot $chroot su $username -s sh -c "cd $dir && exec > dpkg-buildpackage" > > When su receives a signal, it passes it on to its child process (it has > to go to unusual lengths here because it starts new sessions). However, > it only kills its immediate child, not the associated process group. [...] > Could su please kill the process group associated with its immediate > child process instead? This should just be a matter of negating the pid > passed to kill. If it did that, then I think it would do a much better > job of cleaning up after itself.
I think that this started to become a problem in 4.1.5 when su stopped letting its child process have a controlling terminal, so the whole child process group doesn't automatically get the signal. My proposed approach still seems reasonable. Here's a suggested patch. Index: debian/changelog =================================================================== --- debian/changelog (revision 3748) +++ debian/changelog (working copy) @@ -1,3 +1,12 @@ +shadow (1:4.1.5.1-1.1) UNRELEASED; urgency=low + + * debian/patches/496_su_kill_process_group: Kill the child process group, + rather than just the immediate child; this is needed now that su no + longer starts a controlling terminal when not running an interactive + shell (closes: #713979). + + -- Colin Watson <[email protected]> Fri, 26 Jul 2013 16:55:52 +0100 + shadow (1:4.1.5.1-1) unstable; urgency=low * The "Gruyère" release. Index: debian/patches/496_su_kill_process_group =================================================================== --- debian/patches/496_su_kill_process_group (revision 0) +++ debian/patches/496_su_kill_process_group (working copy) @@ -0,0 +1,29 @@ +Description: su: Kill the child process group, not just the immediate child + This is needed now that su no longer starts a controlling terminal when not + running an interactive shell. +Author: Colin Watson <[email protected]> +Forwarded: no +Last-Update: 2013-07-26 + +Index: b/src/su.c +=================================================================== +--- a/src/su.c ++++ b/src/su.c +@@ -194,7 +194,7 @@ + static RETSIGTYPE kill_child (int unused(s)) + { + if (0 != pid_child) { +- (void) kill (pid_child, SIGKILL); ++ (void) kill (-pid_child, SIGKILL); + (void) fputs (_(" ...killed.\n"), stderr); + } else { + (void) fputs (_(" ...waiting for child to terminate.\n"), +@@ -383,7 +383,7 @@ + (void) fputs ("\n", stderr); + (void) fputs (_("Session terminated, terminating shell..."), + stderr); +- (void) kill (pid_child, caught); ++ (void) kill (-pid_child, caught); + } + + ret = pam_close_session (pamh, 0); Index: debian/patches/series =================================================================== --- debian/patches/series (revision 3748) +++ debian/patches/series (working copy) @@ -16,3 +16,4 @@ 523_su_arguments_are_no_more_concatenated_by_default 508_nologin_in_usr_sbin 505_useradd_recommend_adduser +496_su_kill_process_group Thanks, -- Colin Watson [[email protected]] -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]
