This patch set aims at solving the following use case: appraise files from
the initial ram disk. To do that, IMA checks the signature/hash from the
security.ima xattr. Unfortunately, this use case cannot be implemented
currently, as the CPIO format does not support xattrs.
This proposal consists in including file metadata as additional files named
METADATA!!!, for each file added to the ram disk. The CPIO parser in the
kernel recognizes these special files from the file name, and calls the
appropriate parser to add metadata to the previously extracted file. It has
been proposed to use bit 17:16 of the file mode as a way to recognize files
with metadata, but both the kernel and the cpio tool declare the file mode
as unsigned short.
The difference from v2, v3 (https://lkml.org/lkml/2019/5/9/230,
https://lkml.org/lkml/2019/5/17/466) is that file metadata are stored in
separate files instead of a single file. Given that files with metadata
must immediately follow the files metadata will be added to, image
generators have to be modified in this version.
The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that
all files have the same name. The file metadata are added to is always the
previous one, and the image generator in user space will make sure that
files are in the correct sequence.
The difference with another proposal
(https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be
included in an image without changing the image format. Files with metadata
will appear as regular files. It will be task of the parser in the kernel
to process them.
This patch set extends the format of data defined in patch 9/15 of the last
proposal. It adds header version and type, so that new formats can be
defined and arbitrary metadata types can be processed.
The changes introduced by this patch set don't cause any compatibility
issue: kernels without the metadata parser simply extract the special files
and don't process metadata; kernels with the metadata parser don't process
metadata if the special files are not included in the image.
From the kernel space perspective, backporting this functionality to older
kernels should be very easy. It is sufficient to add two calls to the new
function do_process_metadata() in do_copy(), and to check the file name in
do_name(). From the user space perspective, unlike the previous version of
the patch set, it is required to modify the image generators in order to
include metadata as separate files.
Changelog
v3:
- include file metadata as separate files named METADATA!!!
- add the possibility to include in the ram disk arbitrary metadata types
v2:
- replace ksys_lsetxattr() with kern_path() and vfs_setxattr()
(suggested by Jann Horn)
- replace ksys_open()/ksys_read()/ksys_close() with
filp_open()/kernel_read()/fput()
(suggested by Jann Horn)
- use path variable instead of name_buf in do_readxattrs()
- set last byte of str to 0 in do_readxattrs()
- call do_readxattrs() in do_name() before replacing an existing
.xattr-list
- pass pathname to do_setxattrs()
v1:
- move xattr unmarshaling to CPIO parser
Mimi Zohar (1):
initramfs: add file metadata
Roberto Sassu (2):
initramfs: read metadata from special file METADATA!!!
gen_init_cpio: add support for file metadata
include/linux/initramfs.h | 21 ++++++
init/initramfs.c | 137 +++++++++++++++++++++++++++++++++++++-
usr/Kconfig | 8 +++
usr/Makefile | 4 +-
usr/gen_init_cpio.c | 137 ++++++++++++++++++++++++++++++++++++--
usr/gen_initramfs_list.sh | 10 ++-
6 files changed, 305 insertions(+), 12 deletions(-)
create mode 100644 include/linux/initramfs.h
--
2.17.1
From: Mimi Zohar <[email protected]>
This patch adds metadata to a file from a supplied buffer. The buffer might
contains multiple metadata records. The format of each record is:
<metadata len (ASCII, 8 chars)><version><type><metadata>
For now, only the TYPE_XATTR metadata type is supported. The specific
format of this metadata type is:
<xattr #N name>\0<xattr #N value>
[kamensky: fixed restoring of xattrs for symbolic links by using
sys_lsetxattr() instead of sys_setxattr()]
[sassu: removed state management, kept only do_setxattrs(), added support
for generic file metadata, replaced sys_lsetxattr() with
vfs_setxattr(), added check for entry_size, added check for
hdr->c_size, replaced strlen() with strnlen(); moved do_setxattrs()
before do_name()]
Signed-off-by: Mimi Zohar <[email protected]>
Signed-off-by: Victor Kamensky <[email protected]>
Signed-off-by: Taras Kondratiuk <[email protected]>
Signed-off-by: Roberto Sassu <[email protected]>
---
include/linux/initramfs.h | 21 ++++++++++
init/initramfs.c | 88 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 107 insertions(+), 2 deletions(-)
create mode 100644 include/linux/initramfs.h
diff --git a/include/linux/initramfs.h b/include/linux/initramfs.h
new file mode 100644
index 000000000000..2f8cee441236
--- /dev/null
+++ b/include/linux/initramfs.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * include/linux/initramfs.h
+ *
+ * Include file for file metadata in the initial ram disk.
+ */
+#ifndef _LINUX_INITRAMFS_H
+#define _LINUX_INITRAMFS_H
+
+#define METADATA_FILENAME "METADATA!!!"
+
+enum metadata_types { TYPE_NONE, TYPE_XATTR, TYPE__LAST };
+
+struct metadata_hdr {
+ char c_size[8]; /* total size including c_size field */
+ char c_version; /* header version */
+ char c_type; /* metadata type */
+ char c_metadata[]; /* metadata */
+} __packed;
+
+#endif /*LINUX_INITRAMFS_H*/
diff --git a/init/initramfs.c b/init/initramfs.c
index 178130fd61c2..5de396a6aac0 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -10,6 +10,9 @@
#include <linux/syscalls.h>
#include <linux/utime.h>
#include <linux/file.h>
+#include <linux/namei.h>
+#include <linux/xattr.h>
+#include <linux/initramfs.h>
static ssize_t __init xwrite(int fd, const char *p, size_t count)
{
@@ -146,7 +149,7 @@ static __initdata time64_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, metadata_len;
static __initdata uid_t uid;
static __initdata gid_t gid;
static __initdata unsigned rdev;
@@ -218,7 +221,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, *metadata_buf;
static int __init do_start(void)
{
@@ -315,6 +318,87 @@ static int __init maybe_link(void)
return 0;
}
+static int __init do_setxattrs(char *pathname, char *buf, size_t size)
+{
+ struct path path;
+ char *xattr_name, *xattr_value;
+ size_t xattr_name_size, xattr_value_size;
+ int ret;
+
+ xattr_name = buf;
+ xattr_name_size = strnlen(xattr_name, size);
+ if (xattr_name_size == size) {
+ error("malformed xattrs");
+ return -EINVAL;
+ }
+
+ xattr_value = xattr_name + xattr_name_size + 1;
+ xattr_value_size = buf + size - xattr_value;
+
+ ret = kern_path(pathname, 0, &path);
+ if (!ret) {
+ ret = vfs_setxattr(path.dentry, xattr_name, xattr_value,
+ xattr_value_size, 0);
+
+ path_put(&path);
+ }
+
+ pr_debug("%s: %s size: %lu val: %s (ret: %d)\n", pathname,
+ xattr_name, xattr_value_size, xattr_value, ret);
+
+ return ret;
+}
+
+static int __init __maybe_unused do_parse_metadata(char *pathname)
+{
+ char *buf = metadata_buf;
+ char *bufend = metadata_buf + metadata_len;
+ struct metadata_hdr *hdr;
+ char str[sizeof(hdr->c_size) + 1];
+ size_t entry_size;
+
+ if (!metadata_len)
+ return 0;
+
+ str[sizeof(hdr->c_size)] = 0;
+
+ while (buf < bufend) {
+ int ret;
+
+ if (buf + sizeof(*hdr) > bufend) {
+ error("malformed metadata");
+ break;
+ }
+
+ hdr = (struct metadata_hdr *)buf;
+ if (hdr->c_version != 1) {
+ pr_debug("Unsupported header version\n");
+ break;
+ }
+
+ memcpy(str, hdr->c_size, sizeof(hdr->c_size));
+ ret = kstrtoul(str, 16, &entry_size);
+ if (ret || buf + entry_size > bufend || !entry_size) {
+ error("malformed xattrs");
+ break;
+ }
+
+ switch (hdr->c_type) {
+ case TYPE_XATTR:
+ do_setxattrs(pathname, buf + sizeof(*hdr),
+ entry_size - sizeof(*hdr));
+ break;
+ default:
+ pr_debug("Unsupported metadata type\n");
+ break;
+ }
+
+ buf += entry_size;
+ }
+
+ return 0;
+}
+
static __initdata int wfd;
static int __init do_name(void)
--
2.17.1
Instead of changing the CPIO format, metadata are parsed from regular files
with special name 'METADATA!!!'. This file immediately follows the file
metadata are added to.
This patch checks if the file being extracted has the special name and, if
yes, creates a buffer with the content of that file and calls
do_parse_metadata() to parse metadata from the buffer.
Signed-off-by: Roberto Sassu <[email protected]>
Reported-by: kbuild test robot <[email protected]>
---
init/initramfs.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/init/initramfs.c b/init/initramfs.c
index 5de396a6aac0..862c03123de8 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -222,6 +222,7 @@ static void __init read_into(char *buf, unsigned size, enum state next)
}
static __initdata char *header_buf, *symlink_buf, *name_buf, *metadata_buf;
+static __initdata char *metadata_buf_ptr, *previous_name_buf;
static int __init do_start(void)
{
@@ -400,6 +401,7 @@ static int __init __maybe_unused do_parse_metadata(char *pathname)
}
static __initdata int wfd;
+static __initdata int metadata;
static int __init do_name(void)
{
@@ -408,6 +410,10 @@ static int __init do_name(void)
if (strcmp(collected, "TRAILER!!!") == 0) {
free_hash();
return 0;
+ } else if (strcmp(collected, METADATA_FILENAME) == 0) {
+ metadata = 1;
+ } else {
+ memcpy(previous_name_buf, collected, strlen(collected) + 1);
}
clean_path(collected, mode);
if (S_ISREG(mode)) {
@@ -444,11 +450,47 @@ static int __init do_name(void)
return 0;
}
+static int __init do_process_metadata(char *buf, int len, bool last)
+{
+ int ret = 0;
+
+ if (!metadata_buf) {
+ metadata_buf_ptr = metadata_buf = kmalloc(body_len, GFP_KERNEL);
+ if (!metadata_buf_ptr) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ metadata_len = body_len;
+ }
+
+ if (metadata_buf_ptr + len > metadata_buf + metadata_len) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ memcpy(metadata_buf_ptr, buf, len);
+ metadata_buf_ptr += len;
+
+ if (last)
+ do_parse_metadata(previous_name_buf);
+out:
+ if (ret < 0 || last) {
+ kfree(metadata_buf);
+ metadata_buf = NULL;
+ metadata = 0;
+ }
+
+ return ret;
+}
+
static int __init do_copy(void)
{
if (byte_count >= body_len) {
if (xwrite(wfd, victim, body_len) != body_len)
error("write error");
+ if (metadata)
+ do_process_metadata(victim, body_len, true);
ksys_close(wfd);
do_utime(vcollected, mtime);
kfree(vcollected);
@@ -458,6 +500,8 @@ static int __init do_copy(void)
} else {
if (xwrite(wfd, victim, byte_count) != byte_count)
error("write error");
+ if (metadata)
+ do_process_metadata(victim, byte_count, false);
body_len -= byte_count;
eat(byte_count);
return 1;
@@ -467,6 +511,7 @@ static int __init do_copy(void)
static int __init do_symlink(void)
{
collected[N_ALIGN(name_len) + body_len] = '\0';
+ memcpy(previous_name_buf, collected, strlen(collected) + 1);
clean_path(collected, 0);
ksys_symlink(collected + N_ALIGN(name_len), collected);
ksys_lchown(collected, uid, gid);
@@ -534,8 +579,9 @@ static char * __init unpack_to_rootfs(char *buf, unsigned long len)
header_buf = kmalloc(110, GFP_KERNEL);
symlink_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, GFP_KERNEL);
name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL);
+ previous_name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL);
- if (!header_buf || !symlink_buf || !name_buf)
+ if (!header_buf || !symlink_buf || !name_buf || !previous_name_buf)
panic("can't allocate buffers");
state = Start;
@@ -580,6 +626,7 @@ static char * __init unpack_to_rootfs(char *buf, unsigned long len)
len -= my_inptr;
}
dir_utime();
+ kfree(previous_name_buf);
kfree(name_buf);
kfree(symlink_buf);
kfree(header_buf);
--
2.17.1
This patch adds support for file metadata (only TYPE_XATTR metadata type).
gen_init_cpio has been modified to read xattrs from files that will be
added to the image and to include file metadata as separate files with the
special name 'METADATA!!!'.
This behavior can be selected by setting the desired file metadata type as
value for CONFIG_INITRAMFS_FILE_METADATA.
Signed-off-by: Roberto Sassu <[email protected]>
---
usr/Kconfig | 8 +++
usr/Makefile | 4 +-
usr/gen_init_cpio.c | 137 ++++++++++++++++++++++++++++++++++++--
usr/gen_initramfs_list.sh | 10 ++-
4 files changed, 150 insertions(+), 9 deletions(-)
diff --git a/usr/Kconfig b/usr/Kconfig
index 43658b8a975e..8d9f54a16440 100644
--- a/usr/Kconfig
+++ b/usr/Kconfig
@@ -233,3 +233,11 @@ config INITRAMFS_COMPRESSION
default ".lzma" if RD_LZMA
default ".bz2" if RD_BZIP2
default ""
+
+config INITRAMFS_FILE_METADATA
+ string "File metadata type"
+ default ""
+ help
+ Specify xattr to include xattrs in the image.
+
+ If you are not sure, leave it blank.
diff --git a/usr/Makefile b/usr/Makefile
index 4a70ae43c9cb..7d5eb3c7b713 100644
--- a/usr/Makefile
+++ b/usr/Makefile
@@ -29,7 +29,9 @@ ramfs-input := $(if $(filter-out "",$(CONFIG_INITRAMFS_SOURCE)), \
$(shell echo $(CONFIG_INITRAMFS_SOURCE)),-d)
ramfs-args := \
$(if $(CONFIG_INITRAMFS_ROOT_UID), -u $(CONFIG_INITRAMFS_ROOT_UID)) \
- $(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID))
+ $(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID)) \
+ $(if $(filter-out "",$(CONFIG_INITRAMFS_FILE_METADATA)), \
+ -e $(CONFIG_INITRAMFS_FILE_METADATA))
# $(datafile_d_y) is used to identify all files included
# in initramfs and to detect if any files are added/removed.
diff --git a/usr/gen_init_cpio.c b/usr/gen_init_cpio.c
index 03b21189d58b..e93cb1093e77 100644
--- a/usr/gen_init_cpio.c
+++ b/usr/gen_init_cpio.c
@@ -3,6 +3,7 @@
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
+#include <sys/xattr.h>
#include <string.h>
#include <unistd.h>
#include <time.h>
@@ -10,6 +11,7 @@
#include <errno.h>
#include <ctype.h>
#include <limits.h>
+#include "../include/linux/initramfs.h"
/*
* Original work by Jeff Garzik
@@ -24,6 +26,115 @@
static unsigned int offset;
static unsigned int ino = 721;
static time_t default_mtime;
+static char metadata_path[] = "/tmp/cpio-metadata-XXXXXX";
+static int metadata_fd = -1;
+
+static enum metadata_types parse_metadata_type(char *arg)
+{
+ static char *metadata_type_str[TYPE__LAST] = {
+ [TYPE_NONE] = "none",
+ [TYPE_XATTR] = "xattr",
+ };
+ int i;
+
+ for (i = 0; i < TYPE__LAST; i++)
+ if (!strcmp(metadata_type_str[i], arg))
+ return i;
+
+ return TYPE_NONE;
+}
+
+static int cpio_mkfile(const char *name, const char *location,
+ unsigned int mode, uid_t uid, gid_t gid,
+ unsigned int nlinks);
+
+static int write_xattrs(const char *path)
+{
+ struct metadata_hdr hdr = { .c_version = 1, .c_type = TYPE_XATTR };
+ char str[sizeof(hdr.c_size) + 1];
+ char *xattr_list, *list_ptr, *xattr_value;
+ ssize_t list_len, name_len, value_len, len;
+ int ret = -EINVAL;
+
+ if (metadata_fd < 0)
+ return 0;
+
+ if (path == metadata_path)
+ return 0;
+
+ list_len = listxattr(path, NULL, 0);
+ if (list_len <= 0)
+ return 0;
+
+ list_ptr = xattr_list = malloc(list_len);
+ if (!list_ptr) {
+ fprintf(stderr, "out of memory\n");
+ return ret;
+ }
+
+ len = listxattr(path, xattr_list, list_len);
+ if (len != list_len)
+ goto out;
+
+ if (ftruncate(metadata_fd, 0))
+ goto out;
+
+ lseek(metadata_fd, 0, SEEK_SET);
+
+ while (list_ptr < xattr_list + list_len) {
+ name_len = strlen(list_ptr);
+
+ value_len = getxattr(path, list_ptr, NULL, 0);
+ if (value_len < 0) {
+ fprintf(stderr, "cannot get xattrs\n");
+ break;
+ }
+
+ if (value_len) {
+ xattr_value = malloc(value_len);
+ if (!xattr_value) {
+ fprintf(stderr, "out of memory\n");
+ break;
+ }
+ } else {
+ xattr_value = NULL;
+ }
+
+ len = getxattr(path, list_ptr, xattr_value, value_len);
+ if (len != value_len)
+ break;
+
+ snprintf(str, sizeof(str), "%.8lx",
+ sizeof(hdr) + name_len + 1 + value_len);
+
+ memcpy(hdr.c_size, str, sizeof(hdr.c_size));
+
+ if (write(metadata_fd, &hdr, sizeof(hdr)) != sizeof(hdr))
+ break;
+
+ if (write(metadata_fd, list_ptr, name_len + 1) != name_len + 1)
+ break;
+
+ if (write(metadata_fd, xattr_value, value_len) != value_len)
+ break;
+
+ if (fsync(metadata_fd))
+ break;
+
+ list_ptr += name_len + 1;
+ free(xattr_value);
+ xattr_value = NULL;
+ }
+
+ free(xattr_value);
+out:
+ free(xattr_list);
+
+ if (list_ptr != xattr_list + list_len)
+ return ret;
+
+ return cpio_mkfile(METADATA_FILENAME, metadata_path, S_IFREG, 0, 0, 1);
+}
struct file_handler {
const char *type;
@@ -128,7 +239,7 @@ static int cpio_mkslink(const char *name, const char *target,
push_pad();
push_string(target);
push_pad();
- return 0;
+ return write_xattrs(name);
}
static int cpio_mkslink_line(const char *line)
@@ -174,7 +285,7 @@ static int cpio_mkgeneric(const char *name, unsigned int mode,
0); /* chksum */
push_hdr(s);
push_rest(name);
- return 0;
+ return write_xattrs(name);
}
enum generic_types {
@@ -268,7 +379,7 @@ static int cpio_mknod(const char *name, unsigned int mode,
0); /* chksum */
push_hdr(s);
push_rest(name);
- return 0;
+ return write_xattrs(name);
}
static int cpio_mknod_line(const char *line)
@@ -372,8 +483,7 @@ static int cpio_mkfile(const char *name, const char *location,
name += namesize;
}
ino++;
- rc = 0;
-
+ rc = write_xattrs(location);
error:
if (filebuf) free(filebuf);
if (file >= 0) close(file);
@@ -526,10 +636,11 @@ int main (int argc, char *argv[])
int ec = 0;
int line_nr = 0;
const char *filename;
+ enum metadata_types metadata_type = TYPE_NONE;
default_mtime = time(NULL);
while (1) {
- int opt = getopt(argc, argv, "t:h");
+ int opt = getopt(argc, argv, "t:e:h");
char *invalid;
if (opt == -1)
@@ -544,6 +655,9 @@ int main (int argc, char *argv[])
exit(1);
}
break;
+ case 'e':
+ metadata_type = parse_metadata_type(optarg);
+ break;
case 'h':
case '?':
usage(argv[0]);
@@ -565,6 +679,14 @@ int main (int argc, char *argv[])
exit(1);
}
+ if (metadata_type != TYPE_NONE) {
+ metadata_fd = mkstemp(metadata_path);
+ if (metadata_fd < 0) {
+ fprintf(stderr, "cannot create temporary file\n");
+ exit(1);
+ }
+ }
+
while (fgets(line, LINE_SIZE, cpio_list)) {
int type_idx;
size_t slen = strlen(line);
@@ -620,5 +742,8 @@ int main (int argc, char *argv[])
if (ec == 0)
cpio_trailer();
+ if (metadata_type != TYPE_NONE)
+ close(metadata_fd);
+
exit(ec);
}
diff --git a/usr/gen_initramfs_list.sh b/usr/gen_initramfs_list.sh
index 0aad760fcd8c..0907a4043da9 100755
--- a/usr/gen_initramfs_list.sh
+++ b/usr/gen_initramfs_list.sh
@@ -15,7 +15,7 @@ set -e
usage() {
cat << EOF
Usage:
-$0 [-o <file>] [-u <uid>] [-g <gid>] {-d | <cpio_source>} ...
+$0 [-o <file>] [-u <uid>] [-g <gid>] {-d | <cpio_source>} [-e <type>] ...
-o <file> Create compressed initramfs file named <file> using
gen_init_cpio and compressor depending on the extension
-u <uid> User ID to map to user ID 0 (root).
@@ -28,6 +28,7 @@ $0 [-o <file>] [-u <uid>] [-g <gid>] {-d | <cpio_source>} ...
If <cpio_source> is a .cpio file it will be used
as direct input to initramfs.
-d Output the default cpio list.
+ -e <type> File metadata type to include in the cpio archive.
All options except -o and -l may be repeated and are interpreted
sequentially and immediately. -u and -g states are preserved across
@@ -283,6 +284,10 @@ while [ $# -gt 0 ]; do
default_list="$arg"
${dep_list}default_initramfs
;;
+ "-e") # file metadata type
+ metadata_arg="-e $1"
+ shift
+ ;;
"-h")
usage
exit 0
@@ -312,7 +317,8 @@ 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 $metadata_arg $timestamp \
+ ${cpio_list} > ${cpio_tfile}
else
cpio_tfile=${cpio_file}
fi
--
2.17.1
Any opinion on this patch set?
Thanks
Roberto
On 5/23/2019 2:18 PM, Roberto Sassu wrote:
> This patch set aims at solving the following use case: appraise files from
> the initial ram disk. To do that, IMA checks the signature/hash from the
> security.ima xattr. Unfortunately, this use case cannot be implemented
> currently, as the CPIO format does not support xattrs.
>
> This proposal consists in including file metadata as additional files named
> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
> kernel recognizes these special files from the file name, and calls the
> appropriate parser to add metadata to the previously extracted file. It has
> been proposed to use bit 17:16 of the file mode as a way to recognize files
> with metadata, but both the kernel and the cpio tool declare the file mode
> as unsigned short.
>
> The difference from v2, v3 (https://lkml.org/lkml/2019/5/9/230,
> https://lkml.org/lkml/2019/5/17/466) is that file metadata are stored in
> separate files instead of a single file. Given that files with metadata
> must immediately follow the files metadata will be added to, image
> generators have to be modified in this version.
>
> The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that
> all files have the same name. The file metadata are added to is always the
> previous one, and the image generator in user space will make sure that
> files are in the correct sequence.
>
> The difference with another proposal
> (https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be
> included in an image without changing the image format. Files with metadata
> will appear as regular files. It will be task of the parser in the kernel
> to process them.
>
> This patch set extends the format of data defined in patch 9/15 of the last
> proposal. It adds header version and type, so that new formats can be
> defined and arbitrary metadata types can be processed.
>
> The changes introduced by this patch set don't cause any compatibility
> issue: kernels without the metadata parser simply extract the special files
> and don't process metadata; kernels with the metadata parser don't process
> metadata if the special files are not included in the image.
>
> From the kernel space perspective, backporting this functionality to older
> kernels should be very easy. It is sufficient to add two calls to the new
> function do_process_metadata() in do_copy(), and to check the file name in
> do_name(). From the user space perspective, unlike the previous version of
> the patch set, it is required to modify the image generators in order to
> include metadata as separate files.
>
> Changelog
>
> v3:
> - include file metadata as separate files named METADATA!!!
> - add the possibility to include in the ram disk arbitrary metadata types
>
> v2:
> - replace ksys_lsetxattr() with kern_path() and vfs_setxattr()
> (suggested by Jann Horn)
> - replace ksys_open()/ksys_read()/ksys_close() with
> filp_open()/kernel_read()/fput()
> (suggested by Jann Horn)
> - use path variable instead of name_buf in do_readxattrs()
> - set last byte of str to 0 in do_readxattrs()
> - call do_readxattrs() in do_name() before replacing an existing
> .xattr-list
> - pass pathname to do_setxattrs()
>
> v1:
> - move xattr unmarshaling to CPIO parser
>
>
> Mimi Zohar (1):
> initramfs: add file metadata
>
> Roberto Sassu (2):
> initramfs: read metadata from special file METADATA!!!
> gen_init_cpio: add support for file metadata
>
> include/linux/initramfs.h | 21 ++++++
> init/initramfs.c | 137 +++++++++++++++++++++++++++++++++++++-
> usr/Kconfig | 8 +++
> usr/Makefile | 4 +-
> usr/gen_init_cpio.c | 137 ++++++++++++++++++++++++++++++++++++--
> usr/gen_initramfs_list.sh | 10 ++-
> 6 files changed, 305 insertions(+), 12 deletions(-)
> create mode 100644 include/linux/initramfs.h
>
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI
On 6/3/19 4:31 AM, Roberto Sassu wrote:
>> This patch set aims at solving the following use case: appraise files from
>> the initial ram disk. To do that, IMA checks the signature/hash from the
>> security.ima xattr. Unfortunately, this use case cannot be implemented
>> currently, as the CPIO format does not support xattrs.
>>
>> This proposal consists in including file metadata as additional files named
>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
>> kernel recognizes these special files from the file name, and calls the
>> appropriate parser to add metadata to the previously extracted file. It has
>> been proposed to use bit 17:16 of the file mode as a way to recognize files
>> with metadata, but both the kernel and the cpio tool declare the file mode
>> as unsigned short.
>
> Any opinion on this patch set?
>
> Thanks
>
> Roberto
Sorry, I've had the window open since you posted it but haven't gotten around to
it. I'll try to build it later today.
It does look interesting, and I have no objections to the basic approach. I
should be able to add support to toybox cpio over a weekend once I've got the
kernel doing it to test against.
Rob
On 6/3/2019 8:32 PM, Rob Landley wrote:
> On 6/3/19 4:31 AM, Roberto Sassu wrote:
>>> This patch set aims at solving the following use case: appraise files from
>>> the initial ram disk. To do that, IMA checks the signature/hash from the
>>> security.ima xattr. Unfortunately, this use case cannot be implemented
>>> currently, as the CPIO format does not support xattrs.
>>>
>>> This proposal consists in including file metadata as additional files named
>>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
>>> kernel recognizes these special files from the file name, and calls the
>>> appropriate parser to add metadata to the previously extracted file. It has
>>> been proposed to use bit 17:16 of the file mode as a way to recognize files
>>> with metadata, but both the kernel and the cpio tool declare the file mode
>>> as unsigned short.
>>
>> Any opinion on this patch set?
>>
>> Thanks
>>
>> Roberto
>
> Sorry, I've had the window open since you posted it but haven't gotten around to
> it. I'll try to build it later today.
>
> It does look interesting, and I have no objections to the basic approach. I
> should be able to add support to toybox cpio over a weekend once I've got the
> kernel doing it to test against.
Ok.
Let me give some instructions so that people can test this patch set.
To add xattrs to the ram disk embedded in the kernel it is sufficient
to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration.
To add xattrs to the external ram disk, it is necessary to patch cpio:
https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef
(xattr-v1 branch)
and dracut:
https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b
(digest-lists branch)
The same modification can be done for mkinitramfs (add '-e xattr' to the
cpio command line).
To simplify the test, it would be sufficient to replace only the cpio
binary and the dracut script with the modified versions. For dracut, the
patch should be applied to the local dracut (after it has been renamed
to dracut.sh).
Then, run:
dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk)
Xattrs can be seen by stopping the boot process for example by adding
rd.break to the kernel command line.
Roberto
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI
On Thu, 2019-05-23 at 14:18 +0200, Roberto Sassu wrote:
> diff --git a/usr/Kconfig b/usr/Kconfig
> index 43658b8a975e..8d9f54a16440 100644
> --- a/usr/Kconfig
> +++ b/usr/Kconfig
> @@ -233,3 +233,11 @@ config INITRAMFS_COMPRESSION
> default ".lzma" if RD_LZMA
> default ".bz2" if RD_BZIP2
> default ""
> +
> +config INITRAMFS_FILE_METADATA
> + string "File metadata type"
> + default ""
> + help
> + Specify xattr to include xattrs in the image.
> +
> + If you are not sure, leave it blank.
> fi
Instead of having to specify the metdata type, let's make this a
choice.
Mimi
On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote:
> On 6/3/2019 8:32 PM, Rob Landley wrote:
> > On 6/3/19 4:31 AM, Roberto Sassu wrote:
> >>> This patch set aims at solving the following use case: appraise files from
> >>> the initial ram disk. To do that, IMA checks the signature/hash from the
> >>> security.ima xattr. Unfortunately, this use case cannot be implemented
> >>> currently, as the CPIO format does not support xattrs.
> >>>
> >>> This proposal consists in including file metadata as additional files named
> >>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
> >>> kernel recognizes these special files from the file name, and calls the
> >>> appropriate parser to add metadata to the previously extracted file. It has
> >>> been proposed to use bit 17:16 of the file mode as a way to recognize files
> >>> with metadata, but both the kernel and the cpio tool declare the file mode
> >>> as unsigned short.
> >>
> >> Any opinion on this patch set?
> >>
> >> Thanks
> >>
> >> Roberto
> >
> > Sorry, I've had the window open since you posted it but haven't gotten around to
> > it. I'll try to build it later today.
> >
> > It does look interesting, and I have no objections to the basic approach. I
> > should be able to add support to toybox cpio over a weekend once I've got the
> > kernel doing it to test against.
>
> Ok.
>
> Let me give some instructions so that people can test this patch set.
>
> To add xattrs to the ram disk embedded in the kernel it is sufficient
> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration.
>
> To add xattrs to the external ram disk, it is necessary to patch cpio:
>
> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef
> (xattr-v1 branch)
>
> and dracut:
>
> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b
> (digest-lists branch)
>
> The same modification can be done for mkinitramfs (add '-e xattr' to the
> cpio command line).
>
> To simplify the test, it would be sufficient to replace only the cpio
> binary and the dracut script with the modified versions. For dracut, the
> patch should be applied to the local dracut (after it has been renamed
> to dracut.sh).
>
> Then, run:
>
> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk)
>
> Xattrs can be seen by stopping the boot process for example by adding
> rd.break to the kernel command line.
A simple way of testing, without needing any changes other than the
kernel patches, is to save the dracut temporary directory by supplying
"--keep" on the dracut command line, calling
usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e
xattr" option.
If your filesystem already has and copied the security xattrs to the
dracut temporary directory, the script, below, will include them in
the initramfs file. Otherwise, you'll need to write the desired
security xattrs on the files, using setfattr, in the temporary dracut
directory, before creating the initramfs.
Remember to make sure that the initramfs_list includes "getfattr",
otherwise you'll need to wait until real root is mounted as /sysroot
to see the security xattrs for the rootfs files.
The following script has not been tested on a recent version of
dracut. Some changes might be needed, as well as some code cleanup.
#!/bin/bash
initramfs_name=/boot/initramfs-`uname -r`.img
initramfs_output_name=${initramfs_name/.img/.test.img}
if [ $# -eq 1 ]; then
initramfs_name=$1
fi
if [ ! -f "$initramfs_name" ]; then
echo "Usage; $0 <initramfs pathanem>"
exit 1
fi
tmp=$(dracut -H -f "$initramfs_name" --keep --noprelink --nostrip 2>&1)
suffix=$(echo $tmp | cut -d ' ' -f 3 | cut -d '.' -f 2)
tmpdir="/var/tmp/dracut.$suffix/initramfs"
if [ ! -d "$tmpdir" ]; then
echo "$tmpdir does not exist"
exit 1
fi
usr/gen_initramfs_list.sh ${tmpdir} > usr/initramfs_list
usr/gen_init_cpio -e xattr usr/initramfs_list > usr/initramfs_data.cpio
gzip usr/initramfs_data.cpio
echo "Copying usr/initramfs_data.cpio to $initramfs_output_name"
cp usr/initramfs_data.cpio.gz "$initramfs_output_name"
Mimi
Hi Roberto,
> diff --git a/init/initramfs.c b/init/initramfs.c
> index 5de396a6aac0..862c03123de8 100644
> --- a/init/initramfs.c
> +++ b/init/initramfs.c
> +static int __init do_process_metadata(char *buf, int len, bool last)
> +{
Part of the problem in upstreaming CPIO xattr support has been the
difficulty in reading and understanding the initramfs code due to a
lack of comments. At least for any new code, let's add some comments
to simplify the review. In this case, understanding "last", before
reading the code, would help.
Mimi
> + int ret = 0;
> +
> + if (!metadata_buf) {
> + metadata_buf_ptr = metadata_buf = kmalloc(body_len, GFP_KERNEL);
> + if (!metadata_buf_ptr) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + metadata_len = body_len;
> + }
> +
> + if (metadata_buf_ptr + len > metadata_buf + metadata_len) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + memcpy(metadata_buf_ptr, buf, len);
> + metadata_buf_ptr += len;
> +
> + if (last)
> + do_parse_metadata(previous_name_buf);
> +out:
> + if (ret < 0 || last) {
> + kfree(metadata_buf);
> + metadata_buf = NULL;
> + metadata = 0;
> + }
> +
> + return ret;
> +}
> +
> static int __init do_copy(void)
> {
> if (byte_count >= body_len) {
> if (xwrite(wfd, victim, body_len) != body_len)
> error("write error");
> + if (metadata)
> + do_process_metadata(victim, body_len, true);
> ksys_close(wfd);
> do_utime(vcollected, mtime);
> kfree(vcollected);
> @@ -458,6 +500,8 @@ static int __init do_copy(void)
> } else {
> if (xwrite(wfd, victim, byte_count) != byte_count)
> error("write error");
> + if (metadata)
> + do_process_metadata(victim, byte_count, false);
> body_len -= byte_count;
> eat(byte_count);
> return 1;
>
On Thu, 2019-05-23 at 14:18 +0200, Roberto Sassu wrote:
> This patch set aims at solving the following use case: appraise files from
> the initial ram disk. To do that, IMA checks the signature/hash from the
> security.ima xattr. Unfortunately, this use case cannot be implemented
> currently, as the CPIO format does not support xattrs.
>
> This proposal consists in including file metadata as additional files named
> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
> kernel recognizes these special files from the file name, and calls the
> appropriate parser to add metadata to the previously extracted file. It has
> been proposed to use bit 17:16 of the file mode as a way to recognize files
> with metadata, but both the kernel and the cpio tool declare the file mode
> as unsigned short.
Thanks, Roberto!
Victor, Taras, Rob, Arvind, Peter, if you're good with this latest
design, could we get some Reviewed-by, Acked-by, or Tested-by?
thanks!
Mimi
On 6/30/2019 6:39 PM, Mimi Zohar wrote:
> On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote:
>> On 6/3/2019 8:32 PM, Rob Landley wrote:
>>> On 6/3/19 4:31 AM, Roberto Sassu wrote:
>>>>> This patch set aims at solving the following use case: appraise files from
>>>>> the initial ram disk. To do that, IMA checks the signature/hash from the
>>>>> security.ima xattr. Unfortunately, this use case cannot be implemented
>>>>> currently, as the CPIO format does not support xattrs.
>>>>>
>>>>> This proposal consists in including file metadata as additional files named
>>>>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
>>>>> kernel recognizes these special files from the file name, and calls the
>>>>> appropriate parser to add metadata to the previously extracted file. It has
>>>>> been proposed to use bit 17:16 of the file mode as a way to recognize files
>>>>> with metadata, but both the kernel and the cpio tool declare the file mode
>>>>> as unsigned short.
>>>>
>>>> Any opinion on this patch set?
>>>>
>>>> Thanks
>>>>
>>>> Roberto
>>>
>>> Sorry, I've had the window open since you posted it but haven't gotten around to
>>> it. I'll try to build it later today.
>>>
>>> It does look interesting, and I have no objections to the basic approach. I
>>> should be able to add support to toybox cpio over a weekend once I've got the
>>> kernel doing it to test against.
>>
>> Ok.
>>
>> Let me give some instructions so that people can test this patch set.
>>
>> To add xattrs to the ram disk embedded in the kernel it is sufficient
>> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
>> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration.
>>
>> To add xattrs to the external ram disk, it is necessary to patch cpio:
>>
>> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef
>> (xattr-v1 branch)
>>
>> and dracut:
>>
>> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b
>> (digest-lists branch)
>>
>> The same modification can be done for mkinitramfs (add '-e xattr' to the
>> cpio command line).
>>
>> To simplify the test, it would be sufficient to replace only the cpio
>> binary and the dracut script with the modified versions. For dracut, the
>> patch should be applied to the local dracut (after it has been renamed
>> to dracut.sh).
>>
>> Then, run:
>>
>> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk)
>>
>> Xattrs can be seen by stopping the boot process for example by adding
>> rd.break to the kernel command line.
>
> A simple way of testing, without needing any changes other than the
> kernel patches, is to save the dracut temporary directory by supplying
> "--keep" on the dracut command line, calling
> usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e
> xattr" option.
Alternatively, follow the instructions to create the embedded ram disk
with xattrs, and use the existing external ram disk created with dracut
to check if xattrs are created.
Roberto
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI
On Mon, 2019-07-01 at 16:42 +0300, Roberto Sassu wrote:
> On 6/30/2019 6:39 PM, Mimi Zohar wrote:
> > On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote:
> >> On 6/3/2019 8:32 PM, Rob Landley wrote:
> >>> On 6/3/19 4:31 AM, Roberto Sassu wrote:
> >>>>> This patch set aims at solving the following use case: appraise files from
> >>>>> the initial ram disk. To do that, IMA checks the signature/hash from the
> >>>>> security.ima xattr. Unfortunately, this use case cannot be implemented
> >>>>> currently, as the CPIO format does not support xattrs.
> >>>>>
> >>>>> This proposal consists in including file metadata as additional files named
> >>>>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
> >>>>> kernel recognizes these special files from the file name, and calls the
> >>>>> appropriate parser to add metadata to the previously extracted file. It has
> >>>>> been proposed to use bit 17:16 of the file mode as a way to recognize files
> >>>>> with metadata, but both the kernel and the cpio tool declare the file mode
> >>>>> as unsigned short.
> >>>>
> >>>> Any opinion on this patch set?
> >>>>
> >>>> Thanks
> >>>>
> >>>> Roberto
> >>>
> >>> Sorry, I've had the window open since you posted it but haven't gotten around to
> >>> it. I'll try to build it later today.
> >>>
> >>> It does look interesting, and I have no objections to the basic approach. I
> >>> should be able to add support to toybox cpio over a weekend once I've got the
> >>> kernel doing it to test against.
> >>
> >> Ok.
> >>
> >> Let me give some instructions so that people can test this patch set.
> >>
> >> To add xattrs to the ram disk embedded in the kernel it is sufficient
> >> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
> >> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration.
> >>
> >> To add xattrs to the external ram disk, it is necessary to patch cpio:
> >>
> >> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef
> >> (xattr-v1 branch)
> >>
> >> and dracut:
> >>
> >> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b
> >> (digest-lists branch)
> >>
> >> The same modification can be done for mkinitramfs (add '-e xattr' to the
> >> cpio command line).
> >>
> >> To simplify the test, it would be sufficient to replace only the cpio
> >> binary and the dracut script with the modified versions. For dracut, the
> >> patch should be applied to the local dracut (after it has been renamed
> >> to dracut.sh).
> >>
> >> Then, run:
> >>
> >> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk)
> >>
> >> Xattrs can be seen by stopping the boot process for example by adding
> >> rd.break to the kernel command line.
> >
> > A simple way of testing, without needing any changes other than the
> > kernel patches, is to save the dracut temporary directory by supplying
> > "--keep" on the dracut command line, calling
> > usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e
> > xattr" option.
>
> Alternatively, follow the instructions to create the embedded ram disk
> with xattrs, and use the existing external ram disk created with dracut
> to check if xattrs are created.
True, but this alternative is for those who normally use dracut to
create an initramfs, but don't want to update cpio or dracut.
Mimi
Rob, Peter, Arvind, did you have the chance to have a look at this
version of the patch set?
Thanks
Roberto
On 7/1/2019 4:31 PM, Mimi Zohar wrote:
> On Mon, 2019-07-01 at 16:42 +0300, Roberto Sassu wrote:
>> On 6/30/2019 6:39 PM, Mimi Zohar wrote:
>>> On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote:
>>>> On 6/3/2019 8:32 PM, Rob Landley wrote:
>>>>> On 6/3/19 4:31 AM, Roberto Sassu wrote:
>>>>>>> This patch set aims at solving the following use case: appraise files from
>>>>>>> the initial ram disk. To do that, IMA checks the signature/hash from the
>>>>>>> security.ima xattr. Unfortunately, this use case cannot be implemented
>>>>>>> currently, as the CPIO format does not support xattrs.
>>>>>>>
>>>>>>> This proposal consists in including file metadata as additional files named
>>>>>>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
>>>>>>> kernel recognizes these special files from the file name, and calls the
>>>>>>> appropriate parser to add metadata to the previously extracted file. It has
>>>>>>> been proposed to use bit 17:16 of the file mode as a way to recognize files
>>>>>>> with metadata, but both the kernel and the cpio tool declare the file mode
>>>>>>> as unsigned short.
>>>>>>
>>>>>> Any opinion on this patch set?
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Roberto
>>>>>
>>>>> Sorry, I've had the window open since you posted it but haven't gotten around to
>>>>> it. I'll try to build it later today.
>>>>>
>>>>> It does look interesting, and I have no objections to the basic approach. I
>>>>> should be able to add support to toybox cpio over a weekend once I've got the
>>>>> kernel doing it to test against.
>>>>
>>>> Ok.
>>>>
>>>> Let me give some instructions so that people can test this patch set.
>>>>
>>>> To add xattrs to the ram disk embedded in the kernel it is sufficient
>>>> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
>>>> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration.
>>>>
>>>> To add xattrs to the external ram disk, it is necessary to patch cpio:
>>>>
>>>> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef
>>>> (xattr-v1 branch)
>>>>
>>>> and dracut:
>>>>
>>>> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b
>>>> (digest-lists branch)
>>>>
>>>> The same modification can be done for mkinitramfs (add '-e xattr' to the
>>>> cpio command line).
>>>>
>>>> To simplify the test, it would be sufficient to replace only the cpio
>>>> binary and the dracut script with the modified versions. For dracut, the
>>>> patch should be applied to the local dracut (after it has been renamed
>>>> to dracut.sh).
>>>>
>>>> Then, run:
>>>>
>>>> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk)
>>>>
>>>> Xattrs can be seen by stopping the boot process for example by adding
>>>> rd.break to the kernel command line.
>>>
>>> A simple way of testing, without needing any changes other than the
>>> kernel patches, is to save the dracut temporary directory by supplying
>>> "--keep" on the dracut command line, calling
>>> usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e
>>> xattr" option.
>>
>> Alternatively, follow the instructions to create the embedded ram disk
>> with xattrs, and use the existing external ram disk created with dracut
>> to check if xattrs are created.
>
> True, but this alternative is for those who normally use dracut to
> create an initramfs, but don't want to update cpio or dracut.
>
> Mimi
>
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
Is there anything I didn't address in this patch set, that is delaying
the review? I would appreciate if you can give me a feedback, positive
or negative.
Thanks a lot!
Roberto
On 7/15/2019 6:54 PM, Roberto Sassu wrote:
> Rob, Peter, Arvind, did you have the chance to have a look at this
> version of the patch set?
>
> Thanks
>
> Roberto
>
>
> On 7/1/2019 4:31 PM, Mimi Zohar wrote:
>> On Mon, 2019-07-01 at 16:42 +0300, Roberto Sassu wrote:
>>> On 6/30/2019 6:39 PM, Mimi Zohar wrote:
>>>> On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote:
>>>>> On 6/3/2019 8:32 PM, Rob Landley wrote:
>>>>>> On 6/3/19 4:31 AM, Roberto Sassu wrote:
>>>>>>>> This patch set aims at solving the following use case: appraise
>>>>>>>> files from
>>>>>>>> the initial ram disk. To do that, IMA checks the signature/hash
>>>>>>>> from the
>>>>>>>> security.ima xattr. Unfortunately, this use case cannot be
>>>>>>>> implemented
>>>>>>>> currently, as the CPIO format does not support xattrs.
>>>>>>>>
>>>>>>>> This proposal consists in including file metadata as additional
>>>>>>>> files named
>>>>>>>> METADATA!!!, for each file added to the ram disk. The CPIO
>>>>>>>> parser in the
>>>>>>>> kernel recognizes these special files from the file name, and
>>>>>>>> calls the
>>>>>>>> appropriate parser to add metadata to the previously extracted
>>>>>>>> file. It has
>>>>>>>> been proposed to use bit 17:16 of the file mode as a way to
>>>>>>>> recognize files
>>>>>>>> with metadata, but both the kernel and the cpio tool declare the
>>>>>>>> file mode
>>>>>>>> as unsigned short.
>>>>>>>
>>>>>>> Any opinion on this patch set?
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> Roberto
>>>>>>
>>>>>> Sorry, I've had the window open since you posted it but haven't
>>>>>> gotten around to
>>>>>> it. I'll try to build it later today.
>>>>>>
>>>>>> It does look interesting, and I have no objections to the basic
>>>>>> approach. I
>>>>>> should be able to add support to toybox cpio over a weekend once
>>>>>> I've got the
>>>>>> kernel doing it to test against.
>>>>>
>>>>> Ok.
>>>>>
>>>>> Let me give some instructions so that people can test this patch set.
>>>>>
>>>>> To add xattrs to the ram disk embedded in the kernel it is sufficient
>>>>> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and
>>>>> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel
>>>>> configuration.
>>>>>
>>>>> To add xattrs to the external ram disk, it is necessary to patch cpio:
>>>>>
>>>>> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef
>>>>>
>>>>> (xattr-v1 branch)
>>>>>
>>>>> and dracut:
>>>>>
>>>>> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b
>>>>>
>>>>> (digest-lists branch)
>>>>>
>>>>> The same modification can be done for mkinitramfs (add '-e xattr'
>>>>> to the
>>>>> cpio command line).
>>>>>
>>>>> To simplify the test, it would be sufficient to replace only the cpio
>>>>> binary and the dracut script with the modified versions. For
>>>>> dracut, the
>>>>> patch should be applied to the local dracut (after it has been renamed
>>>>> to dracut.sh).
>>>>>
>>>>> Then, run:
>>>>>
>>>>> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram
>>>>> disk)
>>>>>
>>>>> Xattrs can be seen by stopping the boot process for example by adding
>>>>> rd.break to the kernel command line.
>>>>
>>>> A simple way of testing, without needing any changes other than the
>>>> kernel patches, is to save the dracut temporary directory by supplying
>>>> "--keep" on the dracut command line, calling
>>>> usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e
>>>> xattr" option.
>>>
>>> Alternatively, follow the instructions to create the embedded ram disk
>>> with xattrs, and use the existing external ram disk created with dracut
>>> to check if xattrs are created.
>>
>> True, but this alternative is for those who normally use dracut to
>> create an initramfs, but don't want to update cpio or dracut.
>>
>> Mimi
>>
>
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
Dear Roberto,
Cc: Yamada-san, linux-kbuild
On Mi, Jul 24, 2019 at 05:34:53 +0200, Roberto Sassu wrote:
> Is there anything I didn't address in this patch set, that is delaying
> the review? I would appreciate if you can give me a feedback, positive
> or negative.
>
> Thanks a lot!
>
> Roberto
Some of our users have recently asked for this patch series.
Could you please feedback if this is the latest revision available or
maybe there is a newer one developed and potentially not shared on LKML?
Appreciate your time.
Thanks and Best Regards,
Eugeniu
Hello Roberto,
On Do, Jun 09, 2022 at 11:05:45 +0000, Roberto Sassu wrote:
> > From: Eugeniu Rosca [mailto:[email protected]]
> > Sent: Thursday, June 9, 2022 12:26 PM
> > Dear Roberto,
> > Cc: Yamada-san, linux-kbuild
> >
> > On Mi, Jul 24, 2019 at 05:34:53 +0200, Roberto Sassu wrote:
> > > Is there anything I didn't address in this patch set, that is delaying
> > > the review? I would appreciate if you can give me a feedback, positive
> > > or negative.
> > >
> > > Thanks a lot!
> > >
> > > Roberto
> >
> > Some of our users have recently asked for this patch series.
>
> Hello
>
> thanks for your interest in this patch set.
>
> > Could you please feedback if this is the latest revision available or
> > maybe there is a newer one developed and potentially not shared on LKML?
>
> Yes, it is the latest revision available. There might have been few
> fixes in the final code. You may want to have a look at:
Many thanks for the links to the updated patch revisions. It looks
like the new versions added a couple of bugfixes and refinements.
With more users now using this feature, do you think there is a higher
chance for upstreaming, compared to 2019 (original submission date)?
Best Regards,
Eugeniu
Hello Roberto,
On Fr, Jun 10, 2022 at 03:38:24 +0000, Roberto Sassu wrote:
> I would be happy to address the remaining concerns, or take more
> suggestions, and then develop a new version of the patch set.
I face a number of conflicts when I try to rebase the latest openEuler
commits against vanilla master (v5.19-rc2). Do you think it is possible
to submit the rebased version to ML?
In addition, I can also see some open/unresolved points from Mimi [*].
Did you by chance find some mutual agreement offline or do you think
they would still potentially need some attention?
Maybe we can resume the discussion once you submit the rebased series?
Many thanks and looking forward to it.
[*] Potentially comments which deserve a reply/clarification/resolution
https://lore.kernel.org/lkml/[email protected]/#t
https://lore.kernel.org/lkml/[email protected]/
BR, Eugeniu.
From: Roberto Sassu <[email protected]>
Date: Thu, 23 May 2019 14:18:00 +0200
> This patch set aims at solving the following use case: appraise files from
> the initial ram disk. To do that, IMA checks the signature/hash from the
Hi,
is this[0] relatable somehow?
> security.ima xattr. Unfortunately, this use case cannot be implemented
> currently, as the CPIO format does not support xattrs.
>
> This proposal consists in including file metadata as additional files named
> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
> kernel recognizes these special files from the file name, and calls the
> appropriate parser to add metadata to the previously extracted file. It has
> been proposed to use bit 17:16 of the file mode as a way to recognize files
> with metadata, but both the kernel and the cpio tool declare the file mode
> as unsigned short.
>
> The difference from v2, v3 (https://lkml.org/lkml/2019/5/9/230,
> https://lkml.org/lkml/2019/5/17/466) is that file metadata are stored in
> separate files instead of a single file. Given that files with metadata
> must immediately follow the files metadata will be added to, image
> generators have to be modified in this version.
>
> The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that
> all files have the same name. The file metadata are added to is always the
> previous one, and the image generator in user space will make sure that
> files are in the correct sequence.
>
> The difference with another proposal
> (https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be
> included in an image without changing the image format. Files with metadata
> will appear as regular files. It will be task of the parser in the kernel
> to process them.
>
> This patch set extends the format of data defined in patch 9/15 of the last
> proposal. It adds header version and type, so that new formats can be
> defined and arbitrary metadata types can be processed.
>
> The changes introduced by this patch set don't cause any compatibility
> issue: kernels without the metadata parser simply extract the special files
> and don't process metadata; kernels with the metadata parser don't process
> metadata if the special files are not included in the image.
>
> >>From the kernel space perspective, backporting this functionality to older
> kernels should be very easy. It is sufficient to add two calls to the new
> function do_process_metadata() in do_copy(), and to check the file name in
> do_name(). From the user space perspective, unlike the previous version of
> the patch set, it is required to modify the image generators in order to
> include metadata as separate files.
>
> Changelog
>
> v3:
> - include file metadata as separate files named METADATA!!!
> - add the possibility to include in the ram disk arbitrary metadata types
>
> v2:
> - replace ksys_lsetxattr() with kern_path() and vfs_setxattr()
> (suggested by Jann Horn)
> - replace ksys_open()/ksys_read()/ksys_close() with
> filp_open()/kernel_read()/fput()
> (suggested by Jann Horn)
> - use path variable instead of name_buf in do_readxattrs()
> - set last byte of str to 0 in do_readxattrs()
> - call do_readxattrs() in do_name() before replacing an existing
> .xattr-list
> - pass pathname to do_setxattrs()
>
> v1:
> - move xattr unmarshaling to CPIO parser
>
>
> Mimi Zohar (1):
> initramfs: add file metadata
>
> Roberto Sassu (2):
> initramfs: read metadata from special file METADATA!!!
> gen_init_cpio: add support for file metadata
>
> include/linux/initramfs.h | 21 ++++++
> init/initramfs.c | 137 +++++++++++++++++++++++++++++++++++++-
> usr/Kconfig | 8 +++
> usr/Makefile | 4 +-
> usr/gen_init_cpio.c | 137 ++++++++++++++++++++++++++++++++++++--
> usr/gen_initramfs_list.sh | 10 ++-
> 6 files changed, 305 insertions(+), 12 deletions(-)
> create mode 100644 include/linux/initramfs.h
>
> --
> 2.17.1
[0] https://lore.kernel.org/all/[email protected]
Thanks,
Olek
Hello Roberto,
Hello Mimi,
On Thu, May 23, 2019 at 02:18:01PM +0200, Roberto Sassu wrote:
> From: Mimi Zohar <[email protected]>
>
> This patch adds metadata to a file from a supplied buffer. The buffer might
> contains multiple metadata records. The format of each record is:
>
> <metadata len (ASCII, 8 chars)><version><type><metadata>
>
> For now, only the TYPE_XATTR metadata type is supported. The specific
> format of this metadata type is:
>
> <xattr #N name>\0<xattr #N value>
>
> [kamensky: fixed restoring of xattrs for symbolic links by using
> sys_lsetxattr() instead of sys_setxattr()]
>
> [sassu: removed state management, kept only do_setxattrs(), added support
> for generic file metadata, replaced sys_lsetxattr() with
> vfs_setxattr(), added check for entry_size, added check for
> hdr->c_size, replaced strlen() with strnlen(); moved do_setxattrs()
> before do_name()]
>
> Signed-off-by: Mimi Zohar <[email protected]>
> Signed-off-by: Victor Kamensky <[email protected]>
> Signed-off-by: Taras Kondratiuk <[email protected]>
> Signed-off-by: Roberto Sassu <[email protected]>
> ---
> include/linux/initramfs.h | 21 ++++++++++
> init/initramfs.c | 88 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 107 insertions(+), 2 deletions(-)
> create mode 100644 include/linux/initramfs.h
[..]
> +static int __init do_setxattrs(char *pathname, char *buf, size_t size)
> +{
> + struct path path;
> + char *xattr_name, *xattr_value;
> + size_t xattr_name_size, xattr_value_size;
> + int ret;
> +
> + xattr_name = buf;
> + xattr_name_size = strnlen(xattr_name, size);
> + if (xattr_name_size == size) {
> + error("malformed xattrs");
> + return -EINVAL;
> + }
> +
[..]
> +
> + switch (hdr->c_type) {
> + case TYPE_XATTR:
> + do_setxattrs(pathname, buf + sizeof(*hdr),
> + entry_size - sizeof(*hdr));
Is it on purpose not to check the return value of do_setxattrs?
I think I would have more comfort and piece of mind if I knew
the return value is properly checked and acted upon. Otherwise,
why returning an int from within do_setxattrs() at all?
BR, Eugeniu
Dear Yamada-san,
On Do, Mai 23, 2019 at 02:18:00 +0200, Roberto Sassu wrote:
> This patch set aims at solving the following use case: appraise files from
> the initial ram disk. To do that, IMA checks the signature/hash from the
> security.ima xattr. Unfortunately, this use case cannot be implemented
> currently, as the CPIO format does not support xattrs.
>
> This proposal consists in including file metadata as additional files named
> METADATA!!!, for each file added to the ram disk. The CPIO parser in the
> kernel recognizes these special files from the file name, and calls the
> appropriate parser to add metadata to the previously extracted file. It has
> been proposed to use bit 17:16 of the file mode as a way to recognize files
> with metadata, but both the kernel and the cpio tool declare the file mode
> as unsigned short.
>
> The difference from v2, v3 (https://lkml.org/lkml/2019/5/9/230,
> https://lkml.org/lkml/2019/5/17/466) is that file metadata are stored in
> separate files instead of a single file. Given that files with metadata
> must immediately follow the files metadata will be added to, image
> generators have to be modified in this version.
>
> The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that
> all files have the same name. The file metadata are added to is always the
> previous one, and the image generator in user space will make sure that
> files are in the correct sequence.
>
> The difference with another proposal
> (https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be
> included in an image without changing the image format. Files with metadata
> will appear as regular files. It will be task of the parser in the kernel
> to process them.
>
> This patch set extends the format of data defined in patch 9/15 of the last
> proposal. It adds header version and type, so that new formats can be
> defined and arbitrary metadata types can be processed.
>
> The changes introduced by this patch set don't cause any compatibility
> issue: kernels without the metadata parser simply extract the special files
> and don't process metadata; kernels with the metadata parser don't process
> metadata if the special files are not included in the image.
>
> >From the kernel space perspective, backporting this functionality to older
> kernels should be very easy. It is sufficient to add two calls to the new
> function do_process_metadata() in do_copy(), and to check the file name in
> do_name(). From the user space perspective, unlike the previous version of
> the patch set, it is required to modify the image generators in order to
> include metadata as separate files.
Since this patch series most likely falls under your jurisdiction and
also given your recent commits [*] in the same area, I am curious if
there are any early signs which would prevent your final acceptance
and would potentially result in a no-Go?
Can we have an early confirmation that, upon rebasing and handling of
all the review comments, you would be willing to accept the patches?
[*] Most recent commits touching usr/gen_initramfs.sh
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7168965ec7b10b8a2c7dea1f82f1ebadf44d64ba
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65e00e04e5aea34b256814cfa21b32e3b94a2402
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=469e87e89fd61de804bd29f6dd0380a399b567a7
Thanks,
Eugeniu.
Hello Roberto,
On Do, Mai 23, 2019 at 02:18:03 +0200, Roberto Sassu wrote:
> This patch adds support for file metadata (only TYPE_XATTR metadata type).
> gen_init_cpio has been modified to read xattrs from files that will be
> added to the image and to include file metadata as separate files with the
> special name 'METADATA!!!'.
>
> This behavior can be selected by setting the desired file metadata type as
> value for CONFIG_INITRAMFS_FILE_METADATA.
>
> Signed-off-by: Roberto Sassu <[email protected]>
> ---
> usr/Kconfig | 8 +++
> usr/Makefile | 4 +-
> usr/gen_init_cpio.c | 137 ++++++++++++++++++++++++++++++++++++--
> usr/gen_initramfs_list.sh | 10 ++-
> 4 files changed, 150 insertions(+), 9 deletions(-)
>
> diff --git a/usr/gen_init_cpio.c b/usr/gen_init_cpio.c
> index 03b21189d58b..e93cb1093e77 100644
> --- a/usr/gen_init_cpio.c
> +++ b/usr/gen_init_cpio.c
[..]
> +static int write_xattrs(const char *path)
> +{
[..]
> +
> + snprintf(str, sizeof(str), "%.8lx",
> + sizeof(hdr) + name_len + 1 + value_len);
Cppcheck 2.7 reports at this line:
usr/gen_init_cpio.c:107,portability,invalidPrintfArgType_uint,%lx in format string (no. 1) requires 'unsigned long' but the argument type is 'size_t {aka unsigned long}'.
This can be addressed via 's/lx/zx/', according to https://www.kernel.org/doc/Documentation/printk-formats.txt .
BR, Eugeniu
Hello Roberto,
On Do, Mai 23, 2019 at 02:18:03 +0200, Roberto Sassu wrote:
> This patch adds support for file metadata (only TYPE_XATTR metadata type).
> gen_init_cpio has been modified to read xattrs from files that will be
> added to the image and to include file metadata as separate files with the
> special name 'METADATA!!!'.
>
> This behavior can be selected by setting the desired file metadata type as
> value for CONFIG_INITRAMFS_FILE_METADATA.
>
> Signed-off-by: Roberto Sassu <[email protected]>
> ---
> usr/Kconfig | 8 +++
> usr/Makefile | 4 +-
> usr/gen_init_cpio.c | 137 ++++++++++++++++++++++++++++++++++++--
> usr/gen_initramfs_list.sh | 10 ++-
> 4 files changed, 150 insertions(+), 9 deletions(-)
>
> diff --git a/usr/gen_init_cpio.c b/usr/gen_init_cpio.c
[..]
> +static int write_xattrs(const char *path)
> +{
[..]
> + while (list_ptr < xattr_list + list_len) {
> + name_len = strlen(list_ptr);
PVS-Studio 7.19 reports at this line:
=> usr/gen_init_cpio.c 84 warn V769
=> The 'xattr_list' pointer in the 'xattr_list + list_len' expression could be nullptr.
=> In such case, resulting value will be senseless and it should not be used. Check lines: 84, 69.
I guess the finding is valid and it's due to the fact that
the malloc return value is not being checked/sanitized?
BR, Eugeniu.
Hello Roberto,
On Do, Jun 30, 2022 at 03:06:30 +0000, Roberto Sassu wrote:
> sorry, I'm a bit busy. Will have a look at your comments
> as soon as possible, and maybe I rebase the patches.
No rush. Thanks for keeping in touch.
Best regards,
Eugeniu.
On 15/06/2022 10:27, Eugeniu Rosca wrote:
> Hello Roberto,
>
> On Fr, Jun 10, 2022 at 03:38:24 +0000, Roberto Sassu wrote:
>> I would be happy to address the remaining concerns, or take more
>> suggestions, and then develop a new version of the patch set.
> I face a number of conflicts when I try to rebase the latest openEuler
> commits against vanilla master (v5.19-rc2). Do you think it is possible
> to submit the rebased version to ML?
>
> In addition, I can also see some open/unresolved points from Mimi [*].
> Did you by chance find some mutual agreement offline or do you think
> they would still potentially need some attention?
>
> Maybe we can resume the discussion once you submit the rebased series?
>
> Many thanks and looking forward to it.
>
> [*] Potentially comments which deserve a reply/clarification/resolution
>
> https://lore.kernel.org/lkml/[email protected]/#t
> https://lore.kernel.org/lkml/[email protected]/
>
> BR, Eugeniu.
>
Hello,
I have been testing these patches and do not see the xattr information when
trying to retrieve it within the initramfs, do you have an example of how
you tested this originally?
So far I have set the xattr in the rootfs before creating the cpio file like this:
$ setfattr -n user.comment -v "this is a comment" test.txt
If I access the data here it works:
$ getfattr test.txt
# file: test.txt
user.comment
Then I package it and try to verify it with this command:
$getfattr /test.txt
Which returns to the command line without the data.
I believe the cpio is working because I see the file /METADATA\!\!\! in
the target root filesystem, which shows the following when viewed with cat -e:
00000028^A^Auser.comment^@this is a comment
This matches the data I fed in at the start, so I believe the data is being
transferred correctly but I am accessioning it with the wrong tools.
Thank you for any help.
Best regards,
Jim
Best regards,
*Jim Baxter*
Siemens Digital Industries Software
Automotive Business Unit
DI SW STS ABU
UK
Tel.: +44 (161) 926-1656
mailto:[email protected] <mailto:[email protected]>
sw.siemens.com <https://sw.siemens.com/>
On 18/07/2022 17:49, Roberto Sassu wrote:
>> From: Jim Baxter [mailto:[email protected]]
>> Sent: Monday, July 18, 2022 6:36 PM
>>
>>
>> Hello,
>>
>> I have been testing these patches and do not see the xattr information when
>> trying to retrieve it within the initramfs, do you have an example of how
>> you tested this originally?
>
> Hi Jim, all
>
> apologies, I didn't find yet the time to look at this.
Hello Roberto,
Thank you for your response, I can wait until you have looked at the patches,
I asked the question to make sure it was not something wrong in my
configuration.
>
> Uhm, I guess this could be solved with:
>
> https://github.com/openeuler-mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da
>
> and adding initramtmpfs to the kernel command line. You are
> probably using ramfs, which does not have xattr support.
>
Thank you, I have tested that patch but the problem remained. Here is my
command line, I wonder if there is something wrong.
Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0 initrd=0x500000000 rootwait
I also found that root is always mounted as rootfs in my initramfs system
which I understood to be tmpfs, is that incorrect?
sh-3.2# mount
none on / type rootfs (rw)
>> So far I have set the xattr in the rootfs before creating the cpio file like this:
>> $ setfattr -n user.comment -v "this is a comment" test.txt
>> If I access the data here it works:
>> $ getfattr test.txt
>> # file: test.txt
>> user.comment
>>
>>
>> Then I package it and try to verify it with this command:
>> $getfattr /test.txt
>
> I assume you try to pack/unpack, right? If I remember correctly
> I only implemented the pack part. Unpacking is done by the kernel
> (but you are right, it should be done by user space too).
>
I modified the file before packing. To pack I use the following commands:
$ ./usr/gen_initramfs.sh -l initramfs.list -e xattr ../rootfs > initramfs.cpio
$ gzip initramfs.cpio
$ mkimage -A arm64 -O linux -T ramdisk -d initramfs.cpio.gz uRamdisk
The kernel is loaded using:
booti ${kernaddr} ${initramaddr} ${dtbaddr}
>> Which returns to the command line without the data.
>>
>>
>>
>> I believe the cpio is working because I see the file /METADATA\!\!\! in
>> the target root filesystem, which shows the following when viewed with cat -e:
>> 00000028^A^Auser.comment^@this is a comment
>>
>> This matches the data I fed in at the start, so I believe the data is being
>> transferred correctly but I am accessioning it with the wrong tools.
>
> Yes, xattrs are marshalled in the METADATA!!! file, one per regular file
> xattrs are applied to. Xattrs are applied to the previous regular file.
> That file name was preferred to adding a suffix to the file, to avoid
> reaching the filename size limit.
>
> Roberto
Best regards,
Jim
On 7/18/22 11:49, Roberto Sassu wrote:
> Uhm, I guess this could be solved with:
>
> https://github.com/openeuler-mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da
>
> and adding initramtmpfs to the kernel command line.
It's initmpfs. You can argue about whether it should have two t's (I was
consistent naming it in the patch series adding it), but ramfs and tmpfs are two
different things and saying "initramtmpfs" is like saying "mount -t ext4btrfs".
> You are probably using ramfs, which does not have xattr support.
Do not specify root= in your kernel command line. If you specify root= you're
saying "switch off of initramfs to a different root filesystem", so it doesn't
make the overmounted filesystem tmpfs because you told it you wouldn't be using it.
(The decision of what to mount has to be made before it examines the cpio.gz
contents, so root= is used to signal "we are not keeping this initramfs" because
that's literally what root= means. Your root filesystem is not initramfs, it is
instead this thing to be mounted over initramfs.)
You can tell which you're using via /proc/mounts having a line:
rootfs / rootfs rw,size=121832k,nr_inodes=30458 0 0
If it's got the size= then it's tmpfs: ramfs basically doesn't have bounds
checking and "cat /dev/null > filename" on ramfs will lock your system solid due
to unpinnable memory exhaustion.
If you don't have a "rootfs" line at ALL then root= was used to overmount and
part of the gratuitously magic behavior of root= is it hides the rootfs line
from /proc/mounts even though the filesystem is actually still there, which is
not something it does for ANY OTHER OVERMOUNT:
$ mkdir sub
$ mount -t proc proc sub
$ mount -t ramfs sub sub
$ grep sub /proc/mounts
proc /sub proc rw,relatime 0 0
sub /sub ramfs rw,relatime 0 0
I've never understood why they added that gratuitous special case to hide how
the system actually works, but it's a land mine you have to be told about after
you've stepped on it in order to understand what's going on. Part of the reason
people think initramfs is so "magic" when PID 1 isn't, we don't HIDE the fact
that PID 1 is always there but we hide the fact initramfs is...
Rob
On 7/18/22 13:08, Jim Baxter wrote:
>
>
> Best regards,
>
> *Jim Baxter*
>
> Siemens Digital Industries Software
> Automotive Business Unit
> DI SW STS ABU
> UK
> Tel.: +44 (161) 926-1656
> mailto:[email protected] <mailto:[email protected]>
> sw.siemens.com <https://sw.siemens.com/>
>
> On 18/07/2022 17:49, Roberto Sassu wrote:
>>> From: Jim Baxter [mailto:[email protected]]
>>> Sent: Monday, July 18, 2022 6:36 PM
>>>
>>>
>>> Hello,
>>>
>>> I have been testing these patches and do not see the xattr information when
>>> trying to retrieve it within the initramfs, do you have an example of how
>>> you tested this originally?
>>
>> Hi Jim, all
>>
>> apologies, I didn't find yet the time to look at this.
>
> Hello Roberto,
>
> Thank you for your response, I can wait until you have looked at the patches,
> I asked the question to make sure it was not something wrong in my
> configuration.
>
>>
>> Uhm, I guess this could be solved with:
>>
>> https://github.com/openeuler-mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da
>>
>> and adding initramtmpfs to the kernel command line. You are
>> probably using ramfs, which does not have xattr support.
>>
>
>
> Thank you, I have tested that patch but the problem remained. Here is my
> command line, I wonder if there is something wrong.
>
> Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0 initrd=0x500000000 rootwait
/dev/ram0 is a block device. Trying to provide it to tmpfs is like trying to say:
mount -t proc /dev/sda1 /proc
There's nowhere for the block device to GO because it's not a block backed
filesystem.
There's four types of filesystem: block back, pipe backed, ram backed, and
synthetic.
- Only block backed filesystems take a block device argument. Block backed
filesystems require two drivers: one to handle I/O to the block device and one
to interpret the filesystem format with the block device. You do not "format"
any other kind of filesystem. (There's no mkfs.nfs or mkfs.proc: it doesn't work
that way.)
- Pipe backed ones include network filesystems (nfs, samba), FUSE filesystems,
or hybrid weirdness like https://wiki.qemu.org/Documentation/9psetup . These
drivers talk a protocol over a pipe (or network socket, or char device, or...)
to a server at the far end that serves up the filesystem contents. Usually their
source argument is a server address plus filesystem identification plus login
credentials. Often they have a wrapper program that assembles this argument for you.
- Ram backed filesystems (ramfs, tmpfs) treat the "source" argument to mount(2)
as basically a comment, and ignore it. When you're adding things like size
limitations, it goes in the "data" argument (I.E. mount -o thingy).
- synthetic filesystems are just interfaces to the kernel that make up their
contents programmatically (proc, sys, cgroup...) and no two are alike, although
they generally ignore their "source" argument and look at "data" too.
I wrote up documention about this many years ago...
https://landley.net/toybox/doc/mount.html
> I also found that root is always mounted as rootfs in my initramfs system
> which I understood to be tmpfs, is that incorrect?
Yes, although the kernel tries to hide this by lying in /proc/mounts for bad
reasons.
> I modified the file before packing. To pack I use the following commands:
>
> $ ./usr/gen_initramfs.sh -l initramfs.list -e xattr ../rootfs > initramfs.cpio
> $ gzip initramfs.cpio
> $ mkimage -A arm64 -O linux -T ramdisk -d initramfs.cpio.gz uRamdisk
>
> The kernel is loaded using:
> booti ${kernaddr} ${initramaddr} ${dtbaddr}
Remove the root= argument from your kernel command line. It is explicitly
telling the kernel "we will not be staying in rootfs" and thus it doesn't use
tmpfs for it. In your case, you're saying "we're going to overmount the initial
ramfs with a ram disk block device", which is nonsensical because nothing can
have populated it so it will be all zeroes (unformatted) and thus the filesystem
type detection staircase in
https://github.com/torvalds/linux/blob/v5.18/init/do_mounts_rd.c#L38 won't be
able to find a filesystem type to mount on it and it's guaranteed to fail.
Note: initramfs was introduced in the early 2000s, and back in the 1990s there
was an older "initrd" mechanism that DID use ramdisks (which are a chunk of ram
used as a block device). I wrote documention about THAT too:
https://www.kernel.org/doc/Documentation/filesystems/ramfs-rootfs-initramfs.txt
Basically the mechanism you're feeding init.cpio.gz in through was originally
written to populate a ramdisk, and you'd make an ext2 image or something and
gzip that. These days, the kernel decompresses the first few bytes of the file
and if the result is a cpio signature it calls the initramfs plumbing
(extracting the archive into the ram backed filesystem) and if not it extracts
it into the /dev/ram0 block device and treats it as an initial ram disk. In
NEITHER case do you need root= because that's used AFTER initramfs and initrd
have both failed to find an /init program. (Well initrd looks for /linuxrc
instead of /init because historical cruft, and then there was pivot_root...
Don't go there.)
Rob
On 7/19/22 01:55, Roberto Sassu wrote:
>> Thank you, I have tested that patch but the problem remained. Here is my
>> command line, I wonder if there is something wrong.
>>
>> Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0
>> initrd=0x500000000 rootwait
>
> It is just initramtmpfs, without rootfstype=.
Whoever wrote that patch really doesn't understand how this stuff works. I can
tell from the name.
Technically, initramfs is the loader, I.E. "init ramfs". The filesystem instance
is called "rootfs" (hence the name in /proc/mounts when the insane special case
the kernel added doesn't hide information from people, making all this harder to
understand for no obvious reason).
ramfs and tmpfs are two different filesystems that COULD be used to implement
rootfs. (Last I checked they were the only ram backed filesystems in Linux.)
If a system administrator says they're going to install your server's root
partition using the "reiserxfs" filesystem, I would not be reassured.
> Roberto
Rob
P.S. Note: there IS another boot option, you can have a pipe backed root
filesystem! CONFIG_ROOT_NFS for NFS or CONFIG_CIFS_ROOT for Samba. No, I don't
know why the order isn't consistent.
P.P.S. If you want to run a command other than /init out of initramfs or initrd,
use the rdinit=/run/this option. Note the root= overmount mechanism is
completely different code and uses the init=/run/this argument instead, which
means nothing to initramfs. Again, specifying root= says we are NOT staying in
initramfs.
On 7/19/22 07:26, Roberto Sassu wrote:
>> P.P.S. If you want to run a command other than /init out of initramfs or initrd,
>> use the rdinit=/run/this option. Note the root= overmount mechanism is
>> completely different code and uses the init=/run/this argument instead, which
>> means nothing to initramfs. Again, specifying root= says we are NOT staying in
>> initramfs.
>
> Sorry, it was some time ago. I have to go back and see why we needed
> a separate option.
Did I mention that init/do_mounts.c already has:
__setup("rootfstype=", fs_names_setup);
static char * __initdata root_fs_names;
static int __init fs_names_setup(char *str)
{
root_fs_names = str;
return 1;
}
void __init init_rootfs(void)
{
if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] &&
(!root_fs_names || strstr(root_fs_names, "tmpfs")))
is_tmpfs = true;
}
I thought I'd dealt with this back in commit 6e19eded3684? Hmmm, looks like it
might need something like:
diff --git a/init/do_mounts.c b/init/do_mounts.c
index 7058e14ad5f7..4b4e1ffa20e1 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -665,7 +665,7 @@ struct file_system_type rootfs_fs_type = {
void __init init_rootfs(void)
{
- if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] &&
- (!root_fs_names || strstr(root_fs_names, "tmpfs")))
+ if (IS_ENABLED(CONFIG_TMPFS) && (!root_fs_names ? !saved_root_name[0] :
+ strstr(root_fs_names, "tmpfs"))
is_tmpfs = true;
}
> Maybe omitting root= was impacting on mounting
> the real root filesystem. Will get that information.
I know some old bootloaders hardwire in the command line so people can't
_remove_ the root=.
The reason I didn't just make rootfs always be tmpfs when CONFIG_TMPFS is
enabled is:
A) It uses very slightly more resources, and the common case is overmounting an
empty rootfs. (And then hiding it from /proc/mounts so people don't ask too many
questions.)
B) Some embedded systems use more than 50% of the system's memory for initramfs
contents, which the tmpfs defaults won't allow (fills up at 50%), and I'm not
sure I ever hooked up I don't think I ever hooked up rootflags= ala
root_mount_data to the initramfs mount? (If so, setting size= through that
should work...)
> Intuitively, given that root= is consumed for example by dracut, it seems
> a safer choice to have an option to explicitly choose the desired filesystem.
Sounds like a dracut issue. Have you used dracut in a system running from initramfs?
Lots of systems running from initramfs already DON'T have a root=, so you're
saying dracut being broken when there is no root= is something to work around
rather than fix in dracut, even though it's been easy to create a system without
a root= for a decade and a half already...
> Roberto
Rob
On 19/07/2022 07:55, Roberto Sassu wrote:
>> From: Jim Baxter [mailto:[email protected]]
>> Sent: Monday, July 18, 2022 8:08 PM
>>
>>
>>
>> Best regards,
>>
>> *Jim Baxter*
>>
>> Siemens Digital Industries Software
>> Automotive Business Unit
>> DI SW STS ABU
>> UK
>> Tel.: +44 (161) 926-1656
>> mailto:[email protected] <mailto:[email protected]>
>> sw.siemens.com <https://sw.siemens.com/>
>>
>> On 18/07/2022 17:49, Roberto Sassu wrote:
>>>> From: Jim Baxter [mailto:[email protected]]
>>>> Sent: Monday, July 18, 2022 6:36 PM
>>>>
>>>>
>>>> Hello,
>>>>
>>>> I have been testing these patches and do not see the xattr information when
>>>> trying to retrieve it within the initramfs, do you have an example of how
>>>> you tested this originally?
>>>
>>> Hi Jim, all
>>>
>>> apologies, I didn't find yet the time to look at this.
>>
>> Hello Roberto,
>>
>> Thank you for your response, I can wait until you have looked at the patches,
>> I asked the question to make sure it was not something wrong in my
>> configuration.
>>
>>>
>>> Uhm, I guess this could be solved with:
>>>
>>> https://github.com/openeuler-
>> mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da
>>>
>>> and adding initramtmpfs to the kernel command line. You are
>>> probably using ramfs, which does not have xattr support.
>>>
Can I clarify which filesystem type is supported with this patch series?
Is it tmpfs or perhaps a ramdisk?
>>
>>
>> Thank you, I have tested that patch but the problem remained. Here is my
>> command line, I wonder if there is something wrong.
>>
>> Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0
>> initrd=0x500000000 rootwait
>
> It is just initramtmpfs, without rootfstype=.
>
> Roberto
Best regards,
Jim
On 7/29/22 05:37, Jim Baxter wrote:
>>>> Uhm, I guess this could be solved with:
>>>>
>>>> https://github.com/openeuler-
>>> mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da
>>>>
>>>> and adding initramtmpfs to the kernel command line. You are
>>>> probably using ramfs, which does not have xattr support.
>>>>
Oh, here's the actual tested version of the patch wiring up rootfstype=tmpfs to
force rootfs to be tmpfs even when you specify root=
diff --git a/init/do_mounts.c b/init/do_mounts.c
index 7058e14ad5f7..dedf27fe9044 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -665,7 +665,7 @@ struct file_system_type rootfs_fs_type = {
void __init init_rootfs(void)
{
- if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] &&
- (!root_fs_names || strstr(root_fs_names, "tmpfs")))
+ if (IS_ENABLED(CONFIG_TMPFS) && (!root_fs_names ? !saved_root_name[0] :
+ !!strstr(root_fs_names, "tmpfs")))
is_tmpfs = true;
}
Signed-in-triplicate-by: Rob Landley <[email protected]>
No idea why nobody else has fixed that bug in the past 9 years, seems obvious?
Anyway, here's the testing I did using mkroot (ala
https://landley.net/toybox/faq.html#mkroot):
$ (cd root/x86_64; KARGS='quiet root=potato HANDOFF="/bin/head -n 1
/proc/mounts"' ./run-qemu.sh) | tail -n 3
rootfs / rootfs rw 0 0
reboot: Restarting system
$ (cd root/x86_64; KARGS='quiet HANDOFF="/bin/head -n 1 /proc/mounts"'
./run-qemu.sh) | tail -n 3
rootfs / rootfs rw,size=121828k,nr_inodes=30457 0 0
reboot: Restarting system
$ (cd root/x86_64; KARGS='quiet rootfstype=tmpfs root=potato HANDOFF="/bin/head
-n 1 /proc/mounts"' ./run-qemu.sh) | tail -n 3
rootfs / rootfs rw,size=121828k,nr_inodes=30457 0 0
reboot: Restarting system
I.E. rootfstype=tmpfs neutralized the root= so it was still tmpfs despite the
kernel being explicitly told you weren't going to stay on initramfs (which is
still what root= means). With just root= it's still ramfs, with all the "my log
file got too big and the system livelocked" and "querying available space always
returns zero" that entails.
> Can I clarify which filesystem type is supported with this patch series?
> Is it tmpfs or perhaps a ramdisk?
I believe both tmpfs and ramfs support xattrs? (I know tmpfs does, and
fs/ramfs/file-mmu.c plugs simple_getattr() into ramfs_file_operations.setattr so
it looks like that would too? Haven't tried it.)
This isn't a modification to the filesystem code (ramfs/tmpfs), this is a
modification to the boot-time loader (initramfs) that extracts a cpio.gz file
into the filesystem.
Ramdisks have supported xattrs for years: they fake up a block device out of a
chunk of memory and them format it and mount some other filesystem on it,
meaning the driver for the other filesystem handles the xattr support.
But ramdisks don't use initramfs, they load an image of the preformatted
filesystem into the ramdisk block device. Completely separate mechanism, sharing
no code with initramfs, depending on the block layer, etc.
>>> Thank you, I have tested that patch but the problem remained. Here is my
>>> command line, I wonder if there is something wrong.
>>>
>>> Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0
>>> initrd=0x500000000 rootwait
>>
>> It is just initramtmpfs, without rootfstype=.
The above patch does not go on top of that patch, it's instead of.
Rob