tags 391319 + fixed-upstream pending thanks Kees Cook <[EMAIL PROTECTED]> writes:
> On Wed, May 23, 2007 at 08:38:15PM +0100, Roger Leigh wrote: > This is already true. If no proc items are found to kill, the function > immediately exits. Great. >> I think the while loop needs some optimisation, such as checking if >> the process has terminated so that it can break out of the loop early. >> I think simply repeating the readlink inside the loop would be >> sufficient. So instead of "while kill", so "while readlink", and then >> kill inside the loop. > > I was trading off between a 100% CPU spin on readlink vs 1 second > granularity for each process that fails to immediately die. As it is > written, if the process dies immediately, the "kill -0" will fail and no > waiting will be done. > >> Another nice addition would be to print out which processes are being >> killed if in verbose mode. > > Is this valuable? By the time the user sees the warning, the process is > already dead. Perhaps both the pid and the exe name? > > Attached is an improved patch. This is fantastic. I have separated it from 10mount, as 15killprocs (it will always get run before 10mount on shutdown). This is because you might want to run it even if there are no filesystems to unmount. I have tweaked it slightly in a few places, but it's otherwise the same as your patch. This is mainly using /proc/pid/root as well as /proc/pid/exe (saves a grep invocation), and adding an additional log message. I also used signal names rather than numbers with kill (for portability). I also took the liberty of adding you to the AUTHORS file for this contribution--I hope that's acceptable to you? I have attached the patch I have just committed to SVN. Is this OK, or would you like any further changes making? Thanks, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
Index: debian/changelog
===================================================================
--- debian/changelog (revision 1192)
+++ debian/changelog (working copy)
@@ -2,8 +2,11 @@
* New upstream development release.
* debian/control: Change section of libsbuild-doc to "doc".
+ * Processes running in the chroot on stopping a session are now killed
+ by the 15killprocs setup script (Closes: #391319). Many thanks to
+ Kees Cook for implementing this.
- -- Roger Leigh <[EMAIL PROTECTED]> Sun, 20 May 2007 20:04:37 +0100
+ -- Roger Leigh <[EMAIL PROTECTED]> Mon, 28 May 2007 13:06:05 +0100
schroot (1.1.3-1) unstable; urgency=low
Index: AUTHORS
===================================================================
--- AUTHORS (revision 1192)
+++ AUTHORS (working copy)
@@ -4,6 +4,9 @@
Andreas Bombe <[EMAIL PROTECTED]>
Documentation improvements.
+Kees Cook <[EMAIL PROTECTED]>
+ 15killprocs script to kill processes.
+
Federico Di Gregorio <[EMAIL PROTECTED]>
Init script improvements.
Index: ChangeLog
===================================================================
--- ChangeLog (revision 1192)
+++ ChangeLog (working copy)
@@ -1,3 +1,16 @@
+2007-05-28 Roger Leigh <[EMAIL PROTECTED]>
+
+ * debian/changelog: Close #391319.
+
+ * AUTHORS: Add Kees Cook.
+
+ * bin/schroot/setup/15killprocs: New file. Kill processes in the
+ chroot before unmounting any filesystems. Many thanks to Kees
+ Cook for implementing this.
+
+ * bin/schroot/setup/Makefile.am
+ (setup_SCRIPTS): Add 15killprocs
+
2007-05-22 Roger Leigh <[EMAIL PROTECTED]>
* bin/schroot/setup/10mount
Index: bin/schroot/setup/15killprocs
===================================================================
--- bin/schroot/setup/15killprocs (revision 0)
+++ bin/schroot/setup/15killprocs (revision 0)
@@ -0,0 +1,64 @@
+#!/bin/sh
+# Copyright © 2007 Kees Cook <[EMAIL PROTECTED]>
+# Copyright © 2007 Roger Leigh <[EMAIL PROTECTED]>
+#
+# schroot is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# schroot is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+
+set -e
+
+# Kill all processes that were run from within the chroot environment
+# $1: mount base location
+do_kill_all()
+{
+ if [ "$AUTH_VERBOSITY" = "verbose" ]; then
+ echo "Killing processes run inside $1"
+ fi
+ ls /proc | egrep '^[[:digit:]]+$' |
+ while read pid; do
+ exe=$(readlink /proc/"$pid"/exe || true)
+ root=$(readlink /proc/"$pid"/root || true)
+ if [ "$root" = "$1" ]; then
+ if [ "$AUTH_VERBOSITY" = "verbose" ]; then
+ echo "Killing left-over pid $pid (${exe##$1})"
+ echo " Sending SIGTERM to pid $pid"
+ fi
+ kill -TERM "$pid" 2>/dev/null
+
+ count=0
+ max=5
+ while [ -d /proc/"$pid" ]; do
+ count=$(( $count + 1 ))
+ if [ "$AUTH_VERBOSITY" = "verbose" ]; then
+ echo " Waiting for pid $pid to shut down... ($count/$max)"
+ fi
+ sleep 1
+ # Wait for $max seconds for process to die before -9'ing it
+ if [ "$count" -eq "$max" ]; then
+ if [ "$AUTH_VERBOSITY" = "verbose" ]; then
+ echo " Sending SIGKILL to pid $pid"
+ fi
+ kill -KILL "$pid" 2>/dev/null
+ sleep 1
+ break
+ fi
+ done
+ fi
+ done
+}
+
+if [ $1 = "setup-recover" ] || [ $1 = "setup-stop" ]; then
+ do_kill_all "$CHROOT_MOUNT_LOCATION"
+fi
\ No newline at end of file
Property changes on: bin/schroot/setup/15killprocs
___________________________________________________________________
Name: svn:executable
+ *
Index: bin/schroot/setup/Makefile.am
===================================================================
--- bin/schroot/setup/Makefile.am (revision 1192)
+++ bin/schroot/setup/Makefile.am (working copy)
@@ -29,6 +29,7 @@
05lvm \
05file \
10mount \
+ 15killprocs \
20network \
30passwd \
50chrootname \
pgpT9ganhtrQO.pgp
Description: PGP signature
