sysext: stop storing under /usr/lib[/local]/extensions/

sysexts are meant to extend /usr. All extension images and directories are opened and merged in a
single, read-only overlayfs layer, mounted on /usr.
So far, we had fallback storage directories in /usr/lib/extensions and /usr/local/lib/extensions.
This is problematic for three reasons.

Firstly, technically, for directory-based extensions the kernel will reject
creating such an overlay, as there is a recursion problem. It actively
validates that a lowerdir is not a child of another lowerdir, and fails with
-ELOOP if it is. So having a sysext /usr/lib/extensions/myextdir/ would result
in an overlayfs config lowerdir=/usr/lib/extensions/myextdir/usr/:/usr which is
not allowed, as indicated by Christian the kernel performs this check:

/*
 * Check if this layer root is a descendant of:
 * - another layer of this overlayfs instance
 * - upper/work dir of any overlayfs instance
 */

<...>

/* Walk back ancestors to root (inclusive) looking for traps */
while (!err && parent != next) {
        if (is_lower && ovl_lookup_trap_inode(sb, parent)) {
                err = -ELOOP;
                pr_err("overlapping %s path\n", name);

Secondly, there's a confusing aspect to this recursive storage. If you
have /usr/lib/extensions/myext.raw which contains /usr/lib/extensions/mynested.raw
'systemd-sysext merge' will only pick up the first one, but both will appear in
the merged root under /usr/lib/extensions/. So you have two extension images, both
appear in your merged filesystem, but only one is actually in use.

Finally, there's a conceptual aspect: the idea behind sysexts and hermetic /usr
is that the /usr tree is not modified locally, but owned by the vendor. Dropping
extensions in /usr thus goes contrary to this foundational concept.

diff --git a/man/systemd-sysext.xml b/man/systemd-sysext.xml
index 39a16d8e8fb8035728728ec2314d6d103ec70b6f..258c7142c93938be08ff2b46b82944afdb9cb4b6 100644 (file)
--- a/man/systemd-sysext.xml
+++ b/man/systemd-sysext.xml
@@ -84,9 +84,8 @@
     them they may optionally carry Verity authentication information.</para>
 
     <para>System extensions are automatically looked for in the directories
-    <filename>/etc/extensions/</filename>, <filename>/run/extensions/</filename>,
-    <filename>/var/lib/extensions/</filename>, <filename>/usr/lib/extensions/</filename> and
-    <filename>/usr/local/lib/extensions/</filename>. The first two listed directories are not suitable for
+    <filename>/etc/extensions/</filename>, <filename>/run/extensions/</filename> and
+    <filename>/var/lib/extensions/</filename>. The first two listed directories are not suitable for
     carrying large binary images, however are still useful for carrying symlinks to them. The primary place
     for installing system extensions is <filename>/var/lib/extensions/</filename>. Any directories found in
     these search directories are considered directory based extension images, any files with the

diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c
index fa018cb9123cc12474d96b57111612a2c713fe15..5873741c8c6f0deff08fedaf4814ce02c6f664e9 100644 (file)
--- a/src/shared/discover-image.c
+++ b/src/shared/discover-image.c
@@ -58,11 +58,13 @@ static const char* const image_search_path[_IMAGE_CLASS_MAX] = {
                             "/usr/local/lib/portables\0"
                             "/usr/lib/portables\0",
 
+        /* Note that we don't allow storing extensions under /usr/, unlike with other image types. That's
+         * because extension images are supposed to extend /usr/, so you get into recursive races, especially
+         * with directory-based extensions, as the kernel's OverlayFS explicitly checks for this and errors
+         * out with -ELOOP if it finds that a lowerdir= is a child of another lowerdir=. */
         [IMAGE_EXTENSION] = "/etc/extensions\0"             /* only place symlinks here */
                             "/run/extensions\0"             /* and here too */
-                            "/var/lib/extensions\0"         /* the main place for images */
-                            "/usr/local/lib/extensions\0"
-                            "/usr/lib/extensions\0",
+                            "/var/lib/extensions\0",        /* the main place for images */
 };
 
 static Image *image_free(Image *i) {

diff --git a/units/systemd-sysext.service b/units/systemd-sysext.service
index f8c26f5fbfa3eaed9cda32e90a4f2d00221f569e..9a8d4ebc5f8639630bb04ca8daaf4525742c3927 100644 (file)
--- a/units/systemd-sysext.service
+++ b/units/systemd-sysext.service
@@ -15,8 +15,6 @@ ConditionCapability=CAP_SYS_ADMIN
 ConditionDirectoryNotEmpty=|/etc/extensions
 ConditionDirectoryNotEmpty=|/run/extensions
 ConditionDirectoryNotEmpty=|/var/lib/extensions
-ConditionDirectoryNotEmpty=|/usr/local/lib/extensions
-ConditionDirectoryNotEmpty=|/usr/lib/extensions
 
 DefaultDependencies=no
 After=local-fs.target