pkg/cpio,cmds/core/cpio: handle Inum 0 better, start numbering at 2

For reproducible builds, the u-root command produces cpio entries
with inumber of 0. This has worked fine for years, until recently
an archive was created with an empty, zero-length directory, when it
failed on Harvey.

Zero-length entities in cpio archives indicate a zero length file, OR, a second
hard link to an already-written file, OR a directory.
Commit 301d69f tried to handle
zero-length entries but only got 2 of the 3 cases right. A recent commit,
676b9e5 fixed directories,
but still had a few issues with Inums of 0, seen on Harvey.

The problem boils down to handling entries with an inumber of 0.
That is an illegal value for real file systems, but not illegal
per se. The code now uses it as a sentinal
to gate other tests. The code still leaves in a test which
unconditionally skips directories, as there are cpio archives
which strangely end up with directory inumbers the same as file
inumbers. It pays to be careful.

This change cleans up the last few problems we've seen processing
u-root initramfs cpios on Harvey. Why did this not happen before?
Because until now almnost all processing of u-root initramfs was
done by the Linux kernel, not the u-root cpio command. Further,
the commit mentioned above broke a few things starting in February 2020.

This also fixes a bug in the original cpio package, in which the first inumber
was zero, not a legal value.

From the original Unix docs:
"    The root inode is the root of the file system.  Inode 0 can't be used for
     normal purposes and historically bad blocks were linked to inode 1, thus
     the root inode is 2 (inode 1 is no longer used for this purpose, however
     numerous dump tapes make this assumption, so we are stuck with it)."

Signed-off-by: Ronald G. Minnich <[email protected]>

cmds/core/cpio/cpio.go

@@ -78,20 +78,47 @@ func main() {
 			if err != nil {
 				log.Fatalf("error reading records: %v", err)
 			}
-			debug("Creating %s\n", rec)
+			debug("record name %s ino %d\n", rec.Name, rec.Info.Ino)
 
 			// A file with zero size could be a hard link to another file
-			// in the archive. The file with contents always comes first.
-			if rec.Mode&cpio.S_IFMT != cpio.S_IFDIR && rec.Info.FileSize == 0 {
-				if _, ok := inums[rec.Info.Ino]; ok {
-					err := os.Link(inums[rec.Info.Ino], rec.Name)
+			// in the archive. The file with content always comes first.
+			//
+			// But we should ignore files with Ino of 0; that's an illegal value.
+			// The current most common use of this command is with u-root
+			// initramfs cpio files on Linux and Harvey.
+			// (nobody else cares about cpio any more save kernels).
+			// Those always have Ino of zero for reproducible builds.
+			// Hence doing the Ino != 0 test first saves a bit of work.
+			if rec.Info.Ino != 0 {
+				switch rec.Mode & cpio.S_IFMT {
+				// In any Unix past about V1, you can't do os.Link from user mode.
+				// Except via mkdir of course :-).
+				case cpio.S_IFDIR:
+				default:
+					// FileSize of non-zero means it is the first and possibly
+					// only instance of this file.
+					if rec.Info.FileSize != 0 {
+						break
+					}
+					// If the file is not in []inums it is a true zero-length file,
+					// not a hard link to a file already seen.
+					// (pedantic mode: on Unix all files are hard links;
+					// so what this comment really means is "file with more than one
+					// hard link).
+					ino, ok := inums[rec.Info.Ino]
+					if !ok {
+						break
+					}
+					err := os.Link(ino, rec.Name)
+					debug("Hard linking %s to %s", ino, rec.Name)
 					if err != nil {
 						log.Fatal(err)
 					}
 					continue
 				}
+				inums[rec.Info.Ino] = rec.Name
 			}
-			inums[rec.Info.Ino] = rec.Name
+			debug("Creating file %s", rec.Name)
 			if err := cpio.CreateFile(rec); err != nil {
 				log.Printf("Creating %q failed: %v", rec.Name, err)
 			}

cmds/core/cpio/cpio_test.go

@@ -25,6 +25,7 @@ type dirEnt struct {
 }
 
 func TestCpio(t *testing.T) {
+	debug = t.Logf
 	// Create a temporary directory
 	tempDir, err := ioutil.TempDir("", "TestCpio")
 	if err != nil {
@@ -35,7 +36,7 @@ func TestCpio(t *testing.T) {
 	var targets = []dirEnt{
 		{Name: "file1", Type: "file", Content: "Hello World"},
 		{Name: "file2", Type: "file", Content: ""},
-		{Name: "hardlinked", Type: "hardlink", Target: "file1"},
+		{Name: "hardlinked", Type: "hardlink", Target: "file1", Content: "Hello World"},
 		{Name: "hardlinkedtofile2", Type: "hardlink", Target: "file2"},
 		{Name: "directory1", Type: "dir"},
 	}
@@ -125,10 +126,12 @@ func TestCpio(t *testing.T) {
 		var name = filepath.Join(tempExtractDir, ent.Name)
 		newFileInfo, err := os.Stat(name)
 		if err != nil {
-			t.Fatal(err)
+			t.Error(err)
+			continue
 		}
 		newStatT := newFileInfo.Sys().(*syscall.Stat_t)
 		statT := ent.FileInfo.Sys().(*syscall.Stat_t)
+		t.Logf("Entry %v statT %v old ino %d new Ino %d", ent, newFileInfo, statT.Ino, newStatT.Ino)
 		if ent.FileInfo.Name() != newFileInfo.Name() ||
 			ent.FileInfo.Size() != newFileInfo.Size() ||
 			ent.FileInfo.Mode() != newFileInfo.Mode() ||
@@ -147,16 +150,16 @@ func TestCpio(t *testing.T) {
 			msg += fmt.Sprintf("Uid:     | %10d | %10d\n", statT.Uid, newStatT.Uid)
 			msg += fmt.Sprintf("Gid:     | %10d | %10d\n", statT.Gid, newStatT.Gid)
 			msg += fmt.Sprintf("NLink:   | %10d | %10d\n", statT.Nlink, newStatT.Nlink)
-			t.Fatal(msg)
+			t.Error(msg)
 		}
 
-		if ent.Type == "file" {
+		if ent.Type != "dir" {
 			content, err := ioutil.ReadFile(name)
 			if err != nil {
-				t.Fatal(err)
+				t.Error(err)
 			}
 			if string(content) != ent.Content {
-				t.Fatalf("File %s has mismatched contents", ent.Name)
+				t.Errorf("File %s has mismatched contents", ent.Name)
 			}
 		}
 	}

pkg/cpio/fs_plan9.go

@@ -142,7 +142,7 @@ func (r *Recorder) GetRecord(path string) (Record, error) {
 // single CPIO archive. Do not reuse between CPIOs if you don't know what
 // you're doing.
 func NewRecorder() *Recorder {
-	return &Recorder{}
+	return &Recorder{inumber: 2}
 }
 
 // LSInfoFromRecord converts a Record to be usable with the ls package for

pkg/cpio/fs_unix.go

@@ -256,7 +256,7 @@ func (r *Recorder) GetRecord(path string) (Record, error) {
 // single CPIO archive. Do not reuse between CPIOs if you don't know what
 // you're doing.
 func NewRecorder() *Recorder {
-	return &Recorder{make(map[devInode]Info), 0}
+	return &Recorder{make(map[devInode]Info), 2}
 }
 
 // LSInfoFromRecord converts a Record to be usable with the ls package for