Skip to content

Replace fdisk -l with sysfs-based disk enumeration (2tb+ drive support) #2035

Merged
tlaurion merged 1 commit intolinuxboot:masterfrom
tlaurion:replace_codebase_fdisk-l
Mar 6, 2026
Merged

Replace fdisk -l with sysfs-based disk enumeration (2tb+ drive support) #2035
tlaurion merged 1 commit intolinuxboot:masterfrom
tlaurion:replace_codebase_fdisk-l

Conversation

@tlaurion
Copy link
Collaborator

@tlaurion tlaurion commented Dec 8, 2025

fdisk -l can deal with 2TB max size drives. Use linux sysfs instead for everything not gpt/bios fdisk related.

Future UX improvements for TPM Reset/reseal of TPM DUK, primary handle handling (disk swap detection and TPM never been owned before, and catch and guide user more effetively with proper guidance, not just unguided errors) are implemented in seperate PR #2068 which will be rebased and merged in a bit.

Screenshot_20260306_122159

Tested:

  • qemu oem-factory reset with a canokey and public key exported to virt thumb drive's exclusive ext4 public partition
  • qemu oem-facctory-reset with canokey and in-ram key generation and key to card, creating luks+exfat partition on virt thumb drive
  • qemu-img 8tb qcow2 root attached: system information shows 8tb
  • test on v540tu

Fixes #2034 (should have been the fix for #1884)

Copilot AI review requested due to automatic review settings December 8, 2025 18:26
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces fdisk -l commands with sysfs-based disk enumeration to overcome fdisk's 2TB drive size limitation and improve busybox compatibility. The changes enable proper detection and display of large storage devices (e.g., 8TB drives).

Key changes:

  • Introduces list_block_devices() function that reads from /sys/block/* instead of parsing fdisk -l output
  • Replaces fdisk-based disk information gathering with sysfs-based size calculation
  • Updates partition detection logic to use sysfs directory structure

Reviewed changes

Copilot reviewed 2 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
initrd/etc/gui_functions Adds list_block_devices() function and updates show_system_info() to read disk sizes from sysfs
initrd/etc/functions Adds list_block_devices() function, updates device_has_partitions() to check sysfs for partitions, updates is_gpt_bios_grub() to read partition types from sysfs, and updates detect_boot_device() to use new function
initrd/bin/root-hashes-gui.sh Updates detect_root_device() to use list_block_devices() instead of fdisk
initrd/bin/oem-system-info-xx30 Updates disk information gathering to read from sysfs instead of fdisk
initrd/bin/config-gui.sh Updates device listing to use list_block_devices() instead of fdisk

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tlaurion tlaurion marked this pull request as draft December 8, 2025 18:59
@tlaurion tlaurion force-pushed the replace_codebase_fdisk-l branch 2 times, most recently from b0bdb67 to 879fb3c Compare December 8, 2025 19:34
@tlaurion tlaurion changed the title Replace fdisk -l with sysfs-based disk enumeration for better busybox compatibility Replace fdisk -l with sysfs-based disk enumeration (2tb+ drive support) Dec 8, 2025
@tlaurion tlaurion marked this pull request as ready for review December 8, 2025 22:52
@tlaurion tlaurion force-pushed the replace_codebase_fdisk-l branch 3 times, most recently from b3c8e82 to d296082 Compare December 15, 2025 21:15
@tlaurion tlaurion force-pushed the replace_codebase_fdisk-l branch 3 times, most recently from 12bbb04 to 23bcdbf Compare February 28, 2026 00:54
@tlaurion tlaurion requested a review from Copilot February 28, 2026 00:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

initrd/etc/functions:875

  • disk_info_sysfs() builds disk_info lines using "\n" and then echoes with echo -n, which produces literal backslash-n sequences unless the consumer interprets escapes. To make output consistent across callers, prefer emitting real newlines via printf (e.g., printf 'Disk /dev/%s: %s GB\n' ...) or use printf '%b' when printing an escape-containing buffer.
				local size_gb=$(((size_bytes * sector_size + 500000000) / 1000000000))
				disk_info="${disk_info}Disk /dev/${devname}: ${size_gb} GB\n"
				DEBUG "disk_info_sysfs: /dev/${devname} calculated as ${size_gb} GB (${size_bytes} sectors * ${sector_size} bytes/sector = $((size_bytes * sector_size)) bytes)"
			fi
		fi
	done
	echo -n "$disk_info"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tlaurion tlaurion force-pushed the replace_codebase_fdisk-l branch 2 times, most recently from 2f37ee0 to 2971b52 Compare March 3, 2026 14:21
@tlaurion tlaurion requested a review from Copilot March 3, 2026 14:22
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

initrd/etc/functions:832

  • list_block_devices() always exits 0 (even when it outputs nothing). Callers like config-gui.sh currently treat enumeration failure as an error (if ! list_block_devices ...; then ...), but that branch will never run now—an empty device list will instead reach file_selector, which exits with a different error path. Consider having list_block_devices return non-zero when no devices are found (or update callers to check for an empty output file explicitly).
# List all block devices using sysfs
# Outputs one device path per line (e.g., /dev/sda, /dev/vda, /dev/nvme0n1)
list_block_devices() {
	TRACE_FUNC
	for dev in /sys/block/sd* /sys/block/nvme* /sys/block/vd* /sys/block/hd*; do
		if [ -e "$dev" ]; then
			echo "/dev/$(basename "$dev")"
		fi
	done | sort
}

initrd/etc/functions:1347

  • The comment says “otherwise check for grub type”, but the fdisk-based fallback was removed and the function now only checks PARTTYPENAME=BIOS boot and then returns 1. Please update the comment (or reintroduce an actual fallback) so it matches the current behavior.
	# Check partition type using sysfs if available, otherwise check for grub type
	# For GPT disks, check /sys/class/block/<dev>/<partition>/uevent for PARTTYPENAME
	local part_sys="/sys/class/block/$DEVICE/$(basename "$PART_DEV")"
	if [ -e "$part_sys/uevent" ]; then
		if grep -q "PARTTYPENAME=BIOS boot" "$part_sys/uevent"; then
			TRACE "$PART_DEV is a GPT BIOS grub partition"
			return 0
		fi
	fi

	# Fallback: try to detect using other sysfs attributes if available
	# For MBR disks, we would need to check partition type differently
	DEBUG "$PART_DEV - unable to confirm it's a bios-grub partition via sysfs"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 16 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

initrd/etc/functions:1393

  • The comment says p&lt;num&gt; which looks like an HTML-escaped fragment and is confusing in a shell script comment. Replace it with a plain-text example like p<num> or p1.
	# match nvme style (p&lt;num&gt;) or normal (digit suffix)

initrd/etc/functions:958

  • Avoid logging any information about secrets, including password length. Even in debug logs this can leak sensitive metadata and is unnecessary for normal troubleshooting. Drop this log line or replace it with a generic message that doesn’t include derived properties of the password.
	read -r -s -p $'\nTPM Owner Password: ' tpm_owner_password
	echo
	# Cache the password externally to be reused by who needs it
	DEBUG "password length ${#tpm_owner_password}"

initrd/etc/functions:1484

  • mounted_boot_dev is assigned without local, which can leak/overwrite a global variable in this large shared functions file and make later logic harder to reason about. Declare it as local mounted_boot_dev (and consider local CONFIG_BOOT_DEV behavior) to keep detect_boot_device side effects intentional.
	mounted_boot_dev="$(awk '$2=="/boot" {print $1; exit}' /proc/mounts)"
	if [ -n "$mounted_boot_dev" ] && ls -d /boot/grub* >/dev/null 2>&1; then
		CONFIG_BOOT_DEV="$mounted_boot_dev"
		return 0

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tlaurion tlaurion force-pushed the replace_codebase_fdisk-l branch from 3812659 to 1366571 Compare March 5, 2026 03:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 16 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 20 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tlaurion tlaurion force-pushed the replace_codebase_fdisk-l branch 2 times, most recently from 6013417 to 22994b6 Compare March 6, 2026 15:55
@tlaurion
Copy link
Collaborator Author

tlaurion commented Mar 6, 2026

Reduced this pr to only include sysfs disk size reporting and reuse fdisk -l for gpt/mbr partition table detection is detection of boot device.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tlaurion tlaurion force-pushed the replace_codebase_fdisk-l branch 2 times, most recently from d2798b8 to 2d35c74 Compare March 6, 2026 17:28
…bles

fdisk -l can’t be trusted inside Heads’ initrd: busybox limits it to
2 TiB and parsing its output is fragile.

Changes relative to origin/master:

  * add new function disk_info_sysfs() in initrd/etc/functions
    – walks /sys/block, skips partition entries, and computes a byte
      count (preferring blockdev --getsize64, otherwise size*512)
    – converts to decimal GB, switching to TB for ≥1000 GB
  * update show_system_info() (gui_functions & oem‑system‑info‑xx30) to call the
    helper and no longer invoke `fdisk -l` for size output
  * add TRACE_FUNC/DEBUG logging around the helper invocation

Tested in qemu/debian‑13/PureOS; only the size line differs, other behaviour
is identical to master.

Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tlaurion tlaurion merged commit fc2a52d into linuxboot:master Mar 6, 2026
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8tb drives are reported as 4tb drives under System Information menu

2 participants