[Bug-cpio] Re: Bug#565474: cpio makes device nodes into hard links when copying out of a cramfs image

[Carl Miller]

On Mon, Jan 18, 2010 at 11:52:12AM -0800, Carl Miller wrote:
> 
> So now my question is, why do callers of add_inode() not check the
> st_nlinks for being > 1 first, like callers of add_link_defer()?  Is
> there some reason that it *should* be this way?  If not, I'd propose
> putting that check in before calling add_inode(), and only having files
> with nlinks >= 2 in the inode hash table.  Thoughts?
> 
> (Proposed patch to follow while others think of reasons this might break
> something else...)

It looks to me like copypass.c, the other user of add_inode() only adds
to the hash table if st_nlinks > 1.  That eases my concerns about having
copyout.c do the same.  Here's my proposed patch....


address@hidden:/tmp/cpio_patch/cpio-2.10$ diff -du src/copyout.c{.orig,}
--- src/copyout.c.orig  2009-02-14 10:15:50.000000000 -0800
+++ src/copyout.c       2010-01-18 14:02:20.000000000 -0800
@@ -232,7 +232,8 @@
                           header->c_name);
   warn_if_file_changed(header->c_name, file_hdr.c_filesize, file_hdr.c_mtime);

-  if (archive_format == arf_tar || archive_format == arf_ustar)
+  if ((archive_format == arf_tar || archive_format == arf_ustar)
+      && (file_hdr.c_nlink > 1))
     add_inode (file_hdr.c_ino, file_hdr.c_name, file_hdr.c_dev_maj,
               file_hdr.c_dev_min);

@@ -652,7 +653,7 @@

          if (archive_format == arf_tar || archive_format == arf_ustar)
            {
-             if (file_hdr.c_mode & CP_IFDIR)
+             if ((file_hdr.c_mode & CP_IFMT) == CP_IFDIR)
                {
                  int len = strlen (input_name.ds_string);
                  /* Make sure the name ends with a slash */
@@ -696,7 +697,8 @@
          switch (file_hdr.c_mode & CP_IFMT)
            {
            case CP_IFREG:
-             if (archive_format == arf_tar || archive_format == arf_ustar)
+             if ((archive_format == arf_tar || archive_format == arf_ustar)
+                 && (file_hdr.c_nlink > 1))
                {
                  char *otherfile;
                  if ((otherfile = find_inode_file (file_hdr.c_ino,
@@ -743,7 +745,8 @@
              warn_if_file_changed(orig_file_name, file_hdr.c_filesize,
                                    file_hdr.c_mtime);

-             if (archive_format == arf_tar || archive_format == arf_ustar)
+             if ((archive_format == arf_tar || archive_format == arf_ustar)
+                 && (file_hdr.c_nlink > 1))
                add_inode (file_hdr.c_ino, orig_file_name, file_hdr.c_dev_maj,
                           file_hdr.c_dev_min);

@@ -777,7 +780,7 @@
                         orig_file_name);
                  continue;
                }
-             else if (archive_format == arf_ustar)
+             else if ((archive_format == arf_ustar) && (file_hdr.c_nlink > 1))
                {
                  char *otherfile;
                  if ((otherfile = find_inode_file (file_hdr.c_ino,
----------------

I'll compile it and run the same test case against it now...


                               ------Carl