[RFC][PATCH 0/9] extend initramfs archive format to support xattrs

[RFC][PATCH 0/9] extend initramfs archive format to support xattrs

[Mimi Zohar]

Many of the Linux security/integrity features are dependent on file
metadata, stored as extended attributes (xattrs), for making decisions.
These features need to be initialized during initcall and enabled as
early as possible for complete security coverage. 

The linux kernel creates the rootfs file system and extracts the contents
of the initramfs, a compressed CPIO archive, onto it. If CONFIG_TMPFS is
enabled (and "root=" is not specified on the boot command line), rootfs
will use tmpfs instead of ramfs by default.  Although the tmpfs filesystem
supports xattrs, the CPIO archive specification does not define a method
for including them in the archive.  Other archive formats have added xattr
support (eg. tar).

There are a couple of ways to include and label the rootfs filesystem:
- include a file manifest containing the xattrs in the initramfs
- extend CPIO to support xattrs
- add tar support

This patch set extends the existing newc CPIO archive format to include
xattrs in the initramfs.  The changes are limited to gen_init_cpio, the
kernel build tree tool used to generate an initramfs, and init/initramfs.c,
the parser, with minor changes to IMA and EVM.

Mimi


Mimi Zohar (9):
  initramfs: separate reading cpio method from header
  initramfs: add extended attribute support
  gen_init_cpio: replace inline format string with common variable
  gen_init_cpio: define new CPIO format to support xattrs
  gen_init_cpio: include the file extended attributes
  gen_initramfs_list.sh: include xattrs
  evm: make rootfs a special case
  ima: include tmpfs in ima_appraise_tcb policy
  init: remove "root=" command line option test for tmpfs decision

 init/do_mounts.c                    |   2 +-
 init/initramfs.c                    |  95 +++++++++++++++++++++---
 scripts/gen_initramfs_list.sh       |   2 +-
 security/integrity/evm/evm_main.c   |  12 ++-
 security/integrity/ima/ima_policy.c |   2 +
 usr/gen_init_cpio.c                 | 142 ++++++++++++++++++++++++++++++------
 6 files changed, 218 insertions(+), 37 deletions(-)

-- 
1.8.1.4

[RFC][PATCH 1/9] initramfs: separate reading cpio method from header

[Mimi Zohar]

In preparation for adding xattr support, read the CPIO method separately
from the rest of the header.

Signed-off-by: Mimi Zohar <zohar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 init/initramfs.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index bece48c..527703e 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -187,6 +187,7 @@ static void __init parse_header(char *s)
 
 static __initdata enum state {
 	Start,
+	GotFormat,
 	Collect,
 	GotHeader,
 	SkipIt,
@@ -230,7 +231,7 @@ static __initdata char *header_buf, *symlink_buf, *name_buf;
 
 static int __init do_start(void)
 {
-	read_into(header_buf, 110, GotHeader);
+	read_into(header_buf, 6, GotFormat);
 	return 0;
 }
 
@@ -248,7 +249,7 @@ static int __init do_collect(void)
 	return 0;
 }
 
