linuxboot / heads

A minimal Linux that runs as a coreboot or LinuxBoot ROM payload to provide a secure, flexible boot environment for laptops, workstations and servers.
https://osresearch.net/
GNU General Public License v2.0
1.42k stars 187 forks source link

board configs: remove CONFIG_BOOT_DEV to always detect_boot_device. Skip too small partition mount attempts #1784

Closed tlaurion closed 2 months ago

tlaurion commented 2 months ago

Fixes https://github.com/linuxboot/heads/issues/1783

Tested against https://openqa.qubes-os.org/tests/111529/asset/iso/Qubes-4.3.202409051715-x86_64.iso install

tlaurion commented 2 months ago

Revisiting https://github.com/linuxboot/heads/pull/1602 since /boot (and mostly everything else under Heads) should try exfat last. Shows in DEBUG, might already be silenced without DEBUG, let's see.

The following isn't enough to prevent dmesg errors when attempting to detect boot for sda1 on !4.3 test build:https://github.com/JonathonHall-Purism/heads/blob/a6228b9843348beb09ac77cf066682e74253a3bd/initrd/init#L52-L56

EDIT: that codpath is ok, its just that new BIOS partition is unknown and needs to not be attempted to be mounted.

tlaurion commented 2 months ago

@JonathonHall-Purism alternatively we could fix check for fixated size, unsure what else out there could be skipped one way or the other skipping by size/sectors

tlaurion commented 2 months ago

@JonathonHall-Purism @marmarek : I just added blkid type with low cost under busybox config, and now investigating if we could just simply use blkid output for lines that at least include a UUID prior of doing any mount calls.

In the case of interest for @marmarek, sda1 doesn't even show in blkid. with blkid type added under busybox config, partitions not showing a type should definitely not be attempted to be mounted.

This would be a real fix instead of a bandaid fix, where partition size of less than 2mb might hit us in the butt later.

Thoughts before I go too far in that direction?

JonathonHall-Purism commented 2 months ago

@tlaurion If it helps, that's great, but I was already checking the partition type with fdisk -l (see is_gpt_bios_grub). I'm not sure why this did not exclude @marmarek 's bios-grub partition. Is the check with blkid any different?

marmarek commented 2 months ago

@tlaurion If it helps, that's great, but I was already checking the partition type with fdisk -l (see is_gpt_bios_grub). I'm not sure why this did not exclude @marmarek 's bios-grub partition. Is the check with blkid any different?

That's a very good question, maybe due to the hardcoded CONFIG_BOOT_DEV? If skipping bios boot is already there, perhaps just removing static CONFIG_BOOT_DEV is enough?

tlaurion commented 2 months ago

@tlaurion If it helps, that's great, but I was already checking the partition type with fdisk -l (see is_gpt_bios_grub). I'm not sure why this did not exclude @marmarek 's bios-grub partition. Is the check with blkid any different?

Without blkid type: PXL_20240909_135659227.MP.jpg

With blkid type and fdisk -l output: PXL_20240909_134838477.MP.jpg

JonathonHall-Purism commented 2 months ago

That's a very good question, maybe due to the hardcoded CONFIG_BOOT_DEV? If skipping bios boot is already there, perhaps just removing static CONFIG_BOOT_DEV is enough?

Ah, yes, that could be it. The partition type is probably only checked by the automatic detection, not by the attempt to mount CONFIG_BOOT_DEV.

So @tlaurion maybe we don't need the small partition check, and blkid doesn't seem to be needed either. (I'd still be OK with the small partition check if you want, it doesn't do any harm and is testable. You'd have to create a <2MB partition without the bios-grub flag to test it.)

Removing CONFIG_BOOT_DEV defaults everywhere might be enough to cover this case.

tlaurion commented 2 months ago

Unfortunately, without mount skip and only removing CONFIG_BOOT_DEV from all boards,exfat warnings still there in boot partition detection.

PXL_20240909_141306637.MP.jpg

@JonathonHall-Purism I would go with this PR as is and then attack root problem under #1788

tlaurion commented 2 months ago

@tlaurion If it helps, that's great, but I was already checking the partition type with fdisk -l (see is_gpt_bios_grub). I'm not sure why this did not exclude @marmarek 's bios-grub partition. Is the check with blkid any different?

That's a very good question, maybe due to the hardcoded CONFIG_BOOT_DEV? If skipping bios boot is already there, perhaps just removing static CONFIG_BOOT_DEV is enough?

Not enough as shown above. Below on clean boot (without public key injected nor any reference to CONFIG_BOOT_DEV in config.user nor board config)

PXL_20240909_142224811.MP.jpg

JonathonHall-Purism commented 2 months ago

@JonathonHall-Purism I would go with this PR as is and then attack root problem under #1788

OK, agree. Not sure why the fdisk -l check doesn't pick up this partition, but like I said I'm OK with the 2 MB check as well.

Just need to resolve the misleading DEBUG/TRACE then: https://github.com/linuxboot/heads/pull/1784#discussion_r1750303715

tlaurion commented 2 months ago

@JonathonHall-Purism there was still an indentation error. fixed under faa77d4

JonathonHall-Purism commented 2 months ago

@JonathonHall-Purism there was still an indentation error. fixed under faa77d4

This is the diff I see for the latest force-push:

Screenshot_20240909_104942

The indentation was right before, this introduced an error. But this change showed up just as I was merging, so it already got merged :shrug: