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

[SRM] Dpkg update in stable to fix bug with btrfs?



Hello,

after someone got bitten by #575891 while running a lenny chroot
on a btrfs filesystem, Julien Cristau suggested me to try to fix this bug
in stable.

Would you accept a stable upload with the attached patch ?

It's a relatively simple cherry pick of the fix made in the current
version. It's not tested yet but I would do it if you think it needs to be
fixed in stable.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer ◈ [Flattr=20693]

Follow my Debian News ▶ http://RaphaelHertzog.com (English)
                      ▶ http://RaphaelHertzog.fr (Français)
commit fd2021ab7cf2ca85fd97c74023fbcb3484ae4601
Author: Raphaël Hertzog <hertzog@debian.org>
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.
    
    Acked-by: Guillem Jover <guillem@debian.org>

diff --git a/debian/changelog b/debian/changelog
index c7e6e8b..d1763b9 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+dpkg (1.14.30) stable; urgency=low
+
+  * 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
+
+ -- Raphaël Hertzog <hertzog@debian.org>  Wed, 20 Oct 2010 23:10:29 +0200
+
 dpkg (1.14.29) stable-security; urgency=high
 
   * Modify dpkg-source to error out when it would apply patches containing
diff --git a/src/processarc.c b/src/processarc.c
index 3085962..e5922d7 100644
--- a/src/processarc.c
+++ b/src/processarc.c
@@ -44,6 +44,12 @@
 #include "main.h"
 #include "archives.h"
 
+struct rename_list {
+  struct rename_list *next;
+  char *src;
+  char *dst;
+};
+
 void process_archive(const char *filename) {
   static const struct TarFunctions tf = {
     tarfileread,
@@ -83,6 +89,7 @@ void process_archive(const char *filename) {
   struct dirent *de;
   struct stat stab, oldfs;
   struct packageinlist *deconpil, *deconpiltemp;
+  struct rename_list *rename_head = NULL, *rename_node = NULL;
   
   cleanup_pkg_failed= cleanup_conflictor_failed= 0;
   admindirlen= strlen(admindir);
@@ -796,19 +803,37 @@ 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_node = m_malloc(sizeof(*rename_node));
+    rename_node->next = rename_head;
+    rename_node->src = m_strdup(cidir);
+    rename_node->dst = m_strdup(infofnvb.buf);
+    rename_head = rename_node;
+  }
+  pop_cleanup(ehflag_normaltidy); /* closedir */
+
+  while ((rename_node = rename_head)) {
+    if (!rename(rename_node->src, rename_node->dst)) {
       debug(dbg_scripts, "process_archive info installed %s as %s",
-            cidir, infofnvb.buf);
+            rename_node->src, rename_node->dst);
     } 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_node->dst))
+        ohshite(_("unable to remove obsolete info file `%.250s'"),
+                rename_node->dst);
+      debug(dbg_scripts, "process_archive info unlinked %s", rename_node->dst);
     } else {
-      ohshite(_("unable to install (supposed) new info file `%.250s'"),cidir);
+      ohshite(_("unable to install (supposed) new info file `%.250s'"),
+              rename_node->src);
     }
+    rename_head = rename_node->next;
+    free(rename_node->src);
+    free(rename_node->dst);
+    free(rename_node);
   }
-  pop_cleanup(ehflag_normaltidy); /* closedir */
   
   *cidirrest= 0; /* the directory itself */
   dsd= opendir(cidir);

Reply to: