From 6098b3ce16c797033093567e97e5e060aecbc577 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 10 Aug 2026 08:17:22 +0000 Subject: [PATCH 1/5] utils: shrink buffer in libcrun_initialize_apparmor Only the first byte of the buffer is ever inspected, so a two byte buffer is bigger than needed. Reduce it to a single byte and read sizeof (buf) accordingly. Closes: https://github.com/containers/crun/issues/2160 Signed-off-by: Giuseppe Scrivano Co-Authored-By: Claude Opus 4.8 Signed-off-by: Giuseppe Scrivano --- src/libcrun/utils.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcrun/utils.c b/src/libcrun/utils.c index 514b9ac9fb..019e1751f5 100644 --- a/src/libcrun/utils.c +++ b/src/libcrun/utils.c @@ -815,7 +815,7 @@ libcrun_initialize_apparmor (libcrun_error_t *err) { cleanup_close int fd = -1; int size; - char buf[2]; + char buf[1]; if (apparmor_enabled >= 0) return apparmor_enabled; @@ -832,7 +832,7 @@ libcrun_initialize_apparmor (libcrun_error_t *err) return crun_make_error (err, errno, "open `/sys/module/apparmor/parameters/enabled`"); } - size = TEMP_FAILURE_RETRY (read (fd, buf, 2)); + size = TEMP_FAILURE_RETRY (read (fd, buf, sizeof (buf))); apparmor_enabled = size > 0 && buf[0] == 'Y' ? 1 : 0; From b9f427bd0bef17d1090acac6cce749e827fa101d Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 10 Aug 2026 08:17:40 +0000 Subject: [PATCH 2/5] utils: shrink buffer in is_current_process_confined The 256 byte buffer is much bigger than needed: the code only compares the leading "unconfined" token. Size the buffer to that token plus one byte, which is all the comparison requires. Closes: https://github.com/containers/crun/issues/2161 Signed-off-by: Giuseppe Scrivano Co-Authored-By: Claude Opus 4.8 Signed-off-by: Giuseppe Scrivano --- src/libcrun/utils.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libcrun/utils.c b/src/libcrun/utils.c index 019e1751f5..30acec7992 100644 --- a/src/libcrun/utils.c +++ b/src/libcrun/utils.c @@ -963,9 +963,12 @@ libcrun_is_apparmor_enabled (libcrun_error_t *err) static int is_current_process_confined (libcrun_container_t *container, libcrun_error_t *err) { +#define UNCONFINED "unconfined" +#define UNCONFINED_LEN (ssize_t) (sizeof (UNCONFINED) - 1) cleanup_free const char *attr_path = lsm_attr_path (container, "apparmor", "current", err); cleanup_close int fd = -1; - char buf[256]; + /* Only the "unconfined" token plus one delimiter byte are inspected. */ + char buf[UNCONFINED_LEN + 1]; if (UNLIKELY (attr_path == NULL)) return -1; @@ -981,9 +984,9 @@ is_current_process_confined (libcrun_container_t *container, libcrun_error_t *er if (UNLIKELY (bytes_read < 0)) return crun_make_error (err, errno, "read from `%s`", attr_path); -#define UNCONFINED "unconfined" -#define UNCONFINED_LEN (ssize_t) (sizeof (UNCONFINED) - 1) return bytes_read >= UNCONFINED_LEN && memcmp (buf, UNCONFINED, UNCONFINED_LEN); +#undef UNCONFINED +#undef UNCONFINED_LEN } int From 208e6fe804d1a0c51d3132edb9c56f00c427be28 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 10 Aug 2026 08:17:57 +0000 Subject: [PATCH 3/5] utils: harden the confined check in is_current_process_confined The previous check had a few problems: - a short read (fewer bytes than "unconfined") returned 0, reporting the process as unconfined, when the safe assumption is that it is confined; - a profile named e.g. "unconfined_foo" matched on the first 10 bytes and was reported as unconfined; require the token to be followed by a delimiter (end of data, '\n' or ' '); - the return value was the raw memcmp() result, which can be negative and would be misinterpreted by callers as an error; - the read was not wrapped in TEMP_FAILURE_RETRY, so an EINTR turned into a spurious hard error. Normalize the result to 1 (confined) / 0 (unconfined) and default to confined whenever the token does not match exactly. Closes: https://github.com/containers/crun/issues/2165 Signed-off-by: Giuseppe Scrivano Co-Authored-By: Claude Opus 4.8 Signed-off-by: Giuseppe Scrivano --- src/libcrun/utils.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libcrun/utils.c b/src/libcrun/utils.c index 30acec7992..b0f1ef787c 100644 --- a/src/libcrun/utils.c +++ b/src/libcrun/utils.c @@ -980,11 +980,20 @@ is_current_process_confined (libcrun_container_t *container, libcrun_error_t *er if (UNLIKELY (check_proc_super_magic (fd, attr_path, err))) return -1; - ssize_t bytes_read = read (fd, buf, sizeof (buf) - 1); + ssize_t bytes_read = TEMP_FAILURE_RETRY (read (fd, buf, sizeof (buf))); if (UNLIKELY (bytes_read < 0)) return crun_make_error (err, errno, "read from `%s`", attr_path); - return bytes_read >= UNCONFINED_LEN && memcmp (buf, UNCONFINED, UNCONFINED_LEN); + /* The process is unconfined only when the attribute is exactly the token + "unconfined", optionally followed by a delimiter. Anything shorter (a + short read), or a longer profile name such as "unconfined_foo", means the + process is confined. When in doubt default to confined, which is the safe + assumption for the caller. */ + if (bytes_read < UNCONFINED_LEN || memcmp (buf, UNCONFINED, UNCONFINED_LEN) != 0) + return 1; + if (bytes_read == UNCONFINED_LEN || buf[UNCONFINED_LEN] == '\n' || buf[UNCONFINED_LEN] == ' ') + return 0; + return 1; #undef UNCONFINED #undef UNCONFINED_LEN } From cccf900902e6cd95b1b94a6809758a769b7edf0b Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 10 Aug 2026 08:27:41 +0000 Subject: [PATCH 4/5] linux: replace cached masked dir fd only after all fallible steps mount_masked_dir() aliased the container-owned private_data->maskdir_fd into a cleanup_close local, which created two owners for the same descriptor. On the fstat() failure path the cleanup handler closed the descriptor while private_data kept the stale number (later double closed at teardown); on the fs_move_mount_to() failure path the descriptor was closed explicitly and then a second time by the cleanup handler, with do_mount() running in between and able to reuse the number. Stop aliasing: use private_data->maskdir_fd directly for the fallible operations and let the cleanup_close local own only the newly created bind mount. private_data->maskdir_fd is replaced only at the end, once no further error is possible, so ownership is never shared. Closes: https://github.com/containers/crun/issues/2170 Closes: https://github.com/containers/crun/issues/2171 Signed-off-by: Giuseppe Scrivano Co-Authored-By: Claude Opus 4.8 Signed-off-by: Giuseppe Scrivano --- src/libcrun/linux.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/libcrun/linux.c b/src/libcrun/linux.c index e99a53d99c..d2436f9fa1 100644 --- a/src/libcrun/linux.c +++ b/src/libcrun/linux.c @@ -1144,7 +1144,7 @@ static int mount_masked_dir (libcrun_container_t *container, int pathfd, const char *rel_path, libcrun_error_t *err) { struct private_data_s *private_data = get_private_data (container); - cleanup_close int mountfd = -1; + cleanup_close int newfd = -1; libcrun_error_t tmp_err = NULL; int ret; @@ -1160,24 +1160,23 @@ mount_masked_dir (libcrun_container_t *container, int pathfd, const char *rel_pa goto fallback_to_tmpfs; } - mountfd = private_data->maskdir_fd; - if (mountfd >= 0) + if (private_data->maskdir_fd >= 0) { - int rootfsfd = get_private_data (container)->rootfsfd; + int rootfsfd = private_data->rootfsfd; struct stat before, after; - ret = fstat (mountfd, &before); + ret = fstat (private_data->maskdir_fd, &before); if (UNLIKELY (ret < 0)) return crun_make_error (err, errno, "fstat masked dir `%s`", rel_path); - ret = fs_move_mount_to (mountfd, pathfd, NULL); + ret = fs_move_mount_to (private_data->maskdir_fd, pathfd, NULL); if (LIKELY (ret == 0)) { - mountfd = get_bind_mount (rootfsfd, rel_path, true, true, false, MS_PRIVATE, err); - if (UNLIKELY (mountfd < 0)) - return mountfd; + newfd = get_bind_mount (rootfsfd, rel_path, true, true, false, MS_PRIVATE, err); + if (UNLIKELY (newfd < 0)) + return newfd; - ret = fstat (mountfd, &after); + ret = fstat (newfd, &after); if (UNLIKELY (ret < 0)) return crun_make_error (err, errno, "fstat masked dir `%s`", rel_path); @@ -1185,8 +1184,8 @@ mount_masked_dir (libcrun_container_t *container, int pathfd, const char *rel_pa return crun_make_error (err, 0, "race condition detected remounting masked path `/%s`", rel_path); TEMP_FAILURE_RETRY (close (private_data->maskdir_fd)); - private_data->maskdir_fd = mountfd; - mountfd = -1; + private_data->maskdir_fd = newfd; + newfd = -1; return 0; } From 579ccbf3d8c31f63a83af3f6479ec48e784316fb Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 10 Aug 2026 08:27:55 +0000 Subject: [PATCH 5/5] linux: report the real reason when move_mount masked dir fails On the fs_move_mount_to() failure path in mount_masked_dir() tmp_err is always NULL, so the "tmpfs fallback" warning always printed "unknown error" and the ternary guarding it always took the false branch. Capture the errno from fs_move_mount_to() into tmp_err so the fallback warning reports the actual reason. Save errno before anything else can clobber it and release any previously stored error first, so tmp_err is never overwritten (and leaked) while holding an allocation. Closes: https://github.com/containers/crun/issues/2172 Signed-off-by: Giuseppe Scrivano Co-Authored-By: Claude Opus 4.8 Signed-off-by: Giuseppe Scrivano --- src/libcrun/linux.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libcrun/linux.c b/src/libcrun/linux.c index d2436f9fa1..061718a3e2 100644 --- a/src/libcrun/linux.c +++ b/src/libcrun/linux.c @@ -1190,6 +1190,9 @@ mount_masked_dir (libcrun_container_t *container, int pathfd, const char *rel_pa return 0; } + /* tmp_err is NULL here: get_shared_empty_dir_cached() succeeded and did not set it. */ + crun_make_error (&tmp_err, errno, "move mount masked dir `%s`", rel_path); + TEMP_FAILURE_RETRY (close (private_data->maskdir_fd)); private_data->maskdir_fd = -1; }