[RFC 0/3] extend kexec_file_load system call

[RFC 0/3] extend kexec_file_load system call

[AKASHI Takahiro]

Device tree blob must be passed to a second kernel on DTB-capable
archs, like powerpc and arm64, but the current kernel interface
lacks this support.
  
This patch extends kexec_file_load system call by adding an extra
argument to this syscall so that an arbitrary number of file descriptors
can be handed out from user space to the kernel.

See the background [1].

Please note that the new interface looks quite similar to the current
system call, but that it won't always mean that it provides the "binary
compatibility."

[1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html


AKASHI Takahiro (3):
  syscall: add kexec_file_load to generic unistd.h
  kexec: add dtb info to struct kimage
  kexec: extend kexec_file_load system call

 include/linux/fs.h                |  1 +
 include/linux/kexec.h             |  5 +++-
 include/linux/syscalls.h          |  4 ++-
 include/uapi/asm-generic/unistd.h |  8 ++++-
 include/uapi/linux/kexec.h        | 17 +++++++++++
 kernel/kexec_file.c               | 62 ++++++++++++++++++++++++++++++++++-----
 6 files changed, 87 insertions(+), 10 deletions(-)

-- 
2.9.0

[RFC 1/3] syscall: add kexec_file_load to generic unistd.h

[AKASHI Takahiro]

Currently kexec_file_load is supported only on x86, but it will be
supported on powerpc and arm64 in near future. Since both archs
use asm-generic/unistd.h, this patch adds the entry to this file.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/uapi/asm-generic/unistd.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index a26415b..9ead10f 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -724,9 +724,15 @@ __SYSCALL(__NR_copy_file_range, sys_copy_file_range)
 __SC_COMP(__NR_preadv2, sys_preadv2, compat_sys_preadv2)
 #define __NR_pwritev2 287
 __SC_COMP(__NR_pwritev2, sys_pwritev2, compat_sys_pwritev2)
+#define __NR_kexec_file_load 288
+#ifdef CONFIG_KEXEC_FILE
+__SYSCALL(__NR_kexec_file_load, sys_kexec_file_load)
+#else
+__SYSCALL(__NR_kexec_file_load, sys_ni_syscall)
+#endif /* CONFIG_KEXEC_FILE */
 
 #undef __NR_syscalls
-#define __NR_syscalls 288
+#define __NR_syscalls 289
 
 /*
  * All syscalls below here should go away really,
-- 
2.9.0

[RFC 2/3] kexec: add dtb info to struct kimage

[AKASHI Takahiro]

Device tree blob must be passed to a second kernel on DTB-capable
archs, like powerpc and arm64, but the current kernel interface
lacks this support.

This patch adds dtb buffer information to struct kimage.
When users don't specify dtb explicitly and the one used for the current
kernel can be re-used, this change will be good enough for implementing
kexec_file_load feature.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/linux/kexec.h | 3 +++
 kernel/kexec_file.c   | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index e8acb2b..554c848 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -190,6 +190,9 @@ struct kimage {
 	char *cmdline_buf;
 	unsigned long cmdline_buf_len;
 
+	void *dtb_buf;
+	unsigned long dtb_buf_len;
+
 	/* File operations provided by image loader */
 	struct kexec_file_ops *fops;
 
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 9891464..7278329 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -96,6 +96,11 @@ void kimage_file_post_load_cleanup(struct kimage *image)
 		image->initrd_buf = NULL;
 	}
 
+	if (image->dtb_buf) {
+		vfree(image->dtb_buf);
+		image->dtb_buf = NULL;
+	}
+
 	if (image->cmdline_buf) {
 		kfree(image->cmdline_buf);
 		image->cmdline_buf = NULL;
-- 
2.9.0

[RFC 3/3] kexec: extend kexec_file_load system call

[AKASHI Takahiro]

Device tree blob must be passed to a second kernel on DTB-capable
archs, like powerpc and arm64, but the current kernel interface
lacks this support.

This patch extends kexec_file_load system call by adding an extra
argument to this syscall so that an arbitrary number of file descriptors
can be handed out from user space to the kernel.

	long sys_kexec_file_load(int kernel_fd, int initrd_fd,
				 unsigned long cmdline_len,
				 const char __user *cmdline_ptr,
				 unsigned long flags,
				 const struct kexec_fdset __user *ufdset);

If KEXEC_FILE_EXTRA_FDS is set to the "flags" argument, the "ufdset"
argument points to the following struct buffer:

	struct kexec_fdset {
		int nr_fds;
		struct kexec_file_fd fds[0];
	}

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/linux/fs.h         |  1 +
 include/linux/kexec.h      |  2 +-
 include/linux/syscalls.h   |  4 +++-
 include/uapi/linux/kexec.h | 17 ++++++++++++++
 kernel/kexec_file.c        | 57 ++++++++++++++++++++++++++++++++++++++++------
 5 files changed, 72 insertions(+), 9 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index dd28814..6dd6fdf 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2634,6 +2634,7 @@ extern int do_pipe_flags(int *, int);
 	id(MODULE, kernel-module)		\
 	id(KEXEC_IMAGE, kexec-image)		\
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
+	id(KEXEC_DTB, kexec-dtb)		\
 	id(POLICY, security-policy)		\
 	id(MAX_ID, )
 
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 554c848..5f11bd5 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -277,7 +277,7 @@ extern int kexec_load_disabled;
 
 /* List of defined/legal kexec file flags */
 #define KEXEC_FILE_FLAGS	(KEXEC_FILE_UNLOAD | KEXEC_FILE_ON_CRASH | \
-				 KEXEC_FILE_NO_INITRAMFS)
+				 KEXEC_FILE_NO_INITRAMFS | KEXEC_FILE_EXTRA_FDS)
 
 #define VMCOREINFO_BYTES           (4096)
 #define VMCOREINFO_NOTE_NAME       "VMCOREINFO"
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index d022390..fc072bd 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -66,6 +66,7 @@ struct perf_event_attr;
 struct file_handle;
 struct sigaltstack;
 union bpf_attr;
+struct kexec_fdset;
 
 #include <linux/types.h>
 #include <linux/aio_abi.h>
@@ -321,7 +322,8 @@ asmlinkage long sys_kexec_load(unsigned long entry, unsigned long nr_segments,
 asmlinkage long sys_kexec_file_load(int kernel_fd, int initrd_fd,
 				    unsigned long cmdline_len,
 				    const char __user *cmdline_ptr,
-				    unsigned long flags);
+				    unsigned long flags,
+				    const struct kexec_fdset __user *ufdset);
 
 asmlinkage long sys_exit(int error_code);
 asmlinkage long sys_exit_group(int error_code);
diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
index aae5ebf..adf53b6 100644
--- a/include/uapi/linux/kexec.h
+++ b/include/uapi/linux/kexec.h
@@ -23,6 +23,23 @@
 #define KEXEC_FILE_UNLOAD	0x00000001
 #define KEXEC_FILE_ON_CRASH	0x00000002
 #define KEXEC_FILE_NO_INITRAMFS	0x00000004
+#define KEXEC_FILE_EXTRA_FDS	0x00000008
+
+enum kexec_file_type {
+	KEXEC_FILE_TYPE_KERNEL,
+	KEXEC_FILE_TYPE_INITRAMFS,
+	KEXEC_FILE_TYPE_DTB,
+};
+
+struct kexec_file_fd {
+	enum kexec_file_type type;
+	int fd;
+};
+
+struct kexec_fdset {
+	int nr_fds;
+	struct kexec_file_fd fds[0];
+};
 
 /* These values match the ELF architecture values.
  * Unless there is a good reason that should continue to be the case.
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 7278329..451b4b0 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -137,11 +137,14 @@ void kimage_file_post_load_cleanup(struct kimage *image)
 static int
 kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 			     const char __user *cmdline_ptr,
-			     unsigned long cmdline_len, unsigned flags)
+			     unsigned long cmdline_len, unsigned long flags,
+			     const struct kexec_fdset __user *ufdset)
 {
-	int ret = 0;
+	int ret = 0, nr_fds, i;
 	void *ldata;
 	loff_t size;
+	struct kexec_fdset *fdset = NULL;
+	size_t fdset_size;
 
 	ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
 				       &size, INT_MAX, READING_KEXEC_IMAGE);
@@ -174,6 +177,42 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 		image->initrd_buf_len = size;
 	}
 
+	if (flags & KEXEC_FILE_EXTRA_FDS) {
+		ret = copy_from_user(&nr_fds, ufdset, sizeof(int));
+		if (ret) {
+			ret = -EFAULT;
+			goto out;
+		}
+
+		fdset_size = sizeof(struct kexec_fdset)
+				+ nr_fds * sizeof(struct kexec_file_fd);
+		fdset = kmalloc(fdset_size, GFP_KERNEL);
+		if (!fdset) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		ret = copy_from_user(fdset, ufdset, fdset_size);
+		if (ret) {
+			ret = -EFAULT;
+			goto out;
+		}
+
+		for (i = 0; i < fdset->nr_fds; i++) {
+			if (fdset->fds[i].type == KEXEC_FILE_TYPE_DTB) {
+				ret = kernel_read_file_from_fd(fdset->fds[i].fd,
+						&image->dtb_buf, &size, INT_MAX,
+						READING_KEXEC_DTB);
+				if (ret)
+					goto out;
+				image->dtb_buf_len = size;
+			} else {
+				pr_debug("unknown file type %d failed.\n",
+						fdset->fds[i].type);
+			}
+		}
+	}
+
 	if (cmdline_len) {
 		image->cmdline_buf = kzalloc(cmdline_len, GFP_KERNEL);
 		if (!image->cmdline_buf) {
@@ -208,6 +247,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 	image->image_loader_data = ldata;
 out:
 	/* In case of error, free up all allocated memory in this function */
+	kfree(fdset);
+
 	if (ret)
 		kimage_file_post_load_cleanup(image);
 	return ret;
@@ -216,7 +257,8 @@ out:
 static int
 kimage_file_alloc_init(struct kimage **rimage, int kernel_fd,
 		       int initrd_fd, const char __user *cmdline_ptr,
-		       unsigned long cmdline_len, unsigned long flags)
+		       unsigned long cmdline_len, unsigned long flags,
+		       const struct kexec_fdset __user *ufdset)
 {
 	int ret;
 	struct kimage *image;
@@ -235,7 +277,8 @@ kimage_file_alloc_init(struct kimage **rimage, int kernel_fd,
 	}
 
 	ret = kimage_file_prepare_segments(image, kernel_fd, initrd_fd,
-					   cmdline_ptr, cmdline_len, flags);
+					   cmdline_ptr, cmdline_len, flags,
+					   ufdset);
 	if (ret)
 		goto out_free_image;
 
@@ -270,9 +313,9 @@ out_free_image:
 	return ret;
 }
 
-SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
+SYSCALL_DEFINE6(kexec_file_load, int, kernel_fd, int, initrd_fd,
 		unsigned long, cmdline_len, const char __user *, cmdline_ptr,
-		unsigned long, flags)
+		unsigned long, flags, const struct kexec_fdset __user *, ufdset)
 {
 	int ret = 0, i;
 	struct kimage **dest_image, *image;
@@ -309,7 +352,7 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
 		kimage_free(xchg(&kexec_crash_image, NULL));
 
 	ret = kimage_file_alloc_init(&image, kernel_fd, initrd_fd, cmdline_ptr,
-				     cmdline_len, flags);
+				     cmdline_len, flags, ufdset);
 	if (ret)
 		goto out;
 
-- 
2.9.0

[RFC 0/3] extend kexec_file_load system call

[Eric W. Biederman]

AKASHI Takahiro <takahiro.akashi@linaro.org> writes:

