ACK: [SRU][Noble][PATCH 0/1] Linux 6.8 fails to boot on ARM64 if any param is more than 146 chars

Tue Jul 30 09:23:16 UTC 2024

On Tue, Jul 30, 2024 at 11:28:37AM +1200, Matthew Ruffell wrote:
> BugLink: https://bugs.launchpad.net/bugs/2069534
> 
> [Impact]
> 
> Linux 6.8 kernel fails to boot on ARM64 when any Linux command line param is 
> more than 146 characters.
> 
> This most notably affects MAAS deployments, as MAAS generates very long command
> line parameters for ARM64, e.g.:
> 
> nomodeset root=squash:http://10.254.131.130:5248/images/3b08252fa962c37a47d890fb5fe182b631a0c0478d758bf4573efa859cc2c548/ubuntu/arm64/ga-24.04/noble/stable/squashfs ip=::::sjc01-2b16-u07-mgx01b:BOOTIF ip6=off cc:\{'datasource_list': ['MAAS']\}end_cc cloud-config-url=http://10-254-131-128--25.maas-internal:5248/MAAS/metadata/latest/by-id/de6dn3/?op=get_preseed ro overlayroot=tmpfs overlayroot_cfgdisk=disabled log_host=10.254.131.130 log_port=5247 --- BOOTIF=01-${net_default_mac}
> 
> This was introduced in 6.8-rc1 by:
> 
> commit dc3f5aae06381b43bc9d0d416bd15ee1682940e9
> Author: Ard Biesheuvel <ardb at kernel.org>
> Date: Wed Nov 29 12:16:12 2023 +0100
> Subject: arm64: idreg-override: Avoid parameq() and parameqn()
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=dc3f5aae06381b43bc9d0d416bd15ee1682940e9
> 
> There is no workaround, other than using command line parameters less than 146
> characters. This is not tenable for MAAS users.
> 
> [Fix]
> 
> The fix arrived in a major refactor of early ARM64 init, where they moved from
> assembly to the pi mini c library. The specific commit that fixed the issue is:
> 
> commit e223a449125571daa62debd8249fa4fc2da0a961
> Author: Ard Biesheuvel <ardb at kernel.org>
> Date: Wed Feb 14 13:28:50 2024 +0100
> Subject: arm64: idreg-override: Move to early mini C runtime
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e223a449125571daa62debd8249fa4fc2da0a961
> 
> However, this needs a lot of dependencies, mostly all the "mini c runtime"
> commits in the below merge commit:
> 
> commit 6d75c6f40a03c97e1ecd683ae54e249abb9d922b
> Merge: fe46a7dd189e 1ef21fcd6a50
> Author: Linus Torvalds <torvalds at linux-foundation.org>
> Date: Thu Mar 14 15:35:42 2024 -0700
> Subject: Merge tag 'arm64-upstream' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6d75c6f40a03c97e1ecd683ae54e249abb9d922b
> 
> The amount of code is generally unacceptable for an SRU due to regression risk.
> I also don't think that reverting "arm64: idreg-override: Avoid parameq() and 
> parameqn()" is the right solution either.
> 
> Thankfully, T.J. did some debugging of the root cause in comment #20 [1], and
> found the issue occurs because of memcmp() in include/linux/fortify-string.h 
> detecting an attempted out-of-bounds read when comparing buf and 
> aliases[i].alias.
> 
> That triggers the fortified memcmp()'s:
> 
> if (p_size < size || q_size < size)
> fortify_panic(__func__);
> 
> where q_size == 146, size == 147, and it crashes the kernel.
> 
> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2069534/comments/20
> 
> I know SAUCE patches are to be avoided if possible, but T.J's solution is
> minimal and fixes the root cause without the regression risk of backporting the
> entire mini C runtime, so I suggest we go with T.J's patch.
> 
> commit a4c616d2156c9c4cf7c91e6983c8bf0d51985df1
> Author: Tj <tj.iam.tj at proton.me>
> Date:   Fri Jul 26 13:48:44 2024 +0000
> Subject: UBUNTU: SAUCE: arm64: v6.8: cmdline param >= 146 chars kills kernel
> Link: https://lore.kernel.org/stable/JsQ4W_o2R1NfPFTCCJjjksPED-8TuWGr796GMNeUMAdCh-2NSB_16x6TXcEecXwIfgzVxHzeB_-PMQnvQuDo0gmYE_lye0rC5KkbkDgkUqM=@proton.me/T/#u
> 
> [Testcase]
> 
> 1) Deploy an ARM64 VM or use a bare metal ARM64 board with Noble, running 6.8.
> 2) Edit /boot/grub/grub.cfg and add the following param to any boot entry with 
> Linux 6.8
> 
> testparam=f081c381e7b54edcba27e5f790d47911a4cc3e726d8d256878d3df9175c020e0f081c381e7b54edcba27e5f790d47911a4cc3e726d8d256878d3df9175c020e0f081c381e7b5732f126a62b4232
> 
> 3) Reboot the machine and select the boot entry in grub with the testparam as
> above.
> 4) Observe kernel never boots.
> 
> [Where problems could occur]
> 
> We are changing command line parsing on ARM64 systems, such that we only do a
> memcmp() with aliased entries if the parameter we are parsing has the same
> length as an aliased entry. This really shouldn't have any change in
> functionality at all.
> 
> If a regression were to occur, then command line parsing on ARM64 systems could
> be broken, and it could lead to early boot failures, likely caught on automated
> kernel tests.
> 
> [Other Info]
> 
> This fix is 6.8 specific. It is already fixed upstream by the mini C runtime in
> 6.9 and later. This patch is for noble only.
> 
> Tj (1):
>   UBUNTU: SAUCE: arm64: v6.8: cmdline param >= 146 chars kills kernel
> 
>  arch/arm64/kernel/idreg-override.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> -- 
> 2.45.2
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Acked-by: Manuel Diewald <manuel.diewald at canonical.com>

-- 
 Manuel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240730/9d4d43b1/attachment.sig>