From: Ard Biesheuvel <ardb@kernel.org>

Apart from some sanity checks on the size of setup.bin, the only
remaining task carried out by the arch/x86/boot/tools/build.c build tool
is generating the CRC-32 checksum of the bzImage. This feature was added
in commit

  7d6e737c8d2698b6 ("x86: add a crc32 checksum to the kernel image.")

without any motivation (or any commit log text, for that matter). This
checksum is not verified by any known bootloader, and given that

a) the checksum of the entire bzImage is reported by most tools (zlib,
   rhash) as 0xffffffff and not 0x0 as documented,
b) the checksum is corrupted when the image is signed for secure boot,
   which means that no distro ships x86 images with valid CRCs,

it seems quite unlikely that this checksum is being used, so let's just
drop it, along with the tool that generates it.

Instead, use simple file concatenation and truncation to combine the two
pieces into bzImage, and replace the checks on the size of the setup
block with a couple of ASSERT()s in the linker script.

Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 Documentation/arch/x86/boot.rst        |  10 -
 arch/x86/boot/Makefile                 |   7 +-
 arch/x86/boot/compressed/vmlinux.lds.S |   3 +-
 arch/x86/boot/setup.ld                 |   2 +
 arch/x86/boot/tools/.gitignore         |   2 -
 arch/x86/boot/tools/build.c            | 247 --------------------
 6 files changed, 5 insertions(+), 266 deletions(-)

diff --git a/Documentation/arch/x86/boot.rst b/Documentation/arch/x86/boot.rst
index 76f53d3450e7..77e6163288db 100644
--- a/Documentation/arch/x86/boot.rst
+++ b/Documentation/arch/x86/boot.rst
@@ -1038,16 +1038,6 @@ Offset/size:	0x000c/4
   This field contains maximal allowed type for setup_data and setup_indirect structs.
 
 
-The Image Checksum
-==================
-
-From boot protocol version 2.08 onwards the CRC-32 is calculated over
-the entire file using the characteristic polynomial 0x04C11DB7 and an
-initial remainder of 0xffffffff.  The checksum is appended to the
-file; therefore the CRC of the file up to the limit specified in the
-syssize field of the header is always 0.
-
-
 The Kernel Command Line
 =======================
 
diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 9cc0ff6e9067..8589471b65a1 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -35,7 +35,6 @@ setup-y		+= video-vesa.o
 setup-y		+= video-bios.o
 
 targets		+= $(setup-y)
-hostprogs	:= tools/build
 hostprogs	+= mkcpustr
 
 HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
@@ -61,11 +60,9 @@ KBUILD_CFLAGS	+= $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
 $(obj)/bzImage: asflags-y  := $(SVGA_MODE)
 
 quiet_cmd_image = BUILD   $@
-silent_redirect_image = >/dev/null
-cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin \
-			       $(obj)/zoffset.h $@ $($(quiet)redirect_image)
+      cmd_image = cp $< $@; truncate -s %4K $@; cat $(obj)/vmlinux.bin >>$@
 
-$(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin $(obj)/tools/build FORCE
+$(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin FORCE
 	$(call if_changed,image)
 	@$(kecho) 'Kernel: $@ is ready' ' (#'$(or $(KBUILD_BUILD_VERSION),`cat .version`)')'
 
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 083ec6d7722a..48d0b5184557 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -48,8 +48,7 @@ SECTIONS
 		*(.data)
 		*(.data.*)
 
-		/* Add 4 bytes of extra space for a CRC-32 checksum */
-		. = ALIGN(. + 4, 0x200);
+		. = ALIGN(0x200);
 		_edata = . ;
 	}
 	. = ALIGN(L1_CACHE_BYTES);
diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
index 3a2d1360abb0..e1d594a60204 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -45,6 +45,8 @@ SECTIONS
 
 		setup_size = ALIGN(ABSOLUTE(.), 4096);
 		setup_sects = ABSOLUTE(setup_size / 512);
+		ASSERT(setup_sects >= 5, "The setup must be at least 5 sectors in size");
+		ASSERT(setup_sects <= 64, "The setup must be at most 64 sectors in size");
 	}
 
 	. = ALIGN(16);
diff --git a/arch/x86/boot/tools/.gitignore b/arch/x86/boot/tools/.gitignore
deleted file mode 100644
index ae91f4d0d78b..000000000000
--- a/arch/x86/boot/tools/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-build
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
deleted file mode 100644
index 10311d77c67f..000000000000
--- a/arch/x86/boot/tools/build.c
+++ /dev/null
@@ -1,247 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- *  Copyright (C) 1991, 1992  Linus Torvalds
- *  Copyright (C) 1997 Martin Mares
- *  Copyright (C) 2007 H. Peter Anvin
- */
-
-/*
- * This file builds a disk-image from three different files:
- *
- * - setup: 8086 machine code, sets up system parm
- * - system: 80386 code for actual system
- * - zoffset.h: header with ZO_* defines
- *
- * It does some checking that all files are of the correct type, and writes
- * the result to the specified destination, removing headers and padding to
- * the right amount. It also writes some system data to stdout.
- */
-
-/*
- * Changes by tytso to allow root device specification
- * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
- * Cross compiling fixes by Gertjan van Wingerde, July 1996
- * Rewritten by Martin Mares, April 1997
- * Substantially overhauled by H. Peter Anvin, April 2007
- */
-
-#include <stdio.h>
-#include <string.h>
-#include <stdlib.h>
-#include <stdarg.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <sys/mman.h>
-#include <tools/le_byteshift.h>
-
-typedef unsigned char  u8;
-typedef unsigned short u16;
-typedef unsigned int   u32;
-
-/* Minimal number of setup sectors */
-#define SETUP_SECT_MIN 5
-#define SETUP_SECT_MAX 64
-
-/* This must be large enough to hold the entire setup */
-u8 buf[SETUP_SECT_MAX*512];
-
-static unsigned long _edata;
-
-/*----------------------------------------------------------------------*/
-
-static const u32 crctab32[] = {
-	0x00000000, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419,
-	0x706af48f, 0xe963a535, 0x9e6495a3, 0x0edb8832, 0x79dcb8a4,
-	0xe0d5e91e, 0x97d2d988, 0x09b64c2b, 0x7eb17cbd, 0xe7b82d07,
-	0x90bf1d91, 0x1db71064, 0x6ab020f2, 0xf3b97148, 0x84be41de,
-	0x1adad47d, 0x6ddde4eb, 0xf4d4b551, 0x83d385c7, 0x136c9856,
-	0x646ba8c0, 0xfd62f97a, 0x8a65c9ec, 0x14015c4f, 0x63066cd9,
-	0xfa0f3d63, 0x8d080df5, 0x3b6e20c8, 0x4c69105e, 0xd56041e4,
-	0xa2677172, 0x3c03e4d1, 0x4b04d447, 0xd20d85fd, 0xa50ab56b,
-	0x35b5a8fa, 0x42b2986c, 0xdbbbc9d6, 0xacbcf940, 0x32d86ce3,
-	0x45df5c75, 0xdcd60dcf, 0xabd13d59, 0x26d930ac, 0x51de003a,
-	0xc8d75180, 0xbfd06116, 0x21b4f4b5, 0x56b3c423, 0xcfba9599,
-	0xb8bda50f, 0x2802b89e, 0x5f058808, 0xc60cd9b2, 0xb10be924,
-	0x2f6f7c87, 0x58684c11, 0xc1611dab, 0xb6662d3d, 0x76dc4190,
-	0x01db7106, 0x98d220bc, 0xefd5102a, 0x71b18589, 0x06b6b51f,
-	0x9fbfe4a5, 0xe8b8d433, 0x7807c9a2, 0x0f00f934, 0x9609a88e,
-	0xe10e9818, 0x7f6a0dbb, 0x086d3d2d, 0x91646c97, 0xe6635c01,
-	0x6b6b51f4, 0x1c6c6162, 0x856530d8, 0xf262004e, 0x6c0695ed,
-	0x1b01a57b, 0x8208f4c1, 0xf50fc457, 0x65b0d9c6, 0x12b7e950,
-	0x8bbeb8ea, 0xfcb9887c, 0x62dd1ddf, 0x15da2d49, 0x8cd37cf3,
-	0xfbd44c65, 0x4db26158, 0x3ab551ce, 0xa3bc0074, 0xd4bb30e2,
-	0x4adfa541, 0x3dd895d7, 0xa4d1c46d, 0xd3d6f4fb, 0x4369e96a,
-	0x346ed9fc, 0xad678846, 0xda60b8d0, 0x44042d73, 0x33031de5,
-	0xaa0a4c5f, 0xdd0d7cc9, 0x5005713c, 0x270241aa, 0xbe0b1010,
-	0xc90c2086, 0x5768b525, 0x206f85b3, 0xb966d409, 0xce61e49f,
-	0x5edef90e, 0x29d9c998, 0xb0d09822, 0xc7d7a8b4, 0x59b33d17,
-	0x2eb40d81, 0xb7bd5c3b, 0xc0ba6cad, 0xedb88320, 0x9abfb3b6,
-	0x03b6e20c, 0x74b1d29a, 0xead54739, 0x9dd277af, 0x04db2615,
-	0x73dc1683, 0xe3630b12, 0x94643b84, 0x0d6d6a3e, 0x7a6a5aa8,
-	0xe40ecf0b, 0x9309ff9d, 0x0a00ae27, 0x7d079eb1, 0xf00f9344,
-	0x8708a3d2, 0x1e01f268, 0x6906c2fe, 0xf762575d, 0x806567cb,
-	0x196c3671, 0x6e6b06e7, 0xfed41b76, 0x89d32be0, 0x10da7a5a,
-	0x67dd4acc, 0xf9b9df6f, 0x8ebeeff9, 0x17b7be43, 0x60b08ed5,
-	0xd6d6a3e8, 0xa1d1937e, 0x38d8c2c4, 0x4fdff252, 0xd1bb67f1,
-	0xa6bc5767, 0x3fb506dd, 0x48b2364b, 0xd80d2bda, 0xaf0a1b4c,
-	0x36034af6, 0x41047a60, 0xdf60efc3, 0xa867df55, 0x316e8eef,
-	0x4669be79, 0xcb61b38c, 0xbc66831a, 0x256fd2a0, 0x5268e236,
-	0xcc0c7795, 0xbb0b4703, 0x220216b9, 0x5505262f, 0xc5ba3bbe,
-	0xb2bd0b28, 0x2bb45a92, 0x5cb36a04, 0xc2d7ffa7, 0xb5d0cf31,
-	0x2cd99e8b, 0x5bdeae1d, 0x9b64c2b0, 0xec63f226, 0x756aa39c,
-	0x026d930a, 0x9c0906a9, 0xeb0e363f, 0x72076785, 0x05005713,
-	0x95bf4a82, 0xe2b87a14, 0x7bb12bae, 0x0cb61b38, 0x92d28e9b,
-	0xe5d5be0d, 0x7cdcefb7, 0x0bdbdf21, 0x86d3d2d4, 0xf1d4e242,
-	0x68ddb3f8, 0x1fda836e, 0x81be16cd, 0xf6b9265b, 0x6fb077e1,
-	0x18b74777, 0x88085ae6, 0xff0f6a70, 0x66063bca, 0x11010b5c,
-	0x8f659eff, 0xf862ae69, 0x616bffd3, 0x166ccf45, 0xa00ae278,
-	0xd70dd2ee, 0x4e048354, 0x3903b3c2, 0xa7672661, 0xd06016f7,
-	0x4969474d, 0x3e6e77db, 0xaed16a4a, 0xd9d65adc, 0x40df0b66,
-	0x37d83bf0, 0xa9bcae53, 0xdebb9ec5, 0x47b2cf7f, 0x30b5ffe9,
-	0xbdbdf21c, 0xcabac28a, 0x53b39330, 0x24b4a3a6, 0xbad03605,
-	0xcdd70693, 0x54de5729, 0x23d967bf, 0xb3667a2e, 0xc4614ab8,
-	0x5d681b02, 0x2a6f2b94, 0xb40bbe37, 0xc30c8ea1, 0x5a05df1b,
-	0x2d02ef8d
-};
-
-static u32 partial_crc32_one(u8 c, u32 crc)
-{
-	return crctab32[(crc ^ c) & 0xff] ^ (crc >> 8);
-}
-
-static u32 partial_crc32(const u8 *s, int len, u32 crc)
-{
-	while (len--)
-		crc = partial_crc32_one(*s++, crc);
-	return crc;
-}
-
-static void die(const char * str, ...)
-{
-	va_list args;
-	va_start(args, str);
-	vfprintf(stderr, str, args);
-	va_end(args);
-	fputc('\n', stderr);
-	exit(1);
-}
-
-static void usage(void)
-{
-	die("Usage: build setup system zoffset.h image");
-}
-
-/*
- * Parse zoffset.h and find the entry points. We could just #include zoffset.h
- * but that would mean tools/build would have to be rebuilt every time. It's
- * not as if parsing it is hard...
- */
-#define PARSE_ZOFS(p, sym) do { \
-	if (!strncmp(p, "#define ZO_" #sym " ", 11+sizeof(#sym)))	\
-		sym = strtoul(p + 11 + sizeof(#sym), NULL, 16);		\
-} while (0)
-
-static void parse_zoffset(char *fname)
-{
-	FILE *file;
-	char *p;
-	int c;
-
-	file = fopen(fname, "r");
-	if (!file)
-		die("Unable to open `%s': %m", fname);
-	c = fread(buf, 1, sizeof(buf) - 1, file);
-	if (ferror(file))
-		die("read-error on `zoffset.h'");
-	fclose(file);
-	buf[c] = 0;
-
-	p = (char *)buf;
-
-	while (p && *p) {
-		PARSE_ZOFS(p, _edata);
-
-		p = strchr(p, '\n');
-		while (p && (*p == '\r' || *p == '\n'))
-			p++;
-	}
-}
-
-int main(int argc, char ** argv)
-{
-	unsigned int i, sz, setup_sectors;
-	int c;
-	struct stat sb;
-	FILE *file, *dest;
-	int fd;
-	void *kernel;
-	u32 crc = 0xffffffffUL;
-
-	if (argc != 5)
-		usage();
-	parse_zoffset(argv[3]);
-
-	dest = fopen(argv[4], "w");
-	if (!dest)
-		die("Unable to write `%s': %m", argv[4]);
-
-	/* Copy the setup code */
-	file = fopen(argv[1], "r");
-	if (!file)
-		die("Unable to open `%s': %m", argv[1]);
-	c = fread(buf, 1, sizeof(buf), file);
-	if (ferror(file))
-		die("read-error on `setup'");
-	if (c < 1024)
-		die("The setup must be at least 1024 bytes");
-	if (get_unaligned_le16(&buf[510]) != 0xAA55)
-		die("Boot block hasn't got boot flag (0xAA55)");
-	fclose(file);
-
-	/* Pad unused space with zeros */
-	setup_sectors = (c + 4095) / 4096;
-	setup_sectors *= 8;
-	if (setup_sectors < SETUP_SECT_MIN)
-		setup_sectors = SETUP_SECT_MIN;
-	i = setup_sectors*512;
-	memset(buf+c, 0, i-c);
-
-	/* Open and stat the kernel file */
-	fd = open(argv[2], O_RDONLY);
-	if (fd < 0)
-		die("Unable to open `%s': %m", argv[2]);
-	if (fstat(fd, &sb))
-		die("Unable to stat `%s': %m", argv[2]);
-	if (_edata != sb.st_size)
-		die("Unexpected file size `%s': %u != %u", argv[2], _edata,
-		    sb.st_size);
-	sz = _edata - 4;
-	kernel = mmap(NULL, sz, PROT_READ, MAP_SHARED, fd, 0);
-	if (kernel == MAP_FAILED)
-		die("Unable to mmap '%s': %m", argv[2]);
-
-	crc = partial_crc32(buf, i, crc);
-	if (fwrite(buf, 1, i, dest) != i)
-		die("Writing setup failed");
-
-	/* Copy the kernel code */
-	crc = partial_crc32(kernel, sz, crc);
-	if (fwrite(kernel, 1, sz, dest) != sz)
-		die("Writing kernel failed");
-
-	/* Write the CRC */
-	put_unaligned_le32(crc, buf);
-	if (fwrite(buf, 1, 4, dest) != 4)
-		die("Writing CRC failed");
-
-	/* Catch any delayed write failures */
-	if (fclose(dest))
-		die("Writing image failed");
-
-	close(fd);
-
-	/* Everything is OK */
-	return 0;
-}
-- 
2.49.0.rc0.332.g42c0ae87b1-goog

Ard Biesheuvel <ardb+git@google.com> writes:

> From: Ard Biesheuvel <ardb@kernel.org>
>
> Apart from some sanity checks on the size of setup.bin, the only
> remaining task carried out by the arch/x86/boot/tools/build.c build tool
> is generating the CRC-32 checksum of the bzImage. This feature was added
> in commit
>
>   7d6e737c8d2698b6 ("x86: add a crc32 checksum to the kernel image.")
>
> without any motivation (or any commit log text, for that matter).

Doing some history digging, it seems this was done for Xen:
https://lore.kernel.org/lkml/1202936100-30859-1-git-send-email-ijc@hellion.org.uk/T/#m263fdcf0ff3f31ca8e5cd619aa81c7735fa8be91

Let me Cc: a few Xen folks for visibility.

>  This
> checksum is not verified by any known bootloader, and given that
>
> a) the checksum of the entire bzImage is reported by most tools (zlib,
>    rhash) as 0xffffffff and not 0x0 as documented,
> b) the checksum is corrupted when the image is signed for secure boot,
>    which means that no distro ships x86 images with valid CRCs,

Note that at least Fedora/RHEL/... distros ship keyless HMAC hash
alongside vmlinuz binary, e.g. on my Fedora40:

$ cat /boot/.vmlinuz-6.12.6-100.fc40.x86_64.hmac
19aec080fbd05c5de42db486653a5d9e9d69b59ac8d216ea89850ccf1f538c08ff4ada450989ddf1be81e23fd3c99253939d0a2153d3fd270cc1ed561c04294e  vmlinuz-6.12.6-100.fc40.x86_64

and this is created _after_ SB signing. AFAIU, the need for this is
coming from FIPS requirements; this makes me wornder if the CRC-32 is an
early implementation of the same thing.

>
> it seems quite unlikely that this checksum is being used, so let's just
> drop it, along with the tool that generates it.
>
> Instead, use simple file concatenation and truncation to combine the two
> pieces into bzImage, and replace the checks on the size of the setup
> block with a couple of ASSERT()s in the linker script.
>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  Documentation/arch/x86/boot.rst        |  10 -
>  arch/x86/boot/Makefile                 |   7 +-
>  arch/x86/boot/compressed/vmlinux.lds.S |   3 +-
>  arch/x86/boot/setup.ld                 |   2 +
>  arch/x86/boot/tools/.gitignore         |   2 -
>  arch/x86/boot/tools/build.c            | 247 --------------------
>  6 files changed, 5 insertions(+), 266 deletions(-)
>
> diff --git a/Documentation/arch/x86/boot.rst b/Documentation/arch/x86/boot.rst
> index 76f53d3450e7..77e6163288db 100644
> --- a/Documentation/arch/x86/boot.rst
> +++ b/Documentation/arch/x86/boot.rst
> @@ -1038,16 +1038,6 @@ Offset/size:	0x000c/4
>    This field contains maximal allowed type for setup_data and setup_indirect structs.
>  
>  
> -The Image Checksum
> -==================
> -
> -From boot protocol version 2.08 onwards the CRC-32 is calculated over
> -the entire file using the characteristic polynomial 0x04C11DB7 and an
> -initial remainder of 0xffffffff.  The checksum is appended to the
> -file; therefore the CRC of the file up to the limit specified in the
> -syssize field of the header is always 0.
> -
> -
>  The Kernel Command Line
>  =======================
>  
> diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
> index 9cc0ff6e9067..8589471b65a1 100644
> --- a/arch/x86/boot/Makefile
> +++ b/arch/x86/boot/Makefile
> @@ -35,7 +35,6 @@ setup-y		+= video-vesa.o
>  setup-y		+= video-bios.o
>  
>  targets		+= $(setup-y)
> -hostprogs	:= tools/build
>  hostprogs	+= mkcpustr
>  
>  HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
> @@ -61,11 +60,9 @@ KBUILD_CFLAGS	+= $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>  $(obj)/bzImage: asflags-y  := $(SVGA_MODE)
>  
>  quiet_cmd_image = BUILD   $@
> -silent_redirect_image = >/dev/null
> -cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin \
> -			       $(obj)/zoffset.h $@ $($(quiet)redirect_image)
> +      cmd_image = cp $< $@; truncate -s %4K $@; cat $(obj)/vmlinux.bin >>$@
>  
> -$(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin $(obj)/tools/build FORCE
> +$(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin FORCE
>  	$(call if_changed,image)
>  	@$(kecho) 'Kernel: $@ is ready' ' (#'$(or $(KBUILD_BUILD_VERSION),`cat .version`)')'
>  
> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> index 083ec6d7722a..48d0b5184557 100644
> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> @@ -48,8 +48,7 @@ SECTIONS
>  		*(.data)
>  		*(.data.*)
>  
> -		/* Add 4 bytes of extra space for a CRC-32 checksum */
> -		. = ALIGN(. + 4, 0x200);
> +		. = ALIGN(0x200);
>  		_edata = . ;
>  	}
>  	. = ALIGN(L1_CACHE_BYTES);
> diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
> index 3a2d1360abb0..e1d594a60204 100644
> --- a/arch/x86/boot/setup.ld
> +++ b/arch/x86/boot/setup.ld
> @@ -45,6 +45,8 @@ SECTIONS
>  
>  		setup_size = ALIGN(ABSOLUTE(.), 4096);
>  		setup_sects = ABSOLUTE(setup_size / 512);
> +		ASSERT(setup_sects >= 5, "The setup must be at least 5 sectors in size");
> +		ASSERT(setup_sects <= 64, "The setup must be at most 64 sectors in size");
>  	}
>  
>  	. = ALIGN(16);
> diff --git a/arch/x86/boot/tools/.gitignore b/arch/x86/boot/tools/.gitignore
> deleted file mode 100644
> index ae91f4d0d78b..000000000000
> --- a/arch/x86/boot/tools/.gitignore
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0-only
> -build
> diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
> deleted file mode 100644
> index 10311d77c67f..000000000000
> --- a/arch/x86/boot/tools/build.c
> +++ /dev/null
> @@ -1,247 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - *  Copyright (C) 1991, 1992  Linus Torvalds
> - *  Copyright (C) 1997 Martin Mares
> - *  Copyright (C) 2007 H. Peter Anvin
> - */
> -
> -/*
> - * This file builds a disk-image from three different files:
> - *
> - * - setup: 8086 machine code, sets up system parm
> - * - system: 80386 code for actual system
> - * - zoffset.h: header with ZO_* defines
> - *
> - * It does some checking that all files are of the correct type, and writes
> - * the result to the specified destination, removing headers and padding to
> - * the right amount. It also writes some system data to stdout.
> - */
> -
> -/*
> - * Changes by tytso to allow root device specification
> - * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
> - * Cross compiling fixes by Gertjan van Wingerde, July 1996
> - * Rewritten by Martin Mares, April 1997
> - * Substantially overhauled by H. Peter Anvin, April 2007
> - */
> -
> -#include <stdio.h>
> -#include <string.h>
> -#include <stdlib.h>
> -#include <stdarg.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <unistd.h>
> -#include <fcntl.h>
> -#include <sys/mman.h>
> -#include <tools/le_byteshift.h>
> -
> -typedef unsigned char  u8;
> -typedef unsigned short u16;
> -typedef unsigned int   u32;
> -
> -/* Minimal number of setup sectors */
> -#define SETUP_SECT_MIN 5
> -#define SETUP_SECT_MAX 64
> -
> -/* This must be large enough to hold the entire setup */
> -u8 buf[SETUP_SECT_MAX*512];
> -
> -static unsigned long _edata;
> -
> -/*----------------------------------------------------------------------*/
> -
> -static const u32 crctab32[] = {
> -	0x00000000, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419,
> -	0x706af48f, 0xe963a535, 0x9e6495a3, 0x0edb8832, 0x79dcb8a4,
> -	0xe0d5e91e, 0x97d2d988, 0x09b64c2b, 0x7eb17cbd, 0xe7b82d07,
> -	0x90bf1d91, 0x1db71064, 0x6ab020f2, 0xf3b97148, 0x84be41de,
> -	0x1adad47d, 0x6ddde4eb, 0xf4d4b551, 0x83d385c7, 0x136c9856,
> -	0x646ba8c0, 0xfd62f97a, 0x8a65c9ec, 0x14015c4f, 0x63066cd9,
> -	0xfa0f3d63, 0x8d080df5, 0x3b6e20c8, 0x4c69105e, 0xd56041e4,
> -	0xa2677172, 0x3c03e4d1, 0x4b04d447, 0xd20d85fd, 0xa50ab56b,
> -	0x35b5a8fa, 0x42b2986c, 0xdbbbc9d6, 0xacbcf940, 0x32d86ce3,
> -	0x45df5c75, 0xdcd60dcf, 0xabd13d59, 0x26d930ac, 0x51de003a,
> -	0xc8d75180, 0xbfd06116, 0x21b4f4b5, 0x56b3c423, 0xcfba9599,
> -	0xb8bda50f, 0x2802b89e, 0x5f058808, 0xc60cd9b2, 0xb10be924,
> -	0x2f6f7c87, 0x58684c11, 0xc1611dab, 0xb6662d3d, 0x76dc4190,
> -	0x01db7106, 0x98d220bc, 0xefd5102a, 0x71b18589, 0x06b6b51f,
> -	0x9fbfe4a5, 0xe8b8d433, 0x7807c9a2, 0x0f00f934, 0x9609a88e,
> -	0xe10e9818, 0x7f6a0dbb, 0x086d3d2d, 0x91646c97, 0xe6635c01,
> -	0x6b6b51f4, 0x1c6c6162, 0x856530d8, 0xf262004e, 0x6c0695ed,
> -	0x1b01a57b, 0x8208f4c1, 0xf50fc457, 0x65b0d9c6, 0x12b7e950,
> -	0x8bbeb8ea, 0xfcb9887c, 0x62dd1ddf, 0x15da2d49, 0x8cd37cf3,
> -	0xfbd44c65, 0x4db26158, 0x3ab551ce, 0xa3bc0074, 0xd4bb30e2,
> -	0x4adfa541, 0x3dd895d7, 0xa4d1c46d, 0xd3d6f4fb, 0x4369e96a,
> -	0x346ed9fc, 0xad678846, 0xda60b8d0, 0x44042d73, 0x33031de5,
> -	0xaa0a4c5f, 0xdd0d7cc9, 0x5005713c, 0x270241aa, 0xbe0b1010,
> -	0xc90c2086, 0x5768b525, 0x206f85b3, 0xb966d409, 0xce61e49f,
> -	0x5edef90e, 0x29d9c998, 0xb0d09822, 0xc7d7a8b4, 0x59b33d17,
> -	0x2eb40d81, 0xb7bd5c3b, 0xc0ba6cad, 0xedb88320, 0x9abfb3b6,
> -	0x03b6e20c, 0x74b1d29a, 0xead54739, 0x9dd277af, 0x04db2615,
> -	0x73dc1683, 0xe3630b12, 0x94643b84, 0x0d6d6a3e, 0x7a6a5aa8,
> -	0xe40ecf0b, 0x9309ff9d, 0x0a00ae27, 0x7d079eb1, 0xf00f9344,
> -	0x8708a3d2, 0x1e01f268, 0x6906c2fe, 0xf762575d, 0x806567cb,
> -	0x196c3671, 0x6e6b06e7, 0xfed41b76, 0x89d32be0, 0x10da7a5a,
> -	0x67dd4acc, 0xf9b9df6f, 0x8ebeeff9, 0x17b7be43, 0x60b08ed5,
> -	0xd6d6a3e8, 0xa1d1937e, 0x38d8c2c4, 0x4fdff252, 0xd1bb67f1,
> -	0xa6bc5767, 0x3fb506dd, 0x48b2364b, 0xd80d2bda, 0xaf0a1b4c,
> -	0x36034af6, 0x41047a60, 0xdf60efc3, 0xa867df55, 0x316e8eef,
> -	0x4669be79, 0xcb61b38c, 0xbc66831a, 0x256fd2a0, 0x5268e236,
> -	0xcc0c7795, 0xbb0b4703, 0x220216b9, 0x5505262f, 0xc5ba3bbe,
> -	0xb2bd0b28, 0x2bb45a92, 0x5cb36a04, 0xc2d7ffa7, 0xb5d0cf31,
> -	0x2cd99e8b, 0x5bdeae1d, 0x9b64c2b0, 0xec63f226, 0x756aa39c,
> -	0x026d930a, 0x9c0906a9, 0xeb0e363f, 0x72076785, 0x05005713,
> -	0x95bf4a82, 0xe2b87a14, 0x7bb12bae, 0x0cb61b38, 0x92d28e9b,
> -	0xe5d5be0d, 0x7cdcefb7, 0x0bdbdf21, 0x86d3d2d4, 0xf1d4e242,
> -	0x68ddb3f8, 0x1fda836e, 0x81be16cd, 0xf6b9265b, 0x6fb077e1,
> -	0x18b74777, 0x88085ae6, 0xff0f6a70, 0x66063bca, 0x11010b5c,
> -	0x8f659eff, 0xf862ae69, 0x616bffd3, 0x166ccf45, 0xa00ae278,
> -	0xd70dd2ee, 0x4e048354, 0x3903b3c2, 0xa7672661, 0xd06016f7,
> -	0x4969474d, 0x3e6e77db, 0xaed16a4a, 0xd9d65adc, 0x40df0b66,
> -	0x37d83bf0, 0xa9bcae53, 0xdebb9ec5, 0x47b2cf7f, 0x30b5ffe9,
> -	0xbdbdf21c, 0xcabac28a, 0x53b39330, 0x24b4a3a6, 0xbad03605,
> -	0xcdd70693, 0x54de5729, 0x23d967bf, 0xb3667a2e, 0xc4614ab8,
> -	0x5d681b02, 0x2a6f2b94, 0xb40bbe37, 0xc30c8ea1, 0x5a05df1b,
> -	0x2d02ef8d
> -};
> -
> -static u32 partial_crc32_one(u8 c, u32 crc)
> -{
> -	return crctab32[(crc ^ c) & 0xff] ^ (crc >> 8);
> -}
> -
> -static u32 partial_crc32(const u8 *s, int len, u32 crc)
> -{
> -	while (len--)
> -		crc = partial_crc32_one(*s++, crc);
> -	return crc;
> -}
> -
> -static void die(const char * str, ...)
> -{
> -	va_list args;
> -	va_start(args, str);
> -	vfprintf(stderr, str, args);
> -	va_end(args);
> -	fputc('\n', stderr);
> -	exit(1);
> -}
> -
> -static void usage(void)
> -{
> -	die("Usage: build setup system zoffset.h image");
> -}
> -
> -/*
> - * Parse zoffset.h and find the entry points. We could just #include zoffset.h
> - * but that would mean tools/build would have to be rebuilt every time. It's
> - * not as if parsing it is hard...
> - */
> -#define PARSE_ZOFS(p, sym) do { \
> -	if (!strncmp(p, "#define ZO_" #sym " ", 11+sizeof(#sym)))	\
> -		sym = strtoul(p + 11 + sizeof(#sym), NULL, 16);		\
> -} while (0)
> -
> -static void parse_zoffset(char *fname)
> -{
> -	FILE *file;
> -	char *p;
> -	int c;
> -
> -	file = fopen(fname, "r");
> -	if (!file)
> -		die("Unable to open `%s': %m", fname);
> -	c = fread(buf, 1, sizeof(buf) - 1, file);
> -	if (ferror(file))
> -		die("read-error on `zoffset.h'");
> -	fclose(file);
> -	buf[c] = 0;
> -
> -	p = (char *)buf;
> -
> -	while (p && *p) {
> -		PARSE_ZOFS(p, _edata);
> -
> -		p = strchr(p, '\n');
> -		while (p && (*p == '\r' || *p == '\n'))
> -			p++;
> -	}
> -}
> -
> -int main(int argc, char ** argv)
> -{
> -	unsigned int i, sz, setup_sectors;
> -	int c;
> -	struct stat sb;
> -	FILE *file, *dest;
> -	int fd;
> -	void *kernel;
> -	u32 crc = 0xffffffffUL;
> -
> -	if (argc != 5)
> -		usage();
> -	parse_zoffset(argv[3]);
> -
> -	dest = fopen(argv[4], "w");
> -	if (!dest)
> -		die("Unable to write `%s': %m", argv[4]);
> -
> -	/* Copy the setup code */
> -	file = fopen(argv[1], "r");
> -	if (!file)
> -		die("Unable to open `%s': %m", argv[1]);
> -	c = fread(buf, 1, sizeof(buf), file);
> -	if (ferror(file))
> -		die("read-error on `setup'");
> -	if (c < 1024)
> -		die("The setup must be at least 1024 bytes");
> -	if (get_unaligned_le16(&buf[510]) != 0xAA55)
> -		die("Boot block hasn't got boot flag (0xAA55)");
> -	fclose(file);
> -
> -	/* Pad unused space with zeros */
> -	setup_sectors = (c + 4095) / 4096;
> -	setup_sectors *= 8;
> -	if (setup_sectors < SETUP_SECT_MIN)
> -		setup_sectors = SETUP_SECT_MIN;
> -	i = setup_sectors*512;
> -	memset(buf+c, 0, i-c);
> -
> -	/* Open and stat the kernel file */
> -	fd = open(argv[2], O_RDONLY);
> -	if (fd < 0)
> -		die("Unable to open `%s': %m", argv[2]);
> -	if (fstat(fd, &sb))
> -		die("Unable to stat `%s': %m", argv[2]);
> -	if (_edata != sb.st_size)
> -		die("Unexpected file size `%s': %u != %u", argv[2], _edata,
> -		    sb.st_size);
> -	sz = _edata - 4;
> -	kernel = mmap(NULL, sz, PROT_READ, MAP_SHARED, fd, 0);
> -	if (kernel == MAP_FAILED)
> -		die("Unable to mmap '%s': %m", argv[2]);
> -
> -	crc = partial_crc32(buf, i, crc);
> -	if (fwrite(buf, 1, i, dest) != i)
> -		die("Writing setup failed");
> -
> -	/* Copy the kernel code */
> -	crc = partial_crc32(kernel, sz, crc);
> -	if (fwrite(kernel, 1, sz, dest) != sz)
> -		die("Writing kernel failed");
> -
> -	/* Write the CRC */
> -	put_unaligned_le32(crc, buf);
> -	if (fwrite(buf, 1, 4, dest) != 4)
> -		die("Writing CRC failed");
> -
> -	/* Catch any delayed write failures */
> -	if (fclose(dest))
> -		die("Writing image failed");
> -
> -	close(fd);
> -
> -	/* Everything is OK */
> -	return 0;
> -}

-- 
Vitaly

On March 11, 2025 9:59:08 AM PDT, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>Ard Biesheuvel <ardb+git@google.com> writes:
>
>> From: Ard Biesheuvel <ardb@kernel.org>
>>
>> Apart from some sanity checks on the size of setup.bin, the only
>> remaining task carried out by the arch/x86/boot/tools/build.c build tool
>> is generating the CRC-32 checksum of the bzImage. This feature was added
>> in commit
>>
>>   7d6e737c8d2698b6 ("x86: add a crc32 checksum to the kernel image.")
>>
>> without any motivation (or any commit log text, for that matter).
>
>Doing some history digging, it seems this was done for Xen:
>https://lore.kernel.org/lkml/1202936100-30859-1-git-send-email-ijc@hellion.org.uk/T/#m263fdcf0ff3f31ca8e5cd619aa81c7735fa8be91
>
>Let me Cc: a few Xen folks for visibility.
>
>>  This
>> checksum is not verified by any known bootloader, and given that
>>
>> a) the checksum of the entire bzImage is reported by most tools (zlib,
>>    rhash) as 0xffffffff and not 0x0 as documented,
>> b) the checksum is corrupted when the image is signed for secure boot,
>>    which means that no distro ships x86 images with valid CRCs,
>
>Note that at least Fedora/RHEL/... distros ship keyless HMAC hash
>alongside vmlinuz binary, e.g. on my Fedora40:
>
>$ cat /boot/.vmlinuz-6.12.6-100.fc40.x86_64.hmac
>19aec080fbd05c5de42db486653a5d9e9d69b59ac8d216ea89850ccf1f538c08ff4ada450989ddf1be81e23fd3c99253939d0a2153d3fd270cc1ed561c04294e  vmlinuz-6.12.6-100.fc40.x86_64
>
>and this is created _after_ SB signing. AFAIU, the need for this is
>coming from FIPS requirements; this makes me wornder if the CRC-32 is an
>early implementation of the same thing.
>
>>
>> it seems quite unlikely that this checksum is being used, so let's just
>> drop it, along with the tool that generates it.
>>
>> Instead, use simple file concatenation and truncation to combine the two
>> pieces into bzImage, and replace the checks on the size of the setup
>> block with a couple of ASSERT()s in the linker script.
>>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>> ---
>>  Documentation/arch/x86/boot.rst        |  10 -
>>  arch/x86/boot/Makefile                 |   7 +-
>>  arch/x86/boot/compressed/vmlinux.lds.S |   3 +-
>>  arch/x86/boot/setup.ld                 |   2 +
>>  arch/x86/boot/tools/.gitignore         |   2 -
>>  arch/x86/boot/tools/build.c            | 247 --------------------
>>  6 files changed, 5 insertions(+), 266 deletions(-)
>>
>> diff --git a/Documentation/arch/x86/boot.rst b/Documentation/arch/x86/boot.rst
>> index 76f53d3450e7..77e6163288db 100644
>> --- a/Documentation/arch/x86/boot.rst
>> +++ b/Documentation/arch/x86/boot.rst
>> @@ -1038,16 +1038,6 @@ Offset/size:	0x000c/4
>>    This field contains maximal allowed type for setup_data and setup_indirect structs.
>>  
>>  
>> -The Image Checksum
>> -==================
>> -
>> -From boot protocol version 2.08 onwards the CRC-32 is calculated over
>> -the entire file using the characteristic polynomial 0x04C11DB7 and an
>> -initial remainder of 0xffffffff.  The checksum is appended to the
>> -file; therefore the CRC of the file up to the limit specified in the
>> -syssize field of the header is always 0.
>> -
>> -
>>  The Kernel Command Line
>>  =======================
>>  
>> diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
>> index 9cc0ff6e9067..8589471b65a1 100644
>> --- a/arch/x86/boot/Makefile
>> +++ b/arch/x86/boot/Makefile
>> @@ -35,7 +35,6 @@ setup-y		+= video-vesa.o
>>  setup-y		+= video-bios.o
>>  
>>  targets		+= $(setup-y)
>> -hostprogs	:= tools/build
>>  hostprogs	+= mkcpustr
>>  
>>  HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
>> @@ -61,11 +60,9 @@ KBUILD_CFLAGS	+= $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>>  $(obj)/bzImage: asflags-y  := $(SVGA_MODE)
>>  
>>  quiet_cmd_image = BUILD   $@
>> -silent_redirect_image = >/dev/null
>> -cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin \
>> -			       $(obj)/zoffset.h $@ $($(quiet)redirect_image)
>> +      cmd_image = cp $< $@; truncate -s %4K $@; cat $(obj)/vmlinux.bin >>$@
>>  
>> -$(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin $(obj)/tools/build FORCE
>> +$(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin FORCE
>>  	$(call if_changed,image)
>>  	@$(kecho) 'Kernel: $@ is ready' ' (#'$(or $(KBUILD_BUILD_VERSION),`cat .version`)')'
>>  
>> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
>> index 083ec6d7722a..48d0b5184557 100644
>> --- a/arch/x86/boot/compressed/vmlinux.lds.S
>> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
>> @@ -48,8 +48,7 @@ SECTIONS
>>  		*(.data)
>>  		*(.data.*)
>>  
>> -		/* Add 4 bytes of extra space for a CRC-32 checksum */
>> -		. = ALIGN(. + 4, 0x200);
>> +		. = ALIGN(0x200);
>>  		_edata = . ;
>>  	}
>>  	. = ALIGN(L1_CACHE_BYTES);
>> diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
>> index 3a2d1360abb0..e1d594a60204 100644
>> --- a/arch/x86/boot/setup.ld
>> +++ b/arch/x86/boot/setup.ld
>> @@ -45,6 +45,8 @@ SECTIONS
>>  
>>  		setup_size = ALIGN(ABSOLUTE(.), 4096);
>>  		setup_sects = ABSOLUTE(setup_size / 512);
>> +		ASSERT(setup_sects >= 5, "The setup must be at least 5 sectors in size");
>> +		ASSERT(setup_sects <= 64, "The setup must be at most 64 sectors in size");
>>  	}
>>  
>>  	. = ALIGN(16);
>> diff --git a/arch/x86/boot/tools/.gitignore b/arch/x86/boot/tools/.gitignore
>> deleted file mode 100644
>> index ae91f4d0d78b..000000000000
>> --- a/arch/x86/boot/tools/.gitignore
>> +++ /dev/null
>> @@ -1,2 +0,0 @@
>> -# SPDX-License-Identifier: GPL-2.0-only
>> -build
>> diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
>> deleted file mode 100644
>> index 10311d77c67f..000000000000
>> --- a/arch/x86/boot/tools/build.c
>> +++ /dev/null
>> @@ -1,247 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0
>> -/*
>> - *  Copyright (C) 1991, 1992  Linus Torvalds
>> - *  Copyright (C) 1997 Martin Mares
>> - *  Copyright (C) 2007 H. Peter Anvin
>> - */
>> -
>> -/*
>> - * This file builds a disk-image from three different files:
>> - *
>> - * - setup: 8086 machine code, sets up system parm
>> - * - system: 80386 code for actual system
>> - * - zoffset.h: header with ZO_* defines
>> - *
>> - * It does some checking that all files are of the correct type, and writes
>> - * the result to the specified destination, removing headers and padding to
>> - * the right amount. It also writes some system data to stdout.
>> - */
>> -
>> -/*
>> - * Changes by tytso to allow root device specification
>> - * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
>> - * Cross compiling fixes by Gertjan van Wingerde, July 1996
>> - * Rewritten by Martin Mares, April 1997
>> - * Substantially overhauled by H. Peter Anvin, April 2007
>> - */
>> -
>> -#include <stdio.h>
>> -#include <string.h>
>> -#include <stdlib.h>
>> -#include <stdarg.h>
>> -#include <sys/types.h>
>> -#include <sys/stat.h>
>> -#include <unistd.h>
>> -#include <fcntl.h>
>> -#include <sys/mman.h>
>> -#include <tools/le_byteshift.h>
>> -
>> -typedef unsigned char  u8;
>> -typedef unsigned short u16;
>> -typedef unsigned int   u32;
>> -
>> -/* Minimal number of setup sectors */
>> -#define SETUP_SECT_MIN 5
>> -#define SETUP_SECT_MAX 64
>> -
>> -/* This must be large enough to hold the entire setup */
>> -u8 buf[SETUP_SECT_MAX*512];
>> -
>> -static unsigned long _edata;
>> -
>> -/*----------------------------------------------------------------------*/
>> -
>> -static const u32 crctab32[] = {
>> -	0x00000000, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419,
>> -	0x706af48f, 0xe963a535, 0x9e6495a3, 0x0edb8832, 0x79dcb8a4,
>> -	0xe0d5e91e, 0x97d2d988, 0x09b64c2b, 0x7eb17cbd, 0xe7b82d07,
>> -	0x90bf1d91, 0x1db71064, 0x6ab020f2, 0xf3b97148, 0x84be41de,
>> -	0x1adad47d, 0x6ddde4eb, 0xf4d4b551, 0x83d385c7, 0x136c9856,
>> -	0x646ba8c0, 0xfd62f97a, 0x8a65c9ec, 0x14015c4f, 0x63066cd9,
>> -	0xfa0f3d63, 0x8d080df5, 0x3b6e20c8, 0x4c69105e, 0xd56041e4,
>> -	0xa2677172, 0x3c03e4d1, 0x4b04d447, 0xd20d85fd, 0xa50ab56b,
>> -	0x35b5a8fa, 0x42b2986c, 0xdbbbc9d6, 0xacbcf940, 0x32d86ce3,
>> -	0x45df5c75, 0xdcd60dcf, 0xabd13d59, 0x26d930ac, 0x51de003a,
>> -	0xc8d75180, 0xbfd06116, 0x21b4f4b5, 0x56b3c423, 0xcfba9599,
>> -	0xb8bda50f, 0x2802b89e, 0x5f058808, 0xc60cd9b2, 0xb10be924,
>> -	0x2f6f7c87, 0x58684c11, 0xc1611dab, 0xb6662d3d, 0x76dc4190,
>> -	0x01db7106, 0x98d220bc, 0xefd5102a, 0x71b18589, 0x06b6b51f,
>> -	0x9fbfe4a5, 0xe8b8d433, 0x7807c9a2, 0x0f00f934, 0x9609a88e,
>> -	0xe10e9818, 0x7f6a0dbb, 0x086d3d2d, 0x91646c97, 0xe6635c01,
>> -	0x6b6b51f4, 0x1c6c6162, 0x856530d8, 0xf262004e, 0x6c0695ed,
>> -	0x1b01a57b, 0x8208f4c1, 0xf50fc457, 0x65b0d9c6, 0x12b7e950,
>> -	0x8bbeb8ea, 0xfcb9887c, 0x62dd1ddf, 0x15da2d49, 0x8cd37cf3,
>> -	0xfbd44c65, 0x4db26158, 0x3ab551ce, 0xa3bc0074, 0xd4bb30e2,
>> -	0x4adfa541, 0x3dd895d7, 0xa4d1c46d, 0xd3d6f4fb, 0x4369e96a,
>> -	0x346ed9fc, 0xad678846, 0xda60b8d0, 0x44042d73, 0x33031de5,
>> -	0xaa0a4c5f, 0xdd0d7cc9, 0x5005713c, 0x270241aa, 0xbe0b1010,
>> -	0xc90c2086, 0x5768b525, 0x206f85b3, 0xb966d409, 0xce61e49f,
>> -	0x5edef90e, 0x29d9c998, 0xb0d09822, 0xc7d7a8b4, 0x59b33d17,
>> -	0x2eb40d81, 0xb7bd5c3b, 0xc0ba6cad, 0xedb88320, 0x9abfb3b6,
>> -	0x03b6e20c, 0x74b1d29a, 0xead54739, 0x9dd277af, 0x04db2615,
>> -	0x73dc1683, 0xe3630b12, 0x94643b84, 0x0d6d6a3e, 0x7a6a5aa8,
>> -	0xe40ecf0b, 0x9309ff9d, 0x0a00ae27, 0x7d079eb1, 0xf00f9344,
>> -	0x8708a3d2, 0x1e01f268, 0x6906c2fe, 0xf762575d, 0x806567cb,
>> -	0x196c3671, 0x6e6b06e7, 0xfed41b76, 0x89d32be0, 0x10da7a5a,
>> -	0x67dd4acc, 0xf9b9df6f, 0x8ebeeff9, 0x17b7be43, 0x60b08ed5,
>> -	0xd6d6a3e8, 0xa1d1937e, 0x38d8c2c4, 0x4fdff252, 0xd1bb67f1,
>> -	0xa6bc5767, 0x3fb506dd, 0x48b2364b, 0xd80d2bda, 0xaf0a1b4c,
>> -	0x36034af6, 0x41047a60, 0xdf60efc3, 0xa867df55, 0x316e8eef,
>> -	0x4669be79, 0xcb61b38c, 0xbc66831a, 0x256fd2a0, 0x5268e236,
>> -	0xcc0c7795, 0xbb0b4703, 0x220216b9, 0x5505262f, 0xc5ba3bbe,
>> -	0xb2bd0b28, 0x2bb45a92, 0x5cb36a04, 0xc2d7ffa7, 0xb5d0cf31,
>> -	0x2cd99e8b, 0x5bdeae1d, 0x9b64c2b0, 0xec63f226, 0x756aa39c,
>> -	0x026d930a, 0x9c0906a9, 0xeb0e363f, 0x72076785, 0x05005713,
>> -	0x95bf4a82, 0xe2b87a14, 0x7bb12bae, 0x0cb61b38, 0x92d28e9b,
>> -	0xe5d5be0d, 0x7cdcefb7, 0x0bdbdf21, 0x86d3d2d4, 0xf1d4e242,
>> -	0x68ddb3f8, 0x1fda836e, 0x81be16cd, 0xf6b9265b, 0x6fb077e1,
>> -	0x18b74777, 0x88085ae6, 0xff0f6a70, 0x66063bca, 0x11010b5c,
>> -	0x8f659eff, 0xf862ae69, 0x616bffd3, 0x166ccf45, 0xa00ae278,
>> -	0xd70dd2ee, 0x4e048354, 0x3903b3c2, 0xa7672661, 0xd06016f7,
>> -	0x4969474d, 0x3e6e77db, 0xaed16a4a, 0xd9d65adc, 0x40df0b66,
>> -	0x37d83bf0, 0xa9bcae53, 0xdebb9ec5, 0x47b2cf7f, 0x30b5ffe9,
>> -	0xbdbdf21c, 0xcabac28a, 0x53b39330, 0x24b4a3a6, 0xbad03605,
>> -	0xcdd70693, 0x54de5729, 0x23d967bf, 0xb3667a2e, 0xc4614ab8,
>> -	0x5d681b02, 0x2a6f2b94, 0xb40bbe37, 0xc30c8ea1, 0x5a05df1b,
>> -	0x2d02ef8d
>> -};
>> -
>> -static u32 partial_crc32_one(u8 c, u32 crc)
>> -{
>> -	return crctab32[(crc ^ c) & 0xff] ^ (crc >> 8);
>> -}
>> -
>> -static u32 partial_crc32(const u8 *s, int len, u32 crc)
>> -{
>> -	while (len--)
>> -		crc = partial_crc32_one(*s++, crc);
>> -	return crc;
>> -}
>> -
>> -static void die(const char * str, ...)
>> -{
>> -	va_list args;
>> -	va_start(args, str);
>> -	vfprintf(stderr, str, args);
>> -	va_end(args);
>> -	fputc('\n', stderr);
>> -	exit(1);
>> -}
>> -
>> -static void usage(void)
>> -{
>> -	die("Usage: build setup system zoffset.h image");
>> -}
>> -
>> -/*
>> - * Parse zoffset.h and find the entry points. We could just #include zoffset.h
>> - * but that would mean tools/build would have to be rebuilt every time. It's
>> - * not as if parsing it is hard...
>> - */
>> -#define PARSE_ZOFS(p, sym) do { \
>> -	if (!strncmp(p, "#define ZO_" #sym " ", 11+sizeof(#sym)))	\
>> -		sym = strtoul(p + 11 + sizeof(#sym), NULL, 16);		\
>> -} while (0)
>> -
>> -static void parse_zoffset(char *fname)
>> -{
>> -	FILE *file;
>> -	char *p;
>> -	int c;
>> -
>> -	file = fopen(fname, "r");
>> -	if (!file)
>> -		die("Unable to open `%s': %m", fname);
>> -	c = fread(buf, 1, sizeof(buf) - 1, file);
>> -	if (ferror(file))
>> -		die("read-error on `zoffset.h'");
>> -	fclose(file);
>> -	buf[c] = 0;
>> -
>> -	p = (char *)buf;
>> -
>> -	while (p && *p) {
>> -		PARSE_ZOFS(p, _edata);
>> -
>> -		p = strchr(p, '\n');
>> -		while (p && (*p == '\r' || *p == '\n'))
>> -			p++;
>> -	}
>> -}
>> -
>> -int main(int argc, char ** argv)
>> -{
>> -	unsigned int i, sz, setup_sectors;
>> -	int c;
>> -	struct stat sb;
>> -	FILE *file, *dest;
>> -	int fd;
>> -	void *kernel;
>> -	u32 crc = 0xffffffffUL;
>> -
>> -	if (argc != 5)
>> -		usage();
>> -	parse_zoffset(argv[3]);
>> -
>> -	dest = fopen(argv[4], "w");
>> -	if (!dest)
>> -		die("Unable to write `%s': %m", argv[4]);
>> -
>> -	/* Copy the setup code */
>> -	file = fopen(argv[1], "r");
>> -	if (!file)
>> -		die("Unable to open `%s': %m", argv[1]);
>> -	c = fread(buf, 1, sizeof(buf), file);
>> -	if (ferror(file))
>> -		die("read-error on `setup'");
>> -	if (c < 1024)
>> -		die("The setup must be at least 1024 bytes");
>> -	if (get_unaligned_le16(&buf[510]) != 0xAA55)
>> -		die("Boot block hasn't got boot flag (0xAA55)");
>> -	fclose(file);
>> -
>> -	/* Pad unused space with zeros */
>> -	setup_sectors = (c + 4095) / 4096;
>> -	setup_sectors *= 8;
>> -	if (setup_sectors < SETUP_SECT_MIN)
>> -		setup_sectors = SETUP_SECT_MIN;
>> -	i = setup_sectors*512;
>> -	memset(buf+c, 0, i-c);
>> -
>> -	/* Open and stat the kernel file */
>> -	fd = open(argv[2], O_RDONLY);
>> -	if (fd < 0)
>> -		die("Unable to open `%s': %m", argv[2]);
>> -	if (fstat(fd, &sb))
>> -		die("Unable to stat `%s': %m", argv[2]);
>> -	if (_edata != sb.st_size)
>> -		die("Unexpected file size `%s': %u != %u", argv[2], _edata,
>> -		    sb.st_size);
>> -	sz = _edata - 4;
>> -	kernel = mmap(NULL, sz, PROT_READ, MAP_SHARED, fd, 0);
>> -	if (kernel == MAP_FAILED)
>> -		die("Unable to mmap '%s': %m", argv[2]);
>> -
>> -	crc = partial_crc32(buf, i, crc);
>> -	if (fwrite(buf, 1, i, dest) != i)
>> -		die("Writing setup failed");
>> -
>> -	/* Copy the kernel code */
>> -	crc = partial_crc32(kernel, sz, crc);
>> -	if (fwrite(kernel, 1, sz, dest) != sz)
>> -		die("Writing kernel failed");
>> -
>> -	/* Write the CRC */
>> -	put_unaligned_le32(crc, buf);
>> -	if (fwrite(buf, 1, 4, dest) != 4)
>> -		die("Writing CRC failed");
>> -
>> -	/* Catch any delayed write failures */
>> -	if (fclose(dest))
>> -		die("Writing image failed");
>> -
>> -	close(fd);
>> -
>> -	/* Everything is OK */
>> -	return 0;
>> -}
>

Please leave the bytes in question as explicit zeroes if possible.

On Tue, 11 Mar 2025 at 18:14, H. Peter Anvin <hpa@zytor.com> wrote:
>
> >Ard Biesheuvel <ardb+git@google.com> writes:
> >
...
> >> it seems quite unlikely that this checksum is being used, so let's just
> >> drop it, along with the tool that generates it.
> >>
> >> Instead, use simple file concatenation and truncation to combine the two
> >> pieces into bzImage, and replace the checks on the size of the setup
> >> block with a couple of ASSERT()s in the linker script.
> >>
...
>
> Please leave the bytes in question as explicit zeroes if possible.

Keeping the

. = ALIGN(. + 4, 0x200);

in arch/x86/boot/compressed/vmlinux.lds.S should be sufficient to
guarantee that the last 4 bytes of the file are zero, so it is quite
trivial to implement. However, I'm not quite sure what purpose that
would serve: could you elaborate?

On March 11, 2025 10:25:15 AM PDT, Ard Biesheuvel <ardb@kernel.org> wrote:
>On Tue, 11 Mar 2025 at 18:14, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> >Ard Biesheuvel <ardb+git@google.com> writes:
>> >
>...
>> >> it seems quite unlikely that this checksum is being used, so let's just
>> >> drop it, along with the tool that generates it.
>> >>
>> >> Instead, use simple file concatenation and truncation to combine the two
>> >> pieces into bzImage, and replace the checks on the size of the setup
>> >> block with a couple of ASSERT()s in the linker script.
>> >>
>...
>>
>> Please leave the bytes in question as explicit zeroes if possible.
>
>Keeping the
>
>. = ALIGN(. + 4, 0x200);
>
>in arch/x86/boot/compressed/vmlinux.lds.S should be sufficient to
>guarantee that the last 4 bytes of the file are zero, so it is quite
>trivial to implement. However, I'm not quite sure what purpose that
>would serve: could you elaborate?

It means if someone *does* care it will be easier for them to adjust.

On Tue, 11 Mar 2025 at 18:29, H. Peter Anvin <hpa@zytor.com> wrote:
>
> On March 11, 2025 10:25:15 AM PDT, Ard Biesheuvel <ardb@kernel.org> wrote:
> >On Tue, 11 Mar 2025 at 18:14, H. Peter Anvin <hpa@zytor.com> wrote:
> >>
> >> >Ard Biesheuvel <ardb+git@google.com> writes:
> >> >
> >...
> >> >> it seems quite unlikely that this checksum is being used, so let's just
> >> >> drop it, along with the tool that generates it.
> >> >>
> >> >> Instead, use simple file concatenation and truncation to combine the two
> >> >> pieces into bzImage, and replace the checks on the size of the setup
> >> >> block with a couple of ASSERT()s in the linker script.
> >> >>
> >...
> >>
> >> Please leave the bytes in question as explicit zeroes if possible.
> >
> >Keeping the
> >
> >. = ALIGN(. + 4, 0x200);
> >
> >in arch/x86/boot/compressed/vmlinux.lds.S should be sufficient to
> >guarantee that the last 4 bytes of the file are zero, so it is quite
> >trivial to implement. However, I'm not quite sure what purpose that
> >would serve: could you elaborate?
>
> It means if someone *does* care it will be easier for them to adjust.

I.e., someone can always stick a CRC-32 into the last 4 bytes if they
wanted to? Yeah that makes sense.

On Tue, 11 Mar 2025 18:44:09 +0100
Ard Biesheuvel <ardb@kernel.org> wrote:

> On Tue, 11 Mar 2025 at 18:29, H. Peter Anvin <hpa@zytor.com> wrote:
> >
> > On March 11, 2025 10:25:15 AM PDT, Ard Biesheuvel <ardb@kernel.org> wrote:  
> > >On Tue, 11 Mar 2025 at 18:14, H. Peter Anvin <hpa@zytor.com> wrote:  
> > >>  
> > >> >Ard Biesheuvel <ardb+git@google.com> writes:
> > >> >  
> > >...  
> > >> >> it seems quite unlikely that this checksum is being used, so let's just
> > >> >> drop it, along with the tool that generates it.
> > >> >>
> > >> >> Instead, use simple file concatenation and truncation to combine the two
> > >> >> pieces into bzImage, and replace the checks on the size of the setup
> > >> >> block with a couple of ASSERT()s in the linker script.
> > >> >>  
> > >...  
> > >>
> > >> Please leave the bytes in question as explicit zeroes if possible.  
> > >
> > >Keeping the
> > >
> > >. = ALIGN(. + 4, 0x200);
> > >
> > >in arch/x86/boot/compressed/vmlinux.lds.S should be sufficient to
> > >guarantee that the last 4 bytes of the file are zero, so it is quite
> > >trivial to implement. However, I'm not quite sure what purpose that
> > >would serve: could you elaborate?  
> >
> > It means if someone *does* care it will be easier for them to adjust.  
> 
> I.e., someone can always stick a CRC-32 into the last 4 bytes if they
> wanted to? Yeah that makes sense.

I think you'd need a comment saying the last 4 bytes are reserved in case
anyone wants to add a hash.

	David

>

On March 11, 2025 10:44:09 AM PDT, Ard Biesheuvel <ardb@kernel.org> wrote:
>On Tue, 11 Mar 2025 at 18:29, H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On March 11, 2025 10:25:15 AM PDT, Ard Biesheuvel <ardb@kernel.org> wrote:
>> >On Tue, 11 Mar 2025 at 18:14, H. Peter Anvin <hpa@zytor.com> wrote:
>> >>
>> >> >Ard Biesheuvel <ardb+git@google.com> writes:
>> >> >
>> >...
>> >> >> it seems quite unlikely that this checksum is being used, so let's just
>> >> >> drop it, along with the tool that generates it.
>> >> >>
>> >> >> Instead, use simple file concatenation and truncation to combine the two
>> >> >> pieces into bzImage, and replace the checks on the size of the setup
>> >> >> block with a couple of ASSERT()s in the linker script.
>> >> >>
>> >...
>> >>
>> >> Please leave the bytes in question as explicit zeroes if possible.
>> >
>> >Keeping the
>> >
>> >. = ALIGN(. + 4, 0x200);
>> >
>> >in arch/x86/boot/compressed/vmlinux.lds.S should be sufficient to
>> >guarantee that the last 4 bytes of the file are zero, so it is quite
>> >trivial to implement. However, I'm not quite sure what purpose that
>> >would serve: could you elaborate?
>>
>> It means if someone *does* care it will be easier for them to adjust.
>
>I.e., someone can always stick a CRC-32 into the last 4 bytes if they
>wanted to? Yeah that makes sense.

Yes, or something can look for all zero and know it doesn't mean anything.

The following commit has been merged into the x86/build branch of tip:

Commit-ID:     9c54baab4401db249d6938806b812231e0259380
Gitweb:        https://git.kernel.org/tip/9c54baab4401db249d6938806b812231e0259380
Author:        Ard Biesheuvel <ardb@kernel.org>
AuthorDate:    Fri, 07 Mar 2025 17:48:02 +01:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Fri, 07 Mar 2025 23:59:10 +01:00

x86/boot: Drop CRC-32 checksum and the build tool that generates it

Apart from some sanity checks on the size of setup.bin, the only
remaining task carried out by the arch/x86/boot/tools/build.c build tool
is generating the CRC-32 checksum of the bzImage. This feature was added
in commit

  7d6e737c8d2698b6 ("x86: add a crc32 checksum to the kernel image.")

without any motivation (or any commit log text, for that matter). This
checksum is not verified by any known bootloader, and given that

 a) the checksum of the entire bzImage is reported by most tools (zlib,
    rhash) as 0xffffffff and not 0x0 as documented,

 b) the checksum is corrupted when the image is signed for secure boot,
    which means that no distro ships x86 images with valid CRCs,

it seems quite unlikely that this checksum is being used, so let's just
drop it, along with the tool that generates it.

Instead, use simple file concatenation and truncation to combine the two
pieces into bzImage, and replace the checks on the size of the setup
block with a couple of ASSERT()s in the linker script.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ian Campbell <ijc@hellion.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/20250307164801.885261-2-ardb+git@google.com
---
 Documentation/arch/x86/boot.rst        |  10 +-
 arch/x86/boot/Makefile                 |   7 +-
 arch/x86/boot/compressed/vmlinux.lds.S |   3 +-
 arch/x86/boot/setup.ld                 |   2 +-
 arch/x86/boot/tools/.gitignore         |   2 +-
 arch/x86/boot/tools/build.c            | 247 +------------------------
 6 files changed, 5 insertions(+), 266 deletions(-)
 delete mode 100644 arch/x86/boot/tools/.gitignore
 delete mode 100644 arch/x86/boot/tools/build.c

diff --git a/Documentation/arch/x86/boot.rst b/Documentation/arch/x86/boot.rst
index 76f53d3..77e6163 100644
--- a/Documentation/arch/x86/boot.rst
+++ b/Documentation/arch/x86/boot.rst
@@ -1038,16 +1038,6 @@ Offset/size:	0x000c/4
   This field contains maximal allowed type for setup_data and setup_indirect structs.
 
 
-The Image Checksum
-==================
-
-From boot protocol version 2.08 onwards the CRC-32 is calculated over
-the entire file using the characteristic polynomial 0x04C11DB7 and an
-initial remainder of 0xffffffff.  The checksum is appended to the
-file; therefore the CRC of the file up to the limit specified in the
-syssize field of the header is always 0.
-
-
 The Kernel Command Line
 =======================
 
diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 9cc0ff6..8589471 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -35,7 +35,6 @@ setup-y		+= video-vesa.o
 setup-y		+= video-bios.o
 
 targets		+= $(setup-y)
-hostprogs	:= tools/build
 hostprogs	+= mkcpustr
 
 HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
@@ -61,11 +60,9 @@ KBUILD_CFLAGS	+= $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
 $(obj)/bzImage: asflags-y  := $(SVGA_MODE)
 
 quiet_cmd_image = BUILD   $@
-silent_redirect_image = >/dev/null
-cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin \
-			       $(obj)/zoffset.h $@ $($(quiet)redirect_image)
+      cmd_image = cp $< $@; truncate -s %4K $@; cat $(obj)/vmlinux.bin >>$@
 
-$(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin $(obj)/tools/build FORCE
+$(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin FORCE
 	$(call if_changed,image)
 	@$(kecho) 'Kernel: $@ is ready' ' (#'$(or $(KBUILD_BUILD_VERSION),`cat .version`)')'
 
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 083ec6d..48d0b51 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -48,8 +48,7 @@ SECTIONS
 		*(.data)
 		*(.data.*)
 
-		/* Add 4 bytes of extra space for a CRC-32 checksum */
-		. = ALIGN(. + 4, 0x200);
+		. = ALIGN(0x200);
 		_edata = . ;
 	}
 	. = ALIGN(L1_CACHE_BYTES);
diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
index 3a2d136..e1d594a 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -45,6 +45,8 @@ SECTIONS
 
 		setup_size = ALIGN(ABSOLUTE(.), 4096);
 		setup_sects = ABSOLUTE(setup_size / 512);
+		ASSERT(setup_sects >= 5, "The setup must be at least 5 sectors in size");
+		ASSERT(setup_sects <= 64, "The setup must be at most 64 sectors in size");
 	}
 
 	. = ALIGN(16);
diff --git a/arch/x86/boot/tools/.gitignore b/arch/x86/boot/tools/.gitignore
deleted file mode 100644
index ae91f4d..0000000
--- a/arch/x86/boot/tools/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-build
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
deleted file mode 100644
index 10311d7..0000000
--- a/arch/x86/boot/tools/build.c
+++ /dev/null
@@ -1,247 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- *  Copyright (C) 1991, 1992  Linus Torvalds
- *  Copyright (C) 1997 Martin Mares
- *  Copyright (C) 2007 H. Peter Anvin
- */
-
-/*
- * This file builds a disk-image from three different files:
- *
- * - setup: 8086 machine code, sets up system parm
- * - system: 80386 code for actual system
- * - zoffset.h: header with ZO_* defines
- *
- * It does some checking that all files are of the correct type, and writes
- * the result to the specified destination, removing headers and padding to
- * the right amount. It also writes some system data to stdout.
- */
-
-/*
- * Changes by tytso to allow root device specification
- * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
- * Cross compiling fixes by Gertjan van Wingerde, July 1996
- * Rewritten by Martin Mares, April 1997
- * Substantially overhauled by H. Peter Anvin, April 2007
- */
-
-#include <stdio.h>
-#include <string.h>
-#include <stdlib.h>
-#include <stdarg.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <sys/mman.h>
-#include <tools/le_byteshift.h>
-
-typedef unsigned char  u8;
-typedef unsigned short u16;
-typedef unsigned int   u32;
-
-/* Minimal number of setup sectors */
-#define SETUP_SECT_MIN 5
-#define SETUP_SECT_MAX 64
-
-/* This must be large enough to hold the entire setup */
-u8 buf[SETUP_SECT_MAX*512];
-
-static unsigned long _edata;
-
-/*----------------------------------------------------------------------*/
-
-static const u32 crctab32[] = {
-	0x00000000, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419,
-	0x706af48f, 0xe963a535, 0x9e6495a3, 0x0edb8832, 0x79dcb8a4,
-	0xe0d5e91e, 0x97d2d988, 0x09b64c2b, 0x7eb17cbd, 0xe7b82d07,
-	0x90bf1d91, 0x1db71064, 0x6ab020f2, 0xf3b97148, 0x84be41de,
-	0x1adad47d, 0x6ddde4eb, 0xf4d4b551, 0x83d385c7, 0x136c9856,
-	0x646ba8c0, 0xfd62f97a, 0x8a65c9ec, 0x14015c4f, 0x63066cd9,
-	0xfa0f3d63, 0x8d080df5, 0x3b6e20c8, 0x4c69105e, 0xd56041e4,
-	0xa2677172, 0x3c03e4d1, 0x4b04d447, 0xd20d85fd, 0xa50ab56b,
-	0x35b5a8fa, 0x42b2986c, 0xdbbbc9d6, 0xacbcf940, 0x32d86ce3,
-	0x45df5c75, 0xdcd60dcf, 0xabd13d59, 0x26d930ac, 0x51de003a,
-	0xc8d75180, 0xbfd06116, 0x21b4f4b5, 0x56b3c423, 0xcfba9599,
-	0xb8bda50f, 0x2802b89e, 0x5f058808, 0xc60cd9b2, 0xb10be924,
-	0x2f6f7c87, 0x58684c11, 0xc1611dab, 0xb6662d3d, 0x76dc4190,
-	0x01db7106, 0x98d220bc, 0xefd5102a, 0x71b18589, 0x06b6b51f,
-	0x9fbfe4a5, 0xe8b8d433, 0x7807c9a2, 0x0f00f934, 0x9609a88e,
-	0xe10e9818, 0x7f6a0dbb, 0x086d3d2d, 0x91646c97, 0xe6635c01,
-	0x6b6b51f4, 0x1c6c6162, 0x856530d8, 0xf262004e, 0x6c0695ed,
-	0x1b01a57b, 0x8208f4c1, 0xf50fc457, 0x65b0d9c6, 0x12b7e950,
-	0x8bbeb8ea, 0xfcb9887c, 0x62dd1ddf, 0x15da2d49, 0x8cd37cf3,
-	0xfbd44c65, 0x4db26158, 0x3ab551ce, 0xa3bc0074, 0xd4bb30e2,
-	0x4adfa541, 0x3dd895d7, 0xa4d1c46d, 0xd3d6f4fb, 0x4369e96a,
-	0x346ed9fc, 0xad678846, 0xda60b8d0, 0x44042d73, 0x33031de5,
-	0xaa0a4c5f, 0xdd0d7cc9, 0x5005713c, 0x270241aa, 0xbe0b1010,
-	0xc90c2086, 0x5768b525, 0x206f85b3, 0xb966d409, 0xce61e49f,
-	0x5edef90e, 0x29d9c998, 0xb0d09822, 0xc7d7a8b4, 0x59b33d17,
-	0x2eb40d81, 0xb7bd5c3b, 0xc0ba6cad, 0xedb88320, 0x9abfb3b6,
-	0x03b6e20c, 0x74b1d29a, 0xead54739, 0x9dd277af, 0x04db2615,
-	0x73dc1683, 0xe3630b12, 0x94643b84, 0x0d6d6a3e, 0x7a6a5aa8,
-	0xe40ecf0b, 0x9309ff9d, 0x0a00ae27, 0x7d079eb1, 0xf00f9344,
-	0x8708a3d2, 0x1e01f268, 0x6906c2fe, 0xf762575d, 0x806567cb,
-	0x196c3671, 0x6e6b06e7, 0xfed41b76, 0x89d32be0, 0x10da7a5a,
-	0x67dd4acc, 0xf9b9df6f, 0x8ebeeff9, 0x17b7be43, 0x60b08ed5,
-	0xd6d6a3e8, 0xa1d1937e, 0x38d8c2c4, 0x4fdff252, 0xd1bb67f1,
-	0xa6bc5767, 0x3fb506dd, 0x48b2364b, 0xd80d2bda, 0xaf0a1b4c,
-	0x36034af6, 0x41047a60, 0xdf60efc3, 0xa867df55, 0x316e8eef,
-	0x4669be79, 0xcb61b38c, 0xbc66831a, 0x256fd2a0, 0x5268e236,
-	0xcc0c7795, 0xbb0b4703, 0x220216b9, 0x5505262f, 0xc5ba3bbe,
-	0xb2bd0b28, 0x2bb45a92, 0x5cb36a04, 0xc2d7ffa7, 0xb5d0cf31,
-	0x2cd99e8b, 0x5bdeae1d, 0x9b64c2b0, 0xec63f226, 0x756aa39c,
-	0x026d930a, 0x9c0906a9, 0xeb0e363f, 0x72076785, 0x05005713,
-	0x95bf4a82, 0xe2b87a14, 0x7bb12bae, 0x0cb61b38, 0x92d28e9b,
-	0xe5d5be0d, 0x7cdcefb7, 0x0bdbdf21, 0x86d3d2d4, 0xf1d4e242,
-	0x68ddb3f8, 0x1fda836e, 0x81be16cd, 0xf6b9265b, 0x6fb077e1,
-	0x18b74777, 0x88085ae6, 0xff0f6a70, 0x66063bca, 0x11010b5c,
-	0x8f659eff, 0xf862ae69, 0x616bffd3, 0x166ccf45, 0xa00ae278,
-	0xd70dd2ee, 0x4e048354, 0x3903b3c2, 0xa7672661, 0xd06016f7,
-	0x4969474d, 0x3e6e77db, 0xaed16a4a, 0xd9d65adc, 0x40df0b66,
-	0x37d83bf0, 0xa9bcae53, 0xdebb9ec5, 0x47b2cf7f, 0x30b5ffe9,
-	0xbdbdf21c, 0xcabac28a, 0x53b39330, 0x24b4a3a6, 0xbad03605,
-	0xcdd70693, 0x54de5729, 0x23d967bf, 0xb3667a2e, 0xc4614ab8,
-	0x5d681b02, 0x2a6f2b94, 0xb40bbe37, 0xc30c8ea1, 0x5a05df1b,
-	0x2d02ef8d
-};
-
-static u32 partial_crc32_one(u8 c, u32 crc)
-{
-	return crctab32[(crc ^ c) & 0xff] ^ (crc >> 8);
-}
-
-static u32 partial_crc32(const u8 *s, int len, u32 crc)
-{
-	while (len--)
-		crc = partial_crc32_one(*s++, crc);
-	return crc;
-}
-
-static void die(const char * str, ...)
-{
-	va_list args;
-	va_start(args, str);
-	vfprintf(stderr, str, args);
-	va_end(args);
-	fputc('\n', stderr);
-	exit(1);
-}
-
-static void usage(void)
-{
-	die("Usage: build setup system zoffset.h image");
-}
-
-/*
- * Parse zoffset.h and find the entry points. We could just #include zoffset.h
- * but that would mean tools/build would have to be rebuilt every time. It's
- * not as if parsing it is hard...
- */
-#define PARSE_ZOFS(p, sym) do { \
-	if (!strncmp(p, "#define ZO_" #sym " ", 11+sizeof(#sym)))	\
-		sym = strtoul(p + 11 + sizeof(#sym), NULL, 16);		\
-} while (0)
-
-static void parse_zoffset(char *fname)
-{
-	FILE *file;
-	char *p;
-	int c;
-
-	file = fopen(fname, "r");
-	if (!file)
-		die("Unable to open `%s': %m", fname);
-	c = fread(buf, 1, sizeof(buf) - 1, file);
-	if (ferror(file))
-		die("read-error on `zoffset.h'");
-	fclose(file);
-	buf[c] = 0;
-
-	p = (char *)buf;
-
-	while (p && *p) {
-		PARSE_ZOFS(p, _edata);
-
-		p = strchr(p, '\n');
-		while (p && (*p == '\r' || *p == '\n'))
-			p++;
-	}
-}
-
-int main(int argc, char ** argv)
-{
-	unsigned int i, sz, setup_sectors;
-	int c;
-	struct stat sb;
-	FILE *file, *dest;
-	int fd;
-	void *kernel;
-	u32 crc = 0xffffffffUL;
-
-	if (argc != 5)
-		usage();
-	parse_zoffset(argv[3]);
-
-	dest = fopen(argv[4], "w");
-	if (!dest)
-		die("Unable to write `%s': %m", argv[4]);
-
-	/* Copy the setup code */
-	file = fopen(argv[1], "r");
-	if (!file)
-		die("Unable to open `%s': %m", argv[1]);
-	c = fread(buf, 1, sizeof(buf), file);
-	if (ferror(file))
-		die("read-error on `setup'");
-	if (c < 1024)
-		die("The setup must be at least 1024 bytes");
-	if (get_unaligned_le16(&buf[510]) != 0xAA55)
-		die("Boot block hasn't got boot flag (0xAA55)");
-	fclose(file);
-
-	/* Pad unused space with zeros */
-	setup_sectors = (c + 4095) / 4096;
-	setup_sectors *= 8;
-	if (setup_sectors < SETUP_SECT_MIN)
-		setup_sectors = SETUP_SECT_MIN;
-	i = setup_sectors*512;
-	memset(buf+c, 0, i-c);
-
-	/* Open and stat the kernel file */
-	fd = open(argv[2], O_RDONLY);
-	if (fd < 0)
-		die("Unable to open `%s': %m", argv[2]);
-	if (fstat(fd, &sb))
-		die("Unable to stat `%s': %m", argv[2]);
-	if (_edata != sb.st_size)
-		die("Unexpected file size `%s': %u != %u", argv[2], _edata,
-		    sb.st_size);
-	sz = _edata - 4;
-	kernel = mmap(NULL, sz, PROT_READ, MAP_SHARED, fd, 0);
-	if (kernel == MAP_FAILED)
-		die("Unable to mmap '%s': %m", argv[2]);
-
-	crc = partial_crc32(buf, i, crc);
-	if (fwrite(buf, 1, i, dest) != i)
-		die("Writing setup failed");
-
-	/* Copy the kernel code */
-	crc = partial_crc32(kernel, sz, crc);
-	if (fwrite(kernel, 1, sz, dest) != sz)
-		die("Writing kernel failed");
-
-	/* Write the CRC */
-	put_unaligned_le32(crc, buf);
-	if (fwrite(buf, 1, 4, dest) != 4)
-		die("Writing CRC failed");
-
-	/* Catch any delayed write failures */
-	if (fclose(dest))
-		die("Writing image failed");
-
-	close(fd);
-
-	/* Everything is OK */
-	return 0;
-}

On 3/7/25 15:11, tip-bot2 for Ard Biesheuvel wrote:
> x86/boot: Drop CRC-32 checksum and the build tool that generates it

This is breaking really early boot for me across a whole bunch of i386
and 64-bit builds. I had to bisect it because this does not seem like
the kind of thing that would break the boot. It's either hanging the
kernel *really* early, or making KVM angry:

KVM internal error. Suberror: 1
extra data[0]: 0x0000000000000000
extra data[1]: 0x0000000000000030
extra data[2]: 0x0000000000000784
extra data[3]: 0x0000000000000000
extra data[4]: 0x0000000000000000
extra data[5]: 0x0000000000000000
emulation failure
EAX=00000000 EBX=00000000 ECX=00000000 EDX=00083238
ESI=00000000 EDI=00000000 EBP=00000000 ESP=0000fff5
EIP=00008300 EFL=00010002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
...

I'm going to go start backing out individual hunks like the
arch/x86/boot/compressed/vmlinux.lds.S changes, but I figured I'd see if
anyone else is having problems or has more of a clue than I do.

The only weird thing I'm doing is booting the kernel with qemu's -kernel
argument.

On 4/11/25 12:33, Dave Hansen wrote:
...
> The only weird thing I'm doing is booting the kernel with qemu's -kernel
> argument.

I lied. I'm doing other weird things. I have a local script named
"truncate" that's not the same thing as /usr/bin/truncate. Guess what
this patch started doing:

>  quiet_cmd_image = BUILD   $@
> -silent_redirect_image = >/dev/null
> -cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin \
> -			       $(obj)/zoffset.h $@ $($(quiet)redirect_image)
> +      cmd_image = cp $< $@; truncate -s %4K $@; cat $(obj)/vmlinux.bin >>$@

				 ^ right there

I'm an idiot. That was a poorly named script and it cost me a kernel
bisect and poking at the patch for an hour. <sigh>

Sorry for the noise.

* Dave Hansen <dave.hansen@intel.com> wrote:

> On 4/11/25 12:33, Dave Hansen wrote:
> ...
> > The only weird thing I'm doing is booting the kernel with qemu's -kernel
> > argument.
> 
> I lied. I'm doing other weird things. I have a local script named
> "truncate" that's not the same thing as /usr/bin/truncate. Guess what
> this patch started doing:
> 
> >  quiet_cmd_image = BUILD   $@
> > -silent_redirect_image = >/dev/null
> > -cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin \
> > -			       $(obj)/zoffset.h $@ $($(quiet)redirect_image)
> > +      cmd_image = cp $< $@; truncate -s %4K $@; cat $(obj)/vmlinux.bin >>$@
> 
> 				 ^ right there

Oh that sucks ...

> I'm an idiot. That was a poorly named script and it cost me a kernel
> bisect and poking at the patch for an hour. <sigh>
> 
> Sorry for the noise.

I feel your pain, I too once overlaid a well-known utility with my own 
script in ~/bin/. After that incident I started adding the .sh postfix 
to my own scripts, that way there's a much lower chance of namespace 
collisions.

Thanks,

	Ingo

On Sat, 12 Apr 2025 at 10:48, Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Dave Hansen <dave.hansen@intel.com> wrote:
>
> > On 4/11/25 12:33, Dave Hansen wrote:
> > ...
> > > The only weird thing I'm doing is booting the kernel with qemu's -kernel
> > > argument.
> >
> > I lied. I'm doing other weird things. I have a local script named
> > "truncate" that's not the same thing as /usr/bin/truncate. Guess what
> > this patch started doing:
> >
> > >  quiet_cmd_image = BUILD   $@
> > > -silent_redirect_image = >/dev/null
> > > -cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin \
> > > -                          $(obj)/zoffset.h $@ $($(quiet)redirect_image)
> > > +      cmd_image = cp $< $@; truncate -s %4K $@; cat $(obj)/vmlinux.bin >>$@
> >
> >                                ^ right there
>
> Oh that sucks ...
>
> > I'm an idiot. That was a poorly named script and it cost me a kernel
> > bisect and poking at the patch for an hour. <sigh>
> >
> > Sorry for the noise.
>
> I feel your pain, I too once overlaid a well-known utility with my own
> script in ~/bin/. After that incident I started adding the .sh postfix
> to my own scripts, that way there's a much lower chance of namespace
> collisions.
>

There was another report about this, but in that case, the problem was
that busybox's truncate clone does not understand the % notation (even
though the very first original truncate version that I found from 2008
already supported that)

In any case, we might change this to

--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -59,7 +59,7 @@
 $(obj)/bzImage: asflags-y  := $(SVGA_MODE)

 quiet_cmd_image = BUILD   $@
-      cmd_image = cp $< $@; truncate -s %4K $@; cat $(obj)/vmlinux.bin >>$@
+      cmd_image = (dd if=$< bs=4096 conv=sync status=none; cat
$(obj)/vmlinux.bin) >$@

 $(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin FORCE
        $(call if_changed,image)

which is slightly cleaner - I'll send out a patch once I receive
confirmation that busybox dd implements conv=sync (which takes care of
the padding) correctly.