From 8916185e5695c20958cdd03434360f2b0c686764 Mon Sep 17 00:00:00 2001 From: Kai Lueke Date: Mon, 20 Nov 2023 15:47:24 +0100 Subject: [PATCH 1/2] disks: Refuse to modify disks/partitions in use When a partition or the whole disk is in use, sgdisk should not execute the destructive operation. Add a check that errors out when a disk in use or a partition in use is to be destroyed. --- internal/exec/stages/disks/partitions.go | 132 +++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/internal/exec/stages/disks/partitions.go b/internal/exec/stages/disks/partitions.go index cb55c3765..8ae45ccb0 100644 --- a/internal/exec/stages/disks/partitions.go +++ b/internal/exec/stages/disks/partitions.go @@ -19,8 +19,11 @@ package disks import ( + "bufio" "errors" "fmt" + "os" + "path/filepath" "regexp" "sort" "strconv" @@ -30,6 +33,7 @@ import ( "github.com/coreos/ignition/v2/config/v3_5_experimental/types" "github.com/coreos/ignition/v2/internal/exec/util" "github.com/coreos/ignition/v2/internal/sgdisk" + iutil "github.com/coreos/ignition/v2/internal/util" ) var ( @@ -317,11 +321,126 @@ func (p PartitionList) Swap(i, j int) { p[i], p[j] = p[j], p[i] } +// Expects a /dev/xyz path +func blockDevHeld(blockDevResolved string) (bool, error) { + _, blockDevNode := filepath.Split(blockDevResolved) + + holdersDir := fmt.Sprintf("/sys/class/block/%s/holders/", blockDevNode) + entries, err := os.ReadDir(holdersDir) + if err != nil { + return false, fmt.Errorf("failed to retrieve holders of %q: %v", blockDevResolved, err) + } + return len(entries) > 0, nil +} + +// Expects a /dev/xyz path +func blockDevMounted(blockDevResolved string) (bool, error) { + mounts, err := os.Open("/proc/mounts") + if err != nil { + return false, fmt.Errorf("failed to open /proc/mounts: %v", err) + } + scanner := bufio.NewScanner(mounts) + for scanner.Scan() { + mountSource := strings.Split(scanner.Text(), " ")[0] + if strings.HasPrefix(mountSource, "/") { + mountSourceResolved, err := filepath.EvalSymlinks(mountSource) + if err != nil { + return false, fmt.Errorf("failed to resolve %q: %v", mountSource, err) + } + if mountSourceResolved == blockDevResolved { + return true, nil + } + } + } + if err := scanner.Err(); err != nil { + return false, fmt.Errorf("failed to check mounts for %q: %v", blockDevResolved, err) + } + return false, nil +} + +// Expects a /dev/xyz path +func blockDevPartitions(blockDevResolved string) ([]string, error) { + _, blockDevNode := filepath.Split(blockDevResolved) + + // This also works for extended MBR partitions + sysDir := fmt.Sprintf("/sys/class/block/%s/", blockDevNode) + entries, err := os.ReadDir(sysDir) + if err != nil { + return nil, fmt.Errorf("failed to retrieve sysfs entries of %q: %v", blockDevResolved, err) + } + var partitions []string + for _, entry := range entries { + if strings.HasPrefix(entry.Name(), blockDevNode) { + partitions = append(partitions, "/dev/"+entry.Name()) + } + } + + return partitions, nil +} + +// Expects a /dev/xyz path +func blockDevInUse(blockDevResolved string, skipPartitionCheck bool) (bool, []string, error) { + // Note: This ignores swap and LVM usage + inUse := false + held, err := blockDevHeld(blockDevResolved) + if err != nil { + return false, nil, fmt.Errorf("failed to check if %q is held: %v", blockDevResolved, err) + } + mounted, err := blockDevMounted(blockDevResolved) + if err != nil { + return false, nil, fmt.Errorf("failed to check if %q is mounted: %v", blockDevResolved, err) + } + inUse = held || mounted + if skipPartitionCheck { + return inUse, nil, nil + } + partitions, err := blockDevPartitions(blockDevResolved) + if err != nil { + return false, nil, fmt.Errorf("failed to retrieve partitions of %q: %v", blockDevResolved, err) + } + var activePartitions []string + for _, partition := range partitions { + partInUse, _, err := blockDevInUse(partition, true) + if err != nil { + return false, nil, fmt.Errorf("failed to check if partition %q is in use: %v", partition, err) + } + if partInUse { + activePartitions = append(activePartitions, partition) + inUse = true + } + } + return inUse, activePartitions, nil +} + +// Expects a /dev/xyz path +func partitionNumberPrefix(blockDevResolved string) string { + lastChar := blockDevResolved[len(blockDevResolved)-1] + if '0' <= lastChar && lastChar <= '9' { + return "p" + } + return "" +} + // partitionDisk partitions devAlias according to the spec given by dev func (s stage) partitionDisk(dev types.Disk, devAlias string) error { + blockDevResolved, err := filepath.EvalSymlinks(devAlias) + if err != nil { + return fmt.Errorf("failed to resolve %q: %v", devAlias, err) + } + + inUse, activeParts, err := blockDevInUse(blockDevResolved, false) + if err != nil { + return fmt.Errorf("failed usage check on %q: %v", devAlias, err) + } + if inUse && len(activeParts) == 0 { + return fmt.Errorf("refusing to operate on directly active disk %q", devAlias) + } if cutil.IsTrue(dev.WipeTable) { op := sgdisk.Begin(s.Logger, devAlias) s.Logger.Info("wiping partition table requested on %q", devAlias) + if len(activeParts) > 0 { + return fmt.Errorf("refusing to wipe active disk %q", devAlias) + } op.WipeTable(true) if err := op.Commit(); err != nil { // `sgdisk --zap-all` will exit code 2 if the table was corrupted; retry it @@ -343,6 +462,8 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { return err } + prefix := partitionNumberPrefix(blockDevResolved) + // get a list of parititions that have size and start 0 replaced with the real sizes // that would be used if all specified partitions were to be created anew. // Also calculate sectors for all of the start/size values. @@ -360,6 +481,9 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { } matches := exists && matchErr == nil wipeEntry := cutil.IsTrue(part.WipePartitionEntry) + partInUse := iutil.StrSliceContains(activeParts, fmt.Sprintf("%s%s%d", blockDevResolved, prefix, part.Number)) + + var modification bool // This is a translation of the matrix in the operator notes. switch { @@ -367,10 +491,12 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { s.Logger.Info("partition %d specified as nonexistant and no partition was found. Success.", part.Number) case !exists && shouldExist: op.CreatePartition(part) + modification = true case exists && !shouldExist && !wipeEntry: return fmt.Errorf("partition %d exists but is specified as nonexistant and wipePartitionEntry is false", part.Number) case exists && !shouldExist && wipeEntry: op.DeletePartition(part.Number) + modification = true case exists && shouldExist && matches: s.Logger.Info("partition %d found with correct specifications", part.Number) case exists && shouldExist && !wipeEntry && !matches: @@ -383,6 +509,7 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { part.Label = &info.Label part.StartSector = &info.StartSector op.CreatePartition(part) + modification = true } else { return fmt.Errorf("Partition %d didn't match: %v", part.Number, matchErr) } @@ -390,10 +517,15 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { s.Logger.Info("partition %d did not meet specifications, wiping partition entry and recreating", part.Number) op.DeletePartition(part.Number) op.CreatePartition(part) + modification = true default: // unfortunatey, golang doesn't check that all cases are handled exhaustively return fmt.Errorf("Unreachable code reached when processing partition %d. golang--", part.Number) } + + if partInUse && modification { + return fmt.Errorf("refusing to modify active partition %d on %q", part.Number, devAlias) + } } if err := op.Commit(); err != nil { From c2cc56cd0208efe8929f59424c2c4b668617f62b Mon Sep 17 00:00:00 2001 From: Kai Lueke Date: Fri, 29 Sep 2023 18:06:09 +0200 Subject: [PATCH 2/2] sgdisk: Run partx after partition changes The sgdisk tool does not update the kernel partition table with BLKPG in contrast to other similar tools but only uses BLKRRPART which fails as soon as one partition of the disk is mounted. Update the kernel partition table with partx when we know that a partition of the disk is in use. --- docs/release-notes.md | 4 +++ dracut/30ignition/module-setup.sh | 1 + internal/distro/distro.go | 2 ++ internal/exec/stages/disks/partitions.go | 34 ++++++++++++++++++++++++ 4 files changed, 41 insertions(+) diff --git a/docs/release-notes.md b/docs/release-notes.md index fc388b85b..8e842f0a2 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -10,8 +10,12 @@ nav_order: 9 ### Features +- Support partitioning disk with mounted partitions + ### Changes +- The Dracut module now installs partx + ### Bug fixes - Fix Akamai Ignition base64 decoding on padded payloads diff --git a/dracut/30ignition/module-setup.sh b/dracut/30ignition/module-setup.sh index e1d3e4806..54867d107 100755 --- a/dracut/30ignition/module-setup.sh +++ b/dracut/30ignition/module-setup.sh @@ -39,6 +39,7 @@ install() { mkfs.fat \ mkfs.xfs \ mkswap \ + partx \ sgdisk \ useradd \ userdel \ diff --git a/internal/distro/distro.go b/internal/distro/distro.go index 4879f805e..e52546492 100644 --- a/internal/distro/distro.go +++ b/internal/distro/distro.go @@ -36,6 +36,7 @@ var ( groupdelCmd = "groupdel" mdadmCmd = "mdadm" mountCmd = "mount" + partxCmd = "partx" sgdiskCmd = "sgdisk" modprobeCmd = "modprobe" udevadmCmd = "udevadm" @@ -92,6 +93,7 @@ func GroupaddCmd() string { return groupaddCmd } func GroupdelCmd() string { return groupdelCmd } func MdadmCmd() string { return mdadmCmd } func MountCmd() string { return mountCmd } +func PartxCmd() string { return partxCmd } func SgdiskCmd() string { return sgdiskCmd } func ModprobeCmd() string { return modprobeCmd } func UdevadmCmd() string { return udevadmCmd } diff --git a/internal/exec/stages/disks/partitions.go b/internal/exec/stages/disks/partitions.go index 8ae45ccb0..d867a5253 100644 --- a/internal/exec/stages/disks/partitions.go +++ b/internal/exec/stages/disks/partitions.go @@ -23,6 +23,7 @@ import ( "errors" "fmt" "os" + "os/exec" "path/filepath" "regexp" "sort" @@ -31,6 +32,7 @@ import ( cutil "github.com/coreos/ignition/v2/config/util" "github.com/coreos/ignition/v2/config/v3_5_experimental/types" + "github.com/coreos/ignition/v2/internal/distro" "github.com/coreos/ignition/v2/internal/exec/util" "github.com/coreos/ignition/v2/internal/sgdisk" iutil "github.com/coreos/ignition/v2/internal/util" @@ -472,6 +474,10 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { return err } + var partxAdd []uint64 + var partxDelete []uint64 + var partxUpdate []uint64 + for _, part := range resolvedPartitions { shouldExist := partitionShouldExist(part) info, exists := diskInfo.GetPartition(part.Number) @@ -492,11 +498,13 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { case !exists && shouldExist: op.CreatePartition(part) modification = true + partxAdd = append(partxAdd, uint64(part.Number)) case exists && !shouldExist && !wipeEntry: return fmt.Errorf("partition %d exists but is specified as nonexistant and wipePartitionEntry is false", part.Number) case exists && !shouldExist && wipeEntry: op.DeletePartition(part.Number) modification = true + partxDelete = append(partxDelete, uint64(part.Number)) case exists && shouldExist && matches: s.Logger.Info("partition %d found with correct specifications", part.Number) case exists && shouldExist && !wipeEntry && !matches: @@ -510,6 +518,7 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { part.StartSector = &info.StartSector op.CreatePartition(part) modification = true + partxUpdate = append(partxUpdate, uint64(part.Number)) } else { return fmt.Errorf("Partition %d didn't match: %v", part.Number, matchErr) } @@ -518,6 +527,7 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { op.DeletePartition(part.Number) op.CreatePartition(part) modification = true + partxUpdate = append(partxUpdate, uint64(part.Number)) default: // unfortunatey, golang doesn't check that all cases are handled exhaustively return fmt.Errorf("Unreachable code reached when processing partition %d. golang--", part.Number) @@ -532,6 +542,30 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { return fmt.Errorf("commit failure: %v", err) } + // In contrast to similar tools, sgdisk does not trigger the update of the + // kernel partition table with BLKPG but only uses BLKRRPART which fails + // as soon as one partition of the disk is mounted + if len(activeParts) > 0 { + runPartxCommand := func(op string, partitions []uint64) error { + for _, partNr := range partitions { + cmd := exec.Command(distro.PartxCmd(), "--"+op, "--nr", strconv.FormatUint(partNr, 10), blockDevResolved) + if _, err := s.Logger.LogCmd(cmd, "triggering partition %d %s on %q", partNr, op, devAlias); err != nil { + return fmt.Errorf("partition %s failed: %v", op, err) + } + } + return nil + } + if err := runPartxCommand("delete", partxDelete); err != nil { + return err + } + if err := runPartxCommand("update", partxUpdate); err != nil { + return err + } + if err := runPartxCommand("add", partxAdd); err != nil { + return err + } + } + // It's best to wait here for the /dev/ABC entries to be // (re)created, not only for other parts of the initramfs but // also because s.waitOnDevices() can still race with udev's