osbuild: ensure loop.Loop() has the required device node

When loop.Loop() is called and a new loop device must be allocated
there is no gurantee that the correct device node is available on
the system. In containers /dev is often just a tmpfs with static
device nodes. So when /dev/loopN is not available when the
container is created the device node will be missing even if
`get_unbound()` create a new loop device for us.

This commit ensures that the device node is available. It creates
it unconditionally and ignores any EEXIST errors to ensure there
is no TOCTOU issue.

Note that the test could have passed a `Loop(dir_fd=open(tmpdir))`
instead of creating/patching loop.DEV_PATH but it seems slightly
nicer to test the flow without a custom dir_path as this is what
the real code that creates a loop device is also using.

osbuild/loop.py

@@ -15,6 +15,9 @@ __all__ = [
     "UnexpectedDevice"
 ]
 
+# can be mocked in tests
+DEV_PATH = "/dev"
+
 
 class UnexpectedDevice(Exception):
     def __init__(self, expected_minor, rdev, mode):
@@ -124,8 +127,14 @@ class Loop:
 
         with contextlib.ExitStack() as stack:
             if not dir_fd:
-                dir_fd = os.open("/dev", os.O_DIRECTORY)
+                dir_fd = os.open(DEV_PATH, os.O_DIRECTORY)
                 stack.callback(lambda: os.close(dir_fd))
+            # ensure the device node is availale, in containers it may
+            # not get dynamically created
+            try:
+                self.mknod(dir_fd)
+            except FileExistsError:
+                pass
             self.fd = os.open(self.devname, os.O_RDWR, dir_fd=dir_fd)
 
         info = os.stat(self.fd)
@@ -534,7 +543,7 @@ class LoopControl:
 
         with contextlib.ExitStack() as stack:
             if not dir_fd:
-                dir_fd = os.open("/dev", os.O_DIRECTORY)
+                dir_fd = os.open(DEV_PATH, os.O_DIRECTORY)
                 stack.callback(lambda: os.close(dir_fd))
 
             self.fd = os.open("loop-control", os.O_RDWR, dir_fd=dir_fd)

test/mod/test_loop.py

@@ -5,9 +5,11 @@
 import contextlib
 import fcntl
 import os
+import pathlib
 import threading
 import time
 from tempfile import TemporaryDirectory, TemporaryFile
+from unittest.mock import patch
 
 import pytest
 
@@ -255,6 +257,17 @@ def test_on_close(tempdir):
         ctl.close()
 
 
-def test_loop_handles_error_in_init():
+@patch("os.open", side_effect=FileNotFoundError)
+def test_loop_handles_error_in_init(mocked_open):
     with pytest.raises(FileNotFoundError):
-        lopo = loop.Loop("non-existing")
+        lopo = loop.Loop(999)
+
+
+@pytest.mark.skipif(os.getuid() != 0, reason="root only")
+def test_loop_create_mknod():
+    # tmpdir must be /var/tmp because /tmp is usually mounted with "nodev"
+    with TemporaryDirectory(dir="/var/tmp") as tmpdir:
+        with patch.object(loop, "DEV_PATH", new=tmpdir) as mocked_dev_path:
+            lopo = loop.Loop(1337)
+            assert lopo.devname == "loop1337"
+            assert pathlib.Path(f"{tmpdir}/loop1337").is_block_device()