From 31c96036f6ef71a4ecd90acd644f7ebbdb40165c Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Fri, 19 Feb 2021 19:29:09 +0000 Subject: [PATCH] runtime: Don't try to glnx_file_copy_at() from a magic symlink In the code path where we copy a file that would not otherwise be visible in the container, the copy doesn't actually work, because glnx_file_copy_at() opens regular files with O_NOFOLLOW and so refuses to read through the "magic symlink", failing with ELOOP. This can be reproduced by having a Vulkan ICD manifest in /etc/vulkan/icd.d/*.json whose `library_path` is just a SONAME. pressure-vessel tries to copy the file, and fails. It turns out glnx_file_copy_at() is not the right function here anyway, because it tries to fchown() the destination file to match the source file, which is going to work poorly if we are copying a file owned by root (which we often are). Just use the important part of it, which is glnx_regfile_copy_bytes(). Resolves: https://github.com/ValveSoftware/steam-runtime/issues/366 Signed-off-by: Simon McVittie <smcv@collabora.com> --- pressure-vessel/runtime.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/pressure-vessel/runtime.c b/pressure-vessel/runtime.c index a5bb64e4e..90ed82324 100644 --- a/pressure-vessel/runtime.c +++ b/pressure-vessel/runtime.c @@ -2084,8 +2084,8 @@ pv_runtime_take_from_provider (PvRuntime *self, if (flags & TAKE_FROM_PROVIDER_FLAGS_COPY_FALLBACK) { glnx_autofd int file_fd = -1; + glnx_autofd int dest_fd = -1; glnx_autofd int sysroot_fd = -1; - struct stat stat_buf; if (!glnx_opendirat (-1, self->provider_in_current_namespace, FALSE, &sysroot_fd, error)) @@ -2102,20 +2102,29 @@ pv_runtime_take_from_provider (PvRuntime *self, return FALSE; } - if (fstat (file_fd, &stat_buf) != 0) - { - return glnx_throw_errno_prefix (error, - "An error occurred using fstat for %s", - source_in_provider); - } - - g_autofree gchar *proc_fd = g_strdup_printf ("/proc/self/fd/%d", file_fd); + /* We already deleted ${parent_dirfd}/${base}, and we don't + * care about atomicity or durability here, so we can just + * write in-place. The permissions are uninteresting because + * we're not expecting other users to read this temporary + * sysroot anyway, so use 0600 just in case the source file + * has restrictive permissions. */ + dest_fd = TEMP_FAILURE_RETRY (openat (parent_dirfd, base, + O_WRONLY|O_CLOEXEC|O_NOCTTY|O_CREAT|O_EXCL, + 0600)); + + if (dest_fd < 0) + return glnx_throw_errno_prefix (error, + "Unable to open \"%s\" for writing", + dest_in_container); + if (glnx_regfile_copy_bytes (file_fd, dest_fd, (off_t) -1) < 0) + return glnx_throw_errno_prefix (error, + "Unable to copy contents of \"%s/%s\" to \"%s\"", + self->provider_in_current_namespace, + source_in_provider, + dest_in_container); - return glnx_file_copy_at (AT_FDCWD, proc_fd, &stat_buf, - parent_dirfd, base, - GLNX_FILE_COPY_OVERWRITE, - NULL, error); + return TRUE; } else { -- GitLab