> Device tree blob must be passed to a second kernel on DTB-capable
> archs, like powerpc and arm64, but the current kernel interface
> lacks this support.
>   
> This patch extends kexec_file_load system call by adding an extra
> argument to this syscall so that an arbitrary number of file descriptors
> can be handed out from user space to the kernel.
>
> See the background [1].
>
> Please note that the new interface looks quite similar to the current
> system call, but that it won't always mean that it provides the "binary
> compatibility."
>
> [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html

So this design is wrong.  The kernel already has the device tree blob,
you should not be extracting it from the kernel munging it, and then
reinserting it in the kernel if you want signatures and everything to
pass.

What x86 does is pass it's equivalent of the device tree blob from one
kernel to another directly and behind the scenes.  It does not go
through userspace for this.

Until a persuasive case can be made for going around the kernel and
probably adding a feature (like code execution) that can be used to
defeat the signature scheme I am going to nack this.

Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>

I am happy to see support for other architectures, but for the sake of
not moving some code in the kernel let's not build an attackable
infrastructure.

Eric

[RFC 0/3] extend kexec_file_load system call

[Thiago Jung Bauermann]

Hello Eric,

Am Dienstag, 12 Juli 2016, 08:25:48 schrieb Eric W. Biederman:
> AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
> > Device tree blob must be passed to a second kernel on DTB-capable
> > archs, like powerpc and arm64, but the current kernel interface
> > lacks this support.
> > 
> > This patch extends kexec_file_load system call by adding an extra
> > argument to this syscall so that an arbitrary number of file descriptors
> > can be handed out from user space to the kernel.
> > 
> > See the background [1].
> > 
> > Please note that the new interface looks quite similar to the current
> > system call, but that it won't always mean that it provides the "binary
> > compatibility."
> > 
> > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
> 
> So this design is wrong.  The kernel already has the device tree blob,
> you should not be extracting it from the kernel munging it, and then
> reinserting it in the kernel if you want signatures and everything to
> pass.
> 
> What x86 does is pass it's equivalent of the device tree blob from one
> kernel to another directly and behind the scenes.  It does not go
> through userspace for this.
> 
> Until a persuasive case can be made for going around the kernel and
> probably adding a feature (like code execution) that can be used to
> defeat the signature scheme I am going to nack this.

There are situations where userspace needs to change things in the device 
tree to be used by the next kernel.

For example, Petitboot (the boot loader used in OpenPOWER machines) is a 
userspace application running in an intermediary Linux instance and uses 
kexec to load the target OS. It has to modify the device tree that will be 
used by the next kernel so that the next kernel uses the same console that 
petitboot was configured to use (i.e., set the /chosen/linux,stdout-path 
property). It also modifies the device tree to allow the kernel to inherit 
Petitboot's Openfirmware framebuffer.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Tue, Jul 12, 2016 at 10:58:09AM -0300, Thiago Jung Bauermann wrote:
> Hello Eric,
> 
> Am Dienstag, 12 Juli 2016, 08:25:48 schrieb Eric W. Biederman:
> > AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
> > > Device tree blob must be passed to a second kernel on DTB-capable
> > > archs, like powerpc and arm64, but the current kernel interface
> > > lacks this support.
> > > 
> > > This patch extends kexec_file_load system call by adding an extra
> > > argument to this syscall so that an arbitrary number of file descriptors
> > > can be handed out from user space to the kernel.
> > > 
> > > See the background [1].
> > > 
> > > Please note that the new interface looks quite similar to the current
> > > system call, but that it won't always mean that it provides the "binary
> > > compatibility."
> > > 
> > > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
> > 
> > So this design is wrong.  The kernel already has the device tree blob,
> > you should not be extracting it from the kernel munging it, and then
> > reinserting it in the kernel if you want signatures and everything to
> > pass.
> > 
> > What x86 does is pass it's equivalent of the device tree blob from one
> > kernel to another directly and behind the scenes.  It does not go
> > through userspace for this.
> > 
> > Until a persuasive case can be made for going around the kernel and
> > probably adding a feature (like code execution) that can be used to
> > defeat the signature scheme I am going to nack this.
> 
> There are situations where userspace needs to change things in the device 
> tree to be used by the next kernel.
> 
> For example, Petitboot (the boot loader used in OpenPOWER machines) is a 
> userspace application running in an intermediary Linux instance and uses 
> kexec to load the target OS. It has to modify the device tree that will be 
> used by the next kernel so that the next kernel uses the same console that 
> petitboot was configured to use (i.e., set the /chosen/linux,stdout-path 
> property). It also modifies the device tree to allow the kernel to inherit 
> Petitboot's Openfirmware framebuffer.

Can some of this be done with the help of kernel command line options for
second kernel?

Vivek

[RFC 0/3] extend kexec_file_load system call

[Arnd Bergmann]

On Tuesday, July 12, 2016 8:25:48 AM CEST Eric W. Biederman wrote:
> AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
> 
> > Device tree blob must be passed to a second kernel on DTB-capable
> > archs, like powerpc and arm64, but the current kernel interface
> > lacks this support.
> >   
> > This patch extends kexec_file_load system call by adding an extra
> > argument to this syscall so that an arbitrary number of file descriptors
> > can be handed out from user space to the kernel.
> >
> > See the background [1].
> >
> > Please note that the new interface looks quite similar to the current
> > system call, but that it won't always mean that it provides the "binary
> > compatibility."
> >
> > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
> 
> So this design is wrong.  The kernel already has the device tree blob,
> you should not be extracting it from the kernel munging it, and then
> reinserting it in the kernel if you want signatures and everything to
> pass.
> 
> What x86 does is pass it's equivalent of the device tree blob from one
> kernel to another directly and behind the scenes.  It does not go
> through userspace for this.
> 
> Until a persuasive case can be made for going around the kernel and
> probably adding a feature (like code execution) that can be used to
> defeat the signature scheme I am going to nack this.
> 
> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> I am happy to see support for other architectures, but for the sake of
> not moving some code in the kernel let's not build an attackable
> infrastructure.
> 

For historic context, the flattened devicetree format that we now use
to pass data about the system from boot loader to kernel was initially
introduced specifically for the purpose of enabling kexec:

On Open Firmware, the DT is extracted from running firmware and copied
into dynamically allocated data structures. After a kexec, the runtime
interface to the firmware is not available, so the flattened DT format
was created as a way to pass the same data in a binary blob to the new
kernel in a format that can be read from the kernel by walking the
directories in /proc/device-tree/*.

There are a couple of reasons for modifying the devicetree:

- For kboot/petitboot, you can have a kernel that is not booted through
  DT at all but hardwired to a particular machine, and that passes
  a DT for the entire hardware to the kernel that you actually want to
  run.

- for kdump, you need to tell the new kernel about the modified location
  of the memory, so the dump kernel doesn't overwrite the contents
  it wants to dump

- we typically ship devicetree sources for embedded machines with the
  kernel sources. As more hardware of the system gets enabled, the
  devicetree gains extra nodes and properties that describe the hardware
  more completely, so we need to use the latest DT blob to use all
  the drivers

- in some cases, kernels will fail to boot at all with an older version
  of the DT, or fail to use the devices that were working on the
  earlier kernel. This is usually considered a bug, but it's not rare

- In some cases, the kernel can update its DT at runtime, and the new
  settings are expected to be available in the new kernel too, though
  there are cases where you actually don't want the modified contents.

	Arnd

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Tue, Jul 12, 2016 at 04:02:46PM +0200, Arnd Bergmann wrote:
> On Tuesday, July 12, 2016 8:25:48 AM CEST Eric W. Biederman wrote:
> > AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
> > 
> > > Device tree blob must be passed to a second kernel on DTB-capable
> > > archs, like powerpc and arm64, but the current kernel interface
> > > lacks this support.
> > >   
> > > This patch extends kexec_file_load system call by adding an extra
> > > argument to this syscall so that an arbitrary number of file descriptors
> > > can be handed out from user space to the kernel.
> > >
> > > See the background [1].
> > >
> > > Please note that the new interface looks quite similar to the current
> > > system call, but that it won't always mean that it provides the "binary
> > > compatibility."
> > >
> > > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
> > 
> > So this design is wrong.  The kernel already has the device tree blob,
> > you should not be extracting it from the kernel munging it, and then
> > reinserting it in the kernel if you want signatures and everything to
> > pass.
> > 
> > What x86 does is pass it's equivalent of the device tree blob from one
> > kernel to another directly and behind the scenes.  It does not go
> > through userspace for this.
> > 
> > Until a persuasive case can be made for going around the kernel and
> > probably adding a feature (like code execution) that can be used to
> > defeat the signature scheme I am going to nack this.
> > 
> > Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> > 
> > I am happy to see support for other architectures, but for the sake of
> > not moving some code in the kernel let's not build an attackable
> > infrastructure.
> > 
> 
> For historic context, the flattened devicetree format that we now use
> to pass data about the system from boot loader to kernel was initially
> introduced specifically for the purpose of enabling kexec:
> 
> On Open Firmware, the DT is extracted from running firmware and copied
> into dynamically allocated data structures. After a kexec, the runtime
> interface to the firmware is not available, so the flattened DT format
> was created as a way to pass the same data in a binary blob to the new
> kernel in a format that can be read from the kernel by walking the
> directories in /proc/device-tree/*.

So this DT is available inside kernel and running kernel can still
retrieve it and pass it to second kernel?

> 
> There are a couple of reasons for modifying the devicetree:
> 
> - For kboot/petitboot, you can have a kernel that is not booted through
>   DT at all but hardwired to a particular machine, and that passes
>   a DT for the entire hardware to the kernel that you actually want to
>   run.
> 
> - for kdump, you need to tell the new kernel about the modified location
>   of the memory, so the dump kernel doesn't overwrite the contents
>   it wants to dump

In x86 we do this with the help of kernel command line options.

> 
> - we typically ship devicetree sources for embedded machines with the
>   kernel sources. As more hardware of the system gets enabled, the
>   devicetree gains extra nodes and properties that describe the hardware
>   more completely, so we need to use the latest DT blob to use all
>   the drivers
> 
> - in some cases, kernels will fail to boot at all with an older version
>   of the DT, or fail to use the devices that were working on the
>   earlier kernel. This is usually considered a bug, but it's not rare
> 
> - In some cases, the kernel can update its DT at runtime, and the new
>   settings are expected to be available in the new kernel too, though
>   there are cases where you actually don't want the modified contents.

I am assuming that modified DT and unmodifed one both are accessible to
kernel. And if user space can make decisions which modfied fields to use
for new kernels and which ones not, then same can be done in kernel too?

Vivek
> 
> 	Arnd

[RFC 0/3] extend kexec_file_load system call

[Arnd Bergmann]

On Tuesday, July 12, 2016 10:18:11 AM CEST Vivek Goyal wrote:
> > 
> > On Open Firmware, the DT is extracted from running firmware and copied
> > into dynamically allocated data structures. After a kexec, the runtime
> > interface to the firmware is not available, so the flattened DT format
> > was created as a way to pass the same data in a binary blob to the new
> > kernel in a format that can be read from the kernel by walking the
> > directories in /proc/device-tree/*.
> 
> So this DT is available inside kernel and running kernel can still
> retrieve it and pass it to second kernel?

The kernel only uses the flattened DT blob at boot time and converts
it into the runtime data structures (struct device_node). The original
dtb is typically overwritten later.

> > - we typically ship devicetree sources for embedded machines with the
> >   kernel sources. As more hardware of the system gets enabled, the
> >   devicetree gains extra nodes and properties that describe the hardware
> >   more completely, so we need to use the latest DT blob to use all
> >   the drivers
> > 
> > - in some cases, kernels will fail to boot at all with an older version
> >   of the DT, or fail to use the devices that were working on the
> >   earlier kernel. This is usually considered a bug, but it's not rare
> > 
> > - In some cases, the kernel can update its DT at runtime, and the new
> >   settings are expected to be available in the new kernel too, though
> >   there are cases where you actually don't want the modified contents.
> 
> I am assuming that modified DT and unmodifed one both are accessible to
> kernel. And if user space can make decisions which modfied fields to use
> for new kernels and which ones not, then same can be done in kernel too?

The unmodified DT can typically be found on disk next to the kernel binary.
The option you have is to either read it from /proc/devicetree or to
read it from from /boot/*.dtb.

	Arnd

[RFC 0/3] extend kexec_file_load system call

[Mark Rutland]

On Tue, Jul 12, 2016 at 04:24:10PM +0200, Arnd Bergmann wrote:
> On Tuesday, July 12, 2016 10:18:11 AM CEST Vivek Goyal wrote:
> > > 
> > > On Open Firmware, the DT is extracted from running firmware and copied
> > > into dynamically allocated data structures. After a kexec, the runtime
> > > interface to the firmware is not available, so the flattened DT format
> > > was created as a way to pass the same data in a binary blob to the new
> > > kernel in a format that can be read from the kernel by walking the
> > > directories in /proc/device-tree/*.
> > 
> > So this DT is available inside kernel and running kernel can still
> > retrieve it and pass it to second kernel?
> 
> The kernel only uses the flattened DT blob at boot time and converts
> it into the runtime data structures (struct device_node). The original
> dtb is typically overwritten later.

On arm64 we deliberately preserved the DTB, so we can take that and
build a new DTB from that kernel-side.

> > > - we typically ship devicetree sources for embedded machines with the
> > >   kernel sources. As more hardware of the system gets enabled, the
> > >   devicetree gains extra nodes and properties that describe the hardware
> > >   more completely, so we need to use the latest DT blob to use all
> > >   the drivers
> > > 
> > > - in some cases, kernels will fail to boot at all with an older version
> > >   of the DT, or fail to use the devices that were working on the
> > >   earlier kernel. This is usually considered a bug, but it's not rare
> > > 
> > > - In some cases, the kernel can update its DT at runtime, and the new
> > >   settings are expected to be available in the new kernel too, though
> > >   there are cases where you actually don't want the modified contents.
> > 
> > I am assuming that modified DT and unmodifed one both are accessible to
> > kernel. And if user space can make decisions which modfied fields to use
> > for new kernels and which ones not, then same can be done in kernel too?
> 
> The unmodified DT can typically be found on disk next to the kernel binary.
> The option you have is to either read it from /proc/devicetree or to
> read it from from /boot/*.dtb.

/proc/devicetree (aka /sys/firmware/devicetree) is a filesystem derived
from the raw DTB (which is exposed at /sys/firmware/fdt).

The blob that was handed to the kernel at boot time is exposed at
/sys/firmware/fdt.

Thanks,
Mark.

[RFC 0/3] extend kexec_file_load system call

[Thiago Jung Bauermann]

Hi Eric,

I'm trying to understand your concerns leading to your nack. I hope you 
don't mind expanding your thoughts on them a bit.

Am Dienstag, 12 Juli 2016, 08:25:48 schrieb Eric W. Biederman:
> AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
> > Device tree blob must be passed to a second kernel on DTB-capable
> > archs, like powerpc and arm64, but the current kernel interface
> > lacks this support.
> > 
> > This patch extends kexec_file_load system call by adding an extra
> > argument to this syscall so that an arbitrary number of file descriptors
> > can be handed out from user space to the kernel.
> > 
> > See the background [1].
> > 
> > Please note that the new interface looks quite similar to the current
> > system call, but that it won't always mean that it provides the "binary
> > compatibility."
> > 
> > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
> 
> So this design is wrong.  The kernel already has the device tree blob,
> you should not be extracting it from the kernel munging it, and then
> reinserting it in the kernel if you want signatures and everything to
> pass.

I don't understand how the kernel signature will be invalidated. 

There are some types of boot images that can embed a device tree blob in 
them, but the kernel can also be handed a separate device tree blob from 
firmware, the boot loader, or kexec. This latter case is what we are 
discussing, so we are not talking about modifying an embedded blob in the 
kernel image.

> What x86 does is pass it's equivalent of the device tree blob from one
> kernel to another directly and behind the scenes.  It does not go
> through userspace for this.
> 
> Until a persuasive case can be made for going around the kernel and
> probably adding a feature (like code execution) that can be used to
> defeat the signature scheme I am going to nack this.

I also don't understand what you mean by code execution. How does passing a 
device tree blob via kexec enables code execution? How can the signature 
scheme be defeated?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

[RFC 0/3] extend kexec_file_load system call

[Petr Tesarik]

On Tue, 12 Jul 2016 13:25:11 -0300
Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> wrote:

> Hi Eric,
> 
> I'm trying to understand your concerns leading to your nack. I hope you 
> don't mind expanding your thoughts on them a bit.
> 
> Am Dienstag, 12 Juli 2016, 08:25:48 schrieb Eric W. Biederman:
> > AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
> > > Device tree blob must be passed to a second kernel on DTB-capable
> > > archs, like powerpc and arm64, but the current kernel interface
> > > lacks this support.
> > > 
> > > This patch extends kexec_file_load system call by adding an extra
> > > argument to this syscall so that an arbitrary number of file descriptors
> > > can be handed out from user space to the kernel.
> > > 
> > > See the background [1].
> > > 
> > > Please note that the new interface looks quite similar to the current
> > > system call, but that it won't always mean that it provides the "binary
> > > compatibility."
> > > 
> > > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
> > 
> > So this design is wrong.  The kernel already has the device tree blob,
> > you should not be extracting it from the kernel munging it, and then
> > reinserting it in the kernel if you want signatures and everything to
> > pass.
> 
> I don't understand how the kernel signature will be invalidated. 
> 
> There are some types of boot images that can embed a device tree blob in 
> them, but the kernel can also be handed a separate device tree blob from 
> firmware, the boot loader, or kexec. This latter case is what we are 
> discussing, so we are not talking about modifying an embedded blob in the 
> kernel image.
> 
> > What x86 does is pass it's equivalent of the device tree blob from one
> > kernel to another directly and behind the scenes.  It does not go
> > through userspace for this.
> > 
> > Until a persuasive case can be made for going around the kernel and
> > probably adding a feature (like code execution) that can be used to
> > defeat the signature scheme I am going to nack this.
> 
> I also don't understand what you mean by code execution. How does passing a 
> device tree blob via kexec enables code execution? How can the signature 
> scheme be defeated?

I'm not an expert on DTB, so I can't provide an example of code
execution, but you have already mentioned the /chosen/linux,stdout-path
property. If an attacker redirects the bootloader to an insecure
console, they may get access to the system that would otherwise be
impossible.

In general, tampering with the hardware inventory of a machine opens up
a security hole, and one must be very cautious which modifications are
allowed. You're giving this power to an (unsigned, hence untrusted)
userspace application; Eric argues that only the kernel should have
this power.

Just my two cents,
Petr T

[RFC 0/3] extend kexec_file_load system call

[Eric W. Biederman]

Petr Tesarik <ptesarik@suse.cz> writes:

> On Tue, 12 Jul 2016 13:25:11 -0300
> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> wrote:
>
>> Hi Eric,
>> 
>> I'm trying to understand your concerns leading to your nack. I hope you 
>> don't mind expanding your thoughts on them a bit.
>> 
>> Am Dienstag, 12 Juli 2016, 08:25:48 schrieb Eric W. Biederman:
>> > AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
>> > > Device tree blob must be passed to a second kernel on DTB-capable
>> > > archs, like powerpc and arm64, but the current kernel interface
>> > > lacks this support.
>> > > 
>> > > This patch extends kexec_file_load system call by adding an extra
>> > > argument to this syscall so that an arbitrary number of file descriptors
>> > > can be handed out from user space to the kernel.
>> > > 
>> > > See the background [1].
>> > > 
>> > > Please note that the new interface looks quite similar to the current
>> > > system call, but that it won't always mean that it provides the "binary
>> > > compatibility."
>> > > 
>> > > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
>> > 
>> > So this design is wrong.  The kernel already has the device tree blob,
>> > you should not be extracting it from the kernel munging it, and then
>> > reinserting it in the kernel if you want signatures and everything to
>> > pass.
>> 
>> I don't understand how the kernel signature will be invalidated. 
>> 
>> There are some types of boot images that can embed a device tree blob in 
>> them, but the kernel can also be handed a separate device tree blob from 
>> firmware, the boot loader, or kexec. This latter case is what we are 
>> discussing, so we are not talking about modifying an embedded blob in the 
>> kernel image.
>> 
>> > What x86 does is pass it's equivalent of the device tree blob from one
>> > kernel to another directly and behind the scenes.  It does not go
>> > through userspace for this.
>> > 
>> > Until a persuasive case can be made for going around the kernel and
>> > probably adding a feature (like code execution) that can be used to
>> > defeat the signature scheme I am going to nack this.
>> 
>> I also don't understand what you mean by code execution. How does passing a 
>> device tree blob via kexec enables code execution? How can the signature 
>> scheme be defeated?
>
> I'm not an expert on DTB, so I can't provide an example of code
> execution, but you have already mentioned the /chosen/linux,stdout-path
> property. If an attacker redirects the bootloader to an insecure
> console, they may get access to the system that would otherwise be
> impossible.
>
> In general, tampering with the hardware inventory of a machine opens up
> a security hole, and one must be very cautious which modifications are
> allowed. You're giving this power to an (unsigned, hence untrusted)
> userspace application; Eric argues that only the kernel should have
> this power.

At the very least it should be signed.  And of course the more signed
images we have in different combinations the more easily someone can
find a combination that does things the people performing the signing
didn't realizing they were allowing.

So if we can not add an extra variable into the mix it would be good.

Eric

[RFC 0/3] extend kexec_file_load system call

[Eric W. Biederman]

ebiederm at xmission.com (Eric W. Biederman) writes:

> Petr Tesarik <ptesarik@suse.cz> writes:
>
>> On Tue, 12 Jul 2016 13:25:11 -0300
>> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> wrote:
>>
>>> Hi Eric,
>>> 
>>> I'm trying to understand your concerns leading to your nack. I hope you 
>>> don't mind expanding your thoughts on them a bit.
>>> 
>>> Am Dienstag, 12 Juli 2016, 08:25:48 schrieb Eric W. Biederman:
>>> > AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
>>> > > Device tree blob must be passed to a second kernel on DTB-capable
>>> > > archs, like powerpc and arm64, but the current kernel interface
>>> > > lacks this support.
>>> > > 
>>> > > This patch extends kexec_file_load system call by adding an extra
>>> > > argument to this syscall so that an arbitrary number of file descriptors
>>> > > can be handed out from user space to the kernel.
>>> > > 
>>> > > See the background [1].
>>> > > 
>>> > > Please note that the new interface looks quite similar to the current
>>> > > system call, but that it won't always mean that it provides the "binary
>>> > > compatibility."
>>> > > 
>>> > > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
>>> > 
>>> > So this design is wrong.  The kernel already has the device tree blob,
>>> > you should not be extracting it from the kernel munging it, and then
>>> > reinserting it in the kernel if you want signatures and everything to
>>> > pass.
>>> 
>>> I don't understand how the kernel signature will be invalidated. 
>>> 
>>> There are some types of boot images that can embed a device tree blob in 
>>> them, but the kernel can also be handed a separate device tree blob from 
>>> firmware, the boot loader, or kexec. This latter case is what we are 
>>> discussing, so we are not talking about modifying an embedded blob in the 
>>> kernel image.
>>> 
>>> > What x86 does is pass it's equivalent of the device tree blob from one
>>> > kernel to another directly and behind the scenes.  It does not go
>>> > through userspace for this.
>>> > 
>>> > Until a persuasive case can be made for going around the kernel and
>>> > probably adding a feature (like code execution) that can be used to
>>> > defeat the signature scheme I am going to nack this.
>>> 
>>> I also don't understand what you mean by code execution. How does passing a 
>>> device tree blob via kexec enables code execution? How can the signature 
>>> scheme be defeated?
>>
>> I'm not an expert on DTB, so I can't provide an example of code
>> execution, but you have already mentioned the /chosen/linux,stdout-path
>> property. If an attacker redirects the bootloader to an insecure
>> console, they may get access to the system that would otherwise be
>> impossible.
>>
>> In general, tampering with the hardware inventory of a machine opens up
>> a security hole, and one must be very cautious which modifications are
>> allowed. You're giving this power to an (unsigned, hence untrusted)
>> userspace application; Eric argues that only the kernel should have
>> this power.
>
> At the very least it should be signed.  And of course the more signed
> images we have in different combinations the more easily someone can
> find a combination that does things the people performing the signing
> didn't realizing they were allowing.
>
> So if we can not add an extra variable into the mix it would be good.

But it is even more than that.  There was a giant design discussion that
lasted months before this code was added on x86.  The facts on the
ground on x86 are substantially similar to ARM64.  So coming up and
saying that oh that design sucks and we want to do something completely
different to achieve the exact same goals and then not even discussing
why the current design can not work in the problem descriptions is
inconsiderate.

Not taking the time to understand how something works and why and then
asking people to explain to them what they are doing wrong is rude.  It
is a waste of everyones time.

I thought I had said something to that effect, but it doesn't look like
I did.  Apologies for not being clear about that.

I have had a lot of that this last little while.  Code with big design
issues that really should be justified given people are aiming to
overturn previous design decisions and not even considering those
previous decisions, and I find it quite tiring.

Especially when we are dealing with design decisions with a security
impact, and peopel want to add code to achieve a security goal I expect
people to be paying attention to what effect their changes have on the
entire ecosystem.  Not just saying the current behavior is inconvinient
and using that as a rational for changing things.

Eric

[RFC 0/3] extend kexec_file_load system call

[Petr Tesarik]

On Tue, 12 Jul 2016 16:22:07 -0500
ebiederm at xmission.com (Eric W. Biederman) wrote:

> Petr Tesarik <ptesarik@suse.cz> writes:
> 
> > On Tue, 12 Jul 2016 13:25:11 -0300
> > Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> wrote:
>[...]
> >> I also don't understand what you mean by code execution. How does passing a 
> >> device tree blob via kexec enables code execution? How can the signature 
> >> scheme be defeated?
> >
> > I'm not an expert on DTB, so I can't provide an example of code
> > execution, but you have already mentioned the /chosen/linux,stdout-path
> > property. If an attacker redirects the bootloader to an insecure
> > console, they may get access to the system that would otherwise be
> > impossible.
> >
> > In general, tampering with the hardware inventory of a machine opens up
> > a security hole, and one must be very cautious which modifications are
> > allowed. You're giving this power to an (unsigned, hence untrusted)
> > userspace application; Eric argues that only the kernel should have
> > this power.
> 
> At the very least it should be signed.  And of course the more signed
> images we have in different combinations the more easily someone can
> find a combination that does things the people performing the signing
> didn't realizing they were allowing.

Exactly. Reminds me of nasty setuid application exploits when one or
more of stdin, stdout and stderr are closed before exec(), so the first
file to be opened gets one of those special file descriptors. Imagine
what happens if the application opens a secret file for reading (now
file descriptor 0), then expects user input on stdin, detects a syntax
error and complains on stderr, including the full input for reference
("%s is not a valid command")...

No one has designed bootloaders to cope with similar unexpected
situations.

> So if we can not add an extra variable into the mix it would be good.

Indeed. Writing boot loaders is difficult enough already. Adding the
same kind of precautions that are necessary to write secure setuid
applications is over the top IMO.

Petr T

[RFC 0/3] extend kexec_file_load system call

[Russell King - ARM Linux]

On Tue, Jul 12, 2016 at 10:58:05PM +0200, Petr Tesarik wrote:
> I'm not an expert on DTB, so I can't provide an example of code
> execution, but you have already mentioned the /chosen/linux,stdout-path
> property. If an attacker redirects the bootloader to an insecure
> console, they may get access to the system that would otherwise be
> impossible.

I fail to see how kexec connects with the boot loader - the DTB image
that's being talked about is one which is passed from the currently
running kernel to the to-be-kexec'd kernel.  For ARM (and I suspect
also ARM64) that's a direct call chain which doesn't involve any
boot loader or firmware, and certainly none that would involve the
passed DTB image.

However, your point is valid as an attacker can redirect the console
and/or mounted root on the to-be-kexec'd kernel if they can modify
the DTB - and there's a whole host of subtle ways to do that, not
necessarily just modification of the kernel command line.

> In general, tampering with the hardware inventory of a machine opens up
> a security hole, and one must be very cautious which modifications are
> allowed. You're giving this power to an (unsigned, hence untrusted)
> userspace application; Eric argues that only the kernel should have
> this power.

Given that, how does crashdump work in this scenario?

crashdump works by adding an elfcorehdr=address argument to the
crash-booted kernel's command line.  If you can add to the kernel
command line, you can redirect the console and do all sorts of
other stuff like specifying a different filesystem to mount, etc.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[RFC 0/3] extend kexec_file_load system call

[Stewart Smith]

Petr Tesarik <ptesarik@suse.cz> writes:
> On Tue, 12 Jul 2016 13:25:11 -0300
> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> wrote:
>
>> Hi Eric,
>> 
>> I'm trying to understand your concerns leading to your nack. I hope you 
>> don't mind expanding your thoughts on them a bit.
>> 
>> Am Dienstag, 12 Juli 2016, 08:25:48 schrieb Eric W. Biederman:
>> > AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
>> > > Device tree blob must be passed to a second kernel on DTB-capable
>> > > archs, like powerpc and arm64, but the current kernel interface
>> > > lacks this support.
>> > > 
>> > > This patch extends kexec_file_load system call by adding an extra
>> > > argument to this syscall so that an arbitrary number of file descriptors
>> > > can be handed out from user space to the kernel.
>> > > 
>> > > See the background [1].
>> > > 
>> > > Please note that the new interface looks quite similar to the current
>> > > system call, but that it won't always mean that it provides the "binary
>> > > compatibility."
>> > > 
>> > > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
>> > 
>> > So this design is wrong.  The kernel already has the device tree blob,
>> > you should not be extracting it from the kernel munging it, and then
>> > reinserting it in the kernel if you want signatures and everything to
>> > pass.
>> 
>> I don't understand how the kernel signature will be invalidated. 
>> 
>> There are some types of boot images that can embed a device tree blob in 
>> them, but the kernel can also be handed a separate device tree blob from 
>> firmware, the boot loader, or kexec. This latter case is what we are 
>> discussing, so we are not talking about modifying an embedded blob in the 
>> kernel image.
>> 
>> > What x86 does is pass it's equivalent of the device tree blob from one
>> > kernel to another directly and behind the scenes.  It does not go
>> > through userspace for this.
>> > 
>> > Until a persuasive case can be made for going around the kernel and
>> > probably adding a feature (like code execution) that can be used to
>> > defeat the signature scheme I am going to nack this.
>> 
>> I also don't understand what you mean by code execution. How does passing a 
>> device tree blob via kexec enables code execution? How can the signature 
>> scheme be defeated?
>
> I'm not an expert on DTB, so I can't provide an example of code
> execution, but you have already mentioned the /chosen/linux,stdout-path
> property. If an attacker redirects the bootloader to an insecure
> console, they may get access to the system that would otherwise be
> impossible.

In this case, the user is sitting at the (or one of the) console(s) of
the machine. There could be petitboot UIs running on the VGA display,
IPMI serial over lan, local serial port. The logic behind setting
/chosen/linux,stdout-path is (currently) mostly to set it for the kernel
to what the user is interacting with. i.e. if you select an OS installer
to boot from the VGA console, you get a graphical installer running and
if you selected it from a text console, you get a text installer running
(on the appropriate console).

So the bootloader (petitboot) needs to work out which console is being
interacted with in order to set up /chosen/linux,stdout-path correctly.

This specific option could be passed as a kernel command line to the
next kernel, yes. However, isn't the kernel command line also an attack
vector? Is *every* command line option safe?

> In general, tampering with the hardware inventory of a machine opens up
> a security hole, and one must be very cautious which modifications are
> allowed. You're giving this power to an (unsigned, hence untrusted)
> userspace application; Eric argues that only the kernel should have
> this power.

In the case of petitboot on OpenPOWER, this (will) be a signed and
trusted kernel and userspace and verified by a previous bit of firmware.

-- 
Stewart Smith
OPAL Architect, IBM.

[RFC 0/3] extend kexec_file_load system call

[Stewart Smith]

Vivek Goyal <vgoyal@redhat.com> writes:
> On Tue, Jul 12, 2016 at 10:58:09AM -0300, Thiago Jung Bauermann wrote:
>> Hello Eric,
>> 
>> Am Dienstag, 12 Juli 2016, 08:25:48 schrieb Eric W. Biederman:
>> > AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
>> > > Device tree blob must be passed to a second kernel on DTB-capable
>> > > archs, like powerpc and arm64, but the current kernel interface
>> > > lacks this support.
>> > > 
>> > > This patch extends kexec_file_load system call by adding an extra
>> > > argument to this syscall so that an arbitrary number of file descriptors
>> > > can be handed out from user space to the kernel.
>> > > 
>> > > See the background [1].
>> > > 
>> > > Please note that the new interface looks quite similar to the current
>> > > system call, but that it won't always mean that it provides the "binary
>> > > compatibility."
>> > > 
>> > > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
>> > 
>> > So this design is wrong.  The kernel already has the device tree blob,
>> > you should not be extracting it from the kernel munging it, and then
>> > reinserting it in the kernel if you want signatures and everything to
>> > pass.
>> > 
>> > What x86 does is pass it's equivalent of the device tree blob from one
>> > kernel to another directly and behind the scenes.  It does not go
>> > through userspace for this.
>> > 
>> > Until a persuasive case can be made for going around the kernel and
>> > probably adding a feature (like code execution) that can be used to
>> > defeat the signature scheme I am going to nack this.
>> 
>> There are situations where userspace needs to change things in the device 
>> tree to be used by the next kernel.
>> 
>> For example, Petitboot (the boot loader used in OpenPOWER machines) is a 
>> userspace application running in an intermediary Linux instance and uses 
>> kexec to load the target OS. It has to modify the device tree that will be 
>> used by the next kernel so that the next kernel uses the same console that 
>> petitboot was configured to use (i.e., set the /chosen/linux,stdout-path 
>> property). It also modifies the device tree to allow the kernel to inherit 
>> Petitboot's Openfirmware framebuffer.
>
> Can some of this be done with the help of kernel command line options for
> second kernel?

how would this be any more secure?

Passing in an address for a framebuffer via command line option means
you could scribble over any bit of memory, which is the same kind of
damage you could do by modifying the device tree.

-- 
Stewart Smith
OPAL Architect, IBM.

[RFC 0/3] extend kexec_file_load system call

[Dave Young]

On 07/12/16 at 03:50pm, Mark Rutland wrote:
> On Tue, Jul 12, 2016 at 04:24:10PM +0200, Arnd Bergmann wrote:
> > On Tuesday, July 12, 2016 10:18:11 AM CEST Vivek Goyal wrote:
> > > > 
> > > > On Open Firmware, the DT is extracted from running firmware and copied
> > > > into dynamically allocated data structures. After a kexec, the runtime
> > > > interface to the firmware is not available, so the flattened DT format
> > > > was created as a way to pass the same data in a binary blob to the new
> > > > kernel in a format that can be read from the kernel by walking the
> > > > directories in /proc/device-tree/*.
> > > 
> > > So this DT is available inside kernel and running kernel can still
> > > retrieve it and pass it to second kernel?
> > 
> > The kernel only uses the flattened DT blob at boot time and converts
> > it into the runtime data structures (struct device_node). The original
> > dtb is typically overwritten later.
> 
> On arm64 we deliberately preserved the DTB, so we can take that and
> build a new DTB from that kernel-side.
> 
> > > > - we typically ship devicetree sources for embedded machines with the
> > > >   kernel sources. As more hardware of the system gets enabled, the
> > > >   devicetree gains extra nodes and properties that describe the hardware
> > > >   more completely, so we need to use the latest DT blob to use all
> > > >   the drivers
> > > > 
> > > > - in some cases, kernels will fail to boot at all with an older version
> > > >   of the DT, or fail to use the devices that were working on the
> > > >   earlier kernel. This is usually considered a bug, but it's not rare
> > > > 
> > > > - In some cases, the kernel can update its DT at runtime, and the new
> > > >   settings are expected to be available in the new kernel too, though
> > > >   there are cases where you actually don't want the modified contents.
> > > 
> > > I am assuming that modified DT and unmodifed one both are accessible to
> > > kernel. And if user space can make decisions which modfied fields to use
> > > for new kernels and which ones not, then same can be done in kernel too?
> > 
> > The unmodified DT can typically be found on disk next to the kernel binary.
> > The option you have is to either read it from /proc/devicetree or to
> > read it from from /boot/*.dtb.
> 
> /proc/devicetree (aka /sys/firmware/devicetree) is a filesystem derived
> from the raw DTB (which is exposed at /sys/firmware/fdt).
> 
> The blob that was handed to the kernel at boot time is exposed at
> /sys/firmware/fdt.

I believe the blob can be read and passed to kexec kernel in kernel code without
the extra fd.

But consider we can kexec to a different kernel and a different initrd so there
will be use cases to pass a total different dtb as well. From my understanding
it is reasonable but yes I think we should think carefully about the design.

Thanks
Dave

> Thanks,
> Mark.
> 
> _______________________________________________
> kexec mailing list
> kexec at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

[RFC 0/3] extend kexec_file_load system call

[Stewart Smith]

Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> On Tue, Jul 12, 2016 at 10:58:05PM +0200, Petr Tesarik wrote:
>> I'm not an expert on DTB, so I can't provide an example of code
>> execution, but you have already mentioned the /chosen/linux,stdout-path
>> property. If an attacker redirects the bootloader to an insecure
>> console, they may get access to the system that would otherwise be
>> impossible.
>
> I fail to see how kexec connects with the boot loader - the DTB image
> that's being talked about is one which is passed from the currently
> running kernel to the to-be-kexec'd kernel.  For ARM (and I suspect
> also ARM64) that's a direct call chain which doesn't involve any
> boot loader or firmware, and certainly none that would involve the
> passed DTB image.

For OpenPOWER machines, kexec is the bootloader. Our bootloader is a
linux kernel and initramfs with a UI (petitboot) - this means we never
have to write a device driver twice: write a kernel one and you're done
(for booting from the device and using it in your OS).

-- 
Stewart Smith
OPAL Architect, IBM.

[RFC 0/3] extend kexec_file_load system call

[Russell King - ARM Linux]

On Wed, Jul 13, 2016 at 02:59:51PM +1000, Stewart Smith wrote:
> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> > On Tue, Jul 12, 2016 at 10:58:05PM +0200, Petr Tesarik wrote:
> >> I'm not an expert on DTB, so I can't provide an example of code
> >> execution, but you have already mentioned the /chosen/linux,stdout-path
> >> property. If an attacker redirects the bootloader to an insecure
> >> console, they may get access to the system that would otherwise be
> >> impossible.
> >
> > I fail to see how kexec connects with the boot loader - the DTB image
> > that's being talked about is one which is passed from the currently
> > running kernel to the to-be-kexec'd kernel.  For ARM (and I suspect
> > also ARM64) that's a direct call chain which doesn't involve any
> > boot loader or firmware, and certainly none that would involve the
> > passed DTB image.
> 
> For OpenPOWER machines, kexec is the bootloader. Our bootloader is a
> linux kernel and initramfs with a UI (petitboot) - this means we never
> have to write a device driver twice: write a kernel one and you're done
> (for booting from the device and using it in your OS).

I think you misunderstood my point.

On ARM, we do not go:

	kernel (kexec'd from) -> boot loader -> kernel (kexec'd to)

but we go:

	kernel (kexec'd from) -> kernel (kexec'd to)

There's no intermediate step involving any bootloader.

Hence, my point is that the dtb loaded by kexec is _only_ used by the
kernel which is being kexec'd to, not by the bootloader, nor indeed
the kernel which it is loaded into.

Moreover, if you read the bit that I quoted (which is what I was
replying to), you'll notice that it is talking about the DTB loaded
by kexec somehow causing the _bootloader_ to be redirected to an
alternative console.  This point is wholely false on ARM.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[RFC 0/3] extend kexec_file_load system call

[Ard Biesheuvel]

On 13 July 2016 at 09:36, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Wed, Jul 13, 2016 at 02:59:51PM +1000, Stewart Smith wrote:
>> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>> > On Tue, Jul 12, 2016 at 10:58:05PM +0200, Petr Tesarik wrote:
>> >> I'm not an expert on DTB, so I can't provide an example of code
>> >> execution, but you have already mentioned the /chosen/linux,stdout-path
>> >> property. If an attacker redirects the bootloader to an insecure
>> >> console, they may get access to the system that would otherwise be
>> >> impossible.
>> >
>> > I fail to see how kexec connects with the boot loader - the DTB image
>> > that's being talked about is one which is passed from the currently
>> > running kernel to the to-be-kexec'd kernel.  For ARM (and I suspect
>> > also ARM64) that's a direct call chain which doesn't involve any
>> > boot loader or firmware, and certainly none that would involve the
>> > passed DTB image.
>>
>> For OpenPOWER machines, kexec is the bootloader. Our bootloader is a
>> linux kernel and initramfs with a UI (petitboot) - this means we never
>> have to write a device driver twice: write a kernel one and you're done
>> (for booting from the device and using it in your OS).
>
> I think you misunderstood my point.
>
> On ARM, we do not go:
>
>         kernel (kexec'd from) -> boot loader -> kernel (kexec'd to)
>
> but we go:
>
>         kernel (kexec'd from) -> kernel (kexec'd to)
>
> There's no intermediate step involving any bootloader.
>
> Hence, my point is that the dtb loaded by kexec is _only_ used by the
> kernel which is being kexec'd to, not by the bootloader, nor indeed
> the kernel which it is loaded into.
>
> Moreover, if you read the bit that I quoted (which is what I was
> replying to), you'll notice that it is talking about the DTB loaded
> by kexec somehow causing the _bootloader_ to be redirected to an
> alternative console.  This point is wholely false on ARM.
>

The particular example may not apply, but the argument that the DTB
-as a description of the hardware topology- needs to be signed if the
kernel is also signed is valid. We do the same in the UEFI stub, i.e.,
it normally takes a dtb= argument to allow the DTB to be overridden,
but this feature is disabled when Secure Boot is in effect. By the
same reasoning, if any kind of kexec kernel image validation is in
effect, we should either validate the DTB image as well, or disallow
external DTBs and only perform kexec with the kernel's current DTB
(the blob it was booted with, not the unflattened data structure)

[RFC 0/3] extend kexec_file_load system call

[Stewart Smith]

Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> On Wed, Jul 13, 2016 at 02:59:51PM +1000, Stewart Smith wrote:
>> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>> > On Tue, Jul 12, 2016 at 10:58:05PM +0200, Petr Tesarik wrote:
>> >> I'm not an expert on DTB, so I can't provide an example of code
>> >> execution, but you have already mentioned the /chosen/linux,stdout-path
>> >> property. If an attacker redirects the bootloader to an insecure
>> >> console, they may get access to the system that would otherwise be
>> >> impossible.
>> >
>> > I fail to see how kexec connects with the boot loader - the DTB image
>> > that's being talked about is one which is passed from the currently
>> > running kernel to the to-be-kexec'd kernel.  For ARM (and I suspect
>> > also ARM64) that's a direct call chain which doesn't involve any
>> > boot loader or firmware, and certainly none that would involve the
>> > passed DTB image.
>> 
>> For OpenPOWER machines, kexec is the bootloader. Our bootloader is a
>> linux kernel and initramfs with a UI (petitboot) - this means we never
>> have to write a device driver twice: write a kernel one and you're done
>> (for booting from the device and using it in your OS).
>
> I think you misunderstood my point.
>
> On ARM, we do not go:
>
> 	kernel (kexec'd from) -> boot loader -> kernel (kexec'd to)
>
> but we go:
>
> 	kernel (kexec'd from) -> kernel (kexec'd to)
>
> There's no intermediate step involving any bootloader.
>
> Hence, my point is that the dtb loaded by kexec is _only_ used by the
> kernel which is being kexec'd to, not by the bootloader, nor indeed
> the kernel which it is loaded into.
>
> Moreover, if you read the bit that I quoted (which is what I was
> replying to), you'll notice that it is talking about the DTB loaded
> by kexec somehow causing the _bootloader_ to be redirected to an
> alternative console.  This point is wholely false on ARM.

Ahh.. I missed the bootloader bit there.

In which case, we're the same on OpenPOWER, there is no intermediate
bootloader - in our case we have linux (with kexec) taking on what uboot
or grub is typically used for on other platforms.


-- 
Stewart Smith
OPAL Architect, IBM.

[RFC 0/3] extend kexec_file_load system call

[Arnd Bergmann]

On Wednesday, July 13, 2016 10:36:14 AM CEST Dave Young wrote:
> On 07/12/16 at 03:50pm, Mark Rutland wrote:
> > On Tue, Jul 12, 2016 at 04:24:10PM +0200, Arnd Bergmann wrote:
> > > On Tuesday, July 12, 2016 10:18:11 AM CEST Vivek Goyal wrote:
> > 
> > /proc/devicetree (aka /sys/firmware/devicetree) is a filesystem derived
> > from the raw DTB (which is exposed at /sys/firmware/fdt).
> > 
> > The blob that was handed to the kernel at boot time is exposed at
> > /sys/firmware/fdt.
> 
> I believe the blob can be read and passed to kexec kernel in kernel code without
> the extra fd.
> 
> But consider we can kexec to a different kernel and a different initrd so there
> will be use cases to pass a total different dtb as well. From my understanding
> it is reasonable but yes I think we should think carefully about the design.

Ok, I can see four interesting use cases here:

- Using the dtb that the kernel has saved at boot time. Ideally this should not
  require an additional step of signing it, since the running kernel already
  trusts it.

- A dtb blob from the file system that was produced along with the kernel image.
  If we require a signature on the kernel, the the same requirement should be
  made on the dtb. Whoever signs the kernel can also sign the dtb.
  The tricky part here is the kernel command line that is part of the dtb
  and that may need to be modified.

- Modifying the dtb at for any of the reasons I listed: This should always
  be possible when we do not use secure boot, just like booting an unsigned
  kernel is.

- kboot/petitboot with all of the user space being part of the trusted boot
  chain: it would be good to allow these to modify the dtb as needed without
  breaking the trust chain, just like we allow grub or u-boot to modify the dtb
  before passing it to the kernel.

	Arnd

[RFC 0/3] extend kexec_file_load system call

[Russell King - ARM Linux]

On Wed, Jul 13, 2016 at 09:47:56AM +0200, Ard Biesheuvel wrote:
> On 13 July 2016 at 09:36, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Wed, Jul 13, 2016 at 02:59:51PM +1000, Stewart Smith wrote:
> >> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> >> > On Tue, Jul 12, 2016 at 10:58:05PM +0200, Petr Tesarik wrote:
> >> >> I'm not an expert on DTB, so I can't provide an example of code
> >> >> execution, but you have already mentioned the /chosen/linux,stdout-path
> >> >> property. If an attacker redirects the bootloader to an insecure
> >> >> console, they may get access to the system that would otherwise be
> >> >> impossible.
> >> >
> >> > I fail to see how kexec connects with the boot loader - the DTB image
> >> > that's being talked about is one which is passed from the currently
> >> > running kernel to the to-be-kexec'd kernel.  For ARM (and I suspect
> >> > also ARM64) that's a direct call chain which doesn't involve any
> >> > boot loader or firmware, and certainly none that would involve the
> >> > passed DTB image.
> >>
> >> For OpenPOWER machines, kexec is the bootloader. Our bootloader is a
> >> linux kernel and initramfs with a UI (petitboot) - this means we never
> >> have to write a device driver twice: write a kernel one and you're done
> >> (for booting from the device and using it in your OS).
> >
> > I think you misunderstood my point.
> >
> > On ARM, we do not go:
> >
> >         kernel (kexec'd from) -> boot loader -> kernel (kexec'd to)
> >
> > but we go:
> >
> >         kernel (kexec'd from) -> kernel (kexec'd to)
> >
> > There's no intermediate step involving any bootloader.
> >
> > Hence, my point is that the dtb loaded by kexec is _only_ used by the
> > kernel which is being kexec'd to, not by the bootloader, nor indeed
> > the kernel which it is loaded into.
> >
> > Moreover, if you read the bit that I quoted (which is what I was
> > replying to), you'll notice that it is talking about the DTB loaded
> > by kexec somehow causing the _bootloader_ to be redirected to an
> > alternative console.  This point is wholely false on ARM.
> >
> 
> The particular example may not apply, but the argument that the DTB
> -as a description of the hardware topology- needs to be signed if the
> kernel is also signed is valid. We do the same in the UEFI stub, i.e.,
> it normally takes a dtb= argument to allow the DTB to be overridden,
> but this feature is disabled when Secure Boot is in effect. By the
> same reasoning, if any kind of kexec kernel image validation is in
> effect, we should either validate the DTB image as well, or disallow
> external DTBs and only perform kexec with the kernel's current DTB
> (the blob it was booted with, not the unflattened data structure)

*Sigh*  yes, I know full well, which is why I said what I said in my
_first_ reply:

"However, your point is valid as an attacker can redirect the console
 and/or mounted root on the to-be-kexec'd kernel if they can modify
 the DTB - and there's a whole host of subtle ways to do that, not
 necessarily just modification of the kernel command line."

and I went on to raise a valid point about the necessity to do that
for crashdump, which has been _completely_ ignored.

So, I just stopped reading your reply after the first three lines,
because we are in fact in agreement... but thanks for trying to waste
my time.

Please, keep with the overall discussion, and stop replying to a single
email as a whole point in isolation to every other email in the thread.
And stop bikeshedding, by picking up on the easy stuff but ignoring the
more fundamental points, like the crashdump issue I mentioned in my
first reply and now this reply.

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[RFC 0/3] extend kexec_file_load system call

[Stewart Smith]

Ard Biesheuvel <ard.biesheuvel@linaro.org> writes:
> On 13 July 2016 at 09:36, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>> On Wed, Jul 13, 2016 at 02:59:51PM +1000, Stewart Smith wrote:
>>> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
>>> > On Tue, Jul 12, 2016 at 10:58:05PM +0200, Petr Tesarik wrote:
>>> >> I'm not an expert on DTB, so I can't provide an example of code
>>> >> execution, but you have already mentioned the /chosen/linux,stdout-path
>>> >> property. If an attacker redirects the bootloader to an insecure
>>> >> console, they may get access to the system that would otherwise be
>>> >> impossible.
>>> >
>>> > I fail to see how kexec connects with the boot loader - the DTB image
>>> > that's being talked about is one which is passed from the currently
>>> > running kernel to the to-be-kexec'd kernel.  For ARM (and I suspect
>>> > also ARM64) that's a direct call chain which doesn't involve any
>>> > boot loader or firmware, and certainly none that would involve the
>>> > passed DTB image.
>>>
>>> For OpenPOWER machines, kexec is the bootloader. Our bootloader is a
>>> linux kernel and initramfs with a UI (petitboot) - this means we never
>>> have to write a device driver twice: write a kernel one and you're done
>>> (for booting from the device and using it in your OS).
>>
>> I think you misunderstood my point.
>>
>> On ARM, we do not go:
>>
>>         kernel (kexec'd from) -> boot loader -> kernel (kexec'd to)
>>
>> but we go:
>>
>>         kernel (kexec'd from) -> kernel (kexec'd to)
>>
>> There's no intermediate step involving any bootloader.
>>
>> Hence, my point is that the dtb loaded by kexec is _only_ used by the
>> kernel which is being kexec'd to, not by the bootloader, nor indeed
>> the kernel which it is loaded into.
>>
>> Moreover, if you read the bit that I quoted (which is what I was
>> replying to), you'll notice that it is talking about the DTB loaded
>> by kexec somehow causing the _bootloader_ to be redirected to an
>> alternative console.  This point is wholely false on ARM.
>>
>
> The particular example may not apply, but the argument that the DTB
> -as a description of the hardware topology- needs to be signed if the
> kernel is also signed is valid. We do the same in the UEFI stub, i.e.,
> it normally takes a dtb= argument to allow the DTB to be overridden,
> but this feature is disabled when Secure Boot is in effect. By the
> same reasoning, if any kind of kexec kernel image validation is in
> effect, we should either validate the DTB image as well, or disallow
> external DTBs and only perform kexec with the kernel's current DTB
> (the blob it was booted with, not the unflattened data structure)

DTB booted with != current description of hardware

We could have had: PCI hotplug, CPU/memory/cache offlined due to
hardware error, change in available pstates / CPU frequencies.

There is merit in having a signed dtb if you're booting a signed kernel
in a secure boot scenario. However, we still need to set up /chosen/ and
we still need a way to do something like the offb hack.

-- 
Stewart Smith
OPAL Architect, IBM.

[RFC 0/3] extend kexec_file_load system call

[Stewart Smith]

Arnd Bergmann <arnd@arndb.de> writes:
> On Wednesday, July 13, 2016 10:36:14 AM CEST Dave Young wrote:
>> On 07/12/16 at 03:50pm, Mark Rutland wrote:
>> > On Tue, Jul 12, 2016 at 04:24:10PM +0200, Arnd Bergmann wrote:
>> > > On Tuesday, July 12, 2016 10:18:11 AM CEST Vivek Goyal wrote:
>> > 
>> > /proc/devicetree (aka /sys/firmware/devicetree) is a filesystem derived
>> > from the raw DTB (which is exposed at /sys/firmware/fdt).
>> > 
>> > The blob that was handed to the kernel at boot time is exposed at
>> > /sys/firmware/fdt.
>> 
>> I believe the blob can be read and passed to kexec kernel in kernel code without
>> the extra fd.
>> 
>> But consider we can kexec to a different kernel and a different initrd so there
>> will be use cases to pass a total different dtb as well. From my understanding
>> it is reasonable but yes I think we should think carefully about the design.
>
> Ok, I can see four interesting use cases here:
>
> - Using the dtb that the kernel has saved at boot time. Ideally this should not
>   require an additional step of signing it, since the running kernel already
>   trusts it.

- using current view of the hardware, flattened into a new dtb.
  This should already be trusted, as it's what we're running now (boot +
  runtime changes)

-- 
Stewart Smith
OPAL Architect, IBM.

[RFC 0/3] extend kexec_file_load system call

[Russell King - ARM Linux]

On Wed, Jul 13, 2016 at 05:55:33PM +1000, Stewart Smith wrote:
> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> > On Wed, Jul 13, 2016 at 02:59:51PM +1000, Stewart Smith wrote:
> >> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> >> > On Tue, Jul 12, 2016 at 10:58:05PM +0200, Petr Tesarik wrote:
> >> >> I'm not an expert on DTB, so I can't provide an example of code
> >> >> execution, but you have already mentioned the /chosen/linux,stdout-path
> >> >> property. If an attacker redirects the bootloader to an insecure
> >> >> console, they may get access to the system that would otherwise be
> >> >> impossible.
> >> >
> >> > I fail to see how kexec connects with the boot loader - the DTB image
> >> > that's being talked about is one which is passed from the currently
> >> > running kernel to the to-be-kexec'd kernel.  For ARM (and I suspect
> >> > also ARM64) that's a direct call chain which doesn't involve any
> >> > boot loader or firmware, and certainly none that would involve the
> >> > passed DTB image.
> >> 
> >> For OpenPOWER machines, kexec is the bootloader. Our bootloader is a
> >> linux kernel and initramfs with a UI (petitboot) - this means we never
> >> have to write a device driver twice: write a kernel one and you're done
> >> (for booting from the device and using it in your OS).
> >
> > I think you misunderstood my point.
> >
> > On ARM, we do not go:
> >
> > 	kernel (kexec'd from) -> boot loader -> kernel (kexec'd to)
> >
> > but we go:
> >
> > 	kernel (kexec'd from) -> kernel (kexec'd to)
> >
> > There's no intermediate step involving any bootloader.
> >
> > Hence, my point is that the dtb loaded by kexec is _only_ used by the
> > kernel which is being kexec'd to, not by the bootloader, nor indeed
> > the kernel which it is loaded into.
> >
> > Moreover, if you read the bit that I quoted (which is what I was
> > replying to), you'll notice that it is talking about the DTB loaded
> > by kexec somehow causing the _bootloader_ to be redirected to an
> > alternative console.  This point is wholely false on ARM.
> 
> Ahh.. I missed the bootloader bit there.
> 
> In which case, we're the same on OpenPOWER, there is no intermediate
> bootloader - in our case we have linux (with kexec) taking on what uboot
> or grub is typically used for on other platforms.

Indeed - maybe Eric knows better, but I can't see any situation where
the dtb we load via kexec should ever affect "the bootloader", unless
the "kernel" that's being loaded into kexec is "the bootloader".

Now, going back to the more fundamental issue raised in my first reply,
about the kernel command line.

On x86, I can see that it _is_ possible for userspace to specify a
command line, and the kernel loading the image provides the command
line to the to-be-kexeced kernel with very little checking.  So, if
your kernel is signed, what stops the "insecure userspace" loading
a signed kernel but giving it an insecure rootfs and/or console?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[RFC 0/3] extend kexec_file_load system call

[Dave Young]

[snip]
> Now, going back to the more fundamental issue raised in my first reply,
> about the kernel command line.
> 
> On x86, I can see that it _is_ possible for userspace to specify a
> command line, and the kernel loading the image provides the command
> line to the to-be-kexeced kernel with very little checking.  So, if
> your kernel is signed, what stops the "insecure userspace" loading
> a signed kernel but giving it an insecure rootfs and/or console?

The kexec_file_load syscall was introduced for secure boot in the first
place. In case UEFI secure boot the signature verification chain only
covers kernel mode binaries. I think there is such problem in both normal
boot and kexec boot.

Thanks
Dave

[RFC 0/3] extend kexec_file_load system call

[Petr Tesarik]

On Wed, 13 Jul 2016 09:26:39 +0100
Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

> On Wed, Jul 13, 2016 at 05:55:33PM +1000, Stewart Smith wrote:
> > Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> > > On Wed, Jul 13, 2016 at 02:59:51PM +1000, Stewart Smith wrote:
> > >> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> > >> > On Tue, Jul 12, 2016 at 10:58:05PM +0200, Petr Tesarik wrote:
> > >> >> I'm not an expert on DTB, so I can't provide an example of code
> > >> >> execution, but you have already mentioned the /chosen/linux,stdout-path
> > >> >> property. If an attacker redirects the bootloader to an insecure
> > >> >> console, they may get access to the system that would otherwise be
> > >> >> impossible.
> > >> >
> > >> > I fail to see how kexec connects with the boot loader - the DTB image
> > >> > that's being talked about is one which is passed from the currently
> > >> > running kernel to the to-be-kexec'd kernel.  For ARM (and I suspect
> > >> > also ARM64) that's a direct call chain which doesn't involve any
> > >> > boot loader or firmware, and certainly none that would involve the
> > >> > passed DTB image.
> > >> 
> > >> For OpenPOWER machines, kexec is the bootloader. Our bootloader is a
> > >> linux kernel and initramfs with a UI (petitboot) - this means we never
> > >> have to write a device driver twice: write a kernel one and you're done
> > >> (for booting from the device and using it in your OS).
> > >
> > > I think you misunderstood my point.
> > >
> > > On ARM, we do not go:
> > >
> > > 	kernel (kexec'd from) -> boot loader -> kernel (kexec'd to)
> > >
> > > but we go:
> > >
> > > 	kernel (kexec'd from) -> kernel (kexec'd to)
> > >
> > > There's no intermediate step involving any bootloader.
> > >
> > > Hence, my point is that the dtb loaded by kexec is _only_ used by the
> > > kernel which is being kexec'd to, not by the bootloader, nor indeed
> > > the kernel which it is loaded into.
> > >
> > > Moreover, if you read the bit that I quoted (which is what I was
> > > replying to), you'll notice that it is talking about the DTB loaded
> > > by kexec somehow causing the _bootloader_ to be redirected to an
> > > alternative console.  This point is wholely false on ARM.
> > 
> > Ahh.. I missed the bootloader bit there.
> > 
> > In which case, we're the same on OpenPOWER, there is no intermediate
> > bootloader - in our case we have linux (with kexec) taking on what uboot
> > or grub is typically used for on other platforms.
> 
> Indeed - maybe Eric knows better, but I can't see any situation where
> the dtb we load via kexec should ever affect "the bootloader", unless
> the "kernel" that's being loaded into kexec is "the bootloader".
> 
> Now, going back to the more fundamental issue raised in my first reply,
> about the kernel command line.
> 
> On x86, I can see that it _is_ possible for userspace to specify a
> command line, and the kernel loading the image provides the command
> line to the to-be-kexeced kernel with very little checking.  So, if
> your kernel is signed, what stops the "insecure userspace" loading
> a signed kernel but giving it an insecure rootfs and/or console?

This is a valid point. If there are kernel options that can be misused
to defeat the purpose of UEFI SecureBoot, then we're in trouble.
Generally, the Linux kernel should treat the command line as untrusted
source.

My point is that modifying the DTB opens a completely new attack
vector. And the goal is not extending the attack surface (because there
are holes in it already), but reducing the attack surface (e.g. by
limiting available kernel command line options).

Petr T

[RFC 0/3] extend kexec_file_load system call

[Mark Rutland]

On Wed, Jul 13, 2016 at 10:36:14AM +0800, Dave Young wrote:
> But consider we can kexec to a different kernel and a different initrd so there
> will be use cases to pass a total different dtb as well.

It depends on what you mean by "a different kernel", and what this
implies for the DTB.

I expect future arm64 Linux kernels to function with today's DTBs, and
the existing boot protocol. The kexec_file_load syscall already has
enough information for the kernel to inject the initrd and bootargs
properties into a DTB.

In practice on x86 today, kexec_file_load only supports booting to a
Linux kernel, because the in-kernel purgatory only implements the x86
Linux boot protocol. Analagously, for arm64 I think that the first
kernel should use its internal copy of the boot DTB, with /chosen fixed
up appropriately, assuming the next kernel is an arm64 Linux image.

If booting another OS, the only parts of the DTB I would expect to
change are the properties under chosen, as everything else *should* be
OS-independent. However the other OS may have a completely different
boot protocol, might not even take a DTB, and will likely need a
compeltely different purgatory implementation. So just allowing the DTB
to be altered isn't sufficient for that case.

There might be cases where we want a different DTB, but as far as I can
tell we have nothing analagous on x86 today. If we do need this, we
should have an idea of what real case(s) were trying to solve.

Thanks,
Mark.

[RFC 0/3] extend kexec_file_load system call

[Mark Rutland]

On Wed, Jul 13, 2016 at 10:01:33AM +0200, Arnd Bergmann wrote:
> On Wednesday, July 13, 2016 10:36:14 AM CEST Dave Young wrote:
> > On 07/12/16 at 03:50pm, Mark Rutland wrote:
> > > On Tue, Jul 12, 2016 at 04:24:10PM +0200, Arnd Bergmann wrote:
> > > > On Tuesday, July 12, 2016 10:18:11 AM CEST Vivek Goyal wrote:
> > > 
> > > /proc/devicetree (aka /sys/firmware/devicetree) is a filesystem derived
> > > from the raw DTB (which is exposed at /sys/firmware/fdt).
> > > 
> > > The blob that was handed to the kernel at boot time is exposed at
> > > /sys/firmware/fdt.
> > 
> > I believe the blob can be read and passed to kexec kernel in kernel code without
> > the extra fd.
> > 
> > But consider we can kexec to a different kernel and a different initrd so there
> > will be use cases to pass a total different dtb as well. From my understanding
> > it is reasonable but yes I think we should think carefully about the design.
> 
> Ok, I can see four interesting use cases here:
> 
> - Using the dtb that the kernel has saved at boot time. Ideally this should not
>   require an additional step of signing it, since the running kernel already
>   trusts it.

We have sufficient information from the existing kexec_file_load syscall
prototype to do this in-kernel.

> - A dtb blob from the file system that was produced along with the kernel image.
>   If we require a signature on the kernel, the the same requirement should be
>   made on the dtb. Whoever signs the kernel can also sign the dtb.
>   The tricky part here is the kernel command line that is part of the dtb
>   and that may need to be modified.

I suspect that for this case, following the example of the existing
sycall, we'd allow the kernel to modify bootargs and initrd properties
after verfiying the signature of the DTB.

The big question is whether this is a realistic case on a secure boot
system.

> - Modifying the dtb at for any of the reasons I listed: This should always
>   be possible when we do not use secure boot, just like booting an unsigned
>   kernel is.

This is possible with the existing kexec_load syscall, for the non
secure boot case.

> - kboot/petitboot with all of the user space being part of the trusted boot
>   chain: it would be good to allow these to modify the dtb as needed without
>   breaking the trust chain, just like we allow grub or u-boot to modify the dtb
>   before passing it to the kernel.

It depends on *what* we need to modify here. We can modify the bootargs
and initrd properties as part of the kexec_file_load syscall, so what
else would we want to alter?

Thanks,
Mark.

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Wed, Jul 13, 2016 at 09:26:39AM +0100, Russell King - ARM Linux wrote:
> On Wed, Jul 13, 2016 at 05:55:33PM +1000, Stewart Smith wrote:
> > Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> > > On Wed, Jul 13, 2016 at 02:59:51PM +1000, Stewart Smith wrote:
> > >> Russell King - ARM Linux <linux@armlinux.org.uk> writes:
> > >> > On Tue, Jul 12, 2016 at 10:58:05PM +0200, Petr Tesarik wrote:
> > >> >> I'm not an expert on DTB, so I can't provide an example of code
> > >> >> execution, but you have already mentioned the /chosen/linux,stdout-path
> > >> >> property. If an attacker redirects the bootloader to an insecure
> > >> >> console, they may get access to the system that would otherwise be
> > >> >> impossible.
> > >> >
> > >> > I fail to see how kexec connects with the boot loader - the DTB image
> > >> > that's being talked about is one which is passed from the currently
> > >> > running kernel to the to-be-kexec'd kernel.  For ARM (and I suspect
> > >> > also ARM64) that's a direct call chain which doesn't involve any
> > >> > boot loader or firmware, and certainly none that would involve the
> > >> > passed DTB image.
> > >> 
> > >> For OpenPOWER machines, kexec is the bootloader. Our bootloader is a
> > >> linux kernel and initramfs with a UI (petitboot) - this means we never
> > >> have to write a device driver twice: write a kernel one and you're done
> > >> (for booting from the device and using it in your OS).
> > >
> > > I think you misunderstood my point.
> > >
> > > On ARM, we do not go:
> > >
> > > 	kernel (kexec'd from) -> boot loader -> kernel (kexec'd to)
> > >
> > > but we go:
> > >
> > > 	kernel (kexec'd from) -> kernel (kexec'd to)
> > >
> > > There's no intermediate step involving any bootloader.
> > >
> > > Hence, my point is that the dtb loaded by kexec is _only_ used by the
> > > kernel which is being kexec'd to, not by the bootloader, nor indeed
> > > the kernel which it is loaded into.
> > >
> > > Moreover, if you read the bit that I quoted (which is what I was
> > > replying to), you'll notice that it is talking about the DTB loaded
> > > by kexec somehow causing the _bootloader_ to be redirected to an
> > > alternative console.  This point is wholely false on ARM.
> > 
> > Ahh.. I missed the bootloader bit there.
> > 
> > In which case, we're the same on OpenPOWER, there is no intermediate
> > bootloader - in our case we have linux (with kexec) taking on what uboot
> > or grub is typically used for on other platforms.
> 
> Indeed - maybe Eric knows better, but I can't see any situation where
> the dtb we load via kexec should ever affect "the bootloader", unless
> the "kernel" that's being loaded into kexec is "the bootloader".
> 
> Now, going back to the more fundamental issue raised in my first reply,
> about the kernel command line.
> 
> On x86, I can see that it _is_ possible for userspace to specify a
> command line, and the kernel loading the image provides the command
> line to the to-be-kexeced kernel with very little checking.  So, if
> your kernel is signed, what stops the "insecure userspace" loading
> a signed kernel but giving it an insecure rootfs and/or console?

It is not kexec specific. I could do this for regular boot too, right?

Command line options are not signed. I thought idea behind secureboot
was to execute only trusted code and command line options don't enforce
you to execute unsigned code.

So it sounds like different class of security problems which you are
referring to and not necessarily covered by secureboot or signed
kernel.

Vivek

[RFC 0/3] extend kexec_file_load system call

[Arnd Bergmann]

On Wednesday, July 13, 2016 10:41:28 AM CEST Mark Rutland wrote:
> On Wed, Jul 13, 2016 at 10:01:33AM +0200, Arnd Bergmann wrote:
> > On Wednesday, July 13, 2016 10:36:14 AM CEST Dave Young wrote:
> > > On 07/12/16 at 03:50pm, Mark Rutland wrote:
> > > > On Tue, Jul 12, 2016 at 04:24:10PM +0200, Arnd Bergmann wrote:
> > > > > On Tuesday, July 12, 2016 10:18:11 AM CEST Vivek Goyal wrote:
> > > > 
> > > > /proc/devicetree (aka /sys/firmware/devicetree) is a filesystem derived
> > > > from the raw DTB (which is exposed at /sys/firmware/fdt).
> > > > 
> > > > The blob that was handed to the kernel at boot time is exposed at
> > > > /sys/firmware/fdt.
> > > 
> > > I believe the blob can be read and passed to kexec kernel in kernel code without
> > > the extra fd.
> > > 
> > > But consider we can kexec to a different kernel and a different initrd so there
> > > will be use cases to pass a total different dtb as well. From my understanding
> > > it is reasonable but yes I think we should think carefully about the design.
> > 
> > Ok, I can see four interesting use cases here:
> > 
> > - Using the dtb that the kernel has saved at boot time. Ideally this should not
> >   require an additional step of signing it, since the running kernel already
> >   trusts it.
> 
> We have sufficient information from the existing kexec_file_load syscall
> prototype to do this in-kernel.

Ok.

> > - A dtb blob from the file system that was produced along with the kernel image.
> >   If we require a signature on the kernel, the the same requirement should be
> >   made on the dtb. Whoever signs the kernel can also sign the dtb.
> >   The tricky part here is the kernel command line that is part of the dtb
> >   and that may need to be modified.
> 
> I suspect that for this case, following the example of the existing
> sycall, we'd allow the kernel to modify bootargs and initrd properties
> after verfiying the signature of the DTB.

Makes sense.
 
> The big question is whether this is a realistic case on a secure boot
> system.

What does x86 do here? I assume changes to the command line are also
limited.

> > - Modifying the dtb at for any of the reasons I listed: This should always
> >   be possible when we do not use secure boot, just like booting an unsigned
> >   kernel is.
> 
> This is possible with the existing kexec_load syscall, for the non
> secure boot case.

Ok, let's skip that then.

> > - kboot/petitboot with all of the user space being part of the trusted boot
> >   chain: it would be good to allow these to modify the dtb as needed without
> >   breaking the trust chain, just like we allow grub or u-boot to modify the dtb
> >   before passing it to the kernel.
> 
> It depends on *what* we need to modify here. We can modify the bootargs
> and initrd properties as part of the kexec_file_load syscall, so what
> else would we want to alter?

I guess petitboot can also just use kexec_load() instead of kexec_file_load(),
as long as the initramfs containing petitboot is trusted by the kernel.

	Arnd

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Wed, Jul 13, 2016 at 09:41:39AM +1000, Stewart Smith wrote:
> Petr Tesarik <ptesarik@suse.cz> writes:
> > On Tue, 12 Jul 2016 13:25:11 -0300
> > Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> wrote:
> >
> >> Hi Eric,
> >> 
> >> I'm trying to understand your concerns leading to your nack. I hope you 
> >> don't mind expanding your thoughts on them a bit.
> >> 
> >> Am Dienstag, 12 Juli 2016, 08:25:48 schrieb Eric W. Biederman:
> >> > AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
> >> > > Device tree blob must be passed to a second kernel on DTB-capable
> >> > > archs, like powerpc and arm64, but the current kernel interface
> >> > > lacks this support.
> >> > > 
> >> > > This patch extends kexec_file_load system call by adding an extra
> >> > > argument to this syscall so that an arbitrary number of file descriptors
> >> > > can be handed out from user space to the kernel.
> >> > > 
> >> > > See the background [1].
> >> > > 
> >> > > Please note that the new interface looks quite similar to the current
> >> > > system call, but that it won't always mean that it provides the "binary
> >> > > compatibility."
> >> > > 
> >> > > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
> >> > 
> >> > So this design is wrong.  The kernel already has the device tree blob,
> >> > you should not be extracting it from the kernel munging it, and then
> >> > reinserting it in the kernel if you want signatures and everything to
> >> > pass.
> >> 
> >> I don't understand how the kernel signature will be invalidated. 
> >> 
> >> There are some types of boot images that can embed a device tree blob in 
> >> them, but the kernel can also be handed a separate device tree blob from 
> >> firmware, the boot loader, or kexec. This latter case is what we are 
> >> discussing, so we are not talking about modifying an embedded blob in the 
> >> kernel image.
> >> 
> >> > What x86 does is pass it's equivalent of the device tree blob from one
> >> > kernel to another directly and behind the scenes.  It does not go
> >> > through userspace for this.
> >> > 
> >> > Until a persuasive case can be made for going around the kernel and
> >> > probably adding a feature (like code execution) that can be used to
> >> > defeat the signature scheme I am going to nack this.
> >> 
> >> I also don't understand what you mean by code execution. How does passing a 
> >> device tree blob via kexec enables code execution? How can the signature 
> >> scheme be defeated?
> >
> > I'm not an expert on DTB, so I can't provide an example of code
> > execution, but you have already mentioned the /chosen/linux,stdout-path
> > property. If an attacker redirects the bootloader to an insecure
> > console, they may get access to the system that would otherwise be
> > impossible.
> 
> In this case, the user is sitting at the (or one of the) console(s) of
> the machine. There could be petitboot UIs running on the VGA display,
> IPMI serial over lan, local serial port. The logic behind setting
> /chosen/linux,stdout-path is (currently) mostly to set it for the kernel
> to what the user is interacting with. i.e. if you select an OS installer
> to boot from the VGA console, you get a graphical installer running and
> if you selected it from a text console, you get a text installer running
> (on the appropriate console).
> 
> So the bootloader (petitboot) needs to work out which console is being
> interacted with in order to set up /chosen/linux,stdout-path correctly.
> 
> This specific option could be passed as a kernel command line to the
> next kernel, yes. However, isn't the kernel command line also an attack
> vector? Is *every* command line option safe?

I don't think kernel command line is signed. And we will have to define
what is considered *unsafe*. I am working on the assumption that a
user should not be able to force execution of unsigned code at provileged
level. And passing console on kernel command line should be safe in
that respect?

Vivek

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Wed, Jul 13, 2016 at 09:45:22AM +1000, Stewart Smith wrote:
> Vivek Goyal <vgoyal@redhat.com> writes:
> > On Tue, Jul 12, 2016 at 10:58:09AM -0300, Thiago Jung Bauermann wrote:
> >> Hello Eric,
> >> 
> >> Am Dienstag, 12 Juli 2016, 08:25:48 schrieb Eric W. Biederman:
> >> > AKASHI Takahiro <takahiro.akashi@linaro.org> writes:
> >> > > Device tree blob must be passed to a second kernel on DTB-capable
> >> > > archs, like powerpc and arm64, but the current kernel interface
> >> > > lacks this support.
> >> > > 
> >> > > This patch extends kexec_file_load system call by adding an extra
> >> > > argument to this syscall so that an arbitrary number of file descriptors
> >> > > can be handed out from user space to the kernel.
> >> > > 
> >> > > See the background [1].
> >> > > 
> >> > > Please note that the new interface looks quite similar to the current
> >> > > system call, but that it won't always mean that it provides the "binary
> >> > > compatibility."
> >> > > 
> >> > > [1] http://lists.infradead.org/pipermail/kexec/2016-June/016276.html
> >> > 
> >> > So this design is wrong.  The kernel already has the device tree blob,
> >> > you should not be extracting it from the kernel munging it, and then
> >> > reinserting it in the kernel if you want signatures and everything to
> >> > pass.
> >> > 
> >> > What x86 does is pass it's equivalent of the device tree blob from one
> >> > kernel to another directly and behind the scenes.  It does not go
> >> > through userspace for this.
> >> > 
> >> > Until a persuasive case can be made for going around the kernel and
> >> > probably adding a feature (like code execution) that can be used to
> >> > defeat the signature scheme I am going to nack this.
> >> 
> >> There are situations where userspace needs to change things in the device 
> >> tree to be used by the next kernel.
> >> 
> >> For example, Petitboot (the boot loader used in OpenPOWER machines) is a 
> >> userspace application running in an intermediary Linux instance and uses 
> >> kexec to load the target OS. It has to modify the device tree that will be 
> >> used by the next kernel so that the next kernel uses the same console that 
> >> petitboot was configured to use (i.e., set the /chosen/linux,stdout-path 
> >> property). It also modifies the device tree to allow the kernel to inherit 
> >> Petitboot's Openfirmware framebuffer.
> >
> > Can some of this be done with the help of kernel command line options for
> > second kernel?
> 
> how would this be any more secure?
> 
> Passing in an address for a framebuffer via command line option means
> you could scribble over any bit of memory, which is the same kind of
> damage you could do by modifying the device tree.

It is not necessarily safer but works with given framework and we don't
have to modify existing system call.

Also it will allow you to pass in only one thing at a time instead of
allowing passing in new unsigned DTB, which can potentially do lot more.

Vivek

[RFC 0/3] extend kexec_file_load system call

[AKASHI Takahiro]

Apologies for the slow response. I'm attending LinuxCon this week.

On Wed, Jul 13, 2016 at 10:34:47AM +0100, Mark Rutland wrote:
> On Wed, Jul 13, 2016 at 10:36:14AM +0800, Dave Young wrote:
> > But consider we can kexec to a different kernel and a different initrd so there
> > will be use cases to pass a total different dtb as well.
> 
> It depends on what you mean by "a different kernel", and what this
> implies for the DTB.
> 
> I expect future arm64 Linux kernels to function with today's DTBs, and
> the existing boot protocol. The kexec_file_load syscall already has
> enough information for the kernel to inject the initrd and bootargs
> properties into a DTB.
> 
> In practice on x86 today, kexec_file_load only supports booting to a
> Linux kernel, because the in-kernel purgatory only implements the x86
> Linux boot protocol. Analagously, for arm64 I think that the first
> kernel should use its internal copy of the boot DTB, with /chosen fixed
> up appropriately, assuming the next kernel is an arm64 Linux image.
> 
> If booting another OS, the only parts of the DTB I would expect to
> change are the properties under chosen, as everything else *should* be
> OS-independent. However the other OS may have a completely different
> boot protocol, might not even take a DTB, and will likely need a
> compeltely different purgatory implementation. So just allowing the DTB
> to be altered isn't sufficient for that case.
> 
> There might be cases where we want a different DTB, but as far as I can
> tell we have nothing analagous on x86 today. If we do need this, we
> should have an idea of what real case(s) were trying to solve.

What I had in my mind was:

- Kdump
  As Russel said, we definitely need to modify dtb.
  In addition to bootargs and initrd proerties (FYI, in my arm64
  implementation for arm64, eflcorehdr info is also passed as DT
  property), we may want to remove unnecessary devices and
  even add a dedicated storage device for storing a core dump image.
- Say, booting BE kernel on ACPI LE kernel
  In this case, there is no useful dtb in the kernel.

Have said that, as Mark said, we may be able to use normal kexec_load
system call if we don't need a "secure" kexec.

BTW, why doesn't the current kexec_load have ability of verifying
a signature of initramfs image? Is IMA/EVM expected to be used
at runtime?

Thanks,
-Takahiro AKASHI
> Thanks,
> Mark.

[RFC 0/3] extend kexec_file_load system call

[Russell King - ARM Linux]

On Wed, Jul 13, 2016 at 09:03:38AM -0400, Vivek Goyal wrote:
> On Wed, Jul 13, 2016 at 09:26:39AM +0100, Russell King - ARM Linux wrote:
> > Indeed - maybe Eric knows better, but I can't see any situation where
> > the dtb we load via kexec should ever affect "the bootloader", unless
> > the "kernel" that's being loaded into kexec is "the bootloader".
> > 
> > Now, going back to the more fundamental issue raised in my first reply,
> > about the kernel command line.
> > 
> > On x86, I can see that it _is_ possible for userspace to specify a
> > command line, and the kernel loading the image provides the command
> > line to the to-be-kexeced kernel with very little checking.  So, if
> > your kernel is signed, what stops the "insecure userspace" loading
> > a signed kernel but giving it an insecure rootfs and/or console?
> 
> It is not kexec specific. I could do this for regular boot too, right?
> 
> Command line options are not signed. I thought idea behind secureboot
> was to execute only trusted code and command line options don't enforce
> you to execute unsigned code.
> 
> So it sounds like different class of security problems which you are
> referring to and not necessarily covered by secureboot or signed
> kernel.

Let me give you an example.

You have a secure boot setup, where the firmware/ROM validates the boot
loader.  Good, the boot loader hasn't been tampered with.

You interrupt the boot loader and are able to modify the command line
for the booted kernel.

The boot loader loads the kernel and verifies the kernel's signature.
Good, the kernel hasn't been tampered with.  The kernel starts running.

You've plugged in a USB drive to the device, and specified a partition
containing a root filesystem that you control to the kernel.  The
validated kernel finds the USB drive, and mounts it, and executes
your own binaries on the USB drive.

You run a shell on the console.  You now have control of the system,
and can mount the real rootfs, inspect it, and work out what it does,
etc.

At this point, what use was all the validation that the secure boot
has done?  Absolutely useless.

If you can change the command line arguments given to the kernel, you
have no security, no matter how much you verify signatures.  It's
the illusion of security, nothing more, nothing less.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[RFC 0/3] extend kexec_file_load system call

[Mark Rutland]

On Thu, Jul 14, 2016 at 02:38:06AM +0900, AKASHI Takahiro wrote:
> Apologies for the slow response. I'm attending LinuxCon this week.
> 
> On Wed, Jul 13, 2016 at 10:34:47AM +0100, Mark Rutland wrote:
> > On Wed, Jul 13, 2016 at 10:36:14AM +0800, Dave Young wrote:
> > > But consider we can kexec to a different kernel and a different initrd so there
> > > will be use cases to pass a total different dtb as well.
> > 
> > It depends on what you mean by "a different kernel", and what this
> > implies for the DTB.
> > 
> > I expect future arm64 Linux kernels to function with today's DTBs, and
> > the existing boot protocol. The kexec_file_load syscall already has
> > enough information for the kernel to inject the initrd and bootargs
> > properties into a DTB.
> > 
> > In practice on x86 today, kexec_file_load only supports booting to a
> > Linux kernel, because the in-kernel purgatory only implements the x86
> > Linux boot protocol. Analagously, for arm64 I think that the first
> > kernel should use its internal copy of the boot DTB, with /chosen fixed
> > up appropriately, assuming the next kernel is an arm64 Linux image.
> > 
> > If booting another OS, the only parts of the DTB I would expect to
> > change are the properties under chosen, as everything else *should* be
> > OS-independent. However the other OS may have a completely different
> > boot protocol, might not even take a DTB, and will likely need a
> > compeltely different purgatory implementation. So just allowing the DTB
> > to be altered isn't sufficient for that case.
> > 
> > There might be cases where we want a different DTB, but as far as I can
> > tell we have nothing analagous on x86 today. If we do need this, we
> > should have an idea of what real case(s) were trying to solve.
> 
> What I had in my mind was:
> 
> - Kdump
>   As Russel said, we definitely need to modify dtb.

I agree that *something* needs to modify the DTB to pass the cmdline and
initrd properties.

What I'm trying to point out that it isn't necessary that *userspace*
does so for the vast majority of kexec_file_load cases.

If userspace where to have to modify things dynamically, then you can't
have a secure deployment. Either you don't verify signatures on things
modified by userspace, giving a backdoor, or each machine has to have a
local copy of (locally) trusted private keys, which comes with other
risks (e.g. offline extraction of the keys).

>   In addition to bootargs and initrd proerties (FYI, in my arm64
>   implementation for arm64, eflcorehdr info is also passed as DT
>   property),

As pointed out, for kexec_file_load we can add code to the kernel can
add bootargs and initrd properties as necessary for this case. The
existing kexec_file_load prototype allows userspace to pass the required
information.

>   we may want to remove unnecessary devices and even add a dedicated
>   storage device for storing a core dump image.

I suspect that bringing up a minimal number of devices is better
controlled by a cmdline option. In general, figuring out what is
necessary and what is not is going to be board specific, so hacking the
FW tables (DTB or ACPI) is not a very portable/reliable approach.

Do we actually add devices in practice? More so than the above that
requires special knowledge of the platform (including things that were
not described in the boot DTB).

In the ACPI case modifying a DTB alone is not sufficient to change the
information regarding devices, as those won't be described in the DTB.
It's not possible to convert ACPI to DTB in general.

> - Say, booting BE kernel on ACPI LE kernel
>   In this case, there is no useful dtb in the kernel.

If the platform only has ACPI, then you cannot boot a BE kernel to begin
with. As above one cannot convert ACPI to DTB, so one would need
extensive platform knowledge for this to work.

I think it's fair to say that this is not a realistic/common case.

> Have said that, as Mark said, we may be able to use normal kexec_load
> system call if we don't need a "secure" kexec.
> 
> BTW, why doesn't the current kexec_load have ability of verifying
> a signature of initramfs image?

I believe the code was written before secure boot was a concern, and in
the absence of secure boot it was expected that a trusted userspace
would verify signatures itself.

> Is IMA/EVM expected to be used at runtime?

Sorry, I'm not sure what those abbreviations mean. Could you expand
them?

Thanks,
Mark.

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Wed, Jul 13, 2016 at 06:40:10PM +0100, Russell King - ARM Linux wrote:
> On Wed, Jul 13, 2016 at 09:03:38AM -0400, Vivek Goyal wrote:
> > On Wed, Jul 13, 2016 at 09:26:39AM +0100, Russell King - ARM Linux wrote:
> > > Indeed - maybe Eric knows better, but I can't see any situation where
> > > the dtb we load via kexec should ever affect "the bootloader", unless
> > > the "kernel" that's being loaded into kexec is "the bootloader".
> > > 
> > > Now, going back to the more fundamental issue raised in my first reply,
> > > about the kernel command line.
> > > 
> > > On x86, I can see that it _is_ possible for userspace to specify a
> > > command line, and the kernel loading the image provides the command
> > > line to the to-be-kexeced kernel with very little checking.  So, if
> > > your kernel is signed, what stops the "insecure userspace" loading
> > > a signed kernel but giving it an insecure rootfs and/or console?
> > 
> > It is not kexec specific. I could do this for regular boot too, right?
> > 
> > Command line options are not signed. I thought idea behind secureboot
> > was to execute only trusted code and command line options don't enforce
> > you to execute unsigned code.
> > 
> > So it sounds like different class of security problems which you are
> > referring to and not necessarily covered by secureboot or signed
> > kernel.
> 
> Let me give you an example.
> 
> You have a secure boot setup, where the firmware/ROM validates the boot
> loader.  Good, the boot loader hasn't been tampered with.
> 
> You interrupt the boot loader and are able to modify the command line
> for the booted kernel.
> 
> The boot loader loads the kernel and verifies the kernel's signature.
> Good, the kernel hasn't been tampered with.  The kernel starts running.
> 
> You've plugged in a USB drive to the device, and specified a partition
> containing a root filesystem that you control to the kernel.  The
> validated kernel finds the USB drive, and mounts it, and executes
> your own binaries on the USB drive.

You will require physical access to the machine to be able to
insert your usb drive. And IIRC, argument was that if attacker has
physical access to machine, all bets are off anyway.

> 
> You run a shell on the console.  You now have control of the system,
> and can mount the real rootfs, inspect it, and work out what it does,
> etc.
> 
> At this point, what use was all the validation that the secure boot
> has done?  Absolutely useless.
> 
> If you can change the command line arguments given to the kernel, you
> have no security, no matter how much you verify signatures.  It's
> the illusion of security, nothing more, nothing less.
> 
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

[RFC 0/3] extend kexec_file_load system call

[Thiago Jung Bauermann]

Am Mittwoch, 13 Juli 2016, 15:13:42 schrieb Arnd Bergmann:
> On Wednesday, July 13, 2016 10:41:28 AM CEST Mark Rutland wrote:
> > On Wed, Jul 13, 2016 at 10:01:33AM +0200, Arnd Bergmann wrote:
> > > - kboot/petitboot with all of the user space being part of the trusted
> > > boot> > 
> > >   chain: it would be good to allow these to modify the dtb as needed
> > >   without breaking the trust chain, just like we allow grub or u-boot
> > >   to modify the dtb before passing it to the kernel.
> > 
> > It depends on *what* we need to modify here. We can modify the bootargs
> > and initrd properties as part of the kexec_file_load syscall, so what
> > else would we want to alter?
> 
> I guess petitboot can also just use kexec_load() instead of
> kexec_file_load(), as long as the initramfs containing petitboot is
> trusted by the kernel.

For secure boot, Petitboot needs to use kexec_file_load, because of the 
following two features which the system call enables:

1. only allow loading of signed kernels.
2. "measure" (i.e., record the hashes of) the kernel, initrd, kernel
   command line and other boot inputs for the Integrity Measurement
   Architecture subsystem.

Those can't be done with kexec_load.

As for what we need to modify, Petitboot does the following modifications to 
the DTB:

1. Set /chosen/linux,stdout-path based on which console is being used to 
interact with it, as Stewart mentioned in another email.
2. Set display properties on /pciex at n/.../vga at 0 in machines with an 
OpenFirmware framebuffer.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

[RFC 0/3] extend kexec_file_load system call

[Arnd Bergmann]

On Wednesday, July 13, 2016 6:58:32 PM CEST Mark Rutland wrote:
> 
> >   we may want to remove unnecessary devices and even add a dedicated
> >   storage device for storing a core dump image.
> 
> I suspect that bringing up a minimal number of devices is better
> controlled by a cmdline option. In general, figuring out what is
> necessary and what is not is going to be board specific, so hacking the
> FW tables (DTB or ACPI) is not a very portable/reliable approach.
> 
> Do we actually add devices in practice? More so than the above that
> requires special knowledge of the platform (including things that were
> not described in the boot DTB).
> 
> In the ACPI case modifying a DTB alone is not sufficient to change the
> information regarding devices, as those won't be described in the DTB.
> It's not possible to convert ACPI to DTB in general.

A more likely scenario would be replacing ACPI tables with a DTB that
describes the platform in order to use devices that the ACPI tables
don't contain.

> > - Say, booting BE kernel on ACPI LE kernel
> >   In this case, there is no useful dtb in the kernel.

> If the platform only has ACPI, then you cannot boot a BE kernel to begin
> with. As above one cannot convert ACPI to DTB, so one would need
> extensive platform knowledge for this to work.

I think what he meant was to pass a DTB to the kexec kernel in order
to run BE, while the original kernel can only run LE due to ACPI.

If you boot a LE kernel using DTB, the same DTB should work
for a kexec boot for a BE kernel.

	Arnd

[RFC 0/3] extend kexec_file_load system call

[Arnd Bergmann]

On Wednesday, July 13, 2016 3:45:41 PM CEST Thiago Jung Bauermann wrote:
> Am Mittwoch, 13 Juli 2016, 15:13:42 schrieb Arnd Bergmann:
> > On Wednesday, July 13, 2016 10:41:28 AM CEST Mark Rutland wrote:
> > > On Wed, Jul 13, 2016 at 10:01:33AM +0200, Arnd Bergmann wrote:
> > > > - kboot/petitboot with all of the user space being part of the trusted
> > > > boot> > 
> > > >   chain: it would be good to allow these to modify the dtb as needed
> > > >   without breaking the trust chain, just like we allow grub or u-boot
> > > >   to modify the dtb before passing it to the kernel.
> > > 
> > > It depends on *what* we need to modify here. We can modify the bootargs
> > > and initrd properties as part of the kexec_file_load syscall, so what
> > > else would we want to alter?
> > 
> > I guess petitboot can also just use kexec_load() instead of
> > kexec_file_load(), as long as the initramfs containing petitboot is
> > trusted by the kernel.
> 
> For secure boot, Petitboot needs to use kexec_file_load, because of the 
> following two features which the system call enables:
> 
> 1. only allow loading of signed kernels.
> 2. "measure" (i.e., record the hashes of) the kernel, initrd, kernel
>    command line and other boot inputs for the Integrity Measurement
>    Architecture subsystem.
> 
> Those can't be done with kexec_load.

Can't petitboot do both of these in user space?

	Arnd

[RFC 0/3] extend kexec_file_load system call

[Dave Young]

On 07/13/16 at 10:34am, Mark Rutland wrote:
> On Wed, Jul 13, 2016 at 10:36:14AM +0800, Dave Young wrote:
> > But consider we can kexec to a different kernel and a different initrd so there
> > will be use cases to pass a total different dtb as well.
> 
> It depends on what you mean by "a different kernel", and what this
> implies for the DTB.
> 

I thought about kexec as a boot loader just like other bootloaders.
So just like a normal boot kexec should also accept external dtb.
But acutally kexec is different because it can get original dtb and
use it. So I agreed if we can not find a real use case that we have
to extend it we should keep current interface.

> I expect future arm64 Linux kernels to function with today's DTBs, and
> the existing boot protocol. The kexec_file_load syscall already has
> enough information for the kernel to inject the initrd and bootargs
> properties into a DTB.
> 
> In practice on x86 today, kexec_file_load only supports booting to a
> Linux kernel, because the in-kernel purgatory only implements the x86
> Linux boot protocol. Analagously, for arm64 I think that the first
> kernel should use its internal copy of the boot DTB, with /chosen fixed
> up appropriately, assuming the next kernel is an arm64 Linux image.
> 
> If booting another OS, the only parts of the DTB I would expect to
> change are the properties under chosen, as everything else *should* be
> OS-independent. However the other OS may have a completely different
> boot protocol, might not even take a DTB, and will likely need a
> compeltely different purgatory implementation. So just allowing the DTB
> to be altered isn't sufficient for that case.
> 
> There might be cases where we want a different DTB, but as far as I can
> tell we have nothing analagous on x86 today. If we do need this, we
> should have an idea of what real case(s) were trying to solve.

Agreed.

Thanks
Dave

[RFC 0/3] extend kexec_file_load system call

[Dave Young]

On 07/14/16 at 02:38am, AKASHI Takahiro wrote:
> Apologies for the slow response. I'm attending LinuxCon this week.
> 
> On Wed, Jul 13, 2016 at 10:34:47AM +0100, Mark Rutland wrote:
> > On Wed, Jul 13, 2016 at 10:36:14AM +0800, Dave Young wrote:
> > > But consider we can kexec to a different kernel and a different initrd so there
> > > will be use cases to pass a total different dtb as well.
> > 
> > It depends on what you mean by "a different kernel", and what this
> > implies for the DTB.
> > 
> > I expect future arm64 Linux kernels to function with today's DTBs, and
> > the existing boot protocol. The kexec_file_load syscall already has
> > enough information for the kernel to inject the initrd and bootargs
> > properties into a DTB.
> > 
> > In practice on x86 today, kexec_file_load only supports booting to a
> > Linux kernel, because the in-kernel purgatory only implements the x86
> > Linux boot protocol. Analagously, for arm64 I think that the first
> > kernel should use its internal copy of the boot DTB, with /chosen fixed
> > up appropriately, assuming the next kernel is an arm64 Linux image.
> > 
> > If booting another OS, the only parts of the DTB I would expect to
> > change are the properties under chosen, as everything else *should* be
> > OS-independent. However the other OS may have a completely different
> > boot protocol, might not even take a DTB, and will likely need a
> > compeltely different purgatory implementation. So just allowing the DTB
> > to be altered isn't sufficient for that case.
> > 
> > There might be cases where we want a different DTB, but as far as I can
> > tell we have nothing analagous on x86 today. If we do need this, we
> > should have an idea of what real case(s) were trying to solve.
> 
> What I had in my mind was:
> 
> - Kdump
>   As Russel said, we definitely need to modify dtb.
>   In addition to bootargs and initrd proerties (FYI, in my arm64
>   implementation for arm64, eflcorehdr info is also passed as DT
>   property), we may want to remove unnecessary devices and
>   even add a dedicated storage device for storing a core dump image.
> - Say, booting BE kernel on ACPI LE kernel
>   In this case, there is no useful dtb in the kernel.
> 
> Have said that, as Mark said, we may be able to use normal kexec_load
> system call if we don't need a "secure" kexec.
> 
> BTW, why doesn't the current kexec_load have ability of verifying
> a signature of initramfs image? Is IMA/EVM expected to be used
> at runtime?

I believe there are some limitations for verify signatures in kexec_load.
First kexec-tools need to be trusted, but there's no way to sign and
verify signature of shared libraries. There maybe other limitations I
can not remember which are also reasons why Vivek moved to current
file based syscall.

Thanks
Dave

[RFC 0/3] extend kexec_file_load system call

[Thiago Jung Bauermann]

Am Mittwoch, 13 Juli 2016, 21:59:18 schrieb Arnd Bergmann:
> On Wednesday, July 13, 2016 3:45:41 PM CEST Thiago Jung Bauermann wrote:
> > Am Mittwoch, 13 Juli 2016, 15:13:42 schrieb Arnd Bergmann:
> > > On Wednesday, July 13, 2016 10:41:28 AM CEST Mark Rutland wrote:
> > > > On Wed, Jul 13, 2016 at 10:01:33AM +0200, Arnd Bergmann wrote:
> > > > > - kboot/petitboot with all of the user space being part of the
> > > > > trusted
> > > > > boot> >
> > > > > 
> > > > >   chain: it would be good to allow these to modify the dtb as
> > > > >   needed
> > > > >   without breaking the trust chain, just like we allow grub or
> > > > >   u-boot
> > > > >   to modify the dtb before passing it to the kernel.
> > > > 
> > > > It depends on *what* we need to modify here. We can modify the
> > > > bootargs
> > > > and initrd properties as part of the kexec_file_load syscall, so
> > > > what
> > > > else would we want to alter?
> > > 
> > > I guess petitboot can also just use kexec_load() instead of
> > > kexec_file_load(), as long as the initramfs containing petitboot is
> > > trusted by the kernel.
> > 
> > For secure boot, Petitboot needs to use kexec_file_load, because of the
> > following two features which the system call enables:
> > 
> > 1. only allow loading of signed kernels.
> > 2. "measure" (i.e., record the hashes of) the kernel, initrd, kernel
> > 
> >    command line and other boot inputs for the Integrity Measurement
> >    Architecture subsystem.
> > 
> > Those can't be done with kexec_load.
> 
> Can't petitboot do both of these in user space?

To be honest I'm not sure if it *can't* be done from userspace but if you do 
it from the kernel you can guarantee that any kernel image that is loaded 
gets verified and measured.

Whereas if you verify and measure the kernel in userspace then if there's a 
vulnerability in the system which allows an attacker to upload their own 
binary, then they can use kexec_load directly and bypass the verification 
and measurement.

So it's a more resilient design.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

[RFC 0/3] extend kexec_file_load system call

[Arnd Bergmann]

On Wednesday, July 13, 2016 11:18:04 PM CEST Thiago Jung Bauermann wrote:
> Am Mittwoch, 13 Juli 2016, 21:59:18 schrieb Arnd Bergmann:
> > On Wednesday, July 13, 2016 3:45:41 PM CEST Thiago Jung Bauermann wrote:
> > > Am Mittwoch, 13 Juli 2016, 15:13:42 schrieb Arnd Bergmann:
> > > 
> > > For secure boot, Petitboot needs to use kexec_file_load, because of the
> > > following two features which the system call enables:
> > > 
> > > 1. only allow loading of signed kernels.
> > > 2. "measure" (i.e., record the hashes of) the kernel, initrd, kernel
> > > 
> > >    command line and other boot inputs for the Integrity Measurement
> > >    Architecture subsystem.
> > > 
> > > Those can't be done with kexec_load.
> > 
> > Can't petitboot do both of these in user space?
> 
> To be honest I'm not sure if it *can't* be done from userspace but if you do 
> it from the kernel you can guarantee that any kernel image that is loaded 
> gets verified and measured.
> 
> Whereas if you verify and measure the kernel in userspace then if there's a 
> vulnerability in the system which allows an attacker to upload their own 
> binary, then they can use kexec_load directly and bypass the verification 
> and measurement.
> 
> So it's a more resilient design.

Right, but the question remains whether this helps while you allow the
boot loader to modify the dtb. If an attacker gets in and cannot modify
the kernel or initid but can modify the DT, a successful attack would
be a bit harder than having a modified kernel, but you may still need
to treat the system as compromised.

	Arnd

[RFC 0/3] extend kexec_file_load system call

[Mark Rutland]

On Wed, Jul 13, 2016 at 09:57:28PM +0200, Arnd Bergmann wrote:
> On Wednesday, July 13, 2016 6:58:32 PM CEST Mark Rutland wrote:
> > 
> > >   we may want to remove unnecessary devices and even add a dedicated
> > >   storage device for storing a core dump image.
> > 
> > I suspect that bringing up a minimal number of devices is better
> > controlled by a cmdline option. In general, figuring out what is
> > necessary and what is not is going to be board specific, so hacking the
> > FW tables (DTB or ACPI) is not a very portable/reliable approach.
> > 
> > Do we actually add devices in practice? More so than the above that
> > requires special knowledge of the platform (including things that were
> > not described in the boot DTB).
> > 
> > In the ACPI case modifying a DTB alone is not sufficient to change the
> > information regarding devices, as those won't be described in the DTB.
> > It's not possible to convert ACPI to DTB in general.
> 
> A more likely scenario would be replacing ACPI tables with a DTB that
> describes the platform in order to use devices that the ACPI tables
> don't contain.

To do so, you need special knowledge of the platform, which users are
unlikely to have in practice for ACPI-based platforms.

So, I think that boils down to the same problem, and the same comments
apply.

> > > - Say, booting BE kernel on ACPI LE kernel
> > >   In this case, there is no useful dtb in the kernel.
> 
> > If the platform only has ACPI, then you cannot boot a BE kernel to begin
> > with. As above one cannot convert ACPI to DTB, so one would need
> > extensive platform knowledge for this to work.
> 
> I think what he meant was to pass a DTB to the kexec kernel in order
> to run BE, while the original kernel can only run LE due to ACPI.

I understood that. My point was that to build that DTB, you need to have
knowledge of the platform that you are unlikely to have.

The platform firmware may expect to interact with AML, which you can't
place in DT or run in a BE context.

If you need to run BE kernels on a platform, you would likely get a
platform that could boot a BE kernel from the outset (i.e. one that
provides a DTB), or run that code ina VM under an LE host.

Thanks,
Mark.

[RFC 0/3] extend kexec_file_load system call

[Thiago Jung Bauermann]

Am Donnerstag, 14 Juli 2016, 10:29:11 schrieb Arnd Bergmann:
> On Wednesday, July 13, 2016 11:18:04 PM CEST Thiago Jung Bauermann wrote:
> > Am Mittwoch, 13 Juli 2016, 21:59:18 schrieb Arnd Bergmann:
> > > On Wednesday, July 13, 2016 3:45:41 PM CEST Thiago Jung Bauermann 
wrote:
> > > > Am Mittwoch, 13 Juli 2016, 15:13:42 schrieb Arnd Bergmann:
> > > > 
> > > > For secure boot, Petitboot needs to use kexec_file_load, because of
> > > > the
> > > > following two features which the system call enables:
> > > > 
> > > > 1. only allow loading of signed kernels.
> > > > 2. "measure" (i.e., record the hashes of) the kernel, initrd, kernel
> > > > 
> > > >    command line and other boot inputs for the Integrity Measurement
> > > >    Architecture subsystem.
> > > > 
> > > > Those can't be done with kexec_load.
> > > 
> > > Can't petitboot do both of these in user space?
> > 
> > To be honest I'm not sure if it *can't* be done from userspace but if
> > you do it from the kernel you can guarantee that any kernel image that
> > is loaded gets verified and measured.
> > 
> > Whereas if you verify and measure the kernel in userspace then if
> > there's a vulnerability in the system which allows an attacker to
> > upload their own binary, then they can use kexec_load directly and
> > bypass the verification and measurement.
> > 
> > So it's a more resilient design.
> 
> Right, but the question remains whether this helps while you allow the
> boot loader to modify the dtb. If an attacker gets in and cannot modify
> the kernel or initid but can modify the DT, a successful attack would
> be a bit harder than having a modified kernel, but you may still need
> to treat the system as compromised.

Yes, and the same question also remains regarding the kernel command line.

We can have the kernel perform sanity checks on the device tree, just as the 
kernel needs to sanity check the command line.

There's the point that was raised about not wanting to increase the attack 
surface, and that's a valid point. But at least in the way Petitboot works 
today, it needs to modify the device tree and pass it to the kernel.

One thing that is unavoidable to come from userspace is 
/chosen/linux,stdout-path, because it's Petitboot that knows from which 
console the user is interacting with. The other modification to set 
properties in vga at 0 can be done in the kernel.

Given that on DTB-based systems /chosen is an important and established way 
to pass information to the operating system being booted, I'd like to 
suggest the following, then:

Extend the syscall as shown in this RFC from Takahiro AKASHI, but instead of 
accepting a complete DTB from userspace, the syscall would accept a DTB 
containing only a /chosen node. If the DTB contains any other node, the 
syscall fails with EINVAL. The kernel can then add the properties in /chosen 
to the device tree that it will pass to the next kernel.

What do you think?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

[RFC 0/3] extend kexec_file_load system call

[Arnd Bergmann]

On Thursday, July 14, 2016 10:44:14 PM CEST Thiago Jung Bauermann wrote:
> Am Donnerstag, 14 Juli 2016, 10:29:11 schrieb Arnd Bergmann:

> > 
> > Right, but the question remains whether this helps while you allow the
> > boot loader to modify the dtb. If an attacker gets in and cannot modify
> > the kernel or initid but can modify the DT, a successful attack would
> > be a bit harder than having a modified kernel, but you may still need
> > to treat the system as compromised.
> 
> Yes, and the same question also remains regarding the kernel command line.
> 
> We can have the kernel perform sanity checks on the device tree, just as the 
> kernel needs to sanity check the command line.
> 
> There's the point that was raised about not wanting to increase the attack 
> surface, and that's a valid point. But at least in the way Petitboot works 
> today, it needs to modify the device tree and pass it to the kernel.
> 
> One thing that is unavoidable to come from userspace is 
> /chosen/linux,stdout-path, because it's Petitboot that knows from which 
> console the user is interacting with. The other modification to set 
> properties in vga at 0 can be done in the kernel.
> 
> Given that on DTB-based systems /chosen is an important and established way 
> to pass information to the operating system being booted, I'd like to 
> suggest the following, then:
> 
> Extend the syscall as shown in this RFC from Takahiro AKASHI, but instead of 
> accepting a complete DTB from userspace, the syscall would accept a DTB 
> containing only a /chosen node. If the DTB contains any other node, the 
> syscall fails with EINVAL. The kernel can then add the properties in /chosen 
> to the device tree that it will pass to the next kernel.
> 
> What do you think?

I think that helps, as it makes the problem space correspond to that
of modifying the command line, but I can still come up with countless
attacks based on modifications of the /chosen node and/or the command
line, in fact it's probably easier than any other node.

What methods to we have in place for command line changes today on
other architectures?

	Arnd

[RFC 0/3] extend kexec_file_load system call

[Russell King - ARM Linux]

On Wed, Jul 13, 2016 at 03:13:42PM +0200, Arnd Bergmann wrote:
> On Wednesday, July 13, 2016 10:41:28 AM CEST Mark Rutland wrote:
> > The big question is whether this is a realistic case on a secure boot
> > system.
> 
> What does x86 do here? I assume changes to the command line are also
> limited.

They aren't.  You can specify /anything/ even with a fully-signed kernel
and initrd, which was one of the things I pointed out in my previous
set of responses.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Fri, Jul 15, 2016 at 09:49:25AM +0100, Russell King - ARM Linux wrote:
> On Wed, Jul 13, 2016 at 03:13:42PM +0200, Arnd Bergmann wrote:
> > On Wednesday, July 13, 2016 10:41:28 AM CEST Mark Rutland wrote:
> > > The big question is whether this is a realistic case on a secure boot
> > > system.
> > 
> > What does x86 do here? I assume changes to the command line are also
> > limited.
> 
> They aren't.  You can specify /anything/ even with a fully-signed kernel
> and initrd, which was one of the things I pointed out in my previous
> set of responses.

Yes, kernel command line is not signed. For that matter even initird is
not signed. Just kernel is signed and its signatures are verified. Idea
is an unsigned code should not be able to execute in kernel space.

Vivek

[RFC 3/3] kexec: extend kexec_file_load system call

[Vivek Goyal]

On Tue, Jul 12, 2016 at 10:42:01AM +0900, AKASHI Takahiro wrote:

[..]
> -SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> +SYSCALL_DEFINE6(kexec_file_load, int, kernel_fd, int, initrd_fd,
>  		unsigned long, cmdline_len, const char __user *, cmdline_ptr,
> -		unsigned long, flags)
> +		unsigned long, flags, const struct kexec_fdset __user *, ufdset)

Can one add more parameters to existing syscall. Can it break existing
programs with new kernel? I was of the impression that one can't do that.
But may be I am missing something.

Vivek

[RFC 3/3] kexec: extend kexec_file_load system call

[Mark Rutland]

On Fri, Jul 15, 2016 at 09:09:55AM -0400, Vivek Goyal wrote:
> On Tue, Jul 12, 2016 at 10:42:01AM +0900, AKASHI Takahiro wrote:
> 
> [..]
> > -SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > +SYSCALL_DEFINE6(kexec_file_load, int, kernel_fd, int, initrd_fd,
> >  		unsigned long, cmdline_len, const char __user *, cmdline_ptr,
> > -		unsigned long, flags)
> > +		unsigned long, flags, const struct kexec_fdset __user *, ufdset)
> 
> Can one add more parameters to existing syscall. Can it break existing
> programs with new kernel? I was of the impression that one can't do that.
> But may be I am missing something.

I think the idea was that we would only look at the new params if a new
flags was set, and otherwise it would behave as the old syscall.

Regardless, I think it makes far more sense to add a kexec_file_load2
syscall if we're going to modify the prototype at all. It's a rather
different proposition to the existing syscall, and needs to be treated
as such.

Thanks,
Mark.

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Fri, Jul 15, 2016 at 09:31:02AM +0200, Arnd Bergmann wrote:
> On Thursday, July 14, 2016 10:44:14 PM CEST Thiago Jung Bauermann wrote:
> > Am Donnerstag, 14 Juli 2016, 10:29:11 schrieb Arnd Bergmann:
> 
> > > 
> > > Right, but the question remains whether this helps while you allow the
> > > boot loader to modify the dtb. If an attacker gets in and cannot modify
> > > the kernel or initid but can modify the DT, a successful attack would
> > > be a bit harder than having a modified kernel, but you may still need
> > > to treat the system as compromised.
> > 
> > Yes, and the same question also remains regarding the kernel command line.
> > 
> > We can have the kernel perform sanity checks on the device tree, just as the 
> > kernel needs to sanity check the command line.
> > 
> > There's the point that was raised about not wanting to increase the attack 
> > surface, and that's a valid point. But at least in the way Petitboot works 
> > today, it needs to modify the device tree and pass it to the kernel.
> > 
> > One thing that is unavoidable to come from userspace is 
> > /chosen/linux,stdout-path, because it's Petitboot that knows from which 
> > console the user is interacting with. The other modification to set 
> > properties in vga at 0 can be done in the kernel.
> > 
> > Given that on DTB-based systems /chosen is an important and established way 
> > to pass information to the operating system being booted, I'd like to 
> > suggest the following, then:
> > 
> > Extend the syscall as shown in this RFC from Takahiro AKASHI, but instead of 
> > accepting a complete DTB from userspace, the syscall would accept a DTB 
> > containing only a /chosen node. If the DTB contains any other node, the 
> > syscall fails with EINVAL. The kernel can then add the properties in /chosen 
> > to the device tree that it will pass to the next kernel.
> > 
> > What do you think?
> 
> I think that helps, as it makes the problem space correspond to that
> of modifying the command line, but I can still come up with countless
> attacks based on modifications of the /chosen node and/or the command
> line, in fact it's probably easier than any other node.

I don't know anything about DTB. So here comes a very basic question. Does
DTB allow passing an executable blob to kernel or pass the location of
some unsigned executable code at kernel level. I think from secureboot point of
view that would be a concern. Being able to trick kernel to execute an
unsigned code at privileged level.

Vivek

[RFC 0/3] extend kexec_file_load system call

[Mark Rutland]

On Fri, Jul 15, 2016 at 09:26:10AM -0400, Vivek Goyal wrote:
> On Fri, Jul 15, 2016 at 09:31:02AM +0200, Arnd Bergmann wrote:
> > On Thursday, July 14, 2016 10:44:14 PM CEST Thiago Jung Bauermann wrote:
> > > Am Donnerstag, 14 Juli 2016, 10:29:11 schrieb Arnd Bergmann:
> > 
> > > > 
> > > > Right, but the question remains whether this helps while you allow the
> > > > boot loader to modify the dtb. If an attacker gets in and cannot modify
> > > > the kernel or initid but can modify the DT, a successful attack would
> > > > be a bit harder than having a modified kernel, but you may still need
> > > > to treat the system as compromised.
> > > 
> > > Yes, and the same question also remains regarding the kernel command line.
> > > 
> > > We can have the kernel perform sanity checks on the device tree, just as the 
> > > kernel needs to sanity check the command line.
> > > 
> > > There's the point that was raised about not wanting to increase the attack 
> > > surface, and that's a valid point. But at least in the way Petitboot works 
> > > today, it needs to modify the device tree and pass it to the kernel.
> > > 
> > > One thing that is unavoidable to come from userspace is 
> > > /chosen/linux,stdout-path, because it's Petitboot that knows from which 
> > > console the user is interacting with. The other modification to set 
> > > properties in vga at 0 can be done in the kernel.
> > > 
> > > Given that on DTB-based systems /chosen is an important and established way 
> > > to pass information to the operating system being booted, I'd like to 
> > > suggest the following, then:
> > > 
> > > Extend the syscall as shown in this RFC from Takahiro AKASHI, but instead of 
> > > accepting a complete DTB from userspace, the syscall would accept a DTB 
> > > containing only a /chosen node. If the DTB contains any other node, the 
> > > syscall fails with EINVAL. The kernel can then add the properties in /chosen 
> > > to the device tree that it will pass to the next kernel.
> > > 
> > > What do you think?
> > 
> > I think that helps, as it makes the problem space correspond to that
> > of modifying the command line, but I can still come up with countless
> > attacks based on modifications of the /chosen node and/or the command
> > line, in fact it's probably easier than any other node.
> 
> I don't know anything about DTB. So here comes a very basic question. Does
> DTB allow passing an executable blob to kernel or pass the location of
> some unsigned executable code at kernel level. I think from secureboot point of
> view that would be a concern. Being able to trick kernel to execute an
> unsigned code at privileged level.

The DTB itself won't contain executable code.

However, arbitrary bindings could point kernel at such code. For
instance, /chosen/linux,uefi-system-table could point the kernel at a
faked EFI system table, with pointers to malicious code. So
arbitrary modification of /chosen is not safe.

Bindings describe arbitrary system features (devices, firmware
interfaces, etc), so in general they might provide mechanisms to execute
code.

Thanks,
Mark.

[RFC 0/3] extend kexec_file_load system call

[Russell King - ARM Linux]

On Fri, Jul 15, 2016 at 09:26:10AM -0400, Vivek Goyal wrote:
> On Fri, Jul 15, 2016 at 09:31:02AM +0200, Arnd Bergmann wrote:
> > I think that helps, as it makes the problem space correspond to that
> > of modifying the command line, but I can still come up with countless
> > attacks based on modifications of the /chosen node and/or the command
> > line, in fact it's probably easier than any other node.
> 
> I don't know anything about DTB. So here comes a very basic question. Does
> DTB allow passing an executable blob to kernel or pass the location of
> some unsigned executable code at kernel level.

DT on ARM is a description of the hardware - it can be thought of as a
set of nodes with properties attached.  The properties can describe
anything (we have documentation in Documentation/devicetree/bindings
which describes what we expect the properties to contain.)

On other architectures, DT can also contain open-firmware "functions"
but I don't think there's much support in the kernel for that - maybe
the PPC folk can reply on that point.

It is possible that someone may, at some point, decide to create a
property that points to some executable blob, but I can't think of a
reason why we should ever allow such a monstrosity in mainline kernels.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[RFC 0/3] extend kexec_file_load system call

[Thiago Jung Bauermann]

Am Freitag, 15 Juli 2016, 14:33:47 schrieb Mark Rutland:
> On Fri, Jul 15, 2016 at 09:26:10AM -0400, Vivek Goyal wrote:
> > On Fri, Jul 15, 2016 at 09:31:02AM +0200, Arnd Bergmann wrote:
> > > On Thursday, July 14, 2016 10:44:14 PM CEST Thiago Jung Bauermann 
wrote:
> > > > Am Donnerstag, 14 Juli 2016, 10:29:11 schrieb Arnd Bergmann:
> > > > > Right, but the question remains whether this helps while you allow
> > > > > the
> > > > > boot loader to modify the dtb. If an attacker gets in and cannot
> > > > > modify
> > > > > the kernel or initid but can modify the DT, a successful attack
> > > > > would
> > > > > be a bit harder than having a modified kernel, but you may still
> > > > > need
> > > > > to treat the system as compromised.
> > > > 
> > > > Yes, and the same question also remains regarding the kernel command
> > > > line.
> > > > 
> > > > We can have the kernel perform sanity checks on the device tree,
> > > > just as the kernel needs to sanity check the command line.
> > > > 
> > > > There's the point that was raised about not wanting to increase the
> > > > attack surface, and that's a valid point. But at least in the way
> > > > Petitboot works today, it needs to modify the device tree and pass
> > > > it to the kernel.
> > > > 
> > > > One thing that is unavoidable to come from userspace is
> > > > /chosen/linux,stdout-path, because it's Petitboot that knows from
> > > > which
> > > > console the user is interacting with. The other modification to set
> > > > properties in vga at 0 can be done in the kernel.
> > > > 
> > > > Given that on DTB-based systems /chosen is an important and
> > > > established way to pass information to the operating system being
> > > > booted, I'd like to suggest the following, then:
> > > > 
> > > > Extend the syscall as shown in this RFC from Takahiro AKASHI, but
> > > > instead of accepting a complete DTB from userspace, the syscall
> > > > would accept a DTB containing only a /chosen node. If the DTB
> > > > contains any other node, the syscall fails with EINVAL. The kernel
> > > > can then add the properties in /chosen to the device tree that it
> > > > will pass to the next kernel.
> > > > 
> > > > What do you think?
> > > 
> > > I think that helps, as it makes the problem space correspond to that
> > > of modifying the command line, but I can still come up with countless
> > > attacks based on modifications of the /chosen node and/or the command
> > > line, in fact it's probably easier than any other node.
> > 
> > I don't know anything about DTB. So here comes a very basic question.
> > Does DTB allow passing an executable blob to kernel or pass the
> > location of some unsigned executable code at kernel level. I think from
> > secureboot point of view that would be a concern. Being able to trick
> > kernel to execute an unsigned code at privileged level.
> 
> The DTB itself won't contain executable code.
> 
> However, arbitrary bindings could point kernel at such code. For
> instance, /chosen/linux,uefi-system-table could point the kernel at a
> faked EFI system table, with pointers to malicious code. So
> arbitrary modification of /chosen is not safe.

PowerPC doesn't have UEFI so this option is not a concern in that 
architecture. I'm having a look at what a PowerPC kernel gets from /chosen 
and haven't found anything of concern so far, but I'm still looking.

On the other hand, the kernel command line has the option acpi_rsdp, which 
is used to pass the address of the RSDP. I don't really know much about EFI 
so I'm not sure if it can be used to point to code that the kernel can 
execute, but it does point to tables that contain AML code.

> Bindings describe arbitrary system features (devices, firmware
> interfaces, etc), so in general they might provide mechanisms to execute
> code.

Even bindings in /chosen?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

[RFC 0/3] extend kexec_file_load system call

[Mark Rutland]

On Fri, Jul 15, 2016 at 12:29:09PM -0300, Thiago Jung Bauermann wrote:
> Am Freitag, 15 Juli 2016, 14:33:47 schrieb Mark Rutland:
> > On Fri, Jul 15, 2016 at 09:26:10AM -0400, Vivek Goyal wrote:
> > > I don't know anything about DTB. So here comes a very basic question.
> > > Does DTB allow passing an executable blob to kernel or pass the
> > > location of some unsigned executable code at kernel level. I think from
> > > secureboot point of view that would be a concern. Being able to trick
> > > kernel to execute an unsigned code at privileged level.
> > 
> > The DTB itself won't contain executable code.
> > 
> > However, arbitrary bindings could point kernel at such code. For
> > instance, /chosen/linux,uefi-system-table could point the kernel at a
> > faked EFI system table, with pointers to malicious code. So
> > arbitrary modification of /chosen is not safe.
> 
> PowerPC doesn't have UEFI so this option is not a concern in that 
> architecture. I'm having a look at what a PowerPC kernel gets from /chosen 
> and haven't found anything of concern so far, but I'm still looking.
> 
> On the other hand, the kernel command line has the option acpi_rsdp, which 
> is used to pass the address of the RSDP. I don't really know much about EFI 
> so I'm not sure if it can be used to point to code that the kernel can 
> execute, but it does point to tables that contain AML code.

Please let's not conflate EFI and ACPI, the two are distinct.

I believe that there aren't any ACPI tables which contain native code,
or which contain pointers to native code, but I could be mistaken. It
doesn't seem unlikely that malicious AML is possible, but I'm not
familiar enough with AML to know how we sandbox that.

>From a scan of Documentation/kernel-parameters.txt, it doesn't look like
there are options to override the EFI system table (or related tables),
so it doesn't look like there's a trivial mechanism to trigger arbitrary
code execution. It looks like efi_fake_mem could be used to trick the
kernel to poke things it shouldn't, though that likely brings the system
down entirely.

> > Bindings describe arbitrary system features (devices, firmware
> > interfaces, etc), so in general they might provide mechanisms to execute
> > code.
> 
> Even bindings in /chosen?

Yes, even bindings in /chosen. As above, the linux,uefi-system-table
property lives under /chosen, and provides pointers to native code.
Control over this property could yield arbitrary code execution.

Additionally, there are drivers that just go looking for a compatible
string, and will probe regardless of where the node is in the hierarchy.
e.g. clock controller drivers, memory nodes. So /chosen isn't sandboxed
as such. 

I fear that there are many things that one could place under /chosen
that could make the kernel do the wrong thing. Given the example of
drivers, I'm not sure it's going to be possible to audit all the
relevant code.

Thanks,
Mark.

[RFC 0/3] extend kexec_file_load system call

[Arnd Bergmann]

On Friday, July 15, 2016 2:42:10 PM CEST Russell King - ARM Linux wrote:
> 
> On other architectures, DT can also contain open-firmware "functions"
> but I don't think there's much support in the kernel for that - maybe
> the PPC folk can reply on that point.

The open firmware runtime interface are shut down by the time we have
a flattened device tree, so those are not accessible any more. IIRC
SPARC leaves the open firmware interface live, but it doesn't use
fdt, so that's not relevant here.

However, the powerpc specific RTAS runtime services provide a similar
interface to the UEFI runtime support and allow to call into
binary code from the kernel, which gets mapped from a physical
address in the "linux,rtas-base" property in the rtas device node.

Modifying the /rtas node will definitely give you a backdoor into
priviledged code, but modifying only /chosen should not let you get
in through that specific method.

	Arnd

[RFC 0/3] extend kexec_file_load system call

[Thiago Jung Bauermann]

Am Freitag, 15 Juli 2016, 22:26:09 schrieb Arnd Bergmann:
> On Friday, July 15, 2016 2:42:10 PM CEST Russell King - ARM Linux wrote:
> > On other architectures, DT can also contain open-firmware "functions"
> > but I don't think there's much support in the kernel for that - maybe
> > the PPC folk can reply on that point.
> 
> The open firmware runtime interface are shut down by the time we have
> a flattened device tree, so those are not accessible any more. IIRC
> SPARC leaves the open firmware interface live, but it doesn't use
> fdt, so that's not relevant here.
> 
> However, the powerpc specific RTAS runtime services provide a similar
> interface to the UEFI runtime support and allow to call into
> binary code from the kernel, which gets mapped from a physical
> address in the "linux,rtas-base" property in the rtas device node.
> 
> Modifying the /rtas node will definitely give you a backdoor into
> priviledged code, but modifying only /chosen should not let you get
> in through that specific method.

Except that arch/powerpc/kernel/rtas.c looks for any node in the tree called 
"rtas", so it will try to use /chosen/rtas, or /chosen/foo/rtas.

We can forbid subnodes in /chosen in the dtb passed to kexec_file_load, 
though that means userspace can't use the simple-framebuffer binding via 
this mechanism.

We also have to blacklist the device_type and compatible properties in 
/chosen to avoid the problem Mark mentioned.

Still doable, but not ideal. :-/

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

[RFC 3/3] kexec: extend kexec_file_load system call

[Dave Young]

On 07/15/16 at 02:19pm, Mark Rutland wrote:
> On Fri, Jul 15, 2016 at 09:09:55AM -0400, Vivek Goyal wrote:
> > On Tue, Jul 12, 2016 at 10:42:01AM +0900, AKASHI Takahiro wrote:
> > 
> > [..]
> > > -SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > > +SYSCALL_DEFINE6(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > >  		unsigned long, cmdline_len, const char __user *, cmdline_ptr,
> > > -		unsigned long, flags)
> > > +		unsigned long, flags, const struct kexec_fdset __user *, ufdset)
> > 
> > Can one add more parameters to existing syscall. Can it break existing
> > programs with new kernel? I was of the impression that one can't do that.
> > But may be I am missing something.
> 
> I think the idea was that we would only look at the new params if a new
> flags was set, and otherwise it would behave as the old syscall.
> 
> Regardless, I think it makes far more sense to add a kexec_file_load2
> syscall if we're going to modify the prototype at all. It's a rather
> different proposition to the existing syscall, and needs to be treated
> as such.

I do not think it is worth to add another syscall for extra fds.
We have open(2) as an example for different numbers of arguments
already.

Thanks
Dave

[RFC 3/3] kexec: extend kexec_file_load system call

[Dave Young]

On 07/15/16 at 09:09am, Vivek Goyal wrote:
> On Tue, Jul 12, 2016 at 10:42:01AM +0900, AKASHI Takahiro wrote:
> 
> [..]
> > -SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > +SYSCALL_DEFINE6(kexec_file_load, int, kernel_fd, int, initrd_fd,
> >  		unsigned long, cmdline_len, const char __user *, cmdline_ptr,
> > -		unsigned long, flags)
> > +		unsigned long, flags, const struct kexec_fdset __user *, ufdset)
> 
> Can one add more parameters to existing syscall. Can it break existing
> programs with new kernel? I was of the impression that one can't do that.
> But may be I am missing something.

It will not break existing programs because we can use the new param only
when the new flag is set.

But we have a case below, but I think it is fine?
Originally kexec_file_load with the new flags will fail, but now it will
succeed and will access the new argument.

Thanks
Dave

[RFC 3/3] kexec: extend kexec_file_load system call

[Mark Rutland]

On Mon, Jul 18, 2016 at 10:30:24AM +0800, Dave Young wrote:
> On 07/15/16 at 02:19pm, Mark Rutland wrote:
> > On Fri, Jul 15, 2016 at 09:09:55AM -0400, Vivek Goyal wrote:
> > > On Tue, Jul 12, 2016 at 10:42:01AM +0900, AKASHI Takahiro wrote:
> > > 
> > > [..]
> > > > -SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > > > +SYSCALL_DEFINE6(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > > >  		unsigned long, cmdline_len, const char __user *, cmdline_ptr,
> > > > -		unsigned long, flags)
> > > > +		unsigned long, flags, const struct kexec_fdset __user *, ufdset)
> > > 
> > > Can one add more parameters to existing syscall. Can it break existing
> > > programs with new kernel? I was of the impression that one can't do that.
> > > But may be I am missing something.
> > 
> > I think the idea was that we would only look at the new params if a new
> > flags was set, and otherwise it would behave as the old syscall.
> > 
> > Regardless, I think it makes far more sense to add a kexec_file_load2
> > syscall if we're going to modify the prototype at all. It's a rather
> > different proposition to the existing syscall, and needs to be treated
> > as such.
> 
> I do not think it is worth to add another syscall for extra fds.
> We have open(2) as an example for different numbers of arguments
> already.

Did we change the syscall interface for that?

I was under the impression that there was always one underlying syscall,
and the C library did the right thing to pass the expected information
to the underlying syscall.

That's rather different to changing the underlying syscall.

Regardless of how this is wrapped in userspace, I do not think modifying
the existing prototype is a good idea, and I think this kind of
extension needs to be a new syscall.

Thanks,
Mark.

[RFC 0/3] extend kexec_file_load system call

[Balbir Singh]

On Wed, 2016-07-13 at 14:22 -0400, Vivek Goyal wrote:
> On Wed, Jul 13, 2016 at 06:40:10PM +0100, Russell King - ARM Linux wrote:
> >?
> > On Wed, Jul 13, 2016 at 09:03:38AM -0400, Vivek Goyal wrote:
> > >?
> > > On Wed, Jul 13, 2016 at 09:26:39AM +0100, Russell King - ARM Linux wrote:
> > > >?
> > > > Indeed - maybe Eric knows better, but I can't see any situation where
> > > > the dtb we load via kexec should ever affect "the bootloader", unless
> > > > the "kernel" that's being loaded into kexec is "the bootloader".
> > > >?
> > > > Now, going back to the more fundamental issue raised in my first reply,
> > > > about the kernel command line.
> > > >?
> > > > On x86, I can see that it _is_ possible for userspace to specify a
> > > > command line, and the kernel loading the image provides the command
> > > > line to the to-be-kexeced kernel with very little checking.??So, if
> > > > your kernel is signed, what stops the "insecure userspace" loading
> > > > a signed kernel but giving it an insecure rootfs and/or console?
> > > It is not kexec specific. I could do this for regular boot too, right?
> > >?
> > > Command line options are not signed. I thought idea behind secureboot
> > > was to execute only trusted code and command line options don't enforce
> > > you to execute unsigned code.
> > >?

You can set module.sig_enforce=0 and open up the system a bit assuming
that you can get a module to load with another attack

> > > So it sounds like different class of security problems which you are
> > > referring to and not necessarily covered by secureboot or signed
> > > kernel.
> > Let me give you an example.
> >?
> > You have a secure boot setup, where the firmware/ROM validates the boot
> > loader.??Good, the boot loader hasn't been tampered with.
> >?
> > You interrupt the boot loader and are able to modify the command line
> > for the booted kernel.
> >?
> > The boot loader loads the kernel and verifies the kernel's signature.
> > Good, the kernel hasn't been tampered with.??The kernel starts running.
> >?
> > You've plugged in a USB drive to the device, and specified a partition
> > containing a root filesystem that you control to the kernel.??The
> > validated kernel finds the USB drive, and mounts it, and executes
> > your own binaries on the USB drive.
> You will require physical access to the machine to be able to
> insert your usb drive. And IIRC, argument was that if attacker has
> physical access to machine, all bets are off anyway.
>

You don't need physical access -- your machine controller BMC can
do the magic for you. So its not always physical access, is it?
?
> >?
> >?
> > You run a shell on the console.??You now have control of the system,
> > and can mount the real rootfs, inspect it, and work out what it does,
> > etc.
> >?
> > At this point, what use was all the validation that the secure boot
> > has done???Absolutely useless.
> >?
> > If you can change the command line arguments given to the kernel, you
> > have no security, no matter how much you verify signatures.??It's
> > the illusion of security, nothing more, nothing less.
> >?

I agree, if you can change command line arguments, all bets are of lesser value

Balbir Singh

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Mon, Jul 18, 2016 at 10:46:04PM +1000, Balbir Singh wrote:
> On Wed, 2016-07-13 at 14:22 -0400, Vivek Goyal wrote:
> > On Wed, Jul 13, 2016 at 06:40:10PM +0100, Russell King - ARM Linux wrote:
> > >?
> > > On Wed, Jul 13, 2016 at 09:03:38AM -0400, Vivek Goyal wrote:
> > > >?
> > > > On Wed, Jul 13, 2016 at 09:26:39AM +0100, Russell King - ARM Linux wrote:
> > > > >?
> > > > > Indeed - maybe Eric knows better, but I can't see any situation where
> > > > > the dtb we load via kexec should ever affect "the bootloader", unless
> > > > > the "kernel" that's being loaded into kexec is "the bootloader".
> > > > >?
> > > > > Now, going back to the more fundamental issue raised in my first reply,
> > > > > about the kernel command line.
> > > > >?
> > > > > On x86, I can see that it _is_ possible for userspace to specify a
> > > > > command line, and the kernel loading the image provides the command
> > > > > line to the to-be-kexeced kernel with very little checking.??So, if
> > > > > your kernel is signed, what stops the "insecure userspace" loading
> > > > > a signed kernel but giving it an insecure rootfs and/or console?
> > > > It is not kexec specific. I could do this for regular boot too, right?
> > > >?
> > > > Command line options are not signed. I thought idea behind secureboot
> > > > was to execute only trusted code and command line options don't enforce
> > > > you to execute unsigned code.
> > > >?
> 
> You can set module.sig_enforce=0 and open up the system a bit assuming
> that you can get a module to load with another attack

IIUC, sig_enforce bool_enable_only so it can only be enabled. Default
value of it is 0 if CONFIG_MODULE_SIG_FORCE=n.

IOW, if your kernel forced signature verification, you should not be
able to do sig_enforce=0. If you kernel did not have
CONFIG_MODULE_SIG_FORCE=y, then sig_enforce should be 0 by default anyway
and you are not making it worse using command line.

> 
> > > > So it sounds like different class of security problems which you are
> > > > referring to and not necessarily covered by secureboot or signed
> > > > kernel.
> > > Let me give you an example.
> > >?
> > > You have a secure boot setup, where the firmware/ROM validates the boot
> > > loader.??Good, the boot loader hasn't been tampered with.
> > >?
> > > You interrupt the boot loader and are able to modify the command line
> > > for the booted kernel.
> > >?
> > > The boot loader loads the kernel and verifies the kernel's signature.
> > > Good, the kernel hasn't been tampered with.??The kernel starts running.
> > >?
> > > You've plugged in a USB drive to the device, and specified a partition
> > > containing a root filesystem that you control to the kernel.??The
> > > validated kernel finds the USB drive, and mounts it, and executes
> > > your own binaries on the USB drive.
> > You will require physical access to the machine to be able to
> > insert your usb drive. And IIRC, argument was that if attacker has
> > physical access to machine, all bets are off anyway.
> >
> 
> You don't need physical access -- your machine controller BMC can
> do the magic for you. So its not always physical access, is it?

Well, idea was that if you have physical access to machine, then all
bets are off. If BMC can do something which allows running unsigned
code at ring level 0, its a problem I think from secureboot model of
security.

> ?
> > >?
> > >?
> > > You run a shell on the console.??You now have control of the system,
> > > and can mount the real rootfs, inspect it, and work out what it does,
> > > etc.
> > >?
> > > At this point, what use was all the validation that the secure boot
> > > has done???Absolutely useless.
> > >?
> > > If you can change the command line arguments given to the kernel, you
> > > have no security, no matter how much you verify signatures.??It's
> > > the illusion of security, nothing more, nothing less.
> > >?
> 
> I agree, if you can change command line arguments, all bets are of lesser value

If changing command line allows execution of unsigned code at ring level
0, then it is a problem. Otherwise we are talking of security issues which
are not covered by secureboot model.

Vivek

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Mon, Jul 18, 2016 at 09:26:29AM -0400, Vivek Goyal wrote:
> On Mon, Jul 18, 2016 at 10:46:04PM +1000, Balbir Singh wrote:
> > On Wed, 2016-07-13 at 14:22 -0400, Vivek Goyal wrote:
> > > On Wed, Jul 13, 2016 at 06:40:10PM +0100, Russell King - ARM Linux wrote:
> > > >?
> > > > On Wed, Jul 13, 2016 at 09:03:38AM -0400, Vivek Goyal wrote:
> > > > >?
> > > > > On Wed, Jul 13, 2016 at 09:26:39AM +0100, Russell King - ARM Linux wrote:
> > > > > >?
> > > > > > Indeed - maybe Eric knows better, but I can't see any situation where
> > > > > > the dtb we load via kexec should ever affect "the bootloader", unless
> > > > > > the "kernel" that's being loaded into kexec is "the bootloader".
> > > > > >?
> > > > > > Now, going back to the more fundamental issue raised in my first reply,
> > > > > > about the kernel command line.
> > > > > >?
> > > > > > On x86, I can see that it _is_ possible for userspace to specify a
> > > > > > command line, and the kernel loading the image provides the command
> > > > > > line to the to-be-kexeced kernel with very little checking.??So, if
> > > > > > your kernel is signed, what stops the "insecure userspace" loading
> > > > > > a signed kernel but giving it an insecure rootfs and/or console?
> > > > > It is not kexec specific. I could do this for regular boot too, right?
> > > > >?
> > > > > Command line options are not signed. I thought idea behind secureboot
> > > > > was to execute only trusted code and command line options don't enforce
> > > > > you to execute unsigned code.
> > > > >?
> > 
> > You can set module.sig_enforce=0 and open up the system a bit assuming
> > that you can get a module to load with another attack
> 
> IIUC, sig_enforce bool_enable_only so it can only be enabled. Default
> value of it is 0 if CONFIG_MODULE_SIG_FORCE=n.
> 
> IOW, if your kernel forced signature verification, you should not be
> able to do sig_enforce=0. If you kernel did not have
> CONFIG_MODULE_SIG_FORCE=y, then sig_enforce should be 0 by default anyway
> and you are not making it worse using command line.

[ CC Matthew Garrett ]

I think on top of this there were patches by Matthew Garrett, which
disallowed loading of unsigned modules if booted with secureboot on. I
think those patches never made upstream though.

Vivek

> 
> > 
> > > > > So it sounds like different class of security problems which you are
> > > > > referring to and not necessarily covered by secureboot or signed
> > > > > kernel.
> > > > Let me give you an example.
> > > >?
> > > > You have a secure boot setup, where the firmware/ROM validates the boot
> > > > loader.??Good, the boot loader hasn't been tampered with.
> > > >?
> > > > You interrupt the boot loader and are able to modify the command line
> > > > for the booted kernel.
> > > >?
> > > > The boot loader loads the kernel and verifies the kernel's signature.
> > > > Good, the kernel hasn't been tampered with.??The kernel starts running.
> > > >?
> > > > You've plugged in a USB drive to the device, and specified a partition
> > > > containing a root filesystem that you control to the kernel.??The
> > > > validated kernel finds the USB drive, and mounts it, and executes
> > > > your own binaries on the USB drive.
> > > You will require physical access to the machine to be able to
> > > insert your usb drive. And IIRC, argument was that if attacker has
> > > physical access to machine, all bets are off anyway.
> > >
> > 
> > You don't need physical access -- your machine controller BMC can
> > do the magic for you. So its not always physical access, is it?
> 
> Well, idea was that if you have physical access to machine, then all
> bets are off. If BMC can do something which allows running unsigned
> code at ring level 0, its a problem I think from secureboot model of
> security.
> 
> > ?
> > > >?
> > > >?
> > > > You run a shell on the console.??You now have control of the system,
> > > > and can mount the real rootfs, inspect it, and work out what it does,
> > > > etc.
> > > >?
> > > > At this point, what use was all the validation that the secure boot
> > > > has done???Absolutely useless.
> > > >?
> > > > If you can change the command line arguments given to the kernel, you
> > > > have no security, no matter how much you verify signatures.??It's
> > > > the illusion of security, nothing more, nothing less.
> > > >?
> > 
> > I agree, if you can change command line arguments, all bets are of lesser value
> 
> If changing command line allows execution of unsigned code at ring level
> 0, then it is a problem. Otherwise we are talking of security issues which
> are not covered by secureboot model.
> 
> Vivek

[RFC 3/3] kexec: extend kexec_file_load system call

[Dave Young]

On 07/18/16 at 11:07am, Mark Rutland wrote:
> On Mon, Jul 18, 2016 at 10:30:24AM +0800, Dave Young wrote:
> > On 07/15/16 at 02:19pm, Mark Rutland wrote:
> > > On Fri, Jul 15, 2016 at 09:09:55AM -0400, Vivek Goyal wrote:
> > > > On Tue, Jul 12, 2016 at 10:42:01AM +0900, AKASHI Takahiro wrote:
> > > > 
> > > > [..]
> > > > > -SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > > > > +SYSCALL_DEFINE6(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > > > >  		unsigned long, cmdline_len, const char __user *, cmdline_ptr,
> > > > > -		unsigned long, flags)
> > > > > +		unsigned long, flags, const struct kexec_fdset __user *, ufdset)
> > > > 
> > > > Can one add more parameters to existing syscall. Can it break existing
> > > > programs with new kernel? I was of the impression that one can't do that.
> > > > But may be I am missing something.
> > > 
> > > I think the idea was that we would only look at the new params if a new
> > > flags was set, and otherwise it would behave as the old syscall.
> > > 
> > > Regardless, I think it makes far more sense to add a kexec_file_load2
> > > syscall if we're going to modify the prototype at all. It's a rather
> > > different proposition to the existing syscall, and needs to be treated
> > > as such.
> > 
> > I do not think it is worth to add another syscall for extra fds.
> > We have open(2) as an example for different numbers of arguments
> > already.
> 
> Did we change the syscall interface for that?
> 
> I was under the impression that there was always one underlying syscall,
> and the C library did the right thing to pass the expected information
> to the underlying syscall.

I'm not sure kexec_load and kexec_file_load were included in glibc, we use
syscall directly in kexec-tools.

kexec_load man pages says there are no wrappers for both kexec_load and
kexec_file_load in glibc.

> 
> That's rather different to changing the underlying syscall.
> 
> Regardless of how this is wrapped in userspace, I do not think modifying
> the existing prototype is a good idea, and I think this kind of
> extension needs to be a new syscall.

Hmm, as I replied to Vivek, there is one case about the flags, previously
the new flag will be regarded as invalid, but not we extend it it will be
valid, this maybe the only potential bad case.

Thanks
Dave

[RFC 3/3] kexec: extend kexec_file_load system call

[Mark Rutland]

On Tue, Jul 19, 2016 at 08:55:56AM +0800, Dave Young wrote:
> On 07/18/16 at 11:07am, Mark Rutland wrote:
> > On Mon, Jul 18, 2016 at 10:30:24AM +0800, Dave Young wrote:
> > > I do not think it is worth to add another syscall for extra fds.
> > > We have open(2) as an example for different numbers of arguments
> > > already.
> > 
> > Did we change the syscall interface for that?
> > 
> > I was under the impression that there was always one underlying syscall,
> > and the C library did the right thing to pass the expected information
> > to the underlying syscall.
> 
> I'm not sure kexec_load and kexec_file_load were included in glibc, we use
> syscall directly in kexec-tools.
> 
> kexec_load man pages says there are no wrappers for both kexec_load and
> kexec_file_load in glibc.

For the above, I was talking about how open() was handled.

If there are no userspace wrappers, then the two cases aren't comparable
in the first place...

> > That's rather different to changing the underlying syscall.
> > 
> > Regardless of how this is wrapped in userspace, I do not think modifying
> > the existing prototype is a good idea, and I think this kind of
> > extension needs to be a new syscall.
> 
> Hmm, as I replied to Vivek, there is one case about the flags, previously
> the new flag will be regarded as invalid, but not we extend it it will be
> valid, this maybe the only potential bad case.

It's true that adding suport for new flags will change the behaviour of
what used to be error cases. We generally expect real users to not be
making pointless calls for which they rely on an error being returned in
all cases.

Regardless, this extended syscall changes some underlying assumptions
made with the development of kexec_file_load, and I think treating this
as an extension is not a great idea. From a user's perspective there is
little difference between passing an additional flag or using a
different syscall number, so I don't think that we gain much by altering
the existing prototype relative to allocating a new syscall number.

Thus, I think that if this is necessary it should be treated as a new
syscall.

Thanks,
Mark.

[RFC 3/3] kexec: extend kexec_file_load system call

[Vivek Goyal]

On Tue, Jul 19, 2016 at 11:52:00AM +0100, Mark Rutland wrote:
> On Tue, Jul 19, 2016 at 08:55:56AM +0800, Dave Young wrote:
> > On 07/18/16 at 11:07am, Mark Rutland wrote:
> > > On Mon, Jul 18, 2016 at 10:30:24AM +0800, Dave Young wrote:
> > > > I do not think it is worth to add another syscall for extra fds.
> > > > We have open(2) as an example for different numbers of arguments
> > > > already.
> > > 
> > > Did we change the syscall interface for that?
> > > 
> > > I was under the impression that there was always one underlying syscall,
> > > and the C library did the right thing to pass the expected information
> > > to the underlying syscall.
> > 
> > I'm not sure kexec_load and kexec_file_load were included in glibc, we use
> > syscall directly in kexec-tools.
> > 
> > kexec_load man pages says there are no wrappers for both kexec_load and
> > kexec_file_load in glibc.
> 
> For the above, I was talking about how open() was handled.
> 
> If there are no userspace wrappers, then the two cases aren't comparable
> in the first place...
> 
> > > That's rather different to changing the underlying syscall.
> > > 
> > > Regardless of how this is wrapped in userspace, I do not think modifying
> > > the existing prototype is a good idea, and I think this kind of
> > > extension needs to be a new syscall.
> > 
> > Hmm, as I replied to Vivek, there is one case about the flags, previously
> > the new flag will be regarded as invalid, but not we extend it it will be
> > valid, this maybe the only potential bad case.
> 
> It's true that adding suport for new flags will change the behaviour of
> what used to be error cases. We generally expect real users to not be
> making pointless calls for which they rely on an error being returned in
> all cases.
> 
> Regardless, this extended syscall changes some underlying assumptions
> made with the development of kexec_file_load, and I think treating this
> as an extension is not a great idea. From a user's perspective there is
> little difference between passing an additional flag or using a
> different syscall number, so I don't think that we gain much by altering
> the existing prototype relative to allocating a new syscall number.

If we are providing/opening up additional flags, I can't think what will
it break. Same flag was invalid in old kernel but new kernel supports 
it and will accept it. So it sounds reasonable to me to add new flags.

If existing users are not broken, then I think it might be a good idea
to extend existing syscall. Otherwise userspace will have to be modified
to understand a 3rd syscall also and an additional option will show up
which asks users to specify which syscall to use. So extending existing
syscall might keep it little simple for users.

This is only if conclusion in the end is that DT needs to be passed in
from user space.

BTW, does kexec_load() needs to be modified too to handle DT?

Vivek

[RFC 3/3] kexec: extend kexec_file_load system call

[Mark Rutland]

On Tue, Jul 19, 2016 at 08:24:06AM -0400, Vivek Goyal wrote:
> On Tue, Jul 19, 2016 at 11:52:00AM +0100, Mark Rutland wrote:
> > Regardless, this extended syscall changes some underlying assumptions
> > made with the development of kexec_file_load, and I think treating this
> > as an extension is not a great idea. From a user's perspective there is
> > little difference between passing an additional flag or using a
> > different syscall number, so I don't think that we gain much by altering
> > the existing prototype relative to allocating a new syscall number.
> 
> If we are providing/opening up additional flags, I can't think what will
> it break. Same flag was invalid in old kernel but new kernel supports 
> it and will accept it. So it sounds reasonable to me to add new flags.
> 
> If existing users are not broken, then I think it might be a good idea
> to extend existing syscall. Otherwise userspace will have to be modified
> to understand a 3rd syscall also and an additional option will show up
> which asks users to specify which syscall to use. So extending existing
> syscall might keep it little simple for users.

I don't follow.

To use the new feature, you have to modify userspace anyway, as you
require userspace to pass information which it did not previously pass
(in the new arguments added to the syscall).

The presence of a new syscall does not imply the absence of the old
syscall, so you can always use that be default unless the user asks for
asomething only the new syscall provides. Regardless of the
syscall/flags difference, you still have to detect whether the new
functionality is present somehow.

> BTW, does kexec_load() needs to be modified too to handle DT?

No, at least for arm64. In the kexec_load case userspace provides the
DTB as a raw segment, and the user-provided purgatory sets up registers
to pass that to the new kernel.

Thanks,
Mark.

[RFC 3/3] kexec: extend kexec_file_load system call

[Vivek Goyal]

On Tue, Jul 19, 2016 at 01:47:28PM +0100, Mark Rutland wrote:
> On Tue, Jul 19, 2016 at 08:24:06AM -0400, Vivek Goyal wrote:
> > On Tue, Jul 19, 2016 at 11:52:00AM +0100, Mark Rutland wrote:
> > > Regardless, this extended syscall changes some underlying assumptions
> > > made with the development of kexec_file_load, and I think treating this
> > > as an extension is not a great idea. From a user's perspective there is
> > > little difference between passing an additional flag or using a
> > > different syscall number, so I don't think that we gain much by altering
> > > the existing prototype relative to allocating a new syscall number.
> > 
> > If we are providing/opening up additional flags, I can't think what will
> > it break. Same flag was invalid in old kernel but new kernel supports 
> > it and will accept it. So it sounds reasonable to me to add new flags.
> > 
> > If existing users are not broken, then I think it might be a good idea
> > to extend existing syscall. Otherwise userspace will have to be modified
> > to understand a 3rd syscall also and an additional option will show up
> > which asks users to specify which syscall to use. So extending existing
> > syscall might keep it little simple for users.
> 
> I don't follow.
> 
> To use the new feature, you have to modify userspace anyway, as you
> require userspace to pass information which it did not previously pass
> (in the new arguments added to the syscall).
> 
> The presence of a new syscall does not imply the absence of the old
> syscall, so you can always use that be default unless the user asks for
> asomething only the new syscall provides. Regardless of the
> syscall/flags difference, you still have to detect whether the new
> functionality is present somehow.
> 

Hmm., so current idea is that we have two syscalls() which are *ideally*
supposed to work for all arches. Difference between two is that first
one does not support kernel signature verification while second one does.

By default old syscall is used and user can force using new syscall using
option --kexec-file-load.

If a user DTB is present, I was hoping that it will continue to work the
same way. Both the sycalls can be used and can handle DTB. If we introduce
a 3rd syscall, that means only first and 3rd syscall can handle DTB and
we need to introduce one more option which tells whether to use
kexec_load() or use the 3rd new syscall. And that's what I am trying
to avoid.

Vivek

> > BTW, does kexec_load() needs to be modified too to handle DT?
> 
> No, at least for arm64. In the kexec_load case userspace provides the
> DTB as a raw segment, and the user-provided purgatory sets up registers
> to pass that to the new kernel.
> 
> Thanks,
> Mark.

[RFC 0/3] extend kexec_file_load system call

[Balbir Singh]

>>>>>  
>>>>> Command line options are not signed. I thought idea behind secureboot
>>>>> was to execute only trusted code and command line options don't enforce
>>>>> you to execute unsigned code.
>>>>>  
>>
>> You can set module.sig_enforce=0 and open up the system a bit assuming
>> that you can get a module to load with another attack
> 
> IIUC, sig_enforce bool_enable_only so it can only be enabled. Default
> value of it is 0 if CONFIG_MODULE_SIG_FORCE=n.
> 
> IOW, if your kernel forced signature verification, you should not be
> able to do sig_enforce=0. If you kernel did not have
> CONFIG_MODULE_SIG_FORCE=y, then sig_enforce should be 0 by default anyway
> and you are not making it worse using command line.
> 

OK.. I checked and you are right, but that is an example and there are
other things like security=, thermal.*, nosmep, nosmap that need auditing
for safety and might hurt the system security if used. I still think
think that assuming you can pass any command line without breaking security
is a broken argument.

>>
>>>>> So it sounds like different class of security problems which you are
>>>>> referring to and not necessarily covered by secureboot or signed
>>>>> kernel.
>>>> Let me give you an example.
>>>>  
>>>> You have a secure boot setup, where the firmware/ROM validates the boot
>>>> loader.  Good, the boot loader hasn't been tampered with.
>>>>  
>>>> You interrupt the boot loader and are able to modify the command line
>>>> for the booted kernel.
>>>>  
>>>> The boot loader loads the kernel and verifies the kernel's signature.
>>>> Good, the kernel hasn't been tampered with.  The kernel starts running.
>>>>  
>>>> You've plugged in a USB drive to the device, and specified a partition
>>>> containing a root filesystem that you control to the kernel.  The
>>>> validated kernel finds the USB drive, and mounts it, and executes
>>>> your own binaries on the USB drive.
>>> You will require physical access to the machine to be able to
>>> insert your usb drive. And IIRC, argument was that if attacker has
>>> physical access to machine, all bets are off anyway.
>>>
>>
>> You don't need physical access -- your machine controller BMC can
>> do the magic for you. So its not always physical access, is it?
> 
> Well, idea was that if you have physical access to machine, then all
> bets are off. If BMC can do something which allows running unsigned
> code at ring level 0, its a problem I think from secureboot model of
> security.
> 
>>  
>>>>  
>>>>  
>>>> You run a shell on the console.  You now have control of the system,
>>>> and can mount the real rootfs, inspect it, and work out what it does,
>>>> etc.
>>>>  
>>>> At this point, what use was all the validation that the secure boot
>>>> has done?  Absolutely useless.
>>>>  
>>>> If you can change the command line arguments given to the kernel, you
>>>> have no security, no matter how much you verify signatures.  It's
>>>> the illusion of security, nothing more, nothing less.
>>>>  
>>
>> I agree, if you can change command line arguments, all bets are of lesser value
> 
> If changing command line allows execution of unsigned code at ring level
> 0, then it is a problem. Otherwise we are talking of security issues which
> are not covered by secure


I agree that from what I can see/grep there is nothing that allows unsigned
code to run at boot in ring0, but there are implications like the ones
I've mentioned above.

Attacks are typically built as a chain and every bit might matter. One could
turn off features that might lead to the system being attacked at run-time


Balbir Singh.

[RFC 0/3] extend kexec_file_load system call

[Russell King - ARM Linux]

On Wed, Jul 20, 2016 at 01:45:42PM +1000, Balbir Singh wrote:
> > IOW, if your kernel forced signature verification, you should not be
> > able to do sig_enforce=0. If you kernel did not have
> > CONFIG_MODULE_SIG_FORCE=y, then sig_enforce should be 0 by default anyway
> > and you are not making it worse using command line.
> 
> OK.. I checked and you are right, but that is an example and there are
> other things like security=, thermal.*, nosmep, nosmap that need auditing
> for safety and might hurt the system security if used. I still think
> think that assuming you can pass any command line without breaking security
> is a broken argument.

Quite, and you don't need to run code in a privileged environment to do
any of that.

It's also not trivial to protect against: new kernels gain new arguments
which older kernels may not know about.  No matter how much protection
is built into older kernels, newer kernels can become vulnerable through
the addition of further arguments.

Also, how sure are we that there are no stack overflow issues with kernel
command line parsing?  Can we be sure that there's none?  This is
something which happens early in the kernel boot, before the full memory
protections have been set up.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[RFC 0/3] extend kexec_file_load system call

[Michael Ellerman]

Russell King - ARM Linux <linux@armlinux.org.uk> writes:

> On Wed, Jul 20, 2016 at 01:45:42PM +1000, Balbir Singh wrote:
>> > IOW, if your kernel forced signature verification, you should not be
>> > able to do sig_enforce=0. If you kernel did not have
>> > CONFIG_MODULE_SIG_FORCE=y, then sig_enforce should be 0 by default anyway
>> > and you are not making it worse using command line.
>> 
>> OK.. I checked and you are right, but that is an example and there are
>> other things like security=, thermal.*, nosmep, nosmap that need auditing
>> for safety and might hurt the system security if used. I still think
>> think that assuming you can pass any command line without breaking security
>> is a broken argument.
>
> Quite, and you don't need to run code in a privileged environment to do
> any of that.
>
> It's also not trivial to protect against: new kernels gain new arguments
> which older kernels may not know about.  No matter how much protection
> is built into older kernels, newer kernels can become vulnerable through
> the addition of further arguments.

Indeed. A whitelist of allowed command line arguments is the only option.

But given the existing syscall has shipped without a whitelist of command line
arguments, you can't add a whitelist now without potentially breaking someone's
setup.

Getting back to the device tree, we could similarly have a whitelist of
nodes/properties that we allow to be passed in.

At least for stdout-path, I can't really see how that would significantly help
an attacker, but I'm all ears if anyone has ideas.

> Also, how sure are we that there are no stack overflow issues with kernel
> command line parsing?  Can we be sure that there's none?  This is
> something which happens early in the kernel boot, before the full memory
> protections have been set up.

Yeah that's also a good point. More so for the device tree, because the parsing
is more complicated. I think there has been some work done on fuzzing libfdt,
but we should probably do more.

cheers

[RFC 0/3] extend kexec_file_load system call

[Arnd Bergmann]

On Wednesday, July 20, 2016 8:47:45 PM CEST Michael Ellerman wrote:
> At least for stdout-path, I can't really see how that would significantly help
> an attacker, but I'm all ears if anyone has ideas.

That's actually an easy one that came up before: If an attacker controls
a tty device (e.g. network console) that can be used to enter a debugger
(kdb, kgdb, xmon, ...), enabling that to be the console device
gives you a direct attack vector. The same thing will happen if you
have a piece of software that intentially gives extra rights to the
owner of the console device by treating it as "physical presence".

	Arnd

[RFC 3/3] kexec: extend kexec_file_load system call

[David Laight]

From: Dave Young
> On 07/15/16 at 02:19pm, Mark Rutland wrote:
> > On Fri, Jul 15, 2016 at 09:09:55AM -0400, Vivek Goyal wrote:
> > > On Tue, Jul 12, 2016 at 10:42:01AM +0900, AKASHI Takahiro wrote:
> > >
> > > [..]
> > > > -SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > > > +SYSCALL_DEFINE6(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > > >  		unsigned long, cmdline_len, const char __user *, cmdline_ptr,
> > > > -		unsigned long, flags)
> > > > +		unsigned long, flags, const struct kexec_fdset __user *, ufdset)
> > >
> > > Can one add more parameters to existing syscall. Can it break existing
> > > programs with new kernel? I was of the impression that one can't do that.
> > > But may be I am missing something.
> >
> > I think the idea was that we would only look at the new params if a new
> > flags was set, and otherwise it would behave as the old syscall.
> >
> > Regardless, I think it makes far more sense to add a kexec_file_load2
> > syscall if we're going to modify the prototype at all. It's a rather
> > different proposition to the existing syscall, and needs to be treated
> > as such.
> 
> I do not think it is worth to add another syscall for extra fds.
> We have open(2) as an example for different numbers of arguments
> already.

Probably works 'by luck' and no one has actually thought about why.
That ioctl() works is (probably) even more lucky.

There are ABI that use different calling conventions for varags functions
(eg always stack all the arguments). I guess linux doesn't run on any of them.

ioctl() is a particular problem because the 'arg' might be an integer or a pointer.
Fortunately all the 64bit ABI linux uses pass the arg parameter in a register
(and don't use different registers for pointer and data arguments).

You could have two 'libc' functions that refer to the same system call entry.
Certainly safer than a varargs function.

	David

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Wed, Jul 20, 2016 at 01:45:42PM +1000, Balbir Singh wrote:
> >>>>>  
> >>>>> Command line options are not signed. I thought idea behind secureboot
> >>>>> was to execute only trusted code and command line options don't enforce
> >>>>> you to execute unsigned code.
> >>>>>  
> >>
> >> You can set module.sig_enforce=0 and open up the system a bit assuming
> >> that you can get a module to load with another attack
> > 
> > IIUC, sig_enforce bool_enable_only so it can only be enabled. Default
> > value of it is 0 if CONFIG_MODULE_SIG_FORCE=n.
> > 
> > IOW, if your kernel forced signature verification, you should not be
> > able to do sig_enforce=0. If you kernel did not have
> > CONFIG_MODULE_SIG_FORCE=y, then sig_enforce should be 0 by default anyway
> > and you are not making it worse using command line.
> > 
> 
> OK.. I checked and you are right, but that is an example and there are
> other things like security=, thermal.*, nosmep, nosmap that need auditing
> for safety and might hurt the system security if used. I still think
> think that assuming you can pass any command line without breaking security
> is a broken argument.

I agree that if some command line option allows running unsigned code
at ring 0, then we probably should disable that on secureboot enabled
boot.

In fact, there were bunch of patches which made things tighter on
secureboot enabled machines from matthew garrett. AFAIK, these patches
never went upstream.

Vivek

[RFC 0/3] extend kexec_file_load system call

[Vivek Goyal]

On Wed, Jul 20, 2016 at 09:35:30AM +0100, Russell King - ARM Linux wrote:
> On Wed, Jul 20, 2016 at 01:45:42PM +1000, Balbir Singh wrote:
> > > IOW, if your kernel forced signature verification, you should not be
> > > able to do sig_enforce=0. If you kernel did not have
> > > CONFIG_MODULE_SIG_FORCE=y, then sig_enforce should be 0 by default anyway
> > > and you are not making it worse using command line.
> > 
> > OK.. I checked and you are right, but that is an example and there are
> > other things like security=, thermal.*, nosmep, nosmap that need auditing
> > for safety and might hurt the system security if used. I still think
> > think that assuming you can pass any command line without breaking security
> > is a broken argument.
> 
> Quite, and you don't need to run code in a privileged environment to do
> any of that.
> 
> It's also not trivial to protect against: new kernels gain new arguments
> which older kernels may not know about.  No matter how much protection
> is built into older kernels, newer kernels can become vulnerable through
> the addition of further arguments.

If a new kernel command line option becomes an issue, new kernel can
block that in secureboot environment. That way it helps kexec
boot as well as regular boot.

Vivek

[RFC 0/3] extend kexec_file_load system call

[Thiago Jung Bauermann]

Am Mittwoch, 20 Juli 2016, 13:12:20 schrieb Arnd Bergmann:
> On Wednesday, July 20, 2016 8:47:45 PM CEST Michael Ellerman wrote:
> > At least for stdout-path, I can't really see how that would
> > significantly help an attacker, but I'm all ears if anyone has ideas.
> 
> That's actually an easy one that came up before: If an attacker controls
> a tty device (e.g. network console) that can be used to enter a debugger
> (kdb, kgdb, xmon, ...), enabling that to be the console device
> gives you a direct attack vector. The same thing will happen if you
> have a piece of software that intentially gives extra rights to the
> owner of the console device by treating it as "physical presence".

I think people are talking past each other a bit in these arguments about 
what is relevant to security or not.

For the kexec maintainers, kexec_file_load has one very specific and narrow 
purpose: enable Secure Boot as defined by UEFI.

And from what I understand of their arguments so far, there is one and only 
one security concern: when in Secure Boot mode, a system must not allow 
execution of unsigned code with kernel privileges. So even if one can 
specify a different root filesystem and do a lot of nasty things to the 
system with a rogue userspace in that root filesystem, as long as the kernel 
won't load unsigned modules that's not a problem as far as they're 
concerned.

Also, AFAIK attacks requiring "physical presence" are out of scope for the 
UEFI Secure Boot security model. Thus an attack that involves control of a 
console of plugging an USB device is also not a concern.

One thing I don't know is whether an attack involving a networked IPMI 
console or a USB device that can be "plugged" virtually by a managing system 
(BMC) is considered a physical attack or a remote attack in the context of 
UEFI Secure Boot.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

[RFC 3/3] kexec: extend kexec_file_load system call

[Russell King - ARM Linux]

On Wed, Jul 20, 2016 at 11:41:35AM +0000, David Laight wrote:
> From: Dave Young
> > I do not think it is worth to add another syscall for extra fds.
> > We have open(2) as an example for different numbers of arguments
> > already.
> 
> Probably works 'by luck' and no one has actually thought about why.
> That ioctl() works is (probably) even more lucky.
> 
> There are ABI that use different calling conventions for varags functions
> (eg always stack all the arguments). I guess linux doesn't run on any of them.
> 
> ioctl() is a particular problem because the 'arg' might be an integer or a pointer.
> Fortunately all the 64bit ABI linux uses pass the arg parameter in a register
> (and don't use different registers for pointer and data arguments).
> 
> You could have two 'libc' functions that refer to the same system call entry.
> Certainly safer than a varargs function.

Don't forget that the syscall API is not a normal C function API - it's
special, because there's little point stacking arguments on the userspace
stack and then having the kernel function try and read them off the
kernelspace stack.

If an architecture does such a thing, then it needs special veneers to
handle that (reading off the userspace stack and placing them onto the
kernelspace stack, or the arch needs to define some other method of
handling the situation.)

So, really, the actual C APIs don't matter that much - what matters more
is the definition of a sane way to pass such arguments.  Given the
extensive historical nature of open() and ioctl(), it would be completely
silly not to create something which allows these calls to work.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[RFC 0/3] extend kexec_file_load system call

[Thiago Jung Bauermann]

Am Freitag, 15 Juli 2016, 18:03:35 schrieb Thiago Jung Bauermann:
> Am Freitag, 15 Juli 2016, 22:26:09 schrieb Arnd Bergmann:
> > However, the powerpc specific RTAS runtime services provide a similar
> > interface to the UEFI runtime support and allow to call into
> > binary code from the kernel, which gets mapped from a physical
> > address in the "linux,rtas-base" property in the rtas device node.
> > 
> > Modifying the /rtas node will definitely give you a backdoor into
> > priviledged code, but modifying only /chosen should not let you get
> > in through that specific method.
> 
> Except that arch/powerpc/kernel/rtas.c looks for any node in the tree
> called "rtas", so it will try to use /chosen/rtas, or /chosen/foo/rtas.
> 
> We can forbid subnodes in /chosen in the dtb passed to kexec_file_load,
> though that means userspace can't use the simple-framebuffer binding via
> this mechanism.
> 
> We also have to blacklist the device_type and compatible properties in
> /chosen to avoid the problem Mark mentioned.
> 
> Still doable, but not ideal. :-/

So even if not ideal, the solution above is desirable for powerpc. We would 
like to preserve the ability of allowing userspace to pass parameters to the 
OS via the DTB, even if secure boot is enabled.

I would like to turn the above into a proposal:

Extend the syscall as shown in this RFC from Takahiro AKASHI, but instead of 
accepting a complete DTB from userspace, the syscall accepts a DTB 
containing only a /chosen node. If the DTB contains any other node, the 
syscall fails with EINVAL. If the DTB contains any subnode in /chosen, or if 
there's a compatible or device_type property in /chosen, the syscall fails 
with EINVAL as well.

The kernel can then add the properties in /chosen to the device tree that it 
will pass to the next kernel.

What do you think?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

[RFC 0/3] extend kexec_file_load system call

[Jeremy Kerr]

Hi Thiago,

> So even if not ideal, the solution above is desirable for powerpc. We would 
> like to preserve the ability of allowing userspace to pass parameters to the 
> OS via the DTB, even if secure boot is enabled.
> 
> I would like to turn the above into a proposal:
> 
> Extend the syscall as shown in this RFC from Takahiro AKASHI, but instead of 
> accepting a complete DTB from userspace, the syscall accepts a DTB 
> containing only a /chosen node. If the DTB contains any other node, the 
> syscall fails with EINVAL. If the DTB contains any subnode in /chosen, or if 
> there's a compatible or device_type property in /chosen, the syscall fails 
> with EINVAL as well.

This works for me. We could even have it as just a DTB fragment that is
merged *at* the /chosen/ node of the kernel-device tree - so would not
contain a /chosen node itself, and it would be impossible to provide
nodes outside of /chosen. Either is fine.

Thanks!


Jeremy

[RFC 0/3] extend kexec_file_load system call

[Michael Ellerman]

Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:

> Am Freitag, 15 Juli 2016, 18:03:35 schrieb Thiago Jung Bauermann:
>> Am Freitag, 15 Juli 2016, 22:26:09 schrieb Arnd Bergmann:
>> > However, the powerpc specific RTAS runtime services provide a similar
>> > interface to the UEFI runtime support and allow to call into
>> > binary code from the kernel, which gets mapped from a physical
>> > address in the "linux,rtas-base" property in the rtas device node.
>> > 
>> > Modifying the /rtas node will definitely give you a backdoor into
>> > priviledged code, but modifying only /chosen should not let you get
>> > in through that specific method.
>> 
>> Except that arch/powerpc/kernel/rtas.c looks for any node in the tree
>> called "rtas", so it will try to use /chosen/rtas, or /chosen/foo/rtas.
>> 
>> We can forbid subnodes in /chosen in the dtb passed to kexec_file_load,
>> though that means userspace can't use the simple-framebuffer binding via
>> this mechanism.
>> 
>> We also have to blacklist the device_type and compatible properties in
>> /chosen to avoid the problem Mark mentioned.
>> 
>> Still doable, but not ideal. :-/
>
> So even if not ideal, the solution above is desirable for powerpc. We would 
> like to preserve the ability of allowing userspace to pass parameters to the 
> OS via the DTB, even if secure boot is enabled.
>
> I would like to turn the above into a proposal:
>
> Extend the syscall as shown in this RFC from Takahiro AKASHI, but instead of 
> accepting a complete DTB from userspace, the syscall accepts a DTB 
> containing only a /chosen node. If the DTB contains any other node, the 
> syscall fails with EINVAL. If the DTB contains any subnode in /chosen, or if 
> there's a compatible or device_type property in /chosen, the syscall fails 
> with EINVAL as well.
>
> The kernel can then add the properties in /chosen to the device tree that it 
> will pass to the next kernel.
>
> What do you think?

I think we will inevitably have someone who wants to pass something
other than a child of /chosen.

At that point we would be faced with adding yet another syscall, or at
best a new flag.

I think we'd be better allowing userspace to pass a DTB, and having an
explicit whitelist (in the kernel) of which nodes & properties are
allowed in that DTB.

For starters it would only contain /chosen/stdout-path (for example).
But we would be able to add new nodes & properties in future.

The downside is userspace would have no way of detecting the content of
the white list, other than trial and error. But in practice I'm not sure
that would be a big problem.

cheers

[RFC 0/3] extend kexec_file_load system call

[Thiago Jung Bauermann]

Am Freitag, 22 Juli 2016, 12:54:28 schrieb Michael Ellerman:
> Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> writes:
> > So even if not ideal, the solution above is desirable for powerpc. We
> > would like to preserve the ability of allowing userspace to pass
> > parameters to the OS via the DTB, even if secure boot is enabled.
> > 
> > I would like to turn the above into a proposal:
> > 
> > Extend the syscall as shown in this RFC from Takahiro AKASHI, but
> > instead of accepting a complete DTB from userspace, the syscall accepts
> > a DTB containing only a /chosen node. If the DTB contains any other
> > node, the syscall fails with EINVAL. If the DTB contains any subnode in
> > /chosen, or if there's a compatible or device_type property in /chosen,
> > the syscall fails with EINVAL as well.
> > 
> > The kernel can then add the properties in /chosen to the device tree
> > that it will pass to the next kernel.
> > 
> > What do you think?
> 
> I think we will inevitably have someone who wants to pass something
> other than a child of /chosen.
> 
> At that point we would be faced with adding yet another syscall, or at
> best a new flag.
> 
> I think we'd be better allowing userspace to pass a DTB, and having an
> explicit whitelist (in the kernel) of which nodes & properties are
> allowed in that DTB.

Sounds good to me.

> For starters it would only contain /chosen/stdout-path (for example).
> But we would be able to add new nodes & properties in future.

If we allow things outside of chosen, we can keep the offb.c hook in 
Petitboot and whitelist the framebuffer properties it adds to the vga node.

> The downside is userspace would have no way of detecting the content of
> the white list, other than trial and error. But in practice I'm not sure
> that would be a big problem.

For our use case in OpenPower I don't think it would be a problem, since the 
userspace and the kernel are developed together.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

[PATCH v2 3/3] kexec: extend kexec_file_load system call

[Thiago Jung Bauermann]

Device tree blob must be passed to a second kernel on DTB-capable
archs, like powerpc and arm64, but the current kernel interface
lacks this support.

This patch extends kexec_file_load system call by adding an extra
argument to this syscall so that an arbitrary number of file descriptors
can be handed out from user space to the kernel.

	long sys_kexec_file_load(int kernel_fd, int initrd_fd,
				 unsigned long cmdline_len,
				 const char __user *cmdline_ptr,
				 unsigned long flags,
				 const struct kexec_fdset __user *ufdset);

If KEXEC_FILE_EXTRA_FDS is set to the "flags" argument, the "ufdset"
argument points to the following struct buffer:

	struct kexec_fdset {
		int nr_fds;
		struct kexec_file_fd fds[0];
	}

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---

Notes:
    This is a new version of the last patch in this series which adds
    a function where each architecture can verify if the DTB is safe
    to load:
    
    int __weak arch_kexec_verify_buffer(enum kexec_file_type type,
                                        const void *buf,
                                        unsigned long size)
    {
            return -EINVAL;
    }
    
    I will then provide an implementation in my powerpc patch series
    which checks that the DTB only contains nodes and properties from a
    whitelist. arch_kexec_kernel_image_load will copy these properties
    to the device tree blob the kernel was booted with (and perform
    other changes such as setting /chosen/bootargs, of course).
    
    I made the following additional changes:
    - renamed KEXEC_FILE_TYPE_DTB to KEXEC_FILE_TYPE_PARTIAL_DTB,
    - limited max number of fds to KEXEC_SEGMENT_MAX,
    - changed to use fixed size buffer for fdset instead of allocating it,
    - changed to return -EINVAL if an unknown file type is found in fdset.

 include/linux/fs.h         |  1 +
 include/linux/kexec.h      |  7 ++--
 include/linux/syscalls.h   |  4 ++-
 include/uapi/linux/kexec.h | 22 ++++++++++++
 kernel/kexec_file.c        | 83 ++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 108 insertions(+), 9 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index dd288148a6b1..5e0ee342b457 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2634,6 +2634,7 @@ extern int do_pipe_flags(int *, int);
 	id(MODULE, kernel-module)		\
 	id(KEXEC_IMAGE, kexec-image)		\
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
+	id(KEXEC_PARTIAL_DTB, kexec-partial-dtb)		\
 	id(POLICY, security-policy)		\
 	id(MAX_ID, )
 
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 554c8480dba3..b7eec336e935 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -146,7 +146,10 @@ struct kexec_file_ops {
 	kexec_verify_sig_t *verify_sig;
 #endif
 };
-#endif
+
+int __weak arch_kexec_verify_buffer(enum kexec_file_type type, const void *buf,
+				    unsigned long size);
+#endif /* CONFIG_KEXEC_FILE */
 
 struct kimage {
 	kimage_entry_t head;
@@ -277,7 +280,7 @@ extern int kexec_load_disabled;
 
 /* List of defined/legal kexec file flags */
 #define KEXEC_FILE_FLAGS	(KEXEC_FILE_UNLOAD | KEXEC_FILE_ON_CRASH | \
-				 KEXEC_FILE_NO_INITRAMFS)
+				 KEXEC_FILE_NO_INITRAMFS | KEXEC_FILE_EXTRA_FDS)
 
 #define VMCOREINFO_BYTES           (4096)
 #define VMCOREINFO_NOTE_NAME       "VMCOREINFO"
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index d02239022bd0..fc072bdb74e3 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -66,6 +66,7 @@ struct perf_event_attr;
 struct file_handle;
 struct sigaltstack;
 union bpf_attr;
