Replace fdisk -l with sysfs-based disk enumeration (2tb+ drive support) #2035
Conversation
There was a problem hiding this comment.
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 parsingfdisk -loutput - 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.
b0bdb67 to
879fb3c
Compare
b3c8e82 to
d296082
Compare
12bbb04 to
23bcdbf
Compare
There was a problem hiding this comment.
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.
2f37ee0 to
2971b52
Compare
There was a problem hiding this comment.
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 likeconfig-gui.shcurrently treat enumeration failure as an error (if ! list_block_devices ...; then ...), but that branch will never run now—an empty device list will instead reachfile_selector, which exits with a different error path. Consider havinglist_block_devicesreturn 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 bootand 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.
2971b52 to
3812659
Compare
There was a problem hiding this comment.
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<num>which looks like an HTML-escaped fragment and is confusing in a shell script comment. Replace it with a plain-text example likep<num>orp1.
# match nvme style (p<num>) 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_devis assigned withoutlocal, which can leak/overwrite a global variable in this large sharedfunctionsfile and make later logic harder to reason about. Declare it aslocal mounted_boot_dev(and considerlocal CONFIG_BOOT_DEVbehavior) to keepdetect_boot_deviceside 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.
3812659 to
1366571
Compare
There was a problem hiding this comment.
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.
2f5115f to
9ca52be
Compare
There was a problem hiding this comment.
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.
6013417 to
22994b6
Compare
|
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. |
There was a problem hiding this comment.
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.
22994b6 to
0e07469
Compare
There was a problem hiding this comment.
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.
d2798b8 to
2d35c74
Compare
…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>
2d35c74 to
b3cb325
Compare
There was a problem hiding this comment.
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.
Commit 5168be2494 (PR #2035) changed increment_tpm_counter to pass the TPM owner passphrase as the TPM1 counter's auth value (-pwdc), but check_tpm_counter was left using empty auth (-pwdc ''). This caused every counter increment to compute SHA1(owner_pass) while the counter was created with SHA1("") — persistent TPM_AUTH_FAIL. Per TCG TPM Main Spec Part 3, TPM_CreateCounter uses owner auth (-pwdo) but TPM_IncrementCounter uses the counter's own authData, not the owner password. The correct design for Heads' rollback counter is empty auth: rollback security comes from the signed /boot/kexec_rollback.txt and TPM sealing, not counter access control. The repeated auth failures (3 per boot × ~5 boots) triggered TPM 1.2 dictionary-attack lockout (TPM_DEFEND_LOCK_RUNNING), which persisted through forceclear on some implementations, causing subsequent tpm takeown to fail and TPM reset to abort. Changes: - initrd/bin/tpmr.sh (_tpm_auth_retry, tpm2_counter_inc, tpm2_seal, tpm1_seal): add 'defend' and '0x98e|0x149' to auth detection grep patterns so defend lock and TPM2 RC codes are treated as retryable auth failures rather than fatal errors - initrd/bin/tpmr.sh (tpm1_reset): detect "defend lock" after takeown failure and cycle physical presence to clear the lock state before retrying — a full AC power cycle remains the fallback if software presence is insufficient - initrd/etc/functions.sh (check_tpm_counter): pass -pwdc '' (empty counter auth) instead of -pwdc "${tpm_passphrase:-}" so the counter is created with SHA1("") per TCG spec - initrd/etc/functions.sh (increment_tpm_counter): try -pwdc '' first for TPM1 (correct behavior). If that fails on a readable counter (created by the buggy inter-version code), prompt for owner passphrase and retry as migration fallback - initrd/etc/functions.sh (increment_tpm_counter): remove the TPM1-specific owner-passphrase prompt block added by the regression — no longer needed as new counters use empty auth - doc/tpm.md: document TPM1 boot chain, tpmtotp tool selection, auth retry patterns, defend lock recovery, and physical presence Signed-off-by: Thierry Laurion <insurgo@riseup.net>
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.
Tested:
Fixes #2034 (should have been the fix for #1884)