[PATCH v3 0/5]: initramfs: "crc" cpio format and INITRAMFS_PRESERVE_MTIME

[PATCH v3 0/5]: initramfs: "crc" cpio format and INITRAMFS_PRESERVE_MTIME

[David Disseldorp]

This patchset does some minor refactoring of cpio header magic checking
and corrects documentation.
Patches 4/5 and 5/5 allow cpio entry mtime preservation to be disabled
via a new INITRAMFS_PRESERVE_MTIME Kconfig option.

Changes since previous version:
- curb enthusiasm in 1/5 commit message
- add cover letter

 .../early-userspace/buffer-format.rst         | 24 +++-----
 init/Kconfig                                  | 10 ++++
 init/Makefile                                 |  3 +
 init/initramfs.c                              | 57 +++----------------
 init/initramfs_mtime.c                        | 49 ++++++++++++++++
 init/initramfs_mtime.h                        | 11 ++++
 6 files changed, 88 insertions(+), 66 deletions(-)

[PATCH v3 1/5] initramfs: refactor do_header() cpio magic checks

[David Disseldorp]

do_header() is called for each cpio entry and fails if the first six
bytes don't match "newc" magic. The magic check includes a special case
error message if POSIX.1 ASCII (cpio -H odc) magic is detected. This
special case POSIX.1 check can be nested under the "newc" mismatch code
path to avoid calling memcmp() twice in a non-error case.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 init/initramfs.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index a842c0544745..6897994c60fb 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -257,12 +257,11 @@ static int __init do_collect(void)
 
 static int __init do_header(void)
 {
-	if (memcmp(collected, "070707", 6)==0) {
-		error("incorrect cpio method used: use -H newc option");
-		return 1;
-	}
 	if (memcmp(collected, "070701", 6)) {
-		error("no cpio magic");
+		if (memcmp(collected, "070707", 6) == 0)
+			error("incorrect cpio method used: use -H newc option");
+		else
+			error("no cpio magic");
 		return 1;
 	}
 	parse_header(collected);
-- 
2.31.1

[PATCH v3 2/5] initramfs: print helpful cpio error on "crc" magic

[David Disseldorp]

Contrary to the buffer-format.rst documentation, initramfs cpio
extraction does not support "crc" archives, which carry "070702"
header magic. Make it a little clearer that "newc" (magic="070701") is
the only supported cpio format, by extending the POSIX.1 ASCII
(magic="070707") specific error message to also cover "crc" magic.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 init/initramfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index 6897994c60fb..3bc90ab4e443 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -258,7 +258,7 @@ static int __init do_collect(void)
 static int __init do_header(void)
 {
 	if (memcmp(collected, "070701", 6)) {
-		if (memcmp(collected, "070707", 6) == 0)
+		if (memcmp(collected, "0707", 4) == 0)
 			error("incorrect cpio method used: use -H newc option");
 		else
 			error("no cpio magic");
-- 
2.31.1

[PATCH v3 3/5] docs: remove mention of "crc" cpio format support

[David Disseldorp]

init/initramfs.c only supports extraction of cpio archives carrying the
"newc" header magic ("070701"). Remove statements indicating support for
the "crc" cpio format.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 .../early-userspace/buffer-format.rst         | 24 +++++++------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/Documentation/driver-api/early-userspace/buffer-format.rst b/Documentation/driver-api/early-userspace/buffer-format.rst
index 7f74e301fdf3..0df76bca444c 100644
--- a/Documentation/driver-api/early-userspace/buffer-format.rst
+++ b/Documentation/driver-api/early-userspace/buffer-format.rst
@@ -14,10 +14,10 @@ is different.  The initramfs buffer contains an archive which is
 expanded into a ramfs filesystem; this document details the format of
 the initramfs buffer format.
 
-The initramfs buffer format is based around the "newc" or "crc" CPIO
-formats, and can be created with the cpio(1) utility.  The cpio
-archive can be compressed using gzip(1).  One valid version of an
-initramfs buffer is thus a single .cpio.gz file.
+The initramfs buffer format is based around the "newc" CPIO format, and
+can be created with the cpio(1) utility.  The cpio archive can be
+compressed using gzip(1).  One valid version of an initramfs buffer is
+thus a single .cpio.gz file.
 
 The full format of the initramfs buffer is defined by the following
 grammar, where::
@@ -40,9 +40,8 @@ grammar, where::
 
 
 In human terms, the initramfs buffer contains a collection of
-compressed and/or uncompressed cpio archives (in the "newc" or "crc"
-formats); arbitrary amounts zero bytes (for padding) can be added
-between members.
+compressed and/or uncompressed cpio archives (in the "newc" format),
+with arbitrary amount of zero-byte padding between members.
 
 The cpio "TRAILER!!!" entry (cpio end-of-archive) is optional, but is
 not ignored; see "handling of hard links" below.
@@ -55,7 +54,7 @@ by the ASCII string "000012ac"):
 ============= ================== ==============================================
 Field name    Field size	 Meaning
 ============= ================== ==============================================
-c_magic	      6 bytes		 The string "070701" or "070702"
+c_magic	      6 bytes		 The string "070701"
 c_ino	      8 bytes		 File inode number
 c_mode	      8 bytes		 File mode and permissions
 c_uid	      8 bytes		 File uid
@@ -68,8 +67,7 @@ c_min	      8 bytes		 Minor part of file device number
 c_rmaj	      8 bytes		 Major part of device node reference
 c_rmin	      8 bytes		 Minor part of device node reference
 c_namesize    8 bytes		 Length of filename, including final \0
-c_chksum      8 bytes		 Checksum of data field if c_magic is 070702;
-				 otherwise zero
+c_chksum      8 bytes		 Ignored; reserved for unsupported "crc" format
 ============= ================== ==============================================
 
 The c_mode field matches the contents of st_mode returned by stat(2)
@@ -78,12 +76,6 @@ on Linux, and encodes the file type and file permissions.
 The c_filesize should be zero for any file which is not a regular file
 or symlink.
 
-The c_chksum field contains a simple 32-bit unsigned sum of all the
-bytes in the data field.  cpio(1) refers to this as "crc", which is
-clearly incorrect (a cyclic redundancy check is a different and
-significantly stronger integrity check), however, this is the
-algorithm used.
-
 If the filename is "TRAILER!!!" this is actually an end-of-archive
 marker; the c_filesize for an end-of-archive marker must be zero.
 
-- 
2.31.1

[PATCH v3 4/5] initramfs: use do_utime() wrapper consistently

[David Disseldorp]

vfs_utimes() is called via the do_utime() wrapper everywhere except in
do_copy(). Make it consistent.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 init/initramfs.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index 3bc90ab4e443..c64f819ed120 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -379,13 +379,9 @@ static int __init do_name(void)
 static int __init do_copy(void)
 {
 	if (byte_count >= body_len) {
-		struct timespec64 t[2] = { };
 		if (xwrite(wfile, victim, body_len, &wfile_pos) != body_len)
 			error("write error");
-
-		t[0].tv_sec = mtime;
-		t[1].tv_sec = mtime;
-		vfs_utimes(&wfile->f_path, t);
+		do_utime(collected, mtime);
 
 		fput(wfile);
 		eat(body_len);
-- 
2.31.1

[PATCH v3 5/5] initramfs: add INITRAMFS_PRESERVE_MTIME Kconfig option

[David Disseldorp]

initramfs cpio mtime preservation, as implemented via
889d51a10712b6fd6175196626de2116858394f4, uses a linked list to defer
directory mtime processing until after all other items in the cpio
archive have been processed. This is done to ensure that parent
directory mtimes aren't overwritten via subsequent child creation.

This change adds a new INITRAMFS_PRESERVE_MTIME Kconfig option, which
can be used to disable on-by-default mtime retention and in turn
speed up initramfs extraction, particularly for cpio archives with large
directory counts.

For a cpio archive with ~1M directories, rough 20-run local benchmarks
demonstrated:
				mean extraction time (s)	std dev
INITRAMFS_PRESERVE_MTIME=y		3.789035		0.005474
INITRAMFS_PRESERVE_MTIME unset		3.111508		0.004132

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 init/Kconfig           | 10 +++++++++
 init/Makefile          |  3 +++
 init/initramfs.c       | 42 ++----------------------------------
 init/initramfs_mtime.c | 49 ++++++++++++++++++++++++++++++++++++++++++
 init/initramfs_mtime.h | 11 ++++++++++
 5 files changed, 75 insertions(+), 40 deletions(-)
 create mode 100644 init/initramfs_mtime.c
 create mode 100644 init/initramfs_mtime.h

diff --git a/init/Kconfig b/init/Kconfig
index 11f8a845f259..dc734d72a3c6 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1352,6 +1352,16 @@ config BOOT_CONFIG
 
 	  If unsure, say Y.
 
+config INITRAMFS_PRESERVE_MTIME
+	bool "Preserve cpio archive mtimes in initramfs"
+	default y
+	help
+	  Each entry in an initramfs cpio archive carries an mtime value. When
+	  enabled, extracted cpio items take this mtime, with directory mtime
+	  setting deferred until after creation of any child entries.
+
+	  If unsure, say Y.
+
 choice
 	prompt "Compiler optimization level"
 	default CC_OPTIMIZE_FOR_PERFORMANCE
diff --git a/init/Makefile b/init/Makefile
index 2846113677ee..d72bf80170ce 100644
--- a/init/Makefile
+++ b/init/Makefile
@@ -11,6 +11,9 @@ obj-y                          += noinitramfs.o
 else
 obj-$(CONFIG_BLK_DEV_INITRD)   += initramfs.o
 endif
+ifeq ($(CONFIG_INITRAMFS_PRESERVE_MTIME),y)
+obj-$(CONFIG_BLK_DEV_INITRD)   += initramfs_mtime.o
+endif
 obj-$(CONFIG_GENERIC_CALIBRATE_DELAY) += calibrate.o
 
 obj-y                          += init_task.o
diff --git a/init/initramfs.c b/init/initramfs.c
index c64f819ed120..aa0f63d9f570 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -17,6 +17,8 @@
 #include <linux/init_syscalls.h>
 #include <linux/umh.h>
 
+#include "initramfs_mtime.h"
+
 static ssize_t __init xwrite(struct file *file, const char *p, size_t count,
 		loff_t *pos)
 {
@@ -116,46 +118,6 @@ static void __init free_hash(void)
 	}
 }
 
-static long __init do_utime(char *filename, time64_t mtime)
-{
-	struct timespec64 t[2];
-
-	t[0].tv_sec = mtime;
-	t[0].tv_nsec = 0;
-	t[1].tv_sec = mtime;
-	t[1].tv_nsec = 0;
-	return init_utimes(filename, t);
-}
-
-static __initdata LIST_HEAD(dir_list);
-struct dir_entry {
-	struct list_head list;
-	char *name;
-	time64_t mtime;
-};
-
-static void __init dir_add(const char *name, time64_t mtime)
-{
-	struct dir_entry *de = kmalloc(sizeof(struct dir_entry), GFP_KERNEL);
-	if (!de)
-		panic_show_mem("can't allocate dir_entry buffer");
-	INIT_LIST_HEAD(&de->list);
-	de->name = kstrdup(name, GFP_KERNEL);
-	de->mtime = mtime;
-	list_add(&de->list, &dir_list);
-}
-
-static void __init dir_utime(void)
-{
-	struct dir_entry *de, *tmp;
-	list_for_each_entry_safe(de, tmp, &dir_list, list) {
-		list_del(&de->list);
-		do_utime(de->name, de->mtime);
-		kfree(de->name);
-		kfree(de);
-	}
-}
-
 static __initdata time64_t mtime;
 
 /* cpio header parsing */
diff --git a/init/initramfs_mtime.c b/init/initramfs_mtime.c
new file mode 100644
index 000000000000..0020deb21f76
--- /dev/null
+++ b/init/initramfs_mtime.c
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/syscalls.h>
+#include <linux/utime.h>
+#include <linux/file.h>
+#include <linux/init_syscalls.h>
+
+#include "initramfs_mtime.h"
+
+long __init do_utime(char *filename, time64_t mtime)
+{
+	struct timespec64 t[2];
+
+	t[0].tv_sec = mtime;
+	t[0].tv_nsec = 0;
+	t[1].tv_sec = mtime;
+	t[1].tv_nsec = 0;
+	return init_utimes(filename, t);
+}
+
+static __initdata LIST_HEAD(dir_list);
+struct dir_entry {
+	struct list_head list;
+	char *name;
+	time64_t mtime;
+};
+
+void __init dir_add(const char *name, time64_t mtime)
+{
+	struct dir_entry *de = kmalloc(sizeof(struct dir_entry), GFP_KERNEL);
+	if (!de)
+		panic("can't allocate dir_entry buffer");
+	INIT_LIST_HEAD(&de->list);
+	de->name = kstrdup(name, GFP_KERNEL);
+	de->mtime = mtime;
+	list_add(&de->list, &dir_list);
+}
+
+void __init dir_utime(void)
+{
+	struct dir_entry *de, *tmp;
+	list_for_each_entry_safe(de, tmp, &dir_list, list) {
+		list_del(&de->list);
+		do_utime(de->name, de->mtime);
+		kfree(de->name);
+		kfree(de);
+	}
+}
diff --git a/init/initramfs_mtime.h b/init/initramfs_mtime.h
new file mode 100644
index 000000000000..6d15c8b1171f
--- /dev/null
+++ b/init/initramfs_mtime.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifdef CONFIG_INITRAMFS_PRESERVE_MTIME
+long do_utime(char *filename, time64_t mtime) __init;
+void dir_add(const char *name, time64_t mtime) __init;
+void dir_utime(void) __init;
+#else
+static long __init do_utime(char *filename, time64_t mtime) { return 0; }
+static void __init dir_add(const char *name, time64_t mtime) {}
+static void __init dir_utime(void) {}
+#endif
-- 
2.31.1

Re: [PATCH v3 0/5]: initramfs: "crc" cpio format and INITRAMFS_PRESERVE_MTIME

[David Disseldorp]

On Fri,  1 Oct 2021 15:42:51 +0200, David Disseldorp wrote:

> This patchset does some minor refactoring of cpio header magic checking
> and corrects documentation.
> Patches 4/5 and 5/5 allow cpio entry mtime preservation to be disabled
> via a new INITRAMFS_PRESERVE_MTIME Kconfig option.

Ping, any feedback for these changes?

Cheers, David

Re: [PATCH v3 0/5]: initramfs: "crc" cpio format and INITRAMFS_PRESERVE_MTIME

[David Disseldorp]

On Fri,  1 Oct 2021 15:42:51 +0200, David Disseldorp wrote:

> This patchset does some minor refactoring of cpio header magic checking
> and corrects documentation.
> Patches 4/5 and 5/5 allow cpio entry mtime preservation to be disabled
> via a new INITRAMFS_PRESERVE_MTIME Kconfig option.

Ping again. I still think this patchset is worthwhile... Any feedback?

Cheers, David