-static int __init do_header(void)
+static int __init do_format(void)
 {
 	if (memcmp(collected, "070707", 6)==0) {
 		error("incorrect cpio method used: use -H newc option");
@@ -258,6 +259,12 @@ static int __init do_header(void)
 		error("no cpio magic");
 		return 1;
 	}
+	read_into(header_buf, 104, GotHeader);
+	return 0;
+}
+
+static int __init do_header(void)
+{
 	parse_header(collected);
 	next_header = this_header + N_ALIGN(name_len) + body_len;
 	next_header = (next_header + 3) & ~3;
@@ -400,6 +407,7 @@ static int __init do_symlink(void)
 
 static __initdata int (*actions[])(void) = {
 	[Start]		= do_start,
+	[GotFormat]	= do_format,
 	[Collect]	= do_collect,
 	[GotHeader]	= do_header,
 	[SkipIt]	= do_skip,
-- 
1.8.1.4

[RFC][PATCH 5/9] gen_init_cpio: include the file extended attributes

[Mimi Zohar]

This patch reads the xattr(s), creating a buffer containing the
number of xattrs, the xattr(s) name, data size, and data. This
buffer size is included in the CPIO header.  The buffer is written
out to the cpio file after the file name.

This patch also defines the '-x' option to enable the inclusion
of the xattrs.

Signed-off-by: Mimi Zohar <zohar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 usr/gen_init_cpio.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 100 insertions(+), 7 deletions(-)

diff --git a/usr/gen_init_cpio.c b/usr/gen_init_cpio.c
index 0d9c6e8..08994d6 100644
--- a/usr/gen_init_cpio.c
+++ b/usr/gen_init_cpio.c
@@ -9,6 +9,7 @@
 #include <errno.h>
 #include <ctype.h>
 #include <limits.h>
+#include <attr/xattr.h>
 
 /*
  * Original work by Jeff Garzik
@@ -36,6 +37,71 @@ struct file_handler {
 	int (*handler)(const char *line);
 };
 
+#define MAX_XATTRNAMES_SIZE 500
+static char xattr_names[MAX_XATTRNAMES_SIZE];
+static char xattr_header[8];	/* number xattrs */
+static ssize_t xattr_nameslen;
+static unsigned int xattrs_buflen;
+
+static char xattr_buf[1000];
+static unsigned int get_xattrs(const char *name)
+{
+    	char xattr_num[9];
+    	char *xname, *buf, *bufend;
+	int xattrsize = 0, num_xattrs = 0;
+
+	xattr_nameslen = listxattr(name, NULL, 0);
+	if (xattr_nameslen <= 0 || xattr_nameslen > MAX_XATTRNAMES_SIZE)
+		return 0;
+
+	xattr_names[xattr_nameslen] = 0;
+	xattr_nameslen = listxattr(name, xattr_names, xattr_nameslen);
+	if (xattr_nameslen <= 0)
+		return 0;
+
+	/* xattr format: name value-len value */
+	buf = xattr_buf + sizeof xattr_header;
+	bufend = xattr_buf + sizeof xattr_buf;
+
+	for (xname = xattr_names; xname < (xattr_names + xattr_nameslen);
+		xname += strlen(xname) + 1) {
+		char sizebuf[9];
+		int offset;
+
+		/* skip security.evm as it is file system specific */
+		if (strcmp(xname, "security.evm") == 0)
+			continue;
+
+		offset = strlen(xname) + 1 + 8;
+		xattrsize = getxattr(name, xname, NULL, 0);
+		if (buf + offset + xattrsize > bufend) {
+			fprintf(stderr, "%s: xattrs too large \n", name);
+			return 0;
+		}
+
+		xattrsize = getxattr(name, xname, buf + offset,
+				     bufend - (buf + offset));
+		if (xattrsize <= 0)
+			continue;
+		
+		num_xattrs++;
+		fprintf(stderr, "%s: %s %x (%d)\n", name, xname, xattrsize,
+			num_xattrs);
+		strcpy(buf, xname);
+		buf += strlen(xname) + 1;
+		sprintf(sizebuf, "%08X", (int)xattrsize); 
+		memcpy(buf, sizebuf, 8);
+		buf += (8 + xattrsize);
+	}
+
+	*buf = 0;
+	buf++;
+	sprintf(xattr_num, "%08X", num_xattrs);
+	memcpy(xattr_buf, xattr_num, 8);
+
+	return buf - xattr_buf;
+}
+
 static void push_string(const char *name)
 {
 	unsigned int name_len = strlen(name) + 1;
@@ -106,11 +172,24 @@ static void cpio_trailer(void)
 	}
 }
 
+static void include_xattrs(void)
+{
+	if (!xattrs_buflen)
+		return;
+	
+	if (fwrite(xattr_buf, xattrs_buflen, 1, stdout) != 1)
+		fprintf(stderr, "writing xattrs failed\n");
+	offset += xattrs_buflen;
+
+	push_pad();
+}
+
 static int cpio_mkslink(const char *name, const char *target,
 			 unsigned int mode, uid_t uid, gid_t gid)
 {
 	char s[256];
 
+	xattrs_buflen = newcx ? get_xattrs(name) : 0;
 	if (name[0] == '/')
 		name++;
 	sprintf(s, newcx ? newcxfmt : newcfmt,
@@ -127,13 +206,15 @@ static int cpio_mkslink(const char *name, const char *target,
 		0,			/* rmajor */
 		0,			/* rminor */
 		(unsigned)strlen(name) + 1,/* namesize */
-		0,			/* xattrs-size */
+		xattrs_buflen,		/* xattrs-size */
 		0);			/* chksum */
 	push_hdr(s);
 	push_string(name);
 	push_pad();
 	push_string(target);
 	push_pad();
+	if (newcx)
+		include_xattrs();
 	return 0;
 }
 
@@ -160,6 +241,7 @@ static int cpio_mkgeneric(const char *name, unsigned int mode,
 {
 	char s[256];
 
+	xattrs_buflen = newcx ? get_xattrs(name) : 0;
 	if (name[0] == '/')
 		name++;
 	sprintf(s, newcx ? newcxfmt : newcfmt,
@@ -176,10 +258,12 @@ static int cpio_mkgeneric(const char *name, unsigned int mode,
 		0,			/* rmajor */
 		0,			/* rminor */
 		(unsigned)strlen(name) + 1,/* namesize */
-		0,			/* xattrs-size */
+		xattrs_buflen,		/* xattrs-size */
 		0);			/* chksum */
 	push_hdr(s);
 	push_rest(name);
+	if (newcx)
+		include_xattrs();
 	return 0;
 }
 
@@ -339,9 +423,14 @@ static int cpio_mkfile(const char *name, const char *location,
 	}
 
 	size = 0;
+	xattrs_buflen = 0;
 	for (i = 1; i <= nlinks; i++) {
 		/* data goes on last link */
-		if (i == nlinks) size = buf.st_size;
+		if (i == nlinks) {
+			size = buf.st_size;
+			if (newcx)
+				xattrs_buflen = get_xattrs(location);
+		}
 
 		if (name[0] == '/')
 			name++;
@@ -360,12 +449,13 @@ static int cpio_mkfile(const char *name, const char *location,
 			0,			/* rmajor */
 			0,			/* rminor */
 			namesize,		/* namesize */
-			0,			/* xattrs-size */
+			xattrs_buflen,		/* xattrs-size */
 			0);			/* chksum */
 		push_hdr(s);
 		push_string(name);
 		push_pad();
-
+		if (newcx)
+			include_xattrs();
 		if (size) {
 			if (fwrite(filebuf, size, 1, stdout) != 1) {
 				fprintf(stderr, "writing filebuf failed\n");
@@ -458,7 +548,7 @@ static int cpio_mkfile_line(const char *line)
 static void usage(const char *prog)
 {
 	fprintf(stderr, "Usage:\n"
-		"\t%s [-t <timestamp>] <cpio_list>\n"
+		"\t%s [-t <timestamp>] [-x] <cpio_list>\n"
 		"\n"
 		"<cpio_list> is a file containing newline separated entries that\n"
 		"describe the files to be included in the initramfs archive:\n"
@@ -535,7 +625,7 @@ int main (int argc, char *argv[])
 
 	default_mtime = time(NULL);
 	while (1) {
-		int opt = getopt(argc, argv, "t:h");
+		int opt = getopt(argc, argv, "t:h:x");
 		char *invalid;
 
 		if (opt == -1)
@@ -550,6 +640,9 @@ int main (int argc, char *argv[])
 				exit(1);
 			}
 			break;
+		case 'x':
+			newcx = 1;
+			break;
 		case 'h':
 		case '?':
 			usage(argv[0]);
-- 
1.8.1.4

[RFC][PATCH 2/9] initramfs: add extended attribute support

[Mimi Zohar]

This patch writes out the extended attributes included in the cpio file.
As the "security.ima" xattr needs to be written after the file data,
this patch separates extracting and setting the xattrs by defining two
new states "GotXattrs" and "SetXattrs".

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 init/initramfs.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 75 insertions(+), 10 deletions(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index 527703e..11ffd3e 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -52,6 +52,7 @@ static void __init error(char *x)
 /* link hash */
 
 #define N_ALIGN(len) ((((len) + 1) & ~3) + 2)
+#define X_ALIGN(len) ((len + 3) & ~3)
 
 static __initdata struct hash {
 	int ino, minor, major;
@@ -154,19 +155,20 @@ static __initdata time_t mtime;
 
 static __initdata unsigned long ino, major, minor, nlink;
 static __initdata umode_t mode;
-static __initdata unsigned long body_len, name_len;
+static __initdata unsigned long body_len, name_len, xattr_buflen;
 static __initdata uid_t uid;
 static __initdata gid_t gid;
 static __initdata unsigned rdev;
+static __initdata int newcx;
 
 static void __init parse_header(char *s)
 {
-	unsigned long parsed[12];
+	unsigned long parsed[13];
 	char buf[9];
 	int i;
 
 	buf[8] = '\0';
-	for (i = 0, s += 6; i < 12; i++, s += 8) {
+	for (i = 0; i < (!newcx ? 12 : 13); i++, s += 8) {
 		memcpy(buf, s, 8);
 		parsed[i] = simple_strtoul(buf, NULL, 16);
 	}
@@ -181,6 +183,7 @@ static void __init parse_header(char *s)
 	minor = parsed[8];
 	rdev = new_encode_dev(MKDEV(parsed[9], parsed[10]));
 	name_len = parsed[11];
+	xattr_buflen = newcx ? parsed[12] : 0;
 }
 
 /* FSM */
@@ -192,7 +195,9 @@ static __initdata enum state {
 	GotHeader,
 	SkipIt,
 	GotName,
+	GotXattrs,
 	CopyFile,
+	SetXattrs,
 	GotSymlink,
 	Reset
 } state, next_state;
@@ -209,6 +214,8 @@ static inline void __init eat(unsigned n)
 }
 
 static __initdata char *vcollected;
+static __initdata char *ncollected;
+static __initdata char *xcollected;
 static __initdata char *collected;
 static long remains __initdata;
 static __initdata char *collect;
@@ -227,7 +234,7 @@ static void __init read_into(char *buf, unsigned size, enum state next)
 	}
 }
 
-static __initdata char *header_buf, *symlink_buf, *name_buf;
+static __initdata char *header_buf, *symlink_buf, *name_buf, *xattr_buf;
 
 static int __init do_start(void)
 {
@@ -251,22 +258,26 @@ static int __init do_collect(void)
 
 static int __init do_format(void)
 {
+	newcx = 0;
 	if (memcmp(collected, "070707", 6)==0) {
 		error("incorrect cpio method used: use -H newc option");
 		return 1;
 	}
-	if (memcmp(collected, "070701", 6)) {
+	if (memcmp(collected, "070703", 6) == 0)
+		newcx = 1;
+	else if (memcmp(collected, "070701", 6)) {
 		error("no cpio magic");
 		return 1;
 	}
-	read_into(header_buf, 104, GotHeader);
+	read_into(header_buf, !newcx ? 104: 112, GotHeader);
 	return 0;
 }
 
 static int __init do_header(void)
 {
 	parse_header(collected);
-	next_header = this_header + N_ALIGN(name_len) + body_len;
+	next_header = this_header + N_ALIGN(name_len) + X_ALIGN(xattr_buflen)
+	    + body_len;
 	next_header = (next_header + 3) & ~3;
 	state = SkipIt;
 	if (name_len <= 0 || name_len > PATH_MAX)
@@ -328,8 +339,52 @@ static void __init clean_path(char *path, umode_t mode)
 	}
 }
 
-static __initdata int wfd;
+static int __init do_xattrs(void)
+{
+	state = next_state;
+	xcollected = kmalloc(xattr_buflen, GFP_KERNEL);
+	if (!xcollected)
+		panic("can't allocate xattr buffer");
+	memcpy(xcollected, collected, xattr_buflen);
+	return 0;
+}
+
+static int __init do_setxattrs(void)
+{
+	char *xattr_name = NULL;
+	int i, offset = 8, num_xattrs = 0;
+	unsigned xattr_value_size;
+	char *buf = xcollected;
+
+	state = SkipIt;
+	next_state = Reset;
 
+	if (!newcx || xattr_buflen == 0 || !buf)
+		return 0;
+
+	sscanf(buf, "%08X", &num_xattrs);
+
+	/* xattr format: name value-len value */
+	for (i = 0; i < num_xattrs; i++) {
+		void *xattr_buf;
+		int rc;
+
+		xattr_name = buf + offset;
+		offset += (strlen(xattr_name) + 1);
+		sscanf(buf + offset, "%08X", &xattr_value_size);
+		xattr_buf = buf + offset + 8;
+		rc = sys_setxattr(ncollected, xattr_name, xattr_buf,
+				  xattr_value_size, 0);
+		pr_debug("%s: %s size: %u (rc: %d)\n", ncollected, xattr_name,
+			xattr_value_size, rc);
+		offset += (8 + xattr_value_size);
+	}
+	kfree(ncollected);
+	kfree(xcollected);
+	return 0;
+}
+
+static __initdata int wfd;
 static int __init do_name(void)
 {
 	state = SkipIt;
@@ -370,6 +425,12 @@ static int __init do_name(void)
 			do_utime(collected, mtime);
 		}
 	}
+
+	if (xattr_buflen > 0) {
+		ncollected = kstrdup(collected, GFP_KERNEL);
+		next_state = (state == SkipIt) ? SetXattrs : state;
+		read_into(xattr_buf, X_ALIGN(xattr_buflen), GotXattrs);
+	}
 	return 0;
 }
 
@@ -382,7 +443,7 @@ static int __init do_copy(void)
 		do_utime(vcollected, mtime);
 		kfree(vcollected);
 		eat(body_len);
-		state = SkipIt;
+		state = (newcx && xattr_buflen > 0)? SetXattrs : SkipIt;
 		return 0;
 	} else {
 		if (xwrite(wfd, victim, count) != count)
@@ -412,7 +473,9 @@ static __initdata int (*actions[])(void) = {
 	[GotHeader]	= do_header,
 	[SkipIt]	= do_skip,
 	[GotName]	= do_name,
+	[GotXattrs]	= do_xattrs,
 	[CopyFile]	= do_copy,
+	[SetXattrs]	= do_setxattrs,
 	[GotSymlink]	= do_symlink,
 	[Reset]		= do_reset,
 };
@@ -461,9 +524,10 @@ static char * __init unpack_to_rootfs(char *buf, unsigned long len)
 	const char *compress_name;
 	static __initdata char msg_buf[64];
 
-	header_buf = kmalloc(110, GFP_KERNEL);
+	header_buf = kmalloc(118, GFP_KERNEL);
 	symlink_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, GFP_KERNEL);
 	name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL);
+	xattr_buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
 
 	if (!header_buf || !symlink_buf || !name_buf)
 		panic("can't allocate buffers");
@@ -510,6 +574,7 @@ static char * __init unpack_to_rootfs(char *buf, unsigned long len)
 		len -= my_inptr;
 	}
 	dir_utime();
+	kfree(xattr_buf);
 	kfree(name_buf);
 	kfree(symlink_buf);
 	kfree(header_buf);
-- 
1.8.1.4

[RFC][PATCH 3/9] gen_init_cpio: replace inline format string with common variable

[Mimi Zohar]

The same printf format string is used in a number of places.  This
patch replaces the inline format string with a single common variable
called newcformat.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 usr/gen_init_cpio.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/usr/gen_init_cpio.c b/usr/gen_init_cpio.c
index 225ad24..ee35361 100644
--- a/usr/gen_init_cpio.c
+++ b/usr/gen_init_cpio.c
@@ -20,6 +20,9 @@
 #define xstr(s) #s
 #define str(s) xstr(s)
 
+static char *newcfmt = "%s%08X%08X%08lX%08lX%08X%08lX"
+			"%08X%08X%08X%08X%08X%08X%08X";
+
 static unsigned int offset;
 static unsigned int ino = 721;
 static time_t default_mtime;
@@ -74,8 +77,7 @@ static void cpio_trailer(void)
 	char s[256];
 	const char name[] = "TRAILER!!!";
 
-	sprintf(s, "%s%08X%08X%08lX%08lX%08X%08lX"
-	       "%08X%08X%08X%08X%08X%08X%08X",
+	sprintf(s, newcfmt,
 		"070701",		/* magic */
 		0,			/* ino */
 		0,			/* mode */
@@ -106,8 +108,7 @@ static int cpio_mkslink(const char *name, const char *target,
 
 	if (name[0] == '/')
 		name++;
-	sprintf(s,"%s%08X%08X%08lX%08lX%08X%08lX"
-	       "%08X%08X%08X%08X%08X%08X%08X",
+	sprintf(s, newcfmt,
 		"070701",		/* magic */
 		ino++,			/* ino */
 		S_IFLNK | mode,		/* mode */
@@ -155,8 +156,7 @@ static int cpio_mkgeneric(const char *name, unsigned int mode,
 
 	if (name[0] == '/')
 		name++;
-	sprintf(s,"%s%08X%08X%08lX%08lX%08X%08lX"
-	       "%08X%08X%08X%08X%08X%08X%08X",
+	sprintf(s, newcfmt,
 		"070701",		/* magic */
 		ino++,			/* ino */
 		mode,			/* mode */
@@ -249,8 +249,7 @@ static int cpio_mknod(const char *name, unsigned int mode,
 
 	if (name[0] == '/')
 		name++;
-	sprintf(s,"%s%08X%08X%08lX%08lX%08X%08lX"
-	       "%08X%08X%08X%08X%08X%08X%08X",
+	sprintf(s, newcfmt,
 		"070701",		/* magic */
 		ino++,			/* ino */
 		mode,			/* mode */
@@ -339,8 +338,7 @@ static int cpio_mkfile(const char *name, const char *location,
 		if (name[0] == '/')
 			name++;
 		namesize = strlen(name) + 1;
-		sprintf(s,"%s%08X%08X%08lX%08lX%08X%08lX"
-		       "%08lX%08X%08X%08X%08X%08X%08X",
+		sprintf(s, newcfmt,
 			"070701",		/* magic */
 			ino,			/* ino */
 			mode,			/* mode */
-- 
1.8.1.4

[RFC][PATCH 4/9] gen_init_cpio: define new CPIO format to support xattrs

[Mimi Zohar]

This patch defines a new CPIO method 070703 for including xattrs.
The new format extends the existing NEWC header to include the
buffer size containing the number of xattrs, the xattr(s) name,
data size, and data.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 usr/gen_init_cpio.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/usr/gen_init_cpio.c b/usr/gen_init_cpio.c
index ee35361..0d9c6e8 100644
--- a/usr/gen_init_cpio.c
+++ b/usr/gen_init_cpio.c
@@ -21,7 +21,11 @@
 #define str(s) xstr(s)
 
 static char *newcfmt = "%s%08X%08X%08lX%08lX%08X%08lX"
-			"%08X%08X%08X%08X%08X%08X%08X";
+			"%08X%08X%08X%08X%08X%08X%15$08X";
+static char *newcxfmt = "%s%08X%08X%08lX%08lX%08X%08lX"
+			"%08X%08X%08X%08X%08X%08X%08X%08X";
+
+static int newcx;
 
 static unsigned int offset;
 static unsigned int ino = 721;
@@ -58,7 +62,7 @@ static void push_rest(const char *name)
 	putchar(0);
 	offset += name_len;
 
-	tmp_ofs = name_len + 110;
+	tmp_ofs = name_len + (newcx ? 118 : 110);
 	while (tmp_ofs & 3) {
 		putchar(0);
 		offset++;
@@ -69,7 +73,7 @@ static void push_rest(const char *name)
 static void push_hdr(const char *s)
 {
 	fputs(s, stdout);
-	offset += 110;
+	offset += newcx ? 118 : 110;
 }
 
 static void cpio_trailer(void)
@@ -77,8 +81,8 @@ static void cpio_trailer(void)
 	char s[256];
 	const char name[] = "TRAILER!!!";
 
-	sprintf(s, newcfmt,
-		"070701",		/* magic */
+	sprintf(s, newcx ? newcxfmt : newcfmt,
+		newcx ? "070703": "070701",/* magic */
 		0,			/* ino */
 		0,			/* mode */
 		(long) 0,		/* uid */
@@ -91,6 +95,7 @@ static void cpio_trailer(void)
 		0,			/* rmajor */
 		0,			/* rminor */
 		(unsigned)strlen(name)+1, /* namesize */
+		0,			/* xattrs-size */
 		0);			/* chksum */
 	push_hdr(s);
 	push_rest(name);
@@ -108,8 +113,8 @@ static int cpio_mkslink(const char *name, const char *target,
 
 	if (name[0] == '/')
 		name++;
-	sprintf(s, newcfmt,
-		"070701",		/* magic */
+	sprintf(s, newcx ? newcxfmt : newcfmt,
+		newcx ? "070703": "070701",/* magic */
 		ino++,			/* ino */
 		S_IFLNK | mode,		/* mode */
 		(long) uid,		/* uid */
@@ -122,6 +127,7 @@ static int cpio_mkslink(const char *name, const char *target,
 		0,			/* rmajor */
 		0,			/* rminor */
 		(unsigned)strlen(name) + 1,/* namesize */
+		0,			/* xattrs-size */
 		0);			/* chksum */
 	push_hdr(s);
 	push_string(name);
@@ -156,8 +162,8 @@ static int cpio_mkgeneric(const char *name, unsigned int mode,
 
 	if (name[0] == '/')
 		name++;
-	sprintf(s, newcfmt,
-		"070701",		/* magic */
+	sprintf(s, newcx ? newcxfmt : newcfmt,
+		newcx ? "070703": "070701",/* magic */
 		ino++,			/* ino */
 		mode,			/* mode */
 		(long) uid,		/* uid */
@@ -170,6 +176,7 @@ static int cpio_mkgeneric(const char *name, unsigned int mode,
 		0,			/* rmajor */
 		0,			/* rminor */
 		(unsigned)strlen(name) + 1,/* namesize */
+		0,			/* xattrs-size */
 		0);			/* chksum */
 	push_hdr(s);
 	push_rest(name);
@@ -249,8 +256,8 @@ static int cpio_mknod(const char *name, unsigned int mode,
 
 	if (name[0] == '/')
 		name++;
-	sprintf(s, newcfmt,
-		"070701",		/* magic */
+	sprintf(s, newcx ? newcxfmt : newcfmt,
+		newcx ? "070703": "070701",/* magic */
 		ino++,			/* ino */
 		mode,			/* mode */
 		(long) uid,		/* uid */
@@ -263,6 +270,7 @@ static int cpio_mknod(const char *name, unsigned int mode,
 		maj,			/* rmajor */
 		min,			/* rminor */
 		(unsigned)strlen(name) + 1,/* namesize */
+		0,			/* xattrs-size */
 		0);			/* chksum */
 	push_hdr(s);
 	push_rest(name);
@@ -338,8 +346,8 @@ static int cpio_mkfile(const char *name, const char *location,
 		if (name[0] == '/')
 			name++;
 		namesize = strlen(name) + 1;
-		sprintf(s, newcfmt,
-			"070701",		/* magic */
+		sprintf(s, newcx ? newcxfmt : newcfmt,
+			newcx ? "070703": "070701",/* magic */
 			ino,			/* ino */
 			mode,			/* mode */
 			(long) uid,		/* uid */
@@ -352,6 +360,7 @@ static int cpio_mkfile(const char *name, const char *location,
 			0,			/* rmajor */
 			0,			/* rminor */
 			namesize,		/* namesize */
+			0,			/* xattrs-size */
 			0);			/* chksum */
 		push_hdr(s);
 		push_string(name);
-- 
1.8.1.4

[RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Mimi Zohar]

This patch modifies the gen_initramfs_list.sh script to include xattrs
in the initramfs.

Dracut creates the initramfs using the cpio tool on the system, not
the kernel's gen_init_cpio script. The following commands, for example,
would create an initramfs containing xattrs.

dracut -H -f /boot/initramfs-3.XX.0+.img 3.XX.0+ -M --keep \
	--noprelink --nostrip
gen_initramfs_list.sh /var/tmp/initramfs.XXXXXX/ > \
	/var/tmp/initramfs_list.XXXXXX

[Sign files here, if not already signed, using evmctl.]

gen_init_cpio -x /var/tmp/initramfs_list.XXXXXX >  \
	/boot/initramfs-3.XX.0+test.img

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 scripts/gen_initramfs_list.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/gen_initramfs_list.sh b/scripts/gen_initramfs_list.sh
index 17fa901..20c3a80 100644
--- a/scripts/gen_initramfs_list.sh
+++ b/scripts/gen_initramfs_list.sh
@@ -307,7 +307,7 @@ if [ ! -z ${output_file} ]; then
 			fi
 		fi
 		cpio_tfile="$(mktemp ${TMPDIR:-/tmp}/cpiofile.XXXXXX)"
-		usr/gen_init_cpio $timestamp ${cpio_list} > ${cpio_tfile}
+		usr/gen_init_cpio $timestamp -x ${cpio_list} > ${cpio_tfile}
 	else
 		cpio_tfile=${cpio_file}
 	fi
-- 
1.8.1.4

Re: [RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Josh Boyer]

On Wed, Jan 7, 2015 at 3:52 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> This patch modifies the gen_initramfs_list.sh script to include xattrs
> in the initramfs.
>
> Dracut creates the initramfs using the cpio tool on the system, not
> the kernel's gen_init_cpio script. The following commands, for example,
> would create an initramfs containing xattrs.
>
> dracut -H -f /boot/initramfs-3.XX.0+.img 3.XX.0+ -M --keep \
>         --noprelink --nostrip
> gen_initramfs_list.sh /var/tmp/initramfs.XXXXXX/ > \
>         /var/tmp/initramfs_list.XXXXXX
>
> [Sign files here, if not already signed, using evmctl.]
>
> gen_init_cpio -x /var/tmp/initramfs_list.XXXXXX >  \
>         /boot/initramfs-3.XX.0+test.img

That's pretty awkward.  I think it highlights the major downside of
this approach in that from a standard distro point of view this
functionality isn't likely to be used.  Do you foresee this feature as
something that should be widely used, or something that would be used
more in custom, locked-down machines?

I can understand not wanting to redefine the newc format in userspace
cpio, but if you want this to be easier to use then perhaps working
with dracut upstream to make it support this out of the box would be a
good idea.

josh

Re: [RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Mimi Zohar]

On Thu, 2015-01-08 at 09:01 -0500, Josh Boyer wrote: 
> On Wed, Jan 7, 2015 at 3:52 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > This patch modifies the gen_initramfs_list.sh script to include xattrs
> > in the initramfs.
> >
> > Dracut creates the initramfs using the cpio tool on the system, not
> > the kernel's gen_init_cpio script. The following commands, for example,
> > would create an initramfs containing xattrs.
> >
> > dracut -H -f /boot/initramfs-3.XX.0+.img 3.XX.0+ -M --keep \
> >         --noprelink --nostrip
> > gen_initramfs_list.sh /var/tmp/initramfs.XXXXXX/ > \
> >         /var/tmp/initramfs_list.XXXXXX
> >
> > [Sign files here, if not already signed, using evmctl.]
> >
> > gen_init_cpio -x /var/tmp/initramfs_list.XXXXXX >  \
> >         /boot/initramfs-3.XX.0+test.img
> 
> That's pretty awkward.  I think it highlights the major downside of
> this approach in that from a standard distro point of view this
> functionality isn't likely to be used.  Do you foresee this feature as
> something that should be widely used, or something that would be used
> more in custom, locked-down machines?

Before distros can start enabling these features, software packages need
to come with file signatures.  Fin Gunter posted (and shortly will
re-post) patches to include file signatures in RPM patches.

Including file signatures in RPM packages (and similarly in other
software package formats) is the direction we, the linux community, IMHO
should be moving.  How long this will take is entirely up to the
distros.

> I can understand not wanting to redefine the newc format in userspace
> cpio, but if you want this to be easier to use then perhaps working
> with dracut upstream to make it support this out of the box would be a
> good idea.

Anyone using dracut/systemd is currently not using tmpfs, as specifying
"root=" on the boot command line reverts to using ramfs.  Rob Landley
suggested userspace apps use "ROOT=" instead.
(http://sourceforge.net/p/linux-ima/mailman/message/33189705/)

This patch set was posted as an RFC.  Assuming this solution for
including xattrs in the rootfs is acceptable, I'll post the
dracut/systemd changes.

Mimi

Re: [RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Rob Landley]

On 01/08/2015 09:13 AM, Mimi Zohar wrote:
> On Thu, 2015-01-08 at 09:01 -0500, Josh Boyer wrote: 
>> On Wed, Jan 7, 2015 at 3:52 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> That's pretty awkward.  I think it highlights the major downside of
>> this approach in that from a standard distro point of view this
>> functionality isn't likely to be used.  Do you foresee this feature as
>> something that should be widely used, or something that would be used
>> more in custom, locked-down machines?
> 
> Before distros can start enabling these features, software packages need
> to come with file signatures.  Fin Gunter posted (and shortly will
> re-post) patches to include file signatures in RPM patches.

My personal lack of caring about Red Hat's bureaucratic "signing
binaries in triplicate" is probably large enough to be seen from space
(obviously no vendor code has ever contained an exploit that could be
used to run arbitrary code in ring 0, and this totally won't be used for
vendor lock-in, but I remain unconvinced because I'm funny that way)...

But I am curious about how you propose to encode xattrs into the cpio
format. (Which Al Viro chose because it's _simple_. There isn't really a
controlling spec since Posix decided to deprecated it in 2001 and  yank
it from SUSv3 onwards. LSB extended several header fields to 8 hex
digits instead of 6, but they still have 32 bit timestamps which seems a
bit short-sighted. If you're going to define a new rev with a new magic
number, there are a couple other things you might wanna fix...)

I ask because I maintain a new from-scratch cpio implementation
(http://landley.net/hg/toybox/file/1571/toys/posix/cpio.c), so I'd
presumably have to add your format extensions to this. Is there any sort
of documentation on them?

The toybox config Android is using has this cpio implementation enabled
(see
https://android.googlesource.com/platform/external/toybox/+/9250c95a8c47/Android.mk)
so I'd rather like to get this sort of detail right...

> Including file signatures in RPM packages (and similarly in other
> software package formats) is the direction we, the linux community, IMHO
> should be moving.  How long this will take is entirely up to the
> distros.

Glued down to a trusted platform module such that obviously nobody can
possibly exploit such a system, from
https://www.youtube.com/watch?v=4loZGYqaZ7I to
https://trmm.net/Thunderstrike_31c3

I see this as way, way more about vendor lock-in than security.

>> I can understand not wanting to redefine the newc format in userspace
>> cpio, but if you want this to be easier to use then perhaps working
>> with dracut upstream to make it support this out of the box would be a
>> good idea.
> 
> Anyone using dracut/systemd is currently not using tmpfs, as specifying
> "root=" on the boot command line reverts to using ramfs.  Rob Landley
> suggested userspace apps use "ROOT=" instead.
> (http://sourceforge.net/p/linux-ima/mailman/message/33189705/)

I'm working on a documentation update, but the old docs I wrote have
gone a bit stale in a number of places so I'm not done yet...

> This patch set was posted as an RFC.  Assuming this solution for
> including xattrs in the rootfs is acceptable, I'll post the
> dracut/systemd changes.

(I'm not particularly interested in systemd either, but good luck with
that...)

> Mimi

Rob

Re: [RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Mimi Zohar]

On Thu, 2015-01-08 at 12:19 -0600, Rob Landley wrote:
> 
> But I am curious about how you propose to encode xattrs into the cpio
> format. (Which Al Viro chose because it's _simple_. There isn't really
> a
> controlling spec since Posix decided to deprecated it in 2001 and
> yank
> it from SUSv3 onwards. LSB extended several header fields to 8 hex
> digits instead of 6, but they still have 32 bit timestamps which seems
> a
> bit short-sighted. If you're going to define a new rev with a new
> magic
> number, there are a couple other things you might wanna fix...)

Sounds like a good opportunity to make the other changes as well.  We
can include the other changes in this patch set.  Is this (initramfs)
the right mailing list for this discussion?  Do other people need to be
included?

> I ask because I maintain a new from-scratch cpio implementation
> (http://landley.net/hg/toybox/file/1571/toys/posix/cpio.c), so I'd
> presumably have to add your format extensions to this. Is there any
> sort
> of documentation on them?
> 
> The toybox config Android is using has this cpio implementation
> enabled
> (see
> https://android.googlesource.com/platform/external/toybox/+/9250c95a8c47/Android.mk)
> so I'd rather like to get this sort of detail right...

The xattr section, which follows the file name, is of the format:
<number of xattrs> { <xattr name> <xattr data size> <xattr data> } for
each xattr, terminated with a NULL byte and padded to a 4 byte boundary.

The header contains an additional field, before the checksum, containing
the xattr section length, including the NULL byte, but without the
padding.

Note that gen_init_cpio does not include "security.evm" as it is file
system dependent.

Mimi

Re: [RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Rob Landley]

On 01/08/2015 04:08 PM, Mimi Zohar wrote:
> On Thu, 2015-01-08 at 12:19 -0600, Rob Landley wrote:
>>
>> But I am curious about how you propose to encode xattrs into the cpio
>> format. (Which Al Viro chose because it's _simple_. There isn't really
>> a
>> controlling spec since Posix decided to deprecated it in 2001 and
>> yank
>> it from SUSv3 onwards. LSB extended several header fields to 8 hex
>> digits instead of 6, but they still have 32 bit timestamps which seems
>> a
>> bit short-sighted. If you're going to define a new rev with a new
>> magic
>> number, there are a couple other things you might wanna fix...)
> 
> Sounds like a good opportunity to make the other changes as well.  We
> can include the other changes in this patch set.  Is this (initramfs)
> the right mailing list for this discussion?

I'd forgotten there was such a list until the email came in. :)

> Do other people need to be included?

In theory including the Austin Group (the posix committee mailing list)
might be useful, but in practice they hear about stuff well after the
fact, and they washed their hands of cpio over a decade ago (shortly
after Linux started heavily using it in rpm, and a bit before initramfs).

I note that there are two data formats of interest here:

1) the cpio file layout.

2) the list of files generated by gen_initramfs_list.sh and consumed by
gen_init_cpio.

The fact you're modifying gen_initramfs_list.sh seems to imply that
you're changing the _second_ format as well as the first...? The second
was never actually specified, but it does get used a lot by various
build systems and breaking it would inconvenience people. (Plus I'd need
to update the documentation, but I need to do that anyway.)

Ss long as you're extending the format could you add a second 32 bits of
time data that gets glued to the top half of a uint64_t, and then store
the actual time value in microseconds (so time*1000000)? (I'd say
"nanoseconds" but 63 bits of nanoseconds is 292 years, which is just
short enough I'm uncomfortable with it. I'm just optimistic enough to
think that might inconvenience somebody.)

The other "huh" is the filesize, but 4 gigs per file seems unlikely to
be more than initramfs needs any time soon? (It's possible that RPM
might care in 15 years or so...)

>> I ask because I maintain a new from-scratch cpio implementation
>> (http://landley.net/hg/toybox/file/1571/toys/posix/cpio.c), so I'd
>> presumably have to add your format extensions to this. Is there any
>> sort
>> of documentation on them?
>>
>> The toybox config Android is using has this cpio implementation
>> enabled
>> (see
>> https://android.googlesource.com/platform/external/toybox/+/9250c95a8c47/Android.mk)
>> so I'd rather like to get this sort of detail right...
> 
> The xattr section, which follows the file name, is of the format:
> <number of xattrs> { <xattr name> <xattr data size> <xattr data> } for
> each xattr, terminated with a NULL byte and padded to a 4 byte boundary.

where <value> is...  8 bytes of ascii hex digits like the header values?
(Every cpio string is padded to a boundary. Sigh, lemme go read your
patch...)

Ok, 2/9, actual file format parsing. New magic string at the start
"070703" for the new version. (Good, that was my first question: easy
way to distinguish this from the previous format).

-	for (i = 0, s += 6; i < 12; i++, s += 8) {
+	for (i = 0; i < (!newcx ? 12 : 13); i++, s += 8) {

You've tested this and the missing s+= 6 didn't cause problems? (Or did
it move somewhere else...? Is that what the 1/9 did grabbing just the
magic instead of the rest of the header data...?)

> The header contains an additional field, before the checksum, containing
> the xattr section length, including the NULL byte, but without the
> padding.

Ah, the old "4 bytes of padding to align to 4 bytes" silliness. (Even
though you can't trust the null to _be_ there and have to set it
yourself after the read.) I'm starting to remember this...

Ok, different header magic, one new 8 hex digit field at the end of the
header (before crc) containing "xattr_bufflen". The start of this buffer
is an 8 digit hex "num_xattrs", which you iterate through and call
strlen() on despite never having assured that the data you read in
actually _does_ contain a null at the end (of the entire buffer). Then
past that supposed null is another 8 digit hex "xattr_value_size", and
that many bytes following you then send to sys_setxattr().

Except for the part about you trusting your input data waaaaay too much,
seems reasonable? I have no idea what sys_setxattr() accepts, but
presumably there's a man page for the system call...

  http://man7.org/linux/man-pages/man2/setxattr.2.html

Ok, that's probably enough data to implement it. (Not sure why that man
page isn't in my ubuntu 14.04 install which has manpages-dev installed?

  $ man setxattr
  No manual entry for setxattr

> Note that gen_init_cpio does not include "security.evm" as it is file
> system dependent.

I have no idea what that is. Should I not include it in the command line
cpio? (Or have a flag?)

The last time I used extended attributes was on OS/2; my only
non-academic interaction with selinux has been looking up how to switch
it off.

I still boggle that Fortune 500 bureaucracies include "must have a
security system designed by the NSA based on the idea of complicating
the system until there are no obvious holes, because after the Snowden
leaks that's clearly a good idea" as part of their certification
processes for reducing the risk of being unable to delegate blame.

I'm also kind of impressed by the longevity of a hack the original Apple
Lisa developers invented in 1981 because their OS didn't have an
equivalent of the unix "file" command, and undoes the central unix
"everything's a flat file" idea to bring us back to the structure
records with magic meanings of the mainframe days.

http://www.folklore.org/StoryView.py?project=Macintosh&story=The_Grand_Unified_Model_The_Finder.txt&sortOrder=Sort+by+Title

Still, dictating what users can and can't do is policy. It exists,
people use it... sigh. I'll try to take a stab at it some evening this week.

> Mimi

Rob

Re: [RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Mimi Zohar]

On Tue, 2015-01-13 at 12:48 -0600, Rob Landley wrote: 
> On 01/08/2015 04:08 PM, Mimi Zohar wrote:
> > On Thu, 2015-01-08 at 12:19 -0600, Rob Landley wrote:
> >>
> >> But I am curious about how you propose to encode xattrs into the cpio
> >> format. (Which Al Viro chose because it's _simple_. There isn't really
> >> a
> >> controlling spec since Posix decided to deprecated it in 2001 and
> >> yank
> >> it from SUSv3 onwards. LSB extended several header fields to 8 hex
> >> digits instead of 6, but they still have 32 bit timestamps which seems
> >> a
> >> bit short-sighted. If you're going to define a new rev with a new
> >> magic
> >> number, there are a couple other things you might wanna fix...)
> > 
> > Sounds like a good opportunity to make the other changes as well.  We
> > can include the other changes in this patch set.  Is this (initramfs)
> > the right mailing list for this discussion?
> 
> I'd forgotten there was such a list until the email came in. :)
> 
> > Do other people need to be included?
> 
> In theory including the Austin Group (the posix committee mailing list)
> might be useful, but in practice they hear about stuff well after the
> fact, and they washed their hands of cpio over a decade ago (shortly
> after Linux started heavily using it in rpm, and a bit before initramfs).
> 
> I note that there are two data formats of interest here:
> 
> 1) the cpio file layout.
> 
> 2) the list of files generated by gen_initramfs_list.sh and consumed by
> gen_init_cpio.
> 
> The fact you're modifying gen_initramfs_list.sh seems to imply that
> you're changing the _second_ format as well as the first...? The second
> was never actually specified, but it does get used a lot by various
> build systems and breaking it would inconvenience people. (Plus I'd need
> to update the documentation, but I need to do that anyway.)

Patch "[PATCH 6/9] gen_initramfs_list.sh: include xattrs" is a one line
change that adds the "-x" option to include xattrs in the initramfs, if
they exist. This patch makes the new method (070703) the default format.

> Ss long as you're extending the format could you add a second 32 bits of
> time data that gets glued to the top half of a uint64_t, and then store
> the actual time value in microseconds (so time*1000000)? (I'd say
> "nanoseconds" but 63 bits of nanoseconds is 292 years, which is just
> short enough I'm uncomfortable with it. I'm just optimistic enough to
> think that might inconvenience somebody.)
> 
> The other "huh" is the filesize, but 4 gigs per file seems unlikely to
> be more than initramfs needs any time soon? (It's possible that RPM
> might care in 15 years or so...)

4 bytes enough?

> >> I ask because I maintain a new from-scratch cpio implementation
> >> (http://landley.net/hg/toybox/file/1571/toys/posix/cpio.c), so I'd
> >> presumably have to add your format extensions to this. Is there any
> >> sort
> >> of documentation on them?
> >>
> >> The toybox config Android is using has this cpio implementation
> >> enabled
> >> (see
> >> https://android.googlesource.com/platform/external/toybox/+/9250c95a8c47/Android.mk)
> >> so I'd rather like to get this sort of detail right...
> > 
> > The xattr section, which follows the file name, is of the format:
> > <number of xattrs> { <xattr name> <xattr data size> <xattr data> } for
> > each xattr, terminated with a NULL byte and padded to a 4 byte boundary.
> 
> where <value> is...  8 bytes of ascii hex digits like the header values?
> (Every cpio string is padded to a boundary. Sigh, lemme go read your
> patch...)
> 
> Ok, 2/9, actual file format parsing. New magic string at the start
> "070703" for the new version. (Good, that was my first question: easy
> way to distinguish this from the previous format).
> 
> -	for (i = 0, s += 6; i < 12; i++, s += 8) {
> +	for (i = 0; i < (!newcx ? 12 : 13); i++, s += 8) {
> 
> You've tested this and the missing s+= 6 didn't cause problems? (Or did
> it move somewhere else...? Is that what the 1/9 did grabbing just the
> magic instead of the rest of the header data...?)

Right, the first patch separates reading the magic string from reading
the rest of the header.

> > The header contains an additional field, before the checksum, containing
> > the xattr section length, including the NULL byte, but without the
> > padding.
> 
> Ah, the old "4 bytes of padding to align to 4 bytes" silliness. (Even
> though you can't trust the null to _be_ there and have to set it
> yourself after the read.) I'm starting to remember this...
> 
> Ok, different header magic, one new 8 hex digit field at the end of the
> header (before crc) containing "xattr_bufflen". The start of this buffer
> is an 8 digit hex "num_xattrs", which you iterate through and call
> strlen() on despite never having assured that the data you read in
> actually _does_ contain a null at the end (of the entire buffer). Then
> past that supposed null is another 8 digit hex "xattr_value_size", and
> that many bytes following you then send to sys_setxattr().
> 
> Except for the part about you trusting your input data waaaaay too much,
> seems reasonable? 

Ok

> I have no idea what sys_setxattr() accepts, but
> presumably there's a man page for the system call...
> 
>   http://man7.org/linux/man-pages/man2/setxattr.2.html
> 
> Ok, that's probably enough data to implement it. (Not sure why that man
> page isn't in my ubuntu 14.04 install which has manpages-dev installed?
> 
>   $ man setxattr
>   No manual entry for setxattr
> 
> > Note that gen_init_cpio does not include "security.evm" as it is file
> > system dependent.
> 
> I have no idea what that is. Should I not include it in the command line
> cpio? (Or have a flag?)

Right, don't include "security.evm".  Both the HMAC and signature format
include the inode number, which is filesystem specific.

> The last time I used extended attributes was on OS/2; my only
> non-academic interaction with selinux has been looking up how to switch
> it off.
> 
> I still boggle that Fortune 500 bureaucracies include "must have a
> security system designed by the NSA based on the idea of complicating
> the system until there are no obvious holes, because after the Snowden
> leaks that's clearly a good idea" as part of their certification
> processes for reducing the risk of being unable to delegate blame.

I'm the linux-integrity subsystem maintainer, not an LSM maintainer (not
that there is anything wrong in being an LSM maintainer).  So, I'm not
quite sure why you keep saying things like this to me.  BTW, there are a
number of LSMs these days, not only SELinux.
  
> I'm also kind of impressed by the longevity of a hack the original Apple
> Lisa developers invented in 1981 because their OS didn't have an
> equivalent of the unix "file" command, and undoes the central unix
> "everything's a flat file" idea to bring us back to the structure
> records with magic meanings of the mainframe days.
> 
> http://www.folklore.org/StoryView.py?project=Macintosh&story=The_Grand_Unified_Model_The_Finder.txt&sortOrder=Sort+by+Title
> 
> Still, dictating what users can and can't do is policy. It exists,
> people use it... 

> sigh. I'll try to take a stab at it some evening this week.
> 
> > Mimi
> 
> Rob

Thanks!

Mimi

Re: [RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Rob Landley]

On 01/13/2015 02:20 PM, Mimi Zohar wrote:
> On Tue, 2015-01-13 at 12:48 -0600, Rob Landley wrote: 
>> I note that there are two data formats of interest here:
>>
>> 1) the cpio file layout.
>>
>> 2) the list of files generated by gen_initramfs_list.sh and consumed by
>> gen_init_cpio.
>>
>> The fact you're modifying gen_initramfs_list.sh seems to imply that
>> you're changing the _second_ format as well as the first...? The second
>> was never actually specified, but it does get used a lot by various
>> build systems and breaking it would inconvenience people. (Plus I'd need
>> to update the documentation, but I need to do that anyway.)
> 
> Patch "[PATCH 6/9] gen_initramfs_list.sh: include xattrs" is a one line
> change that adds the "-x" option to include xattrs in the initramfs, if
> they exist.

Unconditionally. Even if we've configured xattr support out of our
kernel or tmpfs?

> This patch makes the new method (070703) the default format.

So nobody should ever try to build an embedded system (without xattrs)
from something like Red Hat Enterprise (where they just magically show up)?

>> Ss long as you're extending the format could you add a second 32 bits of
>> time data that gets glued to the top half of a uint64_t, and then store
>> the actual time value in microseconds (so time*1000000)? (I'd say
>> "nanoseconds" but 63 bits of nanoseconds is 292 years, which is just
>> short enough I'm uncomfortable with it. I'm just optimistic enough to
>> think that might inconvenience somebody.)
>>
>> The other "huh" is the filesize, but 4 gigs per file seems unlikely to
>> be more than initramfs needs any time soon? (It's possible that RPM
>> might care in 15 years or so...)
> 
> 4 bytes enough?

Eh, as long as we're breaking compatibility anyway, we might as well
extend the file size. It's gzipped so the extra run of consecutive
zeroes isn't really an issue, and if tmpfs is going to support 64 bit
file sizes the thing that's populating them should to just to match.
(You can already have memory bigger than 4g. Some crazy person is going
to put a squashfs in tmpfs and loopback mount it, or have a giant video
there, or... Bootloaders being able to cope with this is not my problem. :)

Probably having the new fields at the end (and gluing them to the
earlier ones) makes more sense than having variable sized fields? I
don't have a strong opinion either way.

>> I have no idea what sys_setxattr() accepts, but
>> presumably there's a man page for the system call...
>>
>>   http://man7.org/linux/man-pages/man2/setxattr.2.html
>>
>> Ok, that's probably enough data to implement it. (Not sure why that man
>> page isn't in my ubuntu 14.04 install which has manpages-dev installed?
>>
>>   $ man setxattr
>>   No manual entry for setxattr
>>
>>> Note that gen_init_cpio does not include "security.evm" as it is file
>>> system dependent.
>>
>> I have no idea what that is. Should I not include it in the command line
>> cpio? (Or have a flag?)
> 
> Right, don't include "security.evm".  Both the HMAC and signature format
> include the inode number, which is filesystem specific.

So save extended attributes but filter out this one magic extended
attribute that we _shouldn't_ save because we know, a priori, that this
data is not portable.

I'm remembering why I didn't want to get this on me.

>> The last time I used extended attributes was on OS/2; my only
>> non-academic interaction with selinux has been looking up how to switch
>> it off.
>>
>> I still boggle that Fortune 500 bureaucracies include "must have a
>> security system designed by the NSA based on the idea of complicating
>> the system until there are no obvious holes, because after the Snowden
>> leaks that's clearly a good idea" as part of their certification
>> processes for reducing the risk of being unable to delegate blame.
> 
> I'm the linux-integrity subsystem maintainer, not an LSM maintainer (not
> that there is anything wrong in being an LSM maintainer).  So, I'm not
> quite sure why you keep saying things like this to me. BTW, there are
> a number of LSMs these days, not only SELinux.

Yes, and I can't keep 'em straight. The android guys are adding SELinux:

https://android.googlesource.com/platform/external/toybox/+/d5c66a9fd36777f80ba05301dcfa6789b103e486

The Tizen guys are adding something called "smack":
https://git.tizen.org/cgit/platform/upstream/toybox.git

Up until about 3 months ago I had successfully avoided all of this. Oh
well, always something new to learn...

Do each of them have their own rules about what extended attribute data
is not portable and should be filtered out? Or is this one magic entry
it? (I'd RTFM but http://man7.org/linux/man-pages/man5/attr.5.html does
not contain the string "evm".)

Rob

Re: [RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Mimi Zohar]

On Tue, 2015-01-13 at 15:42 -0600, Rob Landley wrote: 
> 
> On 01/13/2015 02:20 PM, Mimi Zohar wrote:
> > On Tue, 2015-01-13 at 12:48 -0600, Rob Landley wrote: 
> >> I note that there are two data formats of interest here:
> >>
> >> 1) the cpio file layout.
> >>
> >> 2) the list of files generated by gen_initramfs_list.sh and consumed by
> >> gen_init_cpio.
> >>
> >> The fact you're modifying gen_initramfs_list.sh seems to imply that
> >> you're changing the _second_ format as well as the first...? The second
> >> was never actually specified, but it does get used a lot by various
> >> build systems and breaking it would inconvenience people. (Plus I'd need
> >> to update the documentation, but I need to do that anyway.)
> > 
> > Patch "[PATCH 6/9] gen_initramfs_list.sh: include xattrs" is a one line
> > change that adds the "-x" option to include xattrs in the initramfs, if
> > they exist.
> 
> Unconditionally. Even if we've configured xattr support out of our
> kernel or tmpfs?
> 
> > This patch makes the new method (070703) the default format.
> 
> So nobody should ever try to build an embedded system (without xattrs)
> from something like Red Hat Enterprise (where they just magically show up)?

Good point.  I'll address this in the next post.

> >> Ss long as you're extending the format could you add a second 32 bits of
> >> time data that gets glued to the top half of a uint64_t, and then store
> >> the actual time value in microseconds (so time*1000000)? (I'd say
> >> "nanoseconds" but 63 bits of nanoseconds is 292 years, which is just
> >> short enough I'm uncomfortable with it. I'm just optimistic enough to
> >> think that might inconvenience somebody.)
> >>
> >> The other "huh" is the filesize, but 4 gigs per file seems unlikely to
> >> be more than initramfs needs any time soon? (It's possible that RPM
> >> might care in 15 years or so...)
> > 
> > 4 bytes enough?

> Eh, as long as we're breaking compatibility anyway, we might as well
> extend the file size. It's gzipped so the extra run of consecutive
> zeroes isn't really an issue, and if tmpfs is going to support 64 bit
> file sizes the thing that's populating them should to just to match.
> (You can already have memory bigger than 4g. Some crazy person is going
> to put a squashfs in tmpfs and loopback mount it, or have a giant video
> there, or... Bootloaders being able to cope with this is not my problem. :)

> Probably having the new fields at the end (and gluing them to the
> earlier ones) makes more sense than having variable sized fields? I
> don't have a strong opinion either way.

The current file data size header field is a 8 character hexidecimal
string, which is long enough to store 4g (0xFFFFFFFF).

> >> I have no idea what sys_setxattr() accepts, but
> >> presumably there's a man page for the system call...
> >>
> >>   http://man7.org/linux/man-pages/man2/setxattr.2.html
> >>
> >> Ok, that's probably enough data to implement it. (Not sure why that man
> >> page isn't in my ubuntu 14.04 install which has manpages-dev installed?
> >>
> >>   $ man setxattr
> >>   No manual entry for setxattr
> >>
> >>> Note that gen_init_cpio does not include "security.evm" as it is file
> >>> system dependent.
> >>
> >> I have no idea what that is. Should I not include it in the command line
> >> cpio? (Or have a flag?)
> > 
> > Right, don't include "security.evm".  Both the HMAC and signature format
> > include the inode number, which is filesystem specific.
> 
> So save extended attributes but filter out this one magic extended
> attribute that we _shouldn't_ save because we know, a priori, that this
> data is not portable.
> 
> I'm remembering why I didn't want to get this on me.
> 
> >> The last time I used extended attributes was on OS/2; my only
> >> non-academic interaction with selinux has been looking up how to switch
> >> it off.
> >>
> >> I still boggle that Fortune 500 bureaucracies include "must have a
> >> security system designed by the NSA based on the idea of complicating
> >> the system until there are no obvious holes, because after the Snowden
> >> leaks that's clearly a good idea" as part of their certification
> >> processes for reducing the risk of being unable to delegate blame.
> > 
> > I'm the linux-integrity subsystem maintainer, not an LSM maintainer (not
> > that there is anything wrong in being an LSM maintainer).  So, I'm not
> > quite sure why you keep saying things like this to me. BTW, there are
> > a number of LSMs these days, not only SELinux.
> 
> Yes, and I can't keep 'em straight. The android guys are adding SELinux:
> 
> https://android.googlesource.com/platform/external/toybox/+/d5c66a9fd36777f80ba05301dcfa6789b103e486
> 
> The Tizen guys are adding something called "smack":
> https://git.tizen.org/cgit/platform/upstream/toybox.git
> 
> Up until about 3 months ago I had successfully avoided all of this. Oh
> well, always something new to learn...
> 
> Do each of them have their own rules about what extended attribute data
> is not portable and should be filtered out? Or is this one magic entry
> it? (I'd RTFM but http://man7.org/linux/man-pages/man5/attr.5.html does
> not contain the string "evm".)
> 
> Rob

I would assume only 'security.evm' is not portable as it attempts to
tightly bind the file metadata to the file data.  Casey?  Paul?

Mimi

Re: [RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Rob Landley]

On 01/13/2015 09:23 PM, Mimi Zohar wrote:
> On Tue, 2015-01-13 at 15:42 -0600, Rob Landley wrote: 
>>> 4 bytes enough?
> 
>> Eh, as long as we're breaking compatibility anyway, we might as well
>> extend the file size. It's gzipped so the extra run of consecutive
>> zeroes isn't really an issue, and if tmpfs is going to support 64 bit
>> file sizes the thing that's populating them should to just to match.
>> (You can already have memory bigger than 4g. Some crazy person is going
>> to put a squashfs in tmpfs and loopback mount it, or have a giant video
>> there, or... Bootloaders being able to cope with this is not my problem. :)
> 
>> Probably having the new fields at the end (and gluing them to the
>> earlier ones) makes more sense than having variable sized fields? I
>> don't have a strong opinion either way.
> 
> The current file data size header field is a 8 character hexidecimal
> string, which is long enough to store 4g (0xFFFFFFFF).

The current header fields are all 32 bits, yes. To get a 64 bit field
we'd have to add a second 32 bit field and add it <<32 to the original
one, or else have the header fields be of varying sizes. That was the
"adding a new one to the end" thing mentioned above.

Then again if we add a new field right before the previous size then the
"treat it as 64 bits vs 2 32 bit ones" is an implementation detail
anyway. And for the moment we can just have it be padding that
compresses away and wait for an actual >4g file.

Unless you think nobody will ever need an archive member >4g in
initramfs, even though servers with ~256g are reasonably common today
already?

Rob

Re: [RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Mimi Zohar]

On Tue, 2015-01-13 at 22:34 -0600, Rob Landley wrote: 
> 
> On 01/13/2015 09:23 PM, Mimi Zohar wrote:
> > On Tue, 2015-01-13 at 15:42 -0600, Rob Landley wrote:

> Then again if we add a new field right before the previous size then the
> "treat it as 64 bits vs 2 32 bit ones" is an implementation detail
> anyway. And for the moment we can just have it be padding that
> compresses away and wait for an actual >4g file.

Sounds good.

Mimi

Re: [RFC][PATCH 6/9] gen_initramfs_list.sh: include xattrs

[Paul Moore]

On Tuesday, January 13, 2015 10:23:23 PM Mimi Zohar wrote:
> I would assume only 'security.evm' is not portable as it attempts to
> tightly bind the file metadata to the file data.  Casey?  Paul?

[NOTE: Added the SELinux mailing list to the CC line.]

The SELinux xattr should be portable assuming the security label's semantics 
remain constant across the different security policies.  If the label is 
completely unknown SELinux should handle it correctly, it will be treated as 
unlabeled until a module is loaded which defines the label.

Although, this is just for initramfs, yes?  If so, I'm not sure this matters 
that much from a practical point of view; Stephen or someone else from the 
SELinux list may have some thoughts on this.

-- 
paul moore
security @ redhat

[RFC][PATCH 7/9] evm: make rootfs a special case

[Mimi Zohar]

Both the EVM HMAC and signature xattr formats are file system
specific and can not be copied from one filesystem to another.

EVM differentiates files without any xattrs (INTEGRITY_UNKNOWN)
from those having protected xattrs (INTEGRITY_NOLABEL).  This
patch treats the rootfs filesystem as a special case, returning
INTEGRITY_UNKNOWN.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/evm/evm_main.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index b392fe6..9c71af7 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -128,11 +128,16 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 	if (rc <= 0) {
 		evm_status = INTEGRITY_FAIL;
 		if (rc == -ENODATA) {
+			struct super_block *sb = dentry->d_inode->i_sb;
+
 			rc = evm_find_protected_xattrs(dentry);
-			if (rc > 0)
-				evm_status = INTEGRITY_NOLABEL;
-			else if (rc == 0)
+			if (rc == 0)
 				evm_status = INTEGRITY_NOXATTRS; /* new file */
+			else if (rc > 0 && sb->s_magic == TMPFS_MAGIC
+				 && strcmp(sb->s_id, "rootfs") == 0)
+				evm_status = INTEGRITY_UNKNOWN;
+			else if (rc > 0)
+				evm_status = INTEGRITY_NOLABEL;
 		} else if (rc == -EOPNOTSUPP) {
 			evm_status = INTEGRITY_UNKNOWN;
 		}
-- 
1.8.1.4

[RFC][PATCH 8/9] ima: include tmpfs in ima_appraise_tcb policy

[Mimi Zohar]

Now that the rootfs includes extended attributes, don't
automatically exclude tmpfs file systems from being appraised.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/evm/evm_main.c   | 1 +
 security/integrity/ima/ima_policy.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 9c71af7..e942e63 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/crypto.h>
 #include <linux/audit.h>
+#include <linux/magic.h>
 #include <linux/xattr.h>
 #include <linux/integrity.h>
 #include <linux/evm.h>
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index d1eefb9..7267eac 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -93,7 +93,9 @@ static struct ima_rule_entry default_appraise_rules[] = {
 	{.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
+#ifndef CONFIG_IMA_LOAD_X509
 	{.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
+#endif
 	{.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC},
 	{.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC},
-- 
1.8.1.4

Re: [RFC][PATCH 8/9] ima: include tmpfs in ima_appraise_tcb policy

[Josh Boyer]

On Wed, Jan 7, 2015 at 3:52 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Now that the rootfs includes extended attributes, don't
> automatically exclude tmpfs file systems from being appraised.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
>  security/integrity/evm/evm_main.c   | 1 +
>  security/integrity/ima/ima_policy.c | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 9c71af7..e942e63 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -19,6 +19,7 @@
>  #include <linux/module.h>
>  #include <linux/crypto.h>
>  #include <linux/audit.h>
> +#include <linux/magic.h>
>  #include <linux/xattr.h>
>  #include <linux/integrity.h>
>  #include <linux/evm.h>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index d1eefb9..7267eac 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -93,7 +93,9 @@ static struct ima_rule_entry default_appraise_rules[] = {
>         {.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
>         {.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
>         {.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
> +#ifndef CONFIG_IMA_LOAD_X509
>         {.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
> +#endif

The commit log makes it sound like tmpfs should be appraised
unconditionally, but you only have it being appraised if IMA_LOAD_X509
is set.  Which is correct (and why isn't it based on whether
CONFIG_IMA_APPRAISE is set)?

Also, what happens if someone creates an initramfs that doesn't
include xattrs and has this option set?

Slightly confusing.

josh

Re: [RFC][PATCH 8/9] ima: include tmpfs in ima_appraise_tcb policy

[Mimi Zohar]

On Thu, 2015-01-08 at 08:53 -0500, Josh Boyer wrote: 
> On Wed, Jan 7, 2015 at 3:52 PM, Mimi Zohar <zohar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
> > Now that the rootfs includes extended attributes, don't
> > automatically exclude tmpfs file systems from being appraised.
> >
> > Signed-off-by: Mimi Zohar <zohar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> > ---
> >  security/integrity/evm/evm_main.c   | 1 +
> >  security/integrity/ima/ima_policy.c | 2 ++
> >  2 files changed, 3 insertions(+)
> >
> > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > index 9c71af7..e942e63 100644
> > --- a/security/integrity/evm/evm_main.c
> > +++ b/security/integrity/evm/evm_main.c
> > @@ -19,6 +19,7 @@
> >  #include <linux/module.h>
> >  #include <linux/crypto.h>
> >  #include <linux/audit.h>
> > +#include <linux/magic.h>
> >  #include <linux/xattr.h>
> >  #include <linux/integrity.h>
> >  #include <linux/evm.h>
> > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > index d1eefb9..7267eac 100644
> > --- a/security/integrity/ima/ima_policy.c
> > +++ b/security/integrity/ima/ima_policy.c
> > @@ -93,7 +93,9 @@ static struct ima_rule_entry default_appraise_rules[] = {
> >         {.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> >         {.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC},
> >         {.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC},
> > +#ifndef CONFIG_IMA_LOAD_X509
> >         {.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC},
> > +#endif
> 
> The commit log makes it sound like tmpfs should be appraised
> unconditionally, but you only have it being appraised if IMA_LOAD_X509
> is set.  Which is correct (and why isn't it based on whether
> CONFIG_IMA_APPRAISE is set)?
> 
> Also, what happens if someone creates an initramfs that doesn't
> include xattrs and has this option set?
> 
> Slightly confusing.

Right, with commit fd5f4e90 "ima: load x509 certificate from the
kernel", the IMA appraisal key can be loaded before executing "init",
wherever it might be.

Only if IMA-appraisal is enabled and the ima_appraise_tcb is specified
on the kernel boot command line, are the builtin ima_appraise_tcb policy
rules used.

Mimi

[RFC][PATCH 9/9] init: remove "root=" command line option test for tmpfs decision

[Mimi Zohar]

The "root=" option is interpreted by the kernel sometimes and passed
on to userspace other times.  Based on whether the "root=" boot
command line option is specified, the initramfs uses tmpfs or ramfs
as the rootfs.

This is a temporary patch that removes the "root=" test in the
decision to use tmpfs or ramfs as the rootfs.  Modify userspace
applications (eg. dracut, systemd) to support "ROOT=" in addition
to "root=".

(Not to be upstreamed.)

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 init/do_mounts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init/do_mounts.c b/init/do_mounts.c
index 82f2288..b2f5e7d 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -622,7 +622,7 @@ int __init init_rootfs(void)
 	if (err)
 		return err;
 
-	if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] &&
+	if (IS_ENABLED(CONFIG_TMPFS) &&
 		(!root_fs_names || strstr(root_fs_names, "tmpfs"))) {
 		err = shmem_init();
 		is_tmpfs = true;
-- 
1.8.1.4