+struct kexec_fdset;
 
 #include <linux/types.h>
 #include <linux/aio_abi.h>
@@ -321,7 +322,8 @@ asmlinkage long sys_kexec_load(unsigned long entry, unsigned long nr_segments,
 asmlinkage long sys_kexec_file_load(int kernel_fd, int initrd_fd,
 				    unsigned long cmdline_len,
 				    const char __user *cmdline_ptr,
-				    unsigned long flags);
+				    unsigned long flags,
+				    const struct kexec_fdset __user *ufdset);
 
 asmlinkage long sys_exit(int error_code);
 asmlinkage long sys_exit_group(int error_code);
diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
index 99048e501b88..32e0cefe2000 100644
--- a/include/uapi/linux/kexec.h
+++ b/include/uapi/linux/kexec.h
@@ -23,6 +23,28 @@
 #define KEXEC_FILE_UNLOAD	0x00000001
 #define KEXEC_FILE_ON_CRASH	0x00000002
 #define KEXEC_FILE_NO_INITRAMFS	0x00000004
+#define KEXEC_FILE_EXTRA_FDS	0x00000008
+
+enum kexec_file_type {
+	KEXEC_FILE_TYPE_KERNEL,
+	KEXEC_FILE_TYPE_INITRAMFS,
+
+	/*
+	 * Device Tree Blob containing just the nodes and properties that
+	 * the kexec_file_load caller wants to add or modify.
+	 */
+	KEXEC_FILE_TYPE_PARTIAL_DTB,
+};
+
+struct kexec_file_fd {
+	enum kexec_file_type type;
+	int fd;
+};
+
+struct kexec_fdset {
+	int nr_fds;
+	struct kexec_file_fd fds[0];
+};
 
 /* These values match the ELF architecture values.
  * Unless there is a good reason that should continue to be the case.
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 113af2f219b9..d6803dd884e2 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -25,6 +25,9 @@
 #include <linux/vmalloc.h>
 #include "kexec_internal.h"
 
+#define MAX_FDSET_SIZE	(sizeof(struct kexec_fdset) + \
+				KEXEC_SEGMENT_MAX * sizeof(struct kexec_file_fd))
+
 /*
  * Declare these symbols weak so that if architecture provides a purgatory,
  * these will be overridden.
@@ -116,6 +119,22 @@ void kimage_file_post_load_cleanup(struct kimage *image)
 	image->image_loader_data = NULL;
 }
 
+/**
+ * arch_kexec_verify_buffer() - check that the given kexec file is valid
+ *
+ * Device trees in particular can contain properties that may make the kernel
+ * execute code that it wasn't supposed to (e.g., use the wrong entry point
+ * when calling firmware functions). Because of this, the kernel needs to
+ * verify that it is safe to use the device tree blob passed from userspace.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int __weak arch_kexec_verify_buffer(enum kexec_file_type type, const void *buf,
+				    unsigned long size)
+{
+	return -EINVAL;
+}
+
 /*
  * In file mode list of segments is prepared by kernel. Copy relevant
  * data from user space, do error checking, prepare segment list
@@ -123,7 +142,8 @@ void kimage_file_post_load_cleanup(struct kimage *image)
 static int
 kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 			     const char __user *cmdline_ptr,
-			     unsigned long cmdline_len, unsigned flags)
+			     unsigned long cmdline_len, unsigned long flags,
+			     const struct kexec_fdset __user *ufdset)
 {
 	int ret = 0;
 	void *ldata;
@@ -160,6 +180,55 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 		image->initrd_buf_len = size;
 	}
 
+	if (flags & KEXEC_FILE_EXTRA_FDS) {
+		int nr_fds, i;
+		size_t fdset_size;
+		char fdset_buf[MAX_FDSET_SIZE];
+		struct kexec_fdset *fdset = (struct kexec_fdset *) fdset_buf;
+
+		ret = copy_from_user(&nr_fds, ufdset, sizeof(int));
+		if (ret) {
+			ret = -EFAULT;
+			goto out;
+		}
+
+		if (nr_fds > KEXEC_SEGMENT_MAX) {
+			ret = -E2BIG;
+			goto out;
+		}
+
+		fdset_size = sizeof(struct kexec_fdset)
+				+ nr_fds * sizeof(struct kexec_file_fd);
+
+		ret = copy_from_user(fdset, ufdset, fdset_size);
+		if (ret) {
+			ret = -EFAULT;
+			goto out;
+		}
+
+		for (i = 0; i < fdset->nr_fds; i++) {
+			if (fdset->fds[i].type == KEXEC_FILE_TYPE_PARTIAL_DTB) {
+				ret = kernel_read_file_from_fd(fdset->fds[i].fd,
+						&image->dtb_buf, &size, INT_MAX,
+						READING_KEXEC_PARTIAL_DTB);
+				if (ret)
+					goto out;
+				image->dtb_buf_len = size;
+
+				ret = arch_kexec_verify_buffer(KEXEC_FILE_TYPE_PARTIAL_DTB,
+							       image->dtb_buf,
+							       image->dtb_buf_len);
+				if (ret)
+					goto out;
+			} else {
+				pr_debug("unknown file type %d failed.\n",
+						fdset->fds[i].type);
+				ret = -EINVAL;
+				goto out;
+			}
+		}
+	}
+
 	if (cmdline_len) {
 		image->cmdline_buf = kzalloc(cmdline_len, GFP_KERNEL);
 		if (!image->cmdline_buf) {
@@ -202,7 +271,8 @@ out:
 static int
 kimage_file_alloc_init(struct kimage **rimage, int kernel_fd,
 		       int initrd_fd, const char __user *cmdline_ptr,
-		       unsigned long cmdline_len, unsigned long flags)
+		       unsigned long cmdline_len, unsigned long flags,
+		       const struct kexec_fdset __user *ufdset)
 {
 	int ret;
 	struct kimage *image;
@@ -221,7 +291,8 @@ kimage_file_alloc_init(struct kimage **rimage, int kernel_fd,
 	}
 
 	ret = kimage_file_prepare_segments(image, kernel_fd, initrd_fd,
-					   cmdline_ptr, cmdline_len, flags);
+					   cmdline_ptr, cmdline_len, flags,
+					   ufdset);
 	if (ret)
 		goto out_free_image;
 
@@ -256,9 +327,9 @@ out_free_image:
 	return ret;
 }
 
-SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
+SYSCALL_DEFINE6(kexec_file_load, int, kernel_fd, int, initrd_fd,
 		unsigned long, cmdline_len, const char __user *, cmdline_ptr,
-		unsigned long, flags)
+		unsigned long, flags, const struct kexec_fdset __user *, ufdset)
 {
 	int ret = 0, i;
 	struct kimage **dest_image, *image;
@@ -295,7 +366,7 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
 		kimage_free(xchg(&kexec_crash_image, NULL));
 
 	ret = kimage_file_alloc_init(&image, kernel_fd, initrd_fd, cmdline_ptr,
-				     cmdline_len, flags);
+				     cmdline_len, flags, ufdset);
 	if (ret)
 		goto out;
 
-- 
1.9.1

[PATCH v2 3/3] kexec: extend kexec_file_load system call

[Thiago Jung Bauermann]

Hi,

Am Dienstag, 26 Juli 2016, 21:24:29 schrieb Thiago Jung Bauermann:
> Notes:
>     This is a new version of the last patch in this series which adds
>     a function where each architecture can verify if the DTB is safe
>     to load:
> 
>     int __weak arch_kexec_verify_buffer(enum kexec_file_type type,
>                                         const void *buf,
>                                         unsigned long size)
>     {
>             return -EINVAL;
>     }
> 
>     I will then provide an implementation in my powerpc patch series
>     which checks that the DTB only contains nodes and properties from a
>     whitelist. arch_kexec_kernel_image_load will copy these properties
>     to the device tree blob the kernel was booted with (and perform
>     other changes such as setting /chosen/bootargs, of course).

Is this approach ok? If so, I'll post a patch next week adding an 
arch_kexec_verify_buffer hook for powerpc to enforce the whitelist, and also 
a new version of the patches implementing kexec_file_load for powerpc on top 
of this series.

Eric, does this address your concerns?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center