Simplify become_root()

- Instead of forking, use spawn() to start the newuidmap, newgidmap
processes early
- Instead of multiprocessing.Event(), use flock to handle the necessary
locking

diff --git a/mkosi/__init__.py b/mkosi/__init__.py
index 49141b584536f60209a7e5b5c48e28d1caeb23af..92783e7102a71a633cce35aad903f61c80a9c2ff 100644 (file)
--- a/mkosi/__init__.py
+++ b/mkosi/__init__.py
@@ -30,7 +30,7 @@ from mkosi.config import (
     MkosiConfigParser,
     SecureBootSignTool,
 )
-from mkosi.install import add_dropin_config_from_resource, copy_path, flock
+from mkosi.install import add_dropin_config_from_resource, copy_path
 from mkosi.log import Style, color_error, complete_step, die, log_step
 from mkosi.manifest import Manifest
 from mkosi.mounts import mount_overlay, mount_passwd, mount_tools, scandir_recursive
@@ -47,6 +47,7 @@ from mkosi.util import (
     OutputFormat,
     Verb,
     flatten,
+    flock,
     format_bytes,
     format_rlimit,
     is_apt_distribution,

diff --git a/mkosi/install.py b/mkosi/install.py
index 4f355580e2bd93714749355ae241a8bd2f3a2b3f..7efce6fedfb559189994bed37adb02fe91900210 100644 (file)
--- a/mkosi/install.py
+++ b/mkosi/install.py
@@ -1,10 +1,6 @@
 # SPDX-License-Identifier: LGPL-2.1+
 
-import contextlib
-import fcntl
 import importlib.resources
-import os
-from collections.abc import Iterator
 from pathlib import Path
 from typing import Optional
 
@@ -31,17 +27,6 @@ def add_dropin_config_from_resource(
     write_resource(dropin, resource, key, mode=0o644)
 
 
-@contextlib.contextmanager
-def flock(path: Path) -> Iterator[Path]:
-    fd = os.open(path, os.O_CLOEXEC|os.O_DIRECTORY|os.O_RDONLY)
-    try:
-        fcntl.fcntl(fd, fcntl.FD_CLOEXEC)
-        fcntl.flock(fd, fcntl.LOCK_EX)
-        yield Path(path)
-    finally:
-        os.close(fd)
-
-
 def copy_path(
     src: Path,
     dst: Path,

diff --git a/mkosi/run.py b/mkosi/run.py
index 240bf0d2f3a4355260411a5e69ad397857e72555..2a287bd0185637604e3f25ea6b975c53bdf0fbcc 100644 (file)
--- a/mkosi/run.py
+++ b/mkosi/run.py
@@ -4,8 +4,8 @@ import asyncio
 import asyncio.tasks
 import ctypes
 import ctypes.util
+import fcntl
 import logging
-import multiprocessing
 import os
 import pwd
 import queue
@@ -22,7 +22,7 @@ from typing import Any, Awaitable, Mapping, Optional, Sequence, Tuple, Type, Typ
 
 from mkosi.log import ARG_DEBUG, ARG_DEBUG_SHELL, die
 from mkosi.types import _FILE, CompletedProcess, PathString, Popen
-from mkosi.util import InvokingUser, make_executable
+from mkosi.util import InvokingUser, flock, make_executable
 
 CLONE_NEWNS = 0x00020000
 CLONE_NEWUSER = 0x10000000
@@ -79,42 +79,41 @@ def become_root() -> tuple[int, int]:
     subuid = read_subrange(Path("/etc/subuid"))
     subgid = read_subrange(Path("/etc/subgid"))
 
-    event = multiprocessing.Event()
     pid = os.getpid()
 
-    child = os.fork()
-    if child == 0:
-        event.wait()
-
-        # We map the private UID range configured in /etc/subuid and /etc/subgid into the container using
-        # newuidmap and newgidmap. On top of that, we also make sure to map in the user running mkosi so that
-        # we can run still chown stuff to that user or run stuff as that user which will make sure any
-        # generated files are owned by that user. We don't map to the last user in the range as the last user
-        # is sometimes used in tests as a default value and mapping to that user might break those tests.
-        newuidmap = [
-            "newuidmap", pid,
-            0, subuid, SUBRANGE - 100,
-            SUBRANGE - 100, os.getuid(), 1,
-            SUBRANGE - 100 + 1, subuid + SUBRANGE - 100 + 1, 99
-        ]
-        run([str(x) for x in newuidmap])
-
-        newgidmap = [
-            "newgidmap", pid,
-            0, subgid, SUBRANGE - 100,
-            SUBRANGE - 100, os.getgid(), 1,
-            SUBRANGE - 100 + 1, subgid + SUBRANGE - 100 + 1, 99
-        ]
-        run([str(x) for x in newgidmap])
-
-        sys.stdout.flush()
-        sys.stderr.flush()
+    # We map the private UID range configured in /etc/subuid and /etc/subgid into the container using
+    # newuidmap and newgidmap. On top of that, we also make sure to map in the user running mkosi so that
+    # we can run still chown stuff to that user or run stuff as that user which will make sure any
+    # generated files are owned by that user. We don't map to the last user in the range as the last user
+    # is sometimes used in tests as a default value and mapping to that user might break those tests.
+    newuidmap = [
+        "flock", "--exclusive", "--no-fork", "/etc/subuid", "newuidmap", pid,
+        0, subuid, SUBRANGE - 100,
+        SUBRANGE - 100, os.getuid(), 1,
+        SUBRANGE - 100 + 1, subuid + SUBRANGE - 100 + 1, 99
+    ]
 
-        os._exit(0)
+    newgidmap = [
+        "flock", "--exclusive", "--no-fork", "/etc/subuid", "newgidmap", pid,
+        0, subgid, SUBRANGE - 100,
+        SUBRANGE - 100, os.getgid(), 1,
+        SUBRANGE - 100 + 1, subgid + SUBRANGE - 100 + 1, 99
+    ]
 
-    unshare(CLONE_NEWUSER)
-    event.set()
-    os.waitpid(child, 0)
+    newuidmap = [str(x) for x in newuidmap]
+    newgidmap = [str(x) for x in newgidmap]
+
+    # newuidmap and newgidmap have to run from outside the user namespace to be able to assign a uid mapping
+    # to the process in the user namespace. The mapping can only be assigned after the user namespace has
+    # been unshared. To make this work, we first lock /etc/subuid, then spawn the newuidmap and newgidmap
+    # processes, which we execute using flock so they don't execute before they can get a lock on /etc/subuid,
+    # then we unshare the user namespace and finally we unlock /etc/subuid, which allows the newuidmap and
+    # newgidmap processes to execute. we then wait for the processes to finish before continuing.
+    with flock(Path("/etc/subuid")) as fd, spawn(newuidmap) as uidmap, spawn(newgidmap) as gidmap:
+        unshare(CLONE_NEWUSER)
+        fcntl.flock(fd, fcntl.LOCK_UN)
+        uidmap.wait()
+        gidmap.wait()
 
     # By default, we're root in the user namespace because if we were our current user by default, we
     # wouldn't be able to chown stuff to be owned by root while the reverse is possible.

diff --git a/mkosi/util.py b/mkosi/util.py
index 5b6b0816957695da1c01847b61c71cef705f08f0..3bdf0ed309e511c426e26917dfbc2b4609b140f6 100644 (file)
--- a/mkosi/util.py
+++ b/mkosi/util.py
@@ -4,6 +4,7 @@ import ast
 import contextlib
 import enum
 import errno
+import fcntl
 import functools
 import importlib
 import itertools
@@ -309,3 +310,14 @@ def try_import(module: str) -> None:
         importlib.import_module(module)
     except ModuleNotFoundError:
         pass
+
+
+@contextlib.contextmanager
+def flock(path: Path) -> Iterator[int]:
+    fd = os.open(path, os.O_CLOEXEC|os.O_RDONLY)
+    try:
+        fcntl.fcntl(fd, fcntl.FD_CLOEXEC)
+        fcntl.flock(fd, fcntl.LOCK_EX)
+        yield fd
+    finally:
+        os.close(fd)