Skip to content

fix: unmount the whole subtree below a mapper in Luks2.lock() - #4864

Open
xander-lin wants to merge 1 commit into
archlinux:masterfrom
xander-lin:fix/luks-lock-walks-the-whole-subtree
Open

xander-lin wants to merge 1 commit into
archlinux:masterfrom
xander-lin:fix/luks-lock-walks-the-whole-subtree

Conversation

@xander-lin

Copy link
Copy Markdown

Summary

Luks2.lock() unmounts what is mounted below the mapper before closing it, but it only looks one
level down:

lsblk_info = get_lsblk_info(self.luks_dev_path)

for child in lsblk_info.children:
	for mountpoint in child.mountpoints:
		debug(f'Unmounting {mountpoint}')
		umount(mountpoint, recursive=True)

	debug(f'Closing crypt device {child.name}')
	SysCommand(f'cryptsetup close {child.name}')

On LUKS-on-LVM nothing is mounted at that level. The layout looks like this:

sda1                    part    <- luks_dev_path
└─cryptlvm             crypt   <- child
  └─vg                  lvm     <- child of the child
    ├─vg-root           lvm     <- this is where / is mounted
    └─vg-home           lvm     <- this is where /home is mounted

The mountpoints are on the logical volumes, two levels below the mapper, so child.mountpoints is
empty for every child that matters. Nothing is unmounted, and cryptsetup close refuses to close a
mapper that still has a mounted filesystem below it:

cryptsetup close cryptlvm: Device or resource busy

lock() is what DeviceHandler.umount_all_existing() calls for every existing CRYPTO_LUKS
partition before a new installation starts, so this is the path taken when installing over a
previous LUKS-on-LVM system - the partition cannot be released and the installation stops.

Fix

Walk the whole subtree below the mapper and unmount children before parents, so the order holds
however deep the stack is:

--- a/archinstall/lib/disk/luks.py
+++ b/archinstall/lib/disk/luks.py
+def _lsblk_subtree(info: LsblkInfo) -> list[LsblkInfo]:
+	"""The device and everything stacked below it, parents before children."""
+	nodes = [info]
+
+	for child in info.children:
+		nodes.extend(_lsblk_subtree(child))
+
+	return nodes
+
@@ class Luks2
 	for child in lsblk_info.children:
-		for mountpoint in child.mountpoints:
-			debug(f'Unmounting {mountpoint}')
-			umount(mountpoint, recursive=True)
+		# Everything mounted below the child has to go before the child itself: on
+		# LUKS-on-LVM the mountpoints live on the logical volumes, which are
+		# grandchildren of the mapper, so looking one level down left them mounted
+		# and `cryptsetup close` then found the device busy.
+		for node in reversed(_lsblk_subtree(child)):
+			debug(f'Unmounting {node.name}')
+			umount(node.path, recursive=True)

Unmounting is done by device path rather than by mountpoint, so a mountpoint that lsblk reports as
[SWAP] - a display marker, not a path - cannot reach umount().

Reproducer

The device tree above is what lsblk reports for a LUKS container holding an LVM volume group. To
see the difference without building one, the shape can be handed to lock() directly:

from pathlib import Path
from unittest.mock import MagicMock, patch

from archinstall.lib.disk.luks import Luks2
from archinstall.lib.models.device import LsblkInfo

def node(name, children=None):
	info = MagicMock(spec=LsblkInfo)
	info.name = name
	info.path = Path(f'/dev/{name}')
	info.mountpoints = []
	info.children = children or []
	return info

root_lv = node('vg-root'); root_lv.mountpoints = [Path('/mnt/archinstall')]
home_lv = node('vg-home'); home_lv.mountpoints = [Path('/mnt/archinstall/home')]
vg = node('vg', [root_lv, home_lv])

lsblk_root = MagicMock(spec=LsblkInfo)
lsblk_root.children = [vg]

