From de862276eddbbe76b436213b4d427205356d1886 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Tue, 28 Mar 2023 16:19:47 +0100 Subject: [PATCH] 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. --- man/systemd-sysext.xml | 5 ++--- src/shared/discover-image.c | 8 +++++--- units/systemd-sysext.service | 2 -- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/man/systemd-sysext.xml b/man/systemd-sysext.xml index 39a16d8e8fb..258c7142c93 100644 --- a/man/systemd-sysext.xml +++ b/man/systemd-sysext.xml @@ -84,9 +84,8 @@ them they may optionally carry Verity authentication information. System extensions are automatically looked for in the directories - /etc/extensions/, /run/extensions/, - /var/lib/extensions/, /usr/lib/extensions/ and - /usr/local/lib/extensions/. The first two listed directories are not suitable for + /etc/extensions/, /run/extensions/ and + /var/lib/extensions/. 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 /var/lib/extensions/. 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 fa018cb9123..5873741c8c6 100644 --- 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 f8c26f5fbfa..9a8d4ebc5f8 100644 --- 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