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

[Carl Miller]

On Sat, Jan 16, 2010 at 05:09:45AM +0000, Clint Adams wrote:
> 
> You mean something like this?
> 
> 
> diff --git a/src/copyout.c b/src/copyout.c
> index 98f3895..f0741f7 100644
> --- a/src/copyout.c
> +++ b/src/copyout.c
> @@ -121,7 +121,9 @@ count_defered_links_to_dev_ino (struct cpio_file_stat 
> *file_hdr)
>    for (d = deferouts; d != NULL; d = d->next)
>      {
>        if ( (d->header.c_ino == ino) && (d->header.c_dev_maj == maj)
> -       && (d->header.c_dev_min == min) )
> +       && (d->header.c_dev_min == min)
> +       && ((d->header.c_mode & CP_IFBLK) != CP_IFBLK)
> +       && ((d->header.c_mode & CP_IFCHR) != CP_IFCHR) )
>       ++count;
>      }
>    return count;
> @@ -178,7 +180,9 @@ writeout_other_defers (struct cpio_file_stat *file_hdr, 
> int out_des)
>    while (d != NULL)
>      {
>        if ( (d->header.c_ino == ino) && (d->header.c_dev_maj == maj)
> -       && (d->header.c_dev_min == min) )
> +       && (d->header.c_dev_min == min)
> +       && ((d->header.c_mode & CP_IFBLK) != CP_IFBLK)
> +       && ((d->header.c_mode & CP_IFCHR) != CP_IFCHR) )
>       {
>         struct deferment *d_free;
>         d->header.c_filesize = 0;


OK, I've downloaded the source and started wading.  It seems like the
above patch won't have any effect on my situation, as it looks like
the deferment linked list is only used in the newascii and crcascii
formats.  Further, those formats only add a deferment if the st_nlink
is greater than 1 (line 713 of copyout.c).  So I think I wouldn't be
observing this problem if I were using one of those archive formats.


Here's something I did find, however....

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 11:15:19.000000000 -0800
@@ -652,7 +652,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 */
-------

The check for adding a slash to the end of the filename in tar and ustar
format archives is broken, erroneously adding a slash to block devices
and sockets.  This can be seen in both the strace and the output of the
test case in my original post, and explains why the error code that
prevents the hard links from being made at the end of the test case is
ENOTDIR.

It seems like this patch should definitely be applied.  However, it will
not solve my problem.  Now I'll get four device nodes all with the same
minor number after unpacking the resulting archive, due to the hard links
succeeding. (It seems I was wrong in saying that you can't hardlink to a
device node -- you can; you just can't have a slash at the end of its
name :-)

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...)


                             ------Carl