[PATCH] x86/boot: Add back some padding for the CRC-32 checksum

[PATCH] x86/boot: Add back some padding for the CRC-32 checksum

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Even though no uses of the bzImage CRC-32 checksum are known, ensure
that the last 4 bytes of the image are unused zero bytes, so that the
checksum can be generated post-build if needed.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
As requested by hpa, this applies onto today's tip/x86/build

 arch/x86/boot/compressed/vmlinux.lds.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 48d0b5184557..083ec6d7722a 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -48,7 +48,8 @@ SECTIONS
 		*(.data)
 		*(.data.*)
 
-		. = ALIGN(0x200);
+		/* Add 4 bytes of extra space for a CRC-32 checksum */
+		. = ALIGN(. + 4, 0x200);
 		_edata = . ;
 	}
 	. = ALIGN(L1_CACHE_BYTES);
-- 
2.49.0.rc0.332.g42c0ae87b1-goog

[tip: x86/build] x86/boot: Add back some padding for the CRC-32 checksum

[tip-bot2 for Ard Biesheuvel]

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

Commit-ID:     e471a86a8c523eccdfd1c4745ed7ac7cbdcc1f3f
Gitweb:        https://git.kernel.org/tip/e471a86a8c523eccdfd1c4745ed7ac7cbdcc1f3f
Author:        Ard Biesheuvel <ardb@kernel.org>
AuthorDate:    Wed, 12 Mar 2025 09:12:05 +01:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 12 Mar 2025 13:04:52 +01:00

x86/boot: Add back some padding for the CRC-32 checksum

Even though no uses of the bzImage CRC-32 checksum are known, ensure
that the last 4 bytes of the image are unused zero bytes, so that the
checksum can be generated post-build if needed.

[ mingo: Added the 'obsolete' qualifier to the comment. ]

Suggested-by: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Ian Campbell <ijc@hellion.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/20250312081204.521411-2-ardb+git@google.com
---
 arch/x86/boot/compressed/vmlinux.lds.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 48d0b51..3b2bc61 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -48,7 +48,8 @@ SECTIONS
 		*(.data)
 		*(.data.*)
 
-		. = ALIGN(0x200);
+		/* Add 4 bytes of extra space for the obsolete CRC-32 checksum */
+		. = ALIGN(. + 4, 0x200);
 		_edata = . ;
 	}
 	. = ALIGN(L1_CACHE_BYTES);

Re: [tip: x86/build] x86/boot: Add back some padding for the CRC-32 checksum

[Borislav Petkov]

On Wed, Mar 12, 2025 at 12:09:34PM -0000, tip-bot2 for Ard Biesheuvel wrote:
> The following commit has been merged into the x86/build branch of tip:
> 
> Commit-ID:     e471a86a8c523eccdfd1c4745ed7ac7cbdcc1f3f
> Gitweb:        https://git.kernel.org/tip/e471a86a8c523eccdfd1c4745ed7ac7cbdcc1f3f
> Author:        Ard Biesheuvel <ardb@kernel.org>
> AuthorDate:    Wed, 12 Mar 2025 09:12:05 +01:00
> Committer:     Ingo Molnar <mingo@kernel.org>
> CommitterDate: Wed, 12 Mar 2025 13:04:52 +01:00
> 
> x86/boot: Add back some padding for the CRC-32 checksum
> 
> Even though no uses of the bzImage CRC-32 checksum are known, ensure
> that the last 4 bytes of the image are unused zero bytes, so that the
> checksum can be generated post-build if needed.

Sounds like it is not needed and sounds like we should whack this thing no?

Or are we doing a grace period and then whack it when that grace period
expires?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [tip: x86/build] x86/boot: Add back some padding for the CRC-32 checksum

[Ard Biesheuvel]

On Mon, 14 Apr 2025 at 15:56, Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Mar 12, 2025 at 12:09:34PM -0000, tip-bot2 for Ard Biesheuvel wrote:
> > The following commit has been merged into the x86/build branch of tip:
> >
> > Commit-ID:     e471a86a8c523eccdfd1c4745ed7ac7cbdcc1f3f
> > Gitweb:        https://git.kernel.org/tip/e471a86a8c523eccdfd1c4745ed7ac7cbdcc1f3f
> > Author:        Ard Biesheuvel <ardb@kernel.org>
> > AuthorDate:    Wed, 12 Mar 2025 09:12:05 +01:00
> > Committer:     Ingo Molnar <mingo@kernel.org>
> > CommitterDate: Wed, 12 Mar 2025 13:04:52 +01:00
> >
> > x86/boot: Add back some padding for the CRC-32 checksum
> >
> > Even though no uses of the bzImage CRC-32 checksum are known, ensure
> > that the last 4 bytes of the image are unused zero bytes, so that the
> > checksum can be generated post-build if needed.
>
> Sounds like it is not needed and sounds like we should whack this thing no?
>
> Or are we doing a grace period and then whack it when that grace period
> expires?
>

This was done on hpa's request - maybe he has a duration in mind for
this grace period?

Re: [tip: x86/build] x86/boot: Add back some padding for the CRC-32 checksum

[H. Peter Anvin]

