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

tlaurion · 2024-09-08T14:29:49Z

Remove static CONFIG_BOOT_DEV in board configs
Skip attempting mount for smaller than 2mb (4096 sectors) partitions under /etc/functions:mount_possible_boot_device

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

tlaurion · 2024-09-08T16:27:22Z

Revisiting #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 · 2024-09-08T17:50:32Z

initrd/etc/functions

+	# Get the size of BOOT_DEV in 512-byte sectors
+	sectors=$(blockdev --getsz "$BOOT_DEV")
+
+	# Check if the partition is small (less than 2MB, which is 4096 sectors)


@JonathonHall-Purism @marmarek : kinda a hack, but works preventing exfat error spurring into dmesg fixing #1783.

Thoughts? Otherwise outside of exfat dmesg errors, just removing CONFIG_BOOT_DEV fixes the static /dev/sda1 /dev/nvme0n1p1 being fixated to force autodetection, while /dev/sda1 causes issues because not known as type and exfat being last partition type still is attempted to be mounted.

alternative solution could be looking for fs signature first (blkid?) but I'm not sure if busybox(?) is capable of that...

or explicitly skipping "bios boot" partition (would need to get the partition type - again, not sure if easy in busybox

if alternatives are not easy to do, I'm okay with the size check too - 2MB is small enough that no legitimate /boot would be affected.

I'm OK with this as a band-aid improvement 👍 It does suppress some spurious warnings and I don't see any downside.

I've already made multiple changes trying to suppress these annoying exFAT warnings - IMO, I think in the long run we either need to patch out that warning from the kernel or stop dumping kernel warnings to the console.

This is a great improvement but overall this is looking like a whack-a-mole game trying to address each situation individually.

Again though, let's merge this and I'll open a separate issue for the exFAT warnings.

Actually, @marmarek, it does look for the partition type above (is_gpt_bios_grub) - why didn't that exclude your bios-grub partition?

I still don't have a problem with this sector count check though, it's not going to cause any harm.

Opened #1788 for exFAT warnings in general

tlaurion · 2024-09-08T17:58:23Z

@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

JonathonHall-Purism

This is a great change 👍 The /boot detection is reliable now, and hard-coding a default isn't very reliable any more since it depends on the type of storage installed and the partition layout.

I made one suggestion regarding fixing indentation, otherwise it looks great.

initrd/etc/functions

JonathonHall-Purism · 2024-09-09T13:17:47Z

initrd/etc/functions

+	# Get the size of BOOT_DEV in 512-byte sectors
+	sectors=$(blockdev --getsz "$BOOT_DEV")
+
+	# Check if the partition is small (less than 2MB, which is 4096 sectors)


I'm OK with this as a band-aid improvement 👍 It does suppress some spurious warnings and I don't see any downside.

I've already made multiple changes trying to suppress these annoying exFAT warnings - IMO, I think in the long run we either need to patch out that warning from the kernel or stop dumping kernel warnings to the console.

This is a great improvement but overall this is looking like a whack-a-mole game trying to address each situation individually.

Again though, let's merge this and I'll open a separate issue for the exFAT warnings.

tlaurion · 2024-09-09T13:23:37Z

@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 · 2024-09-09T13:38:17Z

@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 · 2024-09-09T13:48:23Z

@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 · 2024-09-09T13:50:36Z

@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:

With blkid type and fdisk -l output:

JonathonHall-Purism · 2024-09-09T13:50:57Z

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 · 2024-09-09T14:17:15Z

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

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

tlaurion · 2024-09-09T14:23:01Z

@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)

JonathonHall-Purism · 2024-09-09T14:30:37Z

@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: #1784 (comment)

