tag 575891 patch thanks On Tue, 06 Apr 2010, Joey Hess wrote: > cwillu wrote: > > New information on http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=575891 > > > > Turns out to have been an unsafe assumption on dpkg's part with > > apparently astronomic odds of being triggered on most filesystems. > > Cwillu, thanks for following up.
Hi all, I have prepared a patch for this issue, see attachment. I would be glad if some of you could test it. Guillem, a quick ok from you would be nice (feel free to point stylistic issues if you have some). I want to push this patch in the next update (1.15.6.2). Cheers, -- Raphaël Hertzog Like what I do? Sponsor me: http://ouaza.com/wp/2010/01/05/5-years-of-freexian/ My Debian goals: http://ouaza.com/wp/2010/01/09/debian-related-goals-for-2010/
commit f53c2cbd34a65ea10a5419243acccb68be2b9682 Author: Raphaël Hertzog <[email protected]> Date: Fri Apr 9 08:35:47 2010 +0200 dpkg: fix metadata installation by not mixing rename() in a readdir() loop dpkg's process_archive() was doing the improper assumption that a readdir() loop would not return the same filename twice even when the scanned directory has files renamed into it (coming from tmp.ci). The net result of having the same filename returned twice is that the the second time the updated file to install is no longer there and thus dpkg removed the current metadata file believing that it was obsolete. btrfs triggers this bug consistently. All other readdir() occurrences have been reviewed as well for similar problems. But they are all safe, they mainly unlink() files rather than adding new files into the scanned directory. Thanks to Carey Underwood and Chris Mason for their help in diagnosing this problem. diff --git a/debian/changelog b/debian/changelog index adaa95f..f699fea 100644 --- a/debian/changelog +++ b/debian/changelog @@ -14,6 +14,9 @@ dpkg (1.15.6.2) UNRELEASED; urgency=low do not include that file in the generated source package. * Improve explanation of --all option in dpkg-parsechangelog(1). Thanks to Jari Aalto. Closes: #575706 + * Fix dpkg to not lose package metadata on filesystems where readdir() + returns new files added after the opendir() call, btrfs in particular + triggered the problematic behaviour. Closes: #575891 [ Updated dpkg translations ] * German (Sven Joachim). diff --git a/src/archives.h b/src/archives.h index 2390bb8..0ee8057 100644 --- a/src/archives.h +++ b/src/archives.h @@ -35,6 +35,12 @@ struct pkg_deconf_list { struct pkginfo *pkg_removal; }; +struct rename_list { + struct rename_list *next; + char *src; + char *dest; +}; + extern int fnameidlu; extern struct varbuf fnamevb; extern struct varbuf fnametmpvb; diff --git a/src/processarc.c b/src/processarc.c index fe5ca1f..b596c79 100644 --- a/src/processarc.c +++ b/src/processarc.c @@ -118,6 +118,7 @@ void process_archive(const char *filename) { struct dirent *de; struct stat stab, oldfs; struct pkg_deconf_list *deconpil, *deconpiltemp; + struct rename_list *rename_head = NULL, *rename_temp = NULL; cleanup_pkg_failed= cleanup_conflictor_failed= 0; admindirlen= strlen(admindir); @@ -850,19 +851,35 @@ void process_archive(const char *filename) { varbufaddstr(&infofnvb,de->d_name); varbufaddc(&infofnvb,0); strcpy(cidirrest,p); - if (!rename(cidir,infofnvb.buf)) { + /* We keep files to rename in a list as doing the rename immediately + * might influence the current readdir(), the just renamed file might + * be returned a second time as it's actually a new file from the + * point of view of the filesystem. */ + rename_temp = m_malloc(sizeof(struct rename_list)); + rename_temp->next = rename_head; + rename_temp->src = m_strdup(cidir); + rename_temp->dest = m_strdup(infofnvb.buf); + rename_head = rename_temp; + } + pop_cleanup(ehflag_normaltidy); /* closedir */ + + for(rename_temp = rename_head; rename_temp; rename_temp = rename_head) { + if (!rename(rename_temp->src, rename_temp->dest)) { debug(dbg_scripts, "process_archive info installed %s as %s", - cidir, infofnvb.buf); + rename_temp->src, rename_temp->dest); } else if (errno == ENOENT) { /* Right, no new version. */ - if (unlink(infofnvb.buf)) - ohshite(_("unable to remove obsolete info file `%.250s'"),infofnvb.buf); - debug(dbg_scripts, "process_archive info unlinked %s",infofnvb.buf); + if (unlink(rename_temp->dest)) + ohshite(_("unable to remove obsolete info file `%.250s'"), rename_temp->dest); + debug(dbg_scripts, "process_archive info unlinked %s", rename_temp->dest); } else { - ohshite(_("unable to install (supposed) new info file `%.250s'"),cidir); + ohshite(_("unable to install (supposed) new info file `%.250s'"), rename_temp->src); } + rename_head = rename_temp->next; + free(rename_temp->src); + free(rename_temp->dest); + free(rename_temp); } - pop_cleanup(ehflag_normaltidy); /* closedir */ *cidirrest = '\0'; /* the directory itself */ dsd= opendir(cidir);