luks = object.__new__(Luks2)
luks.luks_dev_path = Path('/dev/sda1')

umounted = []

def record(path, **_kwargs):
	umounted.append(Path(path))

with (
	patch('archinstall.lib.disk.luks.get_lsblk_info', return_value=lsblk_root),
	patch('archinstall.lib.disk.luks.umount', side_effect=record),
	patch('archinstall.lib.disk.luks.SysCommand'),
):
	luks.lock()

# the first entry is the mapper itself, released at the top of lock()
print('unmounted below the mapper:', [str(p) for p in umounted if p != luks.luks_dev_path])

On master:

unmounted below the mapper: []

With the patch:

unmounted below the mapper: ['/dev/vg-home', '/dev/vg-root', '/dev/vg']

Verification boundary

This was verified with a stubbed device tree, not by building a real LUKS-on-LVM stack - the
machine this was written on has no lvm2. The lsblk shape it relies on is the one in the
diagram above, which is what lsblk reports for sda1 -> cryptlvm -> vg -> vg-root. Anyone
reviewing with lvm2 available can confirm it in a few commands; the code path is the same one.

Tests

tests/test_luks_lock_subtree.py:

  • the subtree walk returns the device and its grandchildren, parents first;
  • lock() unmounts the logical volumes under a mapper (this is what fails on master);
  • lock() still runs cryptsetup close for each child.

`lock()` looked one level below the mapper and unmounted the mountpoints it
found there. On LUKS-on-LVM nothing is mounted on that level: the mountpoints
live on the logical volumes, which are grandchildren of the mapper. They stayed
mounted, so `cryptsetup close` found the device busy and the mapper could not be
closed - which is what the cleanup before a re-run does.

Walk the whole subtree instead, children before parents.
@Torxed

Torxed commented Oct 28, 2026

Copy link
Copy Markdown
Member

I sense this is LLM contributions, but the isssue is valid and the fix seems reasonable.
But I can't merge it until tested on LVM as well.

h8d13 added a commit to h8d13/archinstoo that referenced this pull request Oct 28, 2026
* fix(disk): skip deleted partitions when mounting and booting

A DELETE entry keeps its dev_path, mountpoint and flags. The menu
sorts deletes first, so on a reinstall the ESP/boot/root lookups
returned the old partitions, and the mount stage mounted or swapon'd
them. Filter is_delete() there and when preparing fs packages.

Port of archlinux/archinstall#4861.

Signed-off-by: h8d13 <hadean-eon-dev@proton.me>

* fix(disk): never offer or accept a zero-length partition

A free gap between off-grain neighbours can round below 1 MiB once
snapped, and a config could carry a 0 B create. Keep only the aligned
span in FreeSpace so the menu's buffer check drops such gaps, and
reject a create without a length.

Port of archlinux/archinstall#4862.

Signed-off-by: h8d13 <hadean-eon-dev@proton.me>

* fix(disk): swapoff and unmount by device during cleanup

[SWAP] is dropped from lsblk mountpoints for the menu, so umount()
never saw an active swap and left it on before partitioning.
Luks2.lock() passed mountpoints to umount(), which lsblk rejects, so
any mounted mapper failed with 'Failed to read disk'. Swapoff swap
devices and unmount the mapper by its path.

Port of archlinux/archinstall#4864 and #4865.

Signed-off-by: h8d13 <hadean-eon-dev@proton.me>

* fix(disk): reject lvm with plain luks, no_encryption is plain lvm

perform_lvm_operations took the encrypted path for any enc config,
which only handles lvm_on_luks/luks_on_lvm, so luks or no_encryption
with lvm_config built no volume group. Reject luks at parse time and
route no_encryption to the plain lvm setup.

Port of archlinux/archinstall#4863 (that part only).

Signed-off-by: h8d13 <hadean-eon-dev@proton.me>

---------

Signed-off-by: h8d13 <hadean-eon-dev@proton.me>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants