Merge lp:~clint-fewbar/ubuntu/natty/upstart/restore-re-exec-code into lp:ubuntu/natty/upstart

[Clint Byrum]

Now that the libc6 version that cancels the telinit u has been accepted into natty, I'm proposing merging this in.

This will cause upstart to re-exec itself on 'telinit u', which sends a SIGTERM sent to pid 1.

This code was dropped for unknown reasons (possibly just by mistake of package importing into bzr) and will solve the issue of / not being able to be remounted read-only during the shutdown process.

[lp:~clint-fewbar/ubuntu/natty/upstart/restore-re-exec-code]

1289. By Kees Cook on 2011-01-14

debian/apparmor-profile-load: allow profiles to be missing for saner
packaging integration.

[Clint Byrum (clint-fewbar)]

FYI, this is insufficient to stop root FS corruption.

I tested this on lucid, and occasionally it still fails the remount of /

This is because the SIGTERM signal is sent by telinit u, but then we don't wait for upstart to re-exec itself before moving on to the umount. This fails about 50% of the time on lucid.

The simple, but incomplete, solution is to add a sleep 1 after telinit u. The more appropriate solution is to have telinit ptrace 1 with PTRACE_O_TRACEEXEC.

[Clint Byrum (clint-fewbar)]

So, the simpler solution to the race between umountroot exitting and the re-exec completing, brought up by James Hunt, is to watch for changes in /proc/1 .. I think /proc/1/maps is the right place to look.

We are working on a patch to sysvinit's umountroot script to take care of that. In the mean time, this patch should probably be uploaded without it, as this patch will at least give us *some* way of telling upstart to re-exec itself, so the sooner people are booting with this version of upstart, the less corrupted /'s there will be.

[Colin Watson (cjwatson)]

This basically seems OK; but one possible failure mode comes to mind. If the old libc6.postinst is run with the new upstart, then it will run 'telinit u' and that will send SIGTERM to upstart, which will faithfully re-exec and will now have no job state. Is it worth adding some kind of packaging metadata, perhaps Breaks, to try to avoid that?

[Sebastien Bacher (seb128)]

the changelog is buggy and needs to be rebased on the current natty version

[Clint Byrum (clint-fewbar)]

Merged latest changes and pushed.

[lp:~clint-fewbar/ubuntu/natty/upstart/restore-re-exec-code]

1290. By Steve Langasek on 2011-01-26

debian/upstart-job: properly handle jobs that are in state 'start/running'
with no PID, by checking only if the goal is 'start'. LP: #603934.

1291. By Steve Langasek on 2011-01-26

hah, get our grep line right, otherwise we break *all* service upgrades

1292. By Steve Langasek on 2011-01-26

add another bug ref

1293. By Steve Langasek on 2011-01-26

releasing version 0.6.7-5

1294. By Kees Cook on 2011-02-03