…t prior of oem-factory-reset usage repro: sed -i '/CONFIG_BOOT_DEV/d' boards/*/*.config unmaintained_boards/*/*.config qemu debug trace on preinstalled OS: [ 3.999725] [U] hello world [ 4.286215] DEBUG: Debug output enabled from board CONFIG_DEBUG_OUTPUT=y option (/etc/config) [ 4.315239] TRACE: Under init [ 4.369379] DEBUG: Applying panic_on_oom setting to sysctl [ 4.588333] TRACE: /bin/cbfs-init(5): main [ 4.728310] TRACE: /bin/cbfs-init(24): main [ 4.867039] DEBUG: TPM: Will extend PCR[7] with hash of filename /.gnupg/pubring.kbx [ 4.946757] TRACE: /bin/tpmr(788): main [ 5.006987] DEBUG: TPM: Extending PCR[7] with hash 7ccf4f64044946cf4e5b0efe3d959f00562227ae [ 5.068692] DEBUG: exec tpm extend -ix 7 -ic /.gnupg/pubring.kbx [ 5.326365] DEBUG: TPM: Will extend PCR[7] hash content of file /.gnupg/pubring.kbx [ 5.399511] TRACE: /bin/tpmr(788): main [ 5.460618] DEBUG: TPM: Extending PCR[7] with hash 547ca343719d3aa62af4763357d8c10cb35eae55 [ 5.524608] DEBUG: exec tpm extend -ix 7 -if /.gnupg/pubring.kbx [ 5.752340] TRACE: /bin/cbfs-init(24): main [ 5.908677] DEBUG: TPM: Will extend PCR[7] with hash of filename /.gnupg/trustdb.gpg [ 5.988169] TRACE: /bin/tpmr(788): main [ 6.044996] DEBUG: TPM: Extending PCR[7] with hash 7236ea8e612c1435259a8a0f8e0a8f1f5dba7042 [ 6.101604] DEBUG: exec tpm extend -ix 7 -ic /.gnupg/trustdb.gpg [ 6.371341] DEBUG: TPM: Will extend PCR[7] hash content of file /.gnupg/trustdb.gpg [ 6.451878] TRACE: /bin/tpmr(788): main [ 6.511948] DEBUG: TPM: Extending PCR[7] with hash 4697c489f359b40dd8aec55df52a33b1f580a3df [ 6.572785] DEBUG: exec tpm extend -ix 7 -if /.gnupg/trustdb.gpg [ 6.879519] TRACE: /bin/key-init(6): main [ 8.239618] TRACE: Under /etc/ash_functions:combine_configs [ 8.323781] TRACE: Under /etc/ash_functions:pause_recovery !!! Hit enter to proceed to recovery shell !!! [ 8.572855] TRACE: /bin/setconsolefont.sh(6): main [ 8.631296] DEBUG: Board does not ship setfont, not checking console font [ 8.887295] TRACE: /bin/gui-init(641): main [ 8.920627] TRACE: /etc/functions(715): detect_boot_device [ 9.251212] TRACE: /etc/functions(682): mount_possible_boot_device [ 9.312602] TRACE: /etc/functions(642): is_gpt_bios_grub [ 9.410830] TRACE: /dev/vda1 is partition 1 of vda [ 9.540007] TRACE: /etc/functions(619): find_lvm_vg_name [ 9.707187] TRACE: Try mounting /dev/vda1 as /boot [ 9.766843] EXT4-fs (vda1): mounted filesystem with ordered data mode. Opts: (null) [ 9.825028] TRACE: /bin/gui-init(319): clean_boot_check Signed-off-by: Thierry Laurion <[email protected]>

…ttempt on partitions <2Mb (4096 sectors) Removes spurious errors thrown for exfat in dmesg in that function. Something better to propose? Signed-off-by: Thierry Laurion <[email protected]>

tlaurion · 2024-09-09T14:46:29Z

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

JonathonHall-Purism · 2024-09-09T14:50:36Z

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

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

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

tlaurion marked this pull request as draft September 8, 2024 14:29

tlaurion commented Sep 8, 2024

View reviewed changes

tlaurion changed the title ~~board configs: remove CONFIG_BOOT_DEV so detect_boot_device detects it prior of oem-factory-reset usage~~ board configs: remove CONFIG_BOOT_DEV so detect_boot_device detects it prior of oem-factory-reset usage fixating it. Sep 8, 2024

tlaurion requested a review from JonathonHall-Purism September 8, 2024 17:57

tlaurion marked this pull request as ready for review September 8, 2024 17:59

tlaurion mentioned this pull request Sep 8, 2024

Heads fails to find /boot partition with Qubes OS 4.3 #1783

Closed

50 tasks

