James Hunt has proposed merging lp:~jamesodhunt/upstart/bug-1302117-remerged into lp:upstart.
Requested reviews: Upstart Reviewers (upstart-reviewers) For more details, see: https://code.launchpad.net/~jamesodhunt/upstart/bug-1302117-remerged/+merge/225177 Just prior to re-exec Upstart serialises all the internal objects. As a result, log_serialise() gets called for reach job process. If a job process has unflushed data, Upstart attempts to persist it prior to the re-exec. However, the entails calling log_file_open() *in the parent*, which was setting the umask to ensure the log file is created with a known permission. The fix was to save the umask, set it, write the log file, then restore the umask. Also, added a new test to avoid regression. -- https://code.launchpad.net/~jamesodhunt/upstart/bug-1302117-remerged/+merge/225177 Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/bug-1302117-remerged into lp:upstart.
=== modified file 'ChangeLog' --- ChangeLog 2014-06-05 10:13:51 +0000 +++ ChangeLog 2014-07-01 15:36:47 +0000 @@ -75,6 +75,13 @@ - Clarify default behaviour of 'respawn' stanza. - Add missing 'respawn limit unlimited' details. +2014-04-10 James Hunt <[email protected]> + + * init/log.c: log_file_open(): Restore umask after opening log file + (LP: #1302117). + * util/tests/test_initctl.c: test_reexec(): New test: + - "ensure re-exec does not disrupt umask". + 2014-03-11 James Hunt <[email protected]> * NEWS: Release 1.12.1 === modified file 'init/log.c' --- init/log.c 2014-06-04 17:37:59 +0000 +++ init/log.c 2014-07-01 15:36:47 +0000 @@ -405,6 +405,7 @@ struct stat statbuf; int ret = -1; int mode = LOG_DEFAULT_MODE; + mode_t old; int flags = (O_CREAT | O_APPEND | O_WRONLY | O_CLOEXEC | O_NOFOLLOW | O_NONBLOCK); @@ -439,7 +440,7 @@ nih_assert (log->fd == -1); /* Impose some sane defaults. */ - umask (LOG_DEFAULT_UMASK); + old = umask (LOG_DEFAULT_UMASK); /* Non-blocking to avoid holding up the main loop. Without * this, we'd probably need to spawn a thread to handle @@ -447,6 +448,9 @@ */ log->fd = open (log->path, flags, mode); + /* Restore */ + umask (old); + log->open_errno = errno; /* Open may have failed due to path being unaccessible === modified file 'util/tests/test_initctl.c' --- util/tests/test_initctl.c 2014-06-05 10:13:51 +0000 +++ util/tests/test_initctl.c 2014-07-01 15:36:47 +0000 @@ -11052,6 +11052,8 @@ FILE *file; int ok; int ret; + mode_t expected_umask; + size_t len; TEST_GROUP ("re-exec support"); @@ -11317,6 +11319,80 @@ STOP_UPSTART (upstart_pid); /*******************************************************************/ + TEST_FEATURE ("ensure re-exec does not disrupt umask"); + + contents = nih_sprintf (NULL, "exec sh -c umask"); + TEST_NE_P (contents, NULL); + + CREATE_FILE (confdir, "umask.conf", contents); + nih_free (contents); + + start_upstart_common (&upstart_pid, TRUE, TRUE, confdir, logdir, NULL); + + cmd = nih_sprintf (NULL, "%s start umask 2>&1", get_initctl ()); + TEST_NE_P (cmd, NULL); + RUN_COMMAND (NULL, cmd, &output, &lines); + nih_free (output); + + logfile = NIH_MUST (nih_sprintf (NULL, "%s/%s", + logdir, + "umask.log")); + + /* Wait for log to be created */ + ok = FALSE; + for (int i = 0; i < 5; i++) { + sleep (1); + if (! stat (logfile, &statbuf)) { + ok = TRUE; + break; + } + } + TEST_EQ (ok, TRUE); + + contents = nih_file_read (NULL, logfile, &len); + TEST_NE_P (contents, NULL); + TEST_TRUE (len); + + /* overwrite '\n' */ + contents[len-1] = '\0'; + + expected_umask = (mode_t)atoi (contents); + assert0 (unlink (logfile)); + nih_free (contents); + + /* Restart */ + REEXEC_UPSTART (upstart_pid, TRUE); + + /* Re-run job */ + RUN_COMMAND (NULL, cmd, &output, &lines); + nih_free (output); + + /* Wait for log to be recreated */ + ok = FALSE; + for (int i = 0; i < 5; i++) { + sleep (1); + if (! stat (logfile, &statbuf)) { + ok = TRUE; + break; + } + } + TEST_EQ (ok, TRUE); + + contents = nih_file_read (NULL, logfile, &len); + TEST_NE_P (contents, NULL); + TEST_TRUE (len); + + /* overwrite '\n' */ + contents[len-1] = '\0'; + + TEST_EQ (expected_umask, (mode_t)atoi (contents)); + + STOP_UPSTART (upstart_pid); + + DELETE_FILE (confdir, "umask.conf"); + assert0 (unlink (logfile)); + + /*******************************************************************/ TEST_DBUS_END (dbus_pid);
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