debian/apparmor-profile-load: check for correct AppArmor profile loading
interface file (LP: #710649).

1295. By Clint Byrum on 2011-02-05

Re-add upstream r977 to allow proper re-exec on shutdown (LP: #672177)

1296. By Clint Byrum on 2011-02-05

* Re-add upstream r977 to allow proper re-exec on shutdown (LP: #672177)
* debian/control: adding Breaks on eglibc version that disables
  telinit u to avoid accidentally installing a version of libc6 that
  will cause upstart to re-exec and lose its state.

[Clint Byrum (clint-fewbar)]

Ok I overwrote the old branch as it got way out of sync w/ the main natty branch.

Please note that this fix is already present in lucid-updates and maverick-updates. Not sure how it is that it didn't get uploaded into natty before that.

[Kees Cook (kees)]

Looks good; thanks! I'll get this uploaded.

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2011-02-03 13:45:32 +0000
+++ debian/changelog	2011-02-05 01:12:11 +0000
@@ -1,3 +1,12 @@
+upstart (0.6.7-7) natty; urgency=low
+
+  * Re-add upstream r977 to allow proper re-exec on shutdown (LP: #672177)
+  * debian/control: adding Breaks on eglibc version that disables 
+    telinit u to avoid accidentally installing a version of libc6 that 
+    will cause upstart to re-exec and lose its state.
+
+ -- Clint Byrum <[e-mail]>  Fri, 21 Jan 2011 08:39:13 -0800
+
 upstart (0.6.7-6) natty; urgency=low
 
   * debian/apparmor-profile-load: check for correct AppArmor profile loading

=== modified file 'debian/control'
--- debian/control	2010-12-14 17:15:57 +0000
+++ debian/control	2011-02-05 01:12:11 +0000
@@ -12,6 +12,7 @@
 Replaces: upstart-job, sysvinit, upstart-compat-sysv, startup-tasks, system-services
 Conflicts: upstart-job, sysvinit, upstart-compat-sysv, startup-tasks, system-services
 Provides: upstart-job, upstart-compat-sysv, startup-tasks, system-services
+Breaks: libc6 ( << 2.12.1-0ubuntu12 )
 Description: event-based init daemon
  upstart is a replacement for the /sbin/init daemon which handles
  starting of tasks and services during boot, stopping them during

=== modified file 'init/main.c'
--- init/main.c	2010-12-14 17:15:57 +0000
+++ init/main.c	2011-02-05 01:12:11 +0000
@@ -63,6 +63,9 @@
 /* Prototypes for static functions */
 #ifndef DEBUG
 static void crash_handler   (int signum);
+#endif /* DEBUG */
+static void term_handler    (void *data, NihSignal *signal);
+#ifndef DEBUG
 static void cad_handler     (void *data, NihSignal *signal);
 static void kbd_handler     (void *data, NihSignal *signal);
 static void pwr_handler     (void *data, NihSignal *signal);
@@ -261,9 +264,15 @@
 	/* SIGUSR1 instructs us to reconnect to D-Bus */
 	nih_signal_set_handler (SIGUSR1, nih_signal_handler);
 	NIH_MUST (nih_signal_add_handler (NULL, SIGUSR1, usr1_handler, NULL));
+
+	/* SIGTERM instructs us to re-exec ourselves; this should be the
+	 * last in the list to ensure that all other signals are handled
+	 * before a SIGTERM.
+	 */
+	nih_signal_set_handler (SIGTERM, nih_signal_handler);
+	NIH_MUST (nih_signal_add_handler (NULL, SIGTERM, term_handler, NULL));
 #endif /* DEBUG */
 
-
 	/* Watch children for events */
 	NIH_MUST (nih_child_add_watch (NULL, -1, NIH_CHILD_ALL,
 				       job_process_handler, NULL));
@@ -476,7 +485,60 @@
 	/* Goodbye, cruel world. */
 	exit (signum);
 }
-
+#endif
+
+/**
+ * term_handler:
+ * @data: unused,
+ * @signal: signal caught.
+ *
+ * This is called when we receive the TERM signal, which instructs us
+ * to reexec ourselves.
+ **/
+static void
+term_handler (void      *data,
+	      NihSignal *signal)
+{
+	NihError   *err;
+	const char *loglevel;
+	sigset_t    mask, oldmask;
+
+	nih_assert (argv0 != NULL);
+	nih_assert (signal != NULL);
+
+	nih_warn (_("Re-executing %s"), argv0);
+
+	/* Block signals while we work.  We're the last signal handler
+	 * installed so this should mean that they're all handled now.
+	 *
+	 * The child must make sure that it unblocks these again when
+	 * it's ready.
+	 */
+	sigfillset (&mask);
+	sigprocmask (SIG_BLOCK, &mask, &oldmask);
+
+	/* Argument list */
+	if (nih_log_priority <= NIH_LOG_DEBUG) {
+		loglevel = "--debug";
+	} else if (nih_log_priority <= NIH_LOG_INFO) {
+		loglevel = "--verbose";
+	} else if (nih_log_priority >= NIH_LOG_ERROR) {
+		loglevel = "--error";
+	} else {
+		loglevel = NULL;
+	}
+	execl (argv0, argv0, "--restart", loglevel, NULL);
+	nih_error_raise_system ();
+
+	err = nih_error_get ();
+	nih_error (_("Failed to re-execute %s: %s"), argv0, err->message);
+	nih_free (err);
+
+	sigprocmask (SIG_SETMASK, &oldmask, NULL);
+}
+
+
+#ifndef DEBUG
 /**
  * cad_handler:
  * @data: unused,