[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[PATCH] dpkg: activate file triggers before letting user modify conffile



A file trigger is guaranteed to be activated before the file in
question is modified by dpkg, ensuring that interested packages
are notified even if dpkg is interrupted immediately after
modifying the file.  But for conffiles modified by the user on
dpkg’s behalf, triggers are being activated _after_ control
returns from the user, and it is possible for the trigger to be
forgotten.

For example, in the following sequence of events:

	configuring package
	dpkg asks user to guide it in merging changes to the
		distributed conffile
	user picks (Z) suspend; dpkg sends itself SIGSTOP
	user modifies conffile to match new version and forgets
		about dpkg
	*clean system shutdown*
	configuring package again
	file matches new version, so nothing to do
	...

although the conffile is updated under dpkg’s watch, no trigger
gets activated.

If we trigger before transferring control to the user, that
problem goes away.  This also makes the code a little more
obvious.

It would be tempting to trigger only on (Z) suspend, but the user
is just as likely to modify the conffile some other way (in
another process, hitting, ^Z, etc) after the prompt is presented.
It is safer to trigger before presenting the prompt, regardless
of the user’s response.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Guillem Jover wrote:
> On that specific case “cfo_keep | cfof_backup”, the user has been
> prompted, dpkg stopped, allowing to modify the installed conffile,

Hmm, maybe activating the trigger closer to the point when the file
is changed would make that more obvious?  As a side effect, this
fixes a small theoretical race.

 src/configure.c |   33 +++++++++++++++++++++++----------
 1 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/src/configure.c b/src/configure.c
index 030bde3..b5595ce 100644
--- a/src/configure.c
+++ b/src/configure.c
@@ -59,7 +59,8 @@ static int conffoptcells[2][2] = {
 static void md5hash(struct pkginfo *pkg, char *hashbuf, const char *fn);
 static void showdiff(const char *old, const char *new);
 static void suspend(void);
-static enum conffopt promptconfaction(struct pkginfo *pkg, const char *cfgfile,
+static enum conffopt promptconfaction(struct pkginfo *pkg,
+                                      struct filenamenode *cfgfile,
                                       const char *realold, const char *realnew,
                                       int useredited, int distedited,
                                       enum conffopt what);
@@ -150,7 +151,7 @@ deferred_configure_conffile(struct pkginfo *pkg, struct conffile *conff)
 	      "deferred_configure '%s' (= '%s') useredited=%d distedited=%d what=%o",
 	      usenode->name, cdr.buf, useredited, distedited, what);
 
-	what = promptconfaction(pkg, usenode->name, cdr.buf, cdr2.buf,
+	what = promptconfaction(pkg, usenode, cdr.buf, cdr2.buf,
 	                        useredited, distedited, what);
 
 	switch (what & ~(cfof_isnew | cfof_userrmd)) {
@@ -163,7 +164,6 @@ deferred_configure_conffile(struct pkginfo *pkg, struct conffile *conff)
 		varbufaddstr(&cdr, DPKGDISTEXT);
 		varbufaddc(&cdr, 0);
 		strcpy(cdr2rest, DPKGNEWEXT);
-		trig_file_activate(usenode, pkg);
 		if (rename(cdr2.buf, cdr.buf))
 			warning(_("%s: failed to rename '%.250s' to '%.250s': %s"),
 			        pkg->name, cdr2.buf, cdr.buf, strerror(errno));
@@ -579,7 +579,7 @@ suspend(void)
  *         default actions as configured by cmdline/configuration options.
  */
 static enum conffopt
-promptconfaction(struct pkginfo *pkg, const char *cfgfile,
+promptconfaction(struct pkginfo *pkg, struct filenamenode *cfgfile,
                  const char *realold, const char *realnew,
                  int useredited, int distedited, enum conffopt what)
 {
@@ -590,15 +590,28 @@ promptconfaction(struct pkginfo *pkg, const char *cfgfile,
 		return what;
 
 	statusfd_send("status: %s : %s : '%s' '%s' %i %i ",
-	              cfgfile, "conffile-prompt",
+	              cfgfile->name, "conffile-prompt",
 	              realold, realnew, useredited, distedited);
 
+	/*
+	 * The user is about to be prompted and dpkg stopped, allowing to
+	 * modify the installed conffile, through dpkg spawning a shell or
+	 * backgrounding itself, or through another external means.  Since
+	 * dpkg does not currently have a way to track if the installed
+	 * conffile gets modified, just unconditionally trigger.
+	 *
+	 * To avoid missing the trigger, we have to activate the trigger
+	 * before the file is modified, anyway.  But maybe some day dpkg will
+	 * learn to cancel the trigger if it escapes this loop with the file
+	 * unchanged.
+	 */
+	trig_file_activate(cfgfile, pkg);
 	do {
 		/* Flush the terminal's input in case the user involuntarily
 		 * typed some characters. */
 		tcflush(STDIN_FILENO, TCIFLUSH);
-		fprintf(stderr, _("\nConfiguration file `%s'"), cfgfile);
-		if (strcmp(cfgfile, realold))
+		fprintf(stderr, _("\nConfiguration file `%s'"), cfgfile->name);
+		if (strcmp(cfgfile->name, realold))
 			fprintf(stderr, _(" (actually `%s')"), realold);
 
 		if (what & cfof_isnew) {
@@ -658,9 +671,9 @@ promptconfaction(struct pkginfo *pkg, const char *cfgfile,
 		else if (what & cfof_install)
 			fprintf(stderr, _(" The default action is to install the new version.\n"));
 
-		s = strrchr(cfgfile, '/');
+		s = strrchr(cfgfile->name, '/');
 		if (!s || !*++s)
-			s = cfgfile;
+			s = cfgfile->name;
 		fprintf(stderr, "*** %s (Y/I/N/O/D/Z) %s ? ",
 		        s,
 		        (what & cfof_keep) ? _("[default=N]") :
@@ -699,7 +712,7 @@ promptconfaction(struct pkginfo *pkg, const char *cfgfile,
 			suspend();
 	} while (!strchr("yino", cc));
 
-	log_message("conffile %s %s", cfgfile,
+	log_message("conffile %s %s", cfgfile->name,
 	            (cc == 'i' || cc == 'y') ? "install" : "keep");
 
 	what &= cfof_userrmd;
-- 
1.6.5.2


Reply to: