Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 2 Jan 2011 23:31:18 +0100
From:      Jilles Tjoelker <jilles@stack.nl>
To:        freebsd-arch@freebsd.org
Subject:   init(8) running rc.shutdown inappropriately, mechanism for halt(8)/reboot(8)
Message-ID:  <20110102223118.GA95551@stack.nl>

next in thread | raw e-mail | index | archive | help

--CE+1k2dSO48ffgeK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

As discussed on the svn-src commits mailing list, I think halt and
reboot should be changed to trigger the shutdown by sending a signal to
init, either directly or by calling shutdown. The new poweroff utility
should do the same as halt -p. A new -f option and the filenames
fastboot and fasthalt keep the current behaviour of killing all
processes and calling reboot(2) directly, and the same applies if one of
the options -d, -n and -q is used (which cannot be passed to init).

I'm aware of shutdown's -o option which makes it call halt(8) or
reboot(8). These invocations should gain the -f option or change to
fasthalt/fastboot.

The current halt and reboot programs do not run /etc/rc.shutdown, which
may cause daemons not to be shut down correctly. At this time, there is
no way to fix this properly, because /etc/rc.shutdown should not be run
from single-user mode (if this is done, /etc/rc.d/mixer overwrites
/var/db/mixer*-state with the defaults, for example) and reboot has no
way to know this.

When rebooting by sending a signal to init (directly, via ctrl+alt+del,
by re-executing /sbin/init or via shutdown), /etc/rc.shutdown is always
executed. The attached patch changes this, executing it only if /etc/rc
has completed.

The bigger-picture goal is to make all the common ways of shutting down
(ctrl+alt+del, shutdown(8), halt(8), reboot(8), poweroff(8)) work
properly both in single-user and multi-user. Currently, only
ctrl+alt+del, shutdown(8) and poweroff(8) work properly in multi-user,
while only halt(8) and reboot(8) work properly in single-user.

The patch also fixes segfaults and other erratic behaviour if init
receives SIGHUP or SIGTSTP while in single-user mode.

The patch does not attempt to fix any badness with signal handlers
(assumption that pointers can be read and written atomically, EINTR race
condition). I believe the patch does not make this badness any worse.

If the attachment does not work, the patch is also here for some time:
http://www.stack.nl/~jilles/unix/init-fixes-20110102.patch

-- 
Jilles Tjoelker

--CE+1k2dSO48ffgeK
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="init-fixes-20110102.patch"

Index: sbin/init/init.c
===================================================================
--- sbin/init/init.c	(revision 216859)
+++ sbin/init/init.c	(working copy)
@@ -122,6 +122,7 @@
 static state_func_t clean_ttys(void);
 static state_func_t catatonia(void);
 static state_func_t death(void);
+static state_func_t death_single(void);
 
 static state_func_t run_script(const char *);
 
@@ -136,6 +137,7 @@
 
 static void transition(state_t);
 static state_t requested_transition;
+static state_t current_state = death_single;
 
 static void setctty(const char *);
 static const char *get_shell(void);
@@ -559,8 +561,9 @@
 transition(state_t s)
 {
 
+	current_state = s;
 	for (;;)
-		s = (state_t) (*s)();
+		current_state = (state_t) (*current_state)();
 }
 
 /*
@@ -796,7 +799,7 @@
  * Returns 0 on success, otherwise the next transition to enter:
  *  - single_user if fork/execv/waitpid failed, or if the script
  *    terminated with a signal or exit code != 0.
- *  - death if a SIGTERM was delivered to init(8).
+ *  - death_single if a SIGTERM was delivered to init(8).
  */
 static state_func_t
 run_script(const char *script)
@@ -852,8 +855,8 @@
 		if ((wpid = waitpid(-1, &status, WUNTRACED)) != -1)
 			collect_child(wpid);
 		if (wpid == -1) {
-			if (requested_transition == death)
-				return (state_func_t) death;
+			if (requested_transition == death_single)
+				return (state_func_t) death_single;
 			if (errno == EINTR)
 				continue;
 			warning("wait for %s on %s failed: %m; going to "
@@ -1306,7 +1309,9 @@
 
 	switch (sig) {
 	case SIGHUP:
-		requested_transition = clean_ttys;
+		if (current_state == read_ttys || current_state == multi_user ||
+		    current_state == clean_ttys || current_state == catatonia)
+			requested_transition = clean_ttys;
 		break;
 	case SIGUSR2:
 		howto = RB_POWEROFF;
@@ -1315,10 +1320,17 @@
 	case SIGINT:
 		Reboot = TRUE;
 	case SIGTERM:
-		requested_transition = death;
+		if (current_state == read_ttys || current_state == multi_user ||
+		    current_state == clean_ttys || current_state == catatonia)
+			requested_transition = death;
+		else
+			requested_transition = death_single;
 		break;
 	case SIGTSTP:
-		requested_transition = catatonia;
+		if (current_state == runcom || current_state == read_ttys ||
+		    current_state == clean_ttys ||
+		    current_state == multi_user || current_state == catatonia)
+			requested_transition = catatonia;
 		break;
 	default:
 		requested_transition = 0;
@@ -1494,9 +1506,6 @@
 {
 	struct utmpx utx;
 	session_t *sp;
-	int i;
-	pid_t pid;
-	static const int death_sigs[2] = { SIGTERM, SIGKILL };
 
 	/* NB: should send a message to the session logger to avoid blocking. */
 	utx.ut_type = SHUTDOWN_TIME;
@@ -1518,6 +1527,22 @@
 	/* Try to run the rc.shutdown script within a period of time */
 	runshutdown();
 
+	return (state_func_t) death_single;
+}
+
+/*
+ * Do what is necessary to reinitialize single user mode or reboot
+ * from an incomplete state.
+ */
+static state_func_t
+death_single(void)
+{
+	int i;
+	pid_t pid;
+	static const int death_sigs[2] = { SIGTERM, SIGKILL };
+
+	revoke(_PATH_CONSOLE);
+
 	for (i = 0; i < 2; ++i) {
 		if (kill(-1, death_sigs[i]) == -1 && errno == ESRCH)
 			return (state_func_t) single_user;

--CE+1k2dSO48ffgeK--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110102223118.GA95551>