tlaurion changed the title ~~board configs: remove CONFIG_BOOT_DEV so detect_boot_device detects it prior of oem-factory-reset usage fixating it.~~ board configs: remove CONFIG_BOOT_DEV to always detect_boot_device. Skip too small partition mount attempts Sep 8, 2024

JonathonHall-Purism requested changes Sep 9, 2024

View reviewed changes

JonathonHall-Purism mentioned this pull request Sep 9, 2024

Stamp out exFAT warnings at the root #1788

Open

tlaurion force-pushed the dynamic_bootpart_detection branch from 0532b5c to 1724584 Compare September 9, 2024 14:42

/etc/functions:mount_possible_boot_device; punch exclusion of mount a…

faa77d4

…ttempt on partitions <2Mb (4096 sectors) Removes spurious errors thrown for exfat in dmesg in that function. Something better to propose? Signed-off-by: Thierry Laurion <[email protected]>

JonathonHall-Purism approved these changes Sep 9, 2024

View reviewed changes

tlaurion force-pushed the dynamic_bootpart_detection branch from 1724584 to faa77d4 Compare September 9, 2024 14:45

JonathonHall-Purism merged commit 3fef9e0 into linuxboot:master Sep 9, 2024
1 of 2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

tlaurion commented Sep 8, 2024 •

edited

Loading

tlaurion commented Sep 8, 2024 •

edited

Loading

tlaurion Sep 8, 2024 •

edited

Loading

marmarek Sep 9, 2024

marmarek Sep 9, 2024

marmarek Sep 9, 2024

JonathonHall-Purism Sep 9, 2024

JonathonHall-Purism Sep 9, 2024

JonathonHall-Purism Sep 9, 2024

tlaurion commented Sep 8, 2024

JonathonHall-Purism left a comment

JonathonHall-Purism Sep 9, 2024

tlaurion commented Sep 9, 2024 •

edited

Loading

JonathonHall-Purism commented Sep 9, 2024

marmarek commented Sep 9, 2024

tlaurion commented Sep 9, 2024 •

edited

Loading

JonathonHall-Purism commented Sep 9, 2024

tlaurion commented Sep 9, 2024 •

edited

Loading

tlaurion commented Sep 9, 2024 •

edited

Loading

JonathonHall-Purism commented Sep 9, 2024

tlaurion commented Sep 9, 2024

JonathonHall-Purism commented Sep 9, 2024

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

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

Conversation

tlaurion commented Sep 8, 2024 • edited Loading

tlaurion commented Sep 8, 2024 • edited Loading

tlaurion Sep 8, 2024 • edited Loading

Choose a reason for hiding this comment

marmarek Sep 9, 2024

Choose a reason for hiding this comment

marmarek Sep 9, 2024

Choose a reason for hiding this comment

marmarek Sep 9, 2024

Choose a reason for hiding this comment

JonathonHall-Purism Sep 9, 2024

Choose a reason for hiding this comment

JonathonHall-Purism Sep 9, 2024

Choose a reason for hiding this comment

JonathonHall-Purism Sep 9, 2024

Choose a reason for hiding this comment

tlaurion commented Sep 8, 2024

JonathonHall-Purism left a comment

Choose a reason for hiding this comment

JonathonHall-Purism Sep 9, 2024

Choose a reason for hiding this comment

tlaurion commented Sep 9, 2024 • edited Loading

JonathonHall-Purism commented Sep 9, 2024

marmarek commented Sep 9, 2024

tlaurion commented Sep 9, 2024 • edited Loading

JonathonHall-Purism commented Sep 9, 2024

tlaurion commented Sep 9, 2024 • edited Loading

tlaurion commented Sep 9, 2024 • edited Loading

JonathonHall-Purism commented Sep 9, 2024

tlaurion commented Sep 9, 2024

JonathonHall-Purism commented Sep 9, 2024

tlaurion commented Sep 8, 2024 •

edited

Loading

tlaurion commented Sep 8, 2024 •

edited

Loading

tlaurion Sep 8, 2024 •

edited

Loading

tlaurion commented Sep 9, 2024 •

edited

Loading

tlaurion commented Sep 9, 2024 •

edited

Loading

tlaurion commented Sep 9, 2024 •

edited

Loading

tlaurion commented Sep 9, 2024 •

edited

Loading