On April 14, 2025 7:07:53 AM PDT, Ard Biesheuvel <ardb@kernel.org> wrote:
>On Mon, 14 Apr 2025 at 15:56, Borislav Petkov <bp@alien8.de> wrote:
>>
>> On Wed, Mar 12, 2025 at 12:09:34PM -0000, tip-bot2 for Ard Biesheuvel wrote:
>> > The following commit has been merged into the x86/build branch of tip:
>> >
>> > Commit-ID:     e471a86a8c523eccdfd1c4745ed7ac7cbdcc1f3f
>> > Gitweb:        https://git.kernel.org/tip/e471a86a8c523eccdfd1c4745ed7ac7cbdcc1f3f
>> > Author:        Ard Biesheuvel <ardb@kernel.org>
>> > AuthorDate:    Wed, 12 Mar 2025 09:12:05 +01:00
>> > Committer:     Ingo Molnar <mingo@kernel.org>
>> > CommitterDate: Wed, 12 Mar 2025 13:04:52 +01:00
>> >
>> > x86/boot: Add back some padding for the CRC-32 checksum
>> >
>> > Even though no uses of the bzImage CRC-32 checksum are known, ensure
>> > that the last 4 bytes of the image are unused zero bytes, so that the
>> > checksum can be generated post-build if needed.
>>
>> Sounds like it is not needed and sounds like we should whack this thing no?
>>
>> Or are we doing a grace period and then whack it when that grace period
>> expires?
>>
>
>This was done on hpa's request - maybe he has a duration in mind for
>this grace period?

I would prefer to leave it indefinitely, because an all zero pattern is far easier to detect than what would look like a false checksum.

So I remembered eventually who wanted it: it was a direct from flash boot loader who wanted to detect a partial flash failure before invoking the kernel, so that it can redirect to a secondary kernel.

That would obviously not be an UEFI environment, so the signing issue is not applicable.

An all zero end field actually works for that purpose (although requires a boot loader patch), because an unprogrammed flash sector contains FFFFFFFF not 00000000.

We have kept the bzImage format backwards compatible – sometimes at considerable effort – and the cost of reasonably continuing to do so is absolutely minimal. This is an incompatible change, so at least it is appropriate to give unambiguous indication thereof.

In other words: it ain't broken, don't try to fix it. It is all downside, no upside.

Re: [tip: x86/build] x86/boot: Add back some padding for the CRC-32 checksum

[Borislav Petkov]

On Tue, Apr 15, 2025 at 10:00:11AM -0700, H. Peter Anvin wrote:
> >This was done on hpa's request - maybe he has a duration in mind for
> >this grace period?
> 
> I would prefer to leave it indefinitely, because an all zero pattern is far easier to detect than what would look like a false checksum.
> 
> So I remembered eventually who wanted it: it was a direct from flash boot loader who wanted to detect a partial flash failure before invoking the kernel, so that it can redirect to a secondary kernel.

What is a "direct from flash boot loader"?

> That would obviously not be an UEFI environment, so the signing issue is not applicable.
> 
> An all zero end field actually works for that purpose (although requires a boot loader patch), because an unprogrammed flash sector contains FFFFFFFF not 00000000.
> 
> We have kept the bzImage format backwards compatible – sometimes at considerable effort – and the cost of reasonably continuing to do so is absolutely minimal. This is an incompatible change, so at least it is appropriate to give unambiguous indication thereof.
> 
> In other words: it ain't broken, don't try to fix it. It is all downside, no upside.

Sure but look at what is there now:

                /* Add 4 bytes of extra space for the obsolete CRC-32 checksum */
                . = ALIGN(. + 4, 0x200);
                _edata = . ;

This basically screams at me: "delete me, delete me!" :-P

So I would probably put the gist of what you say above as a comment there so
that we have the rationale stated there, for future, trigger-happy
generations.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [tip: x86/build] x86/boot: Add back some padding for the CRC-32 checksum

[H. Peter Anvin]

On April 15, 2025 2:54:27 PM PDT, Borislav Petkov <bp@alien8.de> wrote:
>On Tue, Apr 15, 2025 at 10:00:11AM -0700, H. Peter Anvin wrote:
>> >This was done on hpa's request - maybe he has a duration in mind for
>> >this grace period?
>> 
>> I would prefer to leave it indefinitely, because an all zero pattern is far easier to detect than what would look like a false checksum.
>> 
>> So I remembered eventually who wanted it: it was a direct from flash boot loader who wanted to detect a partial flash failure before invoking the kernel, so that it can redirect to a secondary kernel.
>
>What is a "direct from flash boot loader"?
>
>> That would obviously not be an UEFI environment, so the signing issue is not applicable.
>> 
>> An all zero end field actually works for that purpose (although requires a boot loader patch), because an unprogrammed flash sector contains FFFFFFFF not 00000000.
>> 
>> We have kept the bzImage format backwards compatible – sometimes at considerable effort – and the cost of reasonably continuing to do so is absolutely minimal. This is an incompatible change, so at least it is appropriate to give unambiguous indication thereof.
>> 
>> In other words: it ain't broken, don't try to fix it. It is all downside, no upside.
>
>Sure but look at what is there now:
>
>                /* Add 4 bytes of extra space for the obsolete CRC-32 checksum */
>                . = ALIGN(. + 4, 0x200);
>                _edata = . ;
>
>This basically screams at me: "delete me, delete me!" :-P
>
>So I would probably put the gist of what you say above as a comment there so
>that we have the rationale stated there, for future, trigger-happy
>generations.
>
>Thx.
>

It is an embedded boot loader that loads the kernel image from flash (either NAND or NOR), without a file system.