lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 12 Jul 2019 08:56:44 -0700
From:   hpa@...or.com
To:     Daniel Kiper <daniel.kiper@...cle.com>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, x86@...nel.org
CC:     bp@...en8.de, corbet@....net, dpsmith@...rtussolutions.com,
        eric.snowberg@...cle.com, kanth.ghatraju@...cle.com,
        konrad.wilk@...cle.com, mingo@...hat.com,
        ross.philipson@...cle.com, tglx@...utronix.de
Subject: Re: [PATCH v2 2/3] x86/boot: Introduce the setup_indirect

On July 4, 2019 9:36:11 AM PDT, Daniel Kiper <daniel.kiper@...cle.com> wrote:
>The setup_data is a bit awkward to use for extremely large data
>objects,
>both because the setup_data header has to be adjacent to the data
>object
>and because it has a 32-bit length field. However, it is important that
>intermediate stages of the boot process have a way to identify which
>chunks of memory are occupied by kernel data.
>
>Thus we introduce a uniform way to specify such indirect data as
>setup_indirect struct and SETUP_INDIRECT type.
>
>This patch does not bump setup_header version in arch/x86/boot/header.S
>because it will be followed by additional changes coming into the
>Linux/x86 boot protocol.
>
>Suggested-by: H. Peter Anvin <hpa@...or.com>
>Signed-off-by: Daniel Kiper <daniel.kiper@...cle.com>
>Reviewed-by: Eric Snowberg <eric.snowberg@...cle.com>
>Reviewed-by: Ross Philipson <ross.philipson@...cle.com>
>---
>v2 - suggestions/fixes:
>   - add setup_indirect usage example
>     (suggested by Eric Snowberg and Ross Philipson).
>---
>Documentation/x86/boot.rst            | 38
>++++++++++++++++++++++++++++++++++-
> arch/x86/include/uapi/asm/bootparam.h | 11 +++++++++-
> 2 files changed, 47 insertions(+), 2 deletions(-)
>
>diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
>index a934a56f0516..23d3726d54fc 100644
>--- a/Documentation/x86/boot.rst
>+++ b/Documentation/x86/boot.rst
>@@ -73,7 +73,7 @@ Protocol 2.14:	BURNT BY INCORRECT COMMIT
>ae7e1238e68f2a472a125673ab506d49158c188
> 		(x86/boot: Add ACPI RSDP address to setup_header)
> 		DO NOT USE!!! ASSUME SAME AS 2.13.
> 
>-Protocol 2.15:	(Kernel 5.3) Added the kernel_info.
>+Protocol 2.15:	(Kernel 5.3) Added the kernel_info and setup_indirect.
>=============	============================================================
> 
> .. note::
>@@ -827,6 +827,42 @@ Protocol:	2.09+
>   sure to consider the case where the linked list already contains
>   entries.
> 
>+  The setup_data is a bit awkward to use for extremely large data
>objects,
>+  both because the setup_data header has to be adjacent to the data
>object
>+  and because it has a 32-bit length field. However, it is important
>that
>+  intermediate stages of the boot process have a way to identify which
>+  chunks of memory are occupied by kernel data.
>+
>+  Thus setup_indirect struct and SETUP_INDIRECT type were introduced
>in
>+  protocol 2.15.
>+  
>+  struct setup_indirect {
>+    __u32 type;
>+    __u32 reserved;  /* Reserved, must be set to zero. */
>+    __u64 len;
>+    __u64 addr;
>+  };
>+
>+  The type member is itself simply a SETUP_* type. However, it cannot
>be
>+  SETUP_INDIRECT since making the setup_indirect a tree structure
>could
>+  require a lot of stack space in something that needs to parse it and
>+  stack space can be limited in boot contexts.
>+
>+  Let's give an example how to point to SETUP_E820_EXT data using
>setup_indirect.
>+  In this case setup_data and setup_indirect will look like this:
>+
>+  struct setup_data {
>+    __u64 next = 0 or <addr_of_next_setup_data_struct>;
>+    __u32 type = SETUP_INDIRECT;
>+    __u32 len = sizeof(setup_data);
>+    __u8 data[sizeof(setup_indirect)] = struct setup_indirect {
>+      __u32 type = SETUP_E820_EXT;
>+      __u32 reserved = 0;
>+      __u64 len = <len_of_SETUP_E820_EXT_data>;
>+      __u64 addr = <addr_of_SETUP_E820_EXT_data>;
>+    }
>+  }
>+
> ============	============
> Field name:	pref_address
> Type:		read (reloc)
>diff --git a/arch/x86/include/uapi/asm/bootparam.h
>b/arch/x86/include/uapi/asm/bootparam.h
>index b05318112452..aaaa17fa6ad6 100644
>--- a/arch/x86/include/uapi/asm/bootparam.h
>+++ b/arch/x86/include/uapi/asm/bootparam.h
>@@ -2,7 +2,7 @@
> #ifndef _ASM_X86_BOOTPARAM_H
> #define _ASM_X86_BOOTPARAM_H
> 
>-/* setup_data types */
>+/* setup_data/setup_indirect types */
> #define SETUP_NONE			0
> #define SETUP_E820_EXT			1
> #define SETUP_DTB			2
>@@ -10,6 +10,7 @@
> #define SETUP_EFI			4
> #define SETUP_APPLE_PROPERTIES		5
> #define SETUP_JAILHOUSE			6
>+#define SETUP_INDIRECT			7
> 
> /* ram_size flags */
> #define RAMDISK_IMAGE_START_MASK	0x07FF
>@@ -47,6 +48,14 @@ struct setup_data {
> 	__u8 data[0];
> };
> 
>+/* extensible setup indirect data node */
>+struct setup_indirect {
>+	__u32 type;
>+	__u32 reserved;  /* Reserved, must be set to zero. */
>+	__u64 len;
>+	__u64 addr;
>+};
>+
> struct setup_header {
> 	__u8	setup_sects;
> 	__u16	root_flags;

This needs actual implementation; we can't advertise it until the kernel knows how to consume the data! It probably should be moved to after the 3/3 patch.

Implementing this has two parts:

1. The kernel needs to be augmented so it can find current objects via indirection.

2. And this is the main reason for this in the first place: the early code needs to walk the list and map out the memory areas which are occupied so it doesn't clobber anything; this allows this code to be generic as opposed to having to know what is a pointer and what size it might point to.

(The decompressor didn't need this until kaslr entered the picture, but now it does, sadly.)

Optional/future enhancements that might be nice:

3. Add some kind of description (e.g. foo=u64 ; bar=str ; baz=blob) to make it possible to write a bootloader that can load these kinds of objects without specific enabling.

4. Add support for mapping initramfs fragments  this way.

5. Add support for passingload-on-boot kernel modules.

6. ... ?

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