2024-02-20 19:22:07

by Jann Horn

[permalink] [raw]
Subject: [PATCH 0/3] avoid unnecessary recompilations in x86 boot code

It's been bugging me that every time I rebuild the kernel, kaslr.o and
misc.o get rebuilt just because they pull in one or two things from some
headers that change on every build. So this series moves them into a
separate file that should be faster to build.

This doesn't seem to actually make a difference in terms of wall clock
time, I think because these compiler invocations run in parallel with
kernel compression; but when I tested with an earlier version of this
patch series, I saw something like a 500ms reduction in CPU time used.

Not exactly a major win, and I guess CPU time isn't really the metric
that matters here, but still, I think this makes sense as a cleanup?


Jann Horn (3):
x86/boot: fix KASLR hashing to use full input
x86/boot: avoid recompiling misc.c for incremental rebuilds
x86/boot: avoid recompiling kaslr.c for incremental rebuilds

arch/x86/boot/compressed/Makefile | 4 ++--
arch/x86/boot/compressed/dynamic_vars.c | 17 +++++++++++++++
arch/x86/boot/compressed/dynamic_vars.h | 14 ++++++++++++
arch/x86/boot/compressed/kaslr.c | 29 +++++++++++++++----------
arch/x86/boot/compressed/misc.c | 6 ++---
5 files changed, 53 insertions(+), 17 deletions(-)
create mode 100644 arch/x86/boot/compressed/dynamic_vars.c
create mode 100644 arch/x86/boot/compressed/dynamic_vars.h

--
2.44.0.rc0.258.g7320e95886-goog



2024-02-20 19:22:17

by Jann Horn

[permalink] [raw]
Subject: [PATCH 1/3] x86/boot: fix KASLR hashing to use full input

rotate_xor() currently ignores up to 7 bytes of input. That likely doesn't
really matter but it's still kinda wrong, so fix it.

Signed-off-by: Jann Horn <[email protected]>
---
arch/x86/boot/compressed/kaslr.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index dec961c6d16a..3ede59ad67eb 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -42,17 +42,30 @@ extern unsigned long get_cmd_line_ptr(void);
static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;

