cramfs takes a shortcut with device nodes, and assigns them all inode 1. When using cpio to copy files out of a cramfs image, cpio turns the second and all subsequent copied device nodes into hard links to the first copied out device node, based on them all having the same st_dev and st_ino. The resulting archive does not extract properly, because you cannot make a hard link to a device node. Solution: when checking for hard links during copy-out, take the file type into account. The test should be changed from "st_devs equal and st_inos equal" to "st_devs equal and st_inos equal and not block device and not char device". root@mithrandir:/tmp/unpack# mkdir mnt_228 root@mithrandir:/tmp/unpack# mount -t cramfs -o ro,nodev,noexec,loop /usr/local/isengard/share/builds/build228/disk_fs.img mnt_228 root@mithrandir:/tmp/unpack# ls -l mnt_228/dev/loop? brw-r--r-- 1 root root 7, 0 1969-12-31 16:00 mnt_228/dev/loop0 brw-r--r-- 1 root root 7, 1 1969-12-31 16:00 mnt_228/dev/loop1 brw-r--r-- 1 root root 7, 2 1969-12-31 16:00 mnt_228/dev/loop2 brw-r--r-- 1 root root 7, 3 1969-12-31 16:00 mnt_228/dev/loop3 root@mithrandir:/tmp/unpack# cd mnt_228 root@mithrandir:/tmp/unpack/mnt_228# ( echo "dev" ; echo "dev/loop0" ; echo "dev/loop1" ; echo "dev/loop2" ; echo "dev/loop3" ) | cpio -o -F /tmp/looptest.tar -H ustar --quiet root@mithrandir:/tmp/unpack/mnt_228# tar -tf /tmp/looptest.tar tar: Record size = 7 blocks dev/ dev/loop0/ dev/loop1/ dev/loop2/ dev/loop3/ root@mithrandir:/tmp/unpack/mnt_228# cd .. root@mithrandir:/tmp/unpack# tar -xvf /tmp/looptest.tar tar: Record size = 7 blocks dev/ dev/loop0/ tar: dev/loop0: implausibly old time stamp 1969-12-31 16:00:00 dev/loop1/ tar: dev/loop1: Cannot hard link to `dev/loop0/': Not a directory dev/loop2/ tar: dev/loop2: Cannot hard link to `dev/loop0/': Not a directory dev/loop3/ tar: dev/loop3: Cannot hard link to `dev/loop0/': Not a directory tar: dev: implausibly old time stamp 1969-12-31 16:00:00 tar: Exiting with failure status due to previous errors root@mithrandir:/tmp/unpack# ls -l dev total 0 brw-r--r-- 1 root root 7, 0 1969-12-31 16:00 loop0
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;
Most excellent! Thank you, Clint! I'll try it out first thing Monday morning. I hadn't had a chance to download and poke through the cpio source yet, having only pinned down the bug clearly at 7:45 on a Friday evening of a long, crazy day of work. I was wondering if there's any likelihood of being tripped up by this on any other file types (sockets? fifos?). I spent a few minutes today poking through the Linux kernel source, but only found where directories get rejected for hard-linking.
Carl Miller wrote: These tests should look like: (d->header.c_mode & CP_IFMT) != CP_IFBLK Note the use of CP_IFMT to mask the file type (which is a four-bit field). Cheers, Tim
I presume it also assigns nlinks == 1? during copy-out, do not generate hardlink entries if nlinks < 2. Tim
That would appear to be the case. Here's the tail end of an strace of the
cpio in my example running....
14087 lstat("dev/loop0", {st_dev=makedev(7, 0), st_ino=1, st_mode=S_IFBLK|0644, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=4, st_rdev=makedev(7, 0), st_atime=0, st_mtime=0, st_ctime=0}) = 0
14087 write(3, "dev/\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 512) = 512
14087 lstat("dev/loop1", {st_dev=makedev(7, 0), st_ino=1, st_mode=S_IFBLK|0644, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=4, st_rdev=makedev(7, 1), st_atime=0, st_mtime=0, st_ctime=0}) = 0
14087 write(3, "dev/loop0/\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 512) = 512
14087 lstat("dev/loop2", {st_dev=makedev(7, 0), st_ino=1, st_mode=S_IFBLK|0644, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=4, st_rdev=makedev(7, 2), st_atime=0, st_mtime=0, st_ctime=0}) = 0
14087 write(3, "dev/loop1/\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 512) = 512
14087 lstat("dev/loop3", {st_dev=makedev(7, 0), st_ino=1, st_mode=S_IFBLK|0644, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=4, st_rdev=makedev(7, 3), st_atime=0, st_mtime=0, st_ctime=0}) = 0
14087 write(3, "dev/loop2/\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 512) = 512
14087 read(0, "", 4096) = 0
14087 write(3, "dev/loop3/\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 512) = 512
14087 write(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 512) = 512
14087 write(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 512) = 512
14087 close(3) = 0
14087 exit_group(0) = ?
Good thought. Maybe I'll try that first. Thanks, Tim!
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....
chaz@mithrandir:/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...)
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....
chaz@mithrandir:/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...
root@mithrandir:/tmp/unpack# mkdir mnt_228 root@mithrandir:/tmp/unpack# mount -t cramfs -o ro,nodev,noexec,loop /usr/local/isengard/share/builds/build228/disk_fs.img mnt_228 root@mithrandir:/tmp/unpack# ls -l mnt_228/dev/loop? brw-r--r-- 1 root root 7, 0 1969-12-31 16:00 mnt_228/dev/loop0 brw-r--r-- 1 root root 7, 1 1969-12-31 16:00 mnt_228/dev/loop1 brw-r--r-- 1 root root 7, 2 1969-12-31 16:00 mnt_228/dev/loop2 brw-r--r-- 1 root root 7, 3 1969-12-31 16:00 mnt_228/dev/loop3 root@mithrandir:/tmp/unpack# cd mnt_228 root@mithrandir:/tmp/unpack/mnt_228# ( echo "dev" ; echo "dev/loop0" ; echo "dev/loop1" ; echo "dev/loop2" ; echo "dev/loop3" ) | /tmp/cpio_patch/cpio-2.10/src/cpio -o -F /tmp/looptest.tar -H ustar --quiet root@mithrandir:/tmp/unpack/mnt_228# tar -tf /tmp/looptest.tar tar: Record size = 7 blocks dev/ dev/loop0 dev/loop1 dev/loop2 dev/loop3 root@mithrandir:/tmp/unpack/mnt_228# cd .. root@mithrandir:/tmp/unpack# tar -xvf /tmp/looptest.tar tar: Record size = 7 blocks dev/ dev/loop0 tar: dev/loop0: implausibly old time stamp 1969-12-31 16:00:00 dev/loop1 tar: dev/loop1: implausibly old time stamp 1969-12-31 16:00:00 dev/loop2 tar: dev/loop2: implausibly old time stamp 1969-12-31 16:00:00 dev/loop3 tar: dev/loop3: implausibly old time stamp 1969-12-31 16:00:00 tar: dev: implausibly old time stamp 1969-12-31 16:00:00 root@mithrandir:/tmp/unpack# ls -l dev total 0 brw-r--r-- 1 root root 7, 0 1969-12-31 16:00 loop0 brw-r--r-- 1 root root 7, 1 1969-12-31 16:00 loop1 brw-r--r-- 1 root root 7, 2 1969-12-31 16:00 loop2 brw-r--r-- 1 root root 7, 3 1969-12-31 16:00 loop3 root@mithrandir:/tmp/unpack# Much better. No trailing slashes; all archived files unpack, and have the correct st_rdev. I'm going to call that a fix. Thanks much, Clint and Tim!
Ping? The patch given in message #40 of the bug archive solved the problem, and has been running error-free continuously since. Just wondering if it's been made part of the standard patches Debian applies to the original upsteam source yet, or even better, pushed upstream for inclusion in the maintainer's source. Thanks!