+static unsigned long rotate_xor_one(unsigned long hash, unsigned long val)
+{
+ /* Rotate by odd number of bits and XOR. */
+ hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
+ hash ^= val;
+ return hash;
+}
+
static unsigned long rotate_xor(unsigned long hash, const void *area,
size_t size)
{
size_t i;
unsigned long *ptr = (unsigned long *)area;
+ unsigned long rest = 0;
+
+ for (i = 0; i < size / sizeof(hash); i++)
+ hash = rotate_xor_one(hash, ptr[i]);

- for (i = 0; i < size / sizeof(hash); i++) {
- /* Rotate by odd number of bits and XOR. */
- hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
- hash ^= ptr[i];
+ i = i * sizeof(hash);
+ for (; i < size; i++) {
+ rest <<= 8;
+ rest |= ((unsigned char *)area)[i];
}
+ hash = rotate_xor_one(hash, rest);

return hash;
}
--
2.44.0.rc0.258.g7320e95886-goog


2024-02-20 19:22:36

by Jann Horn

[permalink] [raw]
Subject: [PATCH 2/3] x86/boot: avoid recompiling misc.c for incremental rebuilds

Currently, we rebuild misc.c on every incremental compilation because
the generated header voffset.h (containing kernel image offsets)
changes on every build.

Turn the three macros we need from that header into external variables
that we can separately store in another C file.

Signed-off-by: Jann Horn <[email protected]>
---
arch/x86/boot/compressed/Makefile | 4 ++--
arch/x86/boot/compressed/dynamic_vars.c | 9 +++++++++
arch/x86/boot/compressed/dynamic_vars.h | 11 +++++++++++
arch/x86/boot/compressed/misc.c | 6 ++----
4 files changed, 24 insertions(+), 6 deletions(-)
create mode 100644 arch/x86/boot/compressed/dynamic_vars.c
create mode 100644 arch/x86/boot/compressed/dynamic_vars.h

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index f19c038409aa..d18a553c16fa 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -94,11 +94,11 @@ targets += ../voffset.h
$(obj)/../voffset.h: vmlinux FORCE
$(call if_changed,voffset)

-$(obj)/misc.o: $(obj)/../voffset.h
+$(obj)/dynamic_vars.o: $(obj)/../voffset.h

vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/kernel_info.o $(obj)/head_$(BITS).o \
$(obj)/misc.o $(obj)/string.o $(obj)/cmdline.o $(obj)/error.o \
- $(obj)/piggy.o $(obj)/cpuflags.o
+ $(obj)/piggy.o $(obj)/cpuflags.o $(obj)/dynamic_vars.o

vmlinux-objs-$(CONFIG_EARLY_PRINTK) += $(obj)/early_serial_console.o
vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/kaslr.o
diff --git a/arch/x86/boot/compressed/dynamic_vars.c b/arch/x86/boot/compressed/dynamic_vars.c
new file mode 100644
index 000000000000..cda64ff4b6da
--- /dev/null
+++ b/arch/x86/boot/compressed/dynamic_vars.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/const.h>
+#include "dynamic_vars.h"
+#include "../voffset.h"
+
+const unsigned long vo__text = VO__text;
+const unsigned long vo___bss_start = VO___bss_start;
+const unsigned long vo__end = VO__end;
+const unsigned long kernel_total_size = VO__end - VO__text;
diff --git a/arch/x86/boot/compressed/dynamic_vars.h b/arch/x86/boot/compressed/dynamic_vars.h
new file mode 100644
index 000000000000..a0f7dc359cb6
--- /dev/null
+++ b/arch/x86/boot/compressed/dynamic_vars.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header defines some variables that change on every compilation and are
+ * stored separately in the small file dynamic-vars.c, so that we can avoid
+ * rebuilding some of the other C files in this directory on every incremental
+ * rebuild.
+ */
+
+/* Variables containing VO__text, VO___bss_start, VO__end */
+extern const unsigned long vo__text, vo___bss_start, vo__end;
+extern const unsigned long kernel_total_size;
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index b99e08e6815b..ff13cc3e703d 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -15,8 +15,8 @@
#include "misc.h"
#include "error.h"
#include "pgtable.h"
+#include "dynamic_vars.h"
#include "../string.h"
-#include "../voffset.h"
#include <asm/bootparam_utils.h>

/*
@@ -188,7 +188,7 @@ static void handle_relocations(void *output, unsigned long output_len,
int *reloc;
unsigned long delta, map, ptr;
unsigned long min_addr = (unsigned long)output;
- unsigned long max_addr = min_addr + (VO___bss_start - VO__text);
+ unsigned long max_addr = min_addr + (vo___bss_start - vo__text);

/*
* Calculate the delta between where vmlinux was linked to load
@@ -330,8 +330,6 @@ static size_t parse_elf(void *output)
return ehdr.e_entry - LOAD_PHYSICAL_ADDR;
}

-const unsigned long kernel_total_size = VO__end - VO__text;
-
static u8 boot_heap[BOOT_HEAP_SIZE] __aligned(4);

extern unsigned char input_data[];
--
2.44.0.rc0.258.g7320e95886-goog


2024-02-20 19:27:28

by Jann Horn

[permalink] [raw]
Subject: [PATCH 3/3] x86/boot: avoid recompiling kaslr.c for incremental rebuilds

Currently, every kernel rebuild needs to compile kaslr.c again because
UTS_VERSION changes on every rebuild.
Move the build string into a separate object file to speed things up.

Signed-off-by: Jann Horn <[email protected]>
---
arch/x86/boot/compressed/dynamic_vars.c | 8 ++++++++
arch/x86/boot/compressed/dynamic_vars.h | 3 +++
arch/x86/boot/compressed/kaslr.c | 10 ++--------
3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/arch/x86/boot/compressed/dynamic_vars.c b/arch/x86/boot/compressed/dynamic_vars.c
index cda64ff4b6da..15a57fbb05e3 100644
--- a/arch/x86/boot/compressed/dynamic_vars.c
+++ b/arch/x86/boot/compressed/dynamic_vars.c
@@ -1,9 +1,17 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/const.h>
#include "dynamic_vars.h"
+#include <generated/compile.h>
+#include <generated/utsrelease.h>
+#include <generated/utsversion.h>
#include "../voffset.h"

const unsigned long vo__text = VO__text;
const unsigned long vo___bss_start = VO___bss_start;
const unsigned long vo__end = VO__end;
const unsigned long kernel_total_size = VO__end - VO__text;
+
+/* Simplified build-specific string for starting entropy. */
+const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
+ LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
+unsigned long build_str_len = sizeof(build_str)-1;
diff --git a/arch/x86/boot/compressed/dynamic_vars.h b/arch/x86/boot/compressed/dynamic_vars.h
index a0f7dc359cb6..3ebc4a3144d4 100644
--- a/arch/x86/boot/compressed/dynamic_vars.h
+++ b/arch/x86/boot/compressed/dynamic_vars.h
@@ -9,3 +9,6 @@
/* Variables containing VO__text, VO___bss_start, VO__end */
extern const unsigned long vo__text, vo___bss_start, vo__end;
extern const unsigned long kernel_total_size;
+
+extern const char build_str[];
+extern unsigned long build_str_len;
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 3ede59ad67eb..c14e4e7a6b08 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -23,14 +23,12 @@
#include "error.h"
#include "../string.h"
#include "efi.h"
+#include "dynamic_vars.h"

-#include <generated/compile.h>
#include <linux/module.h>
#include <linux/uts.h>
#include <linux/utsname.h>
#include <linux/ctype.h>
-#include <generated/utsversion.h>
-#include <generated/utsrelease.h>

#define _SETUP
#include <asm/setup.h> /* For COMMAND_LINE_SIZE */
@@ -38,10 +36,6 @@

extern unsigned long get_cmd_line_ptr(void);

-/* Simplified build-specific string for starting entropy. */
-static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
- LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
-
static unsigned long rotate_xor_one(unsigned long hash, unsigned long val)
{
/* Rotate by odd number of bits and XOR. */
@@ -75,7 +69,7 @@ static unsigned long get_boot_seed(void)
{
unsigned long hash = 0;

- hash = rotate_xor(hash, build_str, sizeof(build_str));
+ hash = rotate_xor(hash, build_str, build_str_len);
hash = rotate_xor(hash, boot_params_ptr, sizeof(*boot_params_ptr));

return hash;
--
2.44.0.rc0.258.g7320e95886-goog


2024-02-20 19:34:55

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 3/3] x86/boot: avoid recompiling kaslr.c for incremental rebuilds

On Tue, Feb 20, 2024 at 08:21:44PM +0100, Jann Horn wrote:
> Currently, every kernel rebuild needs to compile kaslr.c again because
> UTS_VERSION changes on every rebuild.
> Move the build string into a separate object file to speed things up.

Heh, I think I don't see this because I force my UTS_VERSION to stay
still by default so I can do binary difference comparisons more easily. :P

args += ['KBUILD_BUILD_TIMESTAMP=1970-01-01']
args += ['KBUILD_BUILD_VERSION=1']
args += ['KBUILD_BUILD_USER=user']
args += ['KBUILD_BUILD_HOST=host']


--
Kees Cook

2024-02-23 23:52:15

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 3/3] x86/boot: avoid recompiling kaslr.c for incremental rebuilds

On Tue, Feb 20, 2024 at 08:21:44PM +0100, Jann Horn wrote:
> Currently, every kernel rebuild needs to compile kaslr.c again because
> UTS_VERSION changes on every rebuild.
> Move the build string into a separate object file to speed things up.
>
> Signed-off-by: Jann Horn <[email protected]>
> ---
> arch/x86/boot/compressed/dynamic_vars.c | 8 ++++++++
> arch/x86/boot/compressed/dynamic_vars.h | 3 +++
> arch/x86/boot/compressed/kaslr.c | 10 ++--------
> 3 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/dynamic_vars.c b/arch/x86/boot/compressed/dynamic_vars.c
> index cda64ff4b6da..15a57fbb05e3 100644
> --- a/arch/x86/boot/compressed/dynamic_vars.c
> +++ b/arch/x86/boot/compressed/dynamic_vars.c
> @@ -1,9 +1,17 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <linux/const.h>
> #include "dynamic_vars.h"
> +#include <generated/compile.h>
> +#include <generated/utsrelease.h>
> +#include <generated/utsversion.h>
> #include "../voffset.h"
>
> const unsigned long vo__text = VO__text;
> const unsigned long vo___bss_start = VO___bss_start;
> const unsigned long vo__end = VO__end;
> const unsigned long kernel_total_size = VO__end - VO__text;
> +
> +/* Simplified build-specific string for starting entropy. */
> +const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> + LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
> +unsigned long build_str_len = sizeof(build_str)-1;

This can be const too, yes? (Also, you didn't want to include the
trailing NUL in the xor?

Otherwise, yeah, I like this whole series.

--
Kees Cook