From 98e28335b7da95f1eed6e298196a482d8d990d21 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 9 Dec 2024 13:52:32 +0100 Subject: [PATCH] discover-image: modernize image discovery around O_PATH let's always pin the image fd as early as we can, then derive all properties off it, to have a consistent view on things. --- src/shared/discover-image.c | 160 ++++++++++++++++++++++-------------- 1 file changed, 98 insertions(+), 62 deletions(-) diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index 35386dfc58f..674811ca354 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -328,19 +328,19 @@ static int image_update_quota(Image *i, int fd) { static int image_make( ImageClass c, const char *pretty, - int dfd, - const char *path, + int dir_fd, + const char *dir_path, const char *filename, + int fd, /* O_PATH fd */ const struct stat *st, Image **ret) { - _cleanup_free_ char *pretty_buffer = NULL, *parent = NULL; - struct stat stbuf; + _cleanup_free_ char *pretty_buffer = NULL; bool read_only; int r; - assert(dfd >= 0 || dfd == AT_FDCWD); - assert(path || dfd == AT_FDCWD); + assert(dir_fd >= 0 || dir_fd == AT_FDCWD); + assert(dir_path || dir_fd == AT_FDCWD); assert(filename); /* We explicitly *do* follow symlinks here, since we want to allow symlinking trees, raw files and block @@ -349,23 +349,36 @@ static int image_make( * This function returns -ENOENT if we can't find the image after all, and -EMEDIUMTYPE if it's not a file we * recognize. */ + _cleanup_close_ int _fd = -EBADF; + if (fd < 0) { + /* If we didn't get an fd passed in, then let's pin it via O_PATH now */ + _fd = openat(dir_fd, filename, O_PATH|O_CLOEXEC); + if (_fd < 0) + return -errno; + + fd = _fd; + st = NULL; /* refresh stat() data now that we have the inode pinned */ + } + + struct stat stbuf; if (!st) { - if (fstatat(dfd, filename, &stbuf, 0) < 0) + if (fstat(fd, &stbuf) < 0) return -errno; st = &stbuf; } - if (!path) - (void) fd_get_path(dfd, &parent); + _cleanup_free_ char *parent = NULL; + if (!dir_path) { + (void) fd_get_path(dir_fd, &parent); + dir_path = parent; + } read_only = - (path && path_startswith(path, "/usr")) || - (parent && path_startswith(parent, "/usr")) || - (faccessat(dfd, filename, W_OK, AT_EACCESS) < 0 && errno == EROFS); + (dir_path && path_startswith(dir_path, "/usr")) || + (faccessat(fd, "", W_OK, AT_EACCESS|AT_EMPTY_PATH) < 0 && errno == EROFS); if (S_ISDIR(st->st_mode)) { - _cleanup_close_ int fd = -EBADF; unsigned file_attr = 0; usec_t crtime = 0; @@ -385,10 +398,6 @@ static int image_make( pretty = pretty_buffer; } - fd = openat(dfd, filename, O_CLOEXEC|O_NOCTTY|O_DIRECTORY); - if (fd < 0) - return -errno; - if (btrfs_might_be_subvol(st)) { r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); @@ -406,7 +415,7 @@ static int image_make( r = image_new(IMAGE_SUBVOLUME, c, pretty, - path, + dir_path, filename, info.read_only || read_only, info.otime, @@ -431,7 +440,7 @@ static int image_make( r = image_new(IMAGE_DIRECTORY, c, pretty, - path, + dir_path, filename, read_only || (file_attr & FS_IMMUTABLE_FL), crtime, @@ -450,7 +459,7 @@ static int image_make( if (!ret) return 0; - (void) fd_getcrtime_at(dfd, filename, AT_SYMLINK_FOLLOW, &crtime); + (void) fd_getcrtime(fd, &crtime); if (!pretty) { r = extract_image_basename( @@ -468,7 +477,7 @@ static int image_make( r = image_new(IMAGE_RAW, c, pretty, - path, + dir_path, filename, !(st->st_mode & 0222) || read_only, crtime, @@ -483,7 +492,6 @@ static int image_make( return 0; } else if (S_ISBLK(st->st_mode)) { - _cleanup_close_ int block_fd = -EBADF; uint64_t size = UINT64_MAX; /* A block device */ @@ -504,30 +512,22 @@ static int image_make( pretty = pretty_buffer; } - block_fd = openat(dfd, filename, O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY); + _cleanup_close_ int block_fd = fd_reopen(fd, O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY); if (block_fd < 0) - log_debug_errno(errno, "Failed to open block device %s/%s, ignoring: %m", path ?: strnull(parent), filename); + log_debug_errno(errno, "Failed to open block device %s/%s, ignoring: %m", strnull(dir_path), filename); else { - /* Refresh stat data after opening the node */ - if (fstat(block_fd, &stbuf) < 0) - return -errno; - st = &stbuf; - - if (!S_ISBLK(st->st_mode)) /* Verify that what we opened is actually what we think it is */ - return -ENOTTY; - if (!read_only) { int state = 0; if (ioctl(block_fd, BLKROGET, &state) < 0) - log_debug_errno(errno, "Failed to issue BLKROGET on device %s/%s, ignoring: %m", path ?: strnull(parent), filename); + log_debug_errno(errno, "Failed to issue BLKROGET on device %s/%s, ignoring: %m", strnull(dir_path), filename); else if (state) read_only = true; } r = blockdev_get_device_size(block_fd, &size); if (r < 0) - log_debug_errno(r, "Failed to issue BLKGETSIZE64 on device %s/%s, ignoring: %m", path ?: strnull(parent), filename); + log_debug_errno(r, "Failed to issue BLKGETSIZE64 on device %s/%s, ignoring: %m", strnull(dir_path), filename); block_fd = safe_close(block_fd); } @@ -535,7 +535,7 @@ static int image_make( r = image_new(IMAGE_BLOCK, c, pretty, - path, + dir_path, filename, !(st->st_mode & 0222) || read_only, 0, @@ -597,7 +597,10 @@ int image_find(ImageClass class, const char *root, Image **ret) { - int r; + /* As mentioned above, we follow symlinks on this fstatat(), because we want to permit people to + * symlink block devices into the search path. (For now, we disable that when operating relative to + * some root directory.) */ + int open_flags = root ? O_NOFOLLOW : 0, r; assert(class >= 0); assert(class < _IMAGE_CLASS_MAX); @@ -614,8 +617,6 @@ int image_find(ImageClass class, NULSTR_FOREACH(path, pick_image_search_path(class)) { _cleanup_free_ char *resolved = NULL; _cleanup_closedir_ DIR *d = NULL; - struct stat st; - int flags; r = chase_and_opendir(path, root, CHASE_PREFIX_ROOT, &resolved, &d); if (r == -ENOENT) @@ -623,22 +624,22 @@ int image_find(ImageClass class, if (r < 0) return r; - /* As mentioned above, we follow symlinks on this fstatat(), because we want to permit people - * to symlink block devices into the search path. (For now, we disable that when operating - * relative to some root directory.) */ - flags = root ? AT_SYMLINK_NOFOLLOW : 0; - STRV_FOREACH(n, names) { _cleanup_free_ char *fname_buf = NULL; const char *fname = *n; - if (fstatat(dirfd(d), fname, &st, flags) < 0) { + _cleanup_close_ int fd = openat(dirfd(d), fname, O_PATH|O_CLOEXEC|open_flags); + if (fd < 0) { if (errno != ENOENT) return -errno; - continue; /* Vanished while we were looking at it */ + continue; } + struct stat st; + if (fstat(fd, &st) < 0) + return -errno; + if (endswith(fname, ".raw")) { if (!S_ISREG(st.st_mode)) { log_debug("Ignoring non-regular file '%s' with .raw suffix.", fname); @@ -688,6 +689,7 @@ int image_find(ImageClass class, /* Refresh the stat data for the discovered target */ st = result.st; + fd = safe_close(fd); _cleanup_free_ char *bn = NULL; r = path_extract_filename(result.path, &bn); @@ -707,7 +709,7 @@ int image_find(ImageClass class, continue; } - r = image_make(class, name, dirfd(d), resolved, fname, &st, ret); + r = image_make(class, name, dirfd(d), resolved, fname, fd, &st, ret); if (IN_SET(r, -ENOENT, -EMEDIUMTYPE)) continue; if (r < 0) @@ -721,7 +723,14 @@ int image_find(ImageClass class, } if (class == IMAGE_MACHINE && streq(name, ".host")) { - r = image_make(class, ".host", AT_FDCWD, NULL, empty_to_root(root), NULL, ret); + r = image_make(class, + ".host", + /* dir_fd= */ AT_FDCWD, + /* dir_path= */ NULL, + /* filename= */ empty_to_root(root), + /* fd= */ -EBADF, + /* st= */ NULL, + ret); if (r < 0) return r; @@ -741,9 +750,25 @@ int image_from_path(const char *path, Image **ret) { * overridden by another, different image earlier in the search path */ if (path_equal(path, "/")) - return image_make(IMAGE_MACHINE, ".host", AT_FDCWD, NULL, "/", NULL, ret); - - return image_make(_IMAGE_CLASS_INVALID, NULL, AT_FDCWD, NULL, path, NULL, ret); + return image_make( + IMAGE_MACHINE, + ".host", + /* dir_fd= */ AT_FDCWD, + /* dir_path= */ NULL, + /* filename= */ "/", + /* fd= */ -EBADF, + /* st= */ NULL, + ret); + + return image_make( + _IMAGE_CLASS_INVALID, + /* pretty= */ NULL, + /* dir_fd= */ AT_FDCWD, + /* dir_path= */ NULL, + /* filename= */ path, + /* fd= */ -EBADF, + /* st= */ NULL, + ret); } int image_find_harder(ImageClass class, const char *name_or_path, const char *root, Image **ret) { @@ -758,7 +783,10 @@ int image_discover( const char *root, Hashmap *h) { - int r; + /* As mentioned above, we follow symlinks on this fstatat(), because we want to permit people to + * symlink block devices into the search path. (For now, we disable that when operating relative to + * some root directory.) */ + int open_flags = root ? O_NOFOLLOW : 0, r; assert(class >= 0); assert(class < _IMAGE_CLASS_MAX); @@ -778,22 +806,22 @@ int image_discover( _cleanup_free_ char *pretty = NULL, *fname_buf = NULL; _cleanup_(image_unrefp) Image *image = NULL; const char *fname = de->d_name; - struct stat st; - int flags; if (dot_or_dot_dot(fname)) continue; - /* As mentioned above, we follow symlinks on this fstatat(), because we want to - * permit people to symlink block devices into the search path. */ - flags = root ? AT_SYMLINK_NOFOLLOW : 0; - if (fstatat(dirfd(d), fname, &st, flags) < 0) { - if (errno == ENOENT) - continue; + _cleanup_close_ int fd = openat(dirfd(d), fname, O_PATH|O_CLOEXEC|open_flags); + if (fd < 0) { + if (errno != ENOENT) + return -errno; - return -errno; + continue; /* Vanished while we were looking at it */ } + struct stat st; + if (fstat(fd, &st) < 0) + return -errno; + if (S_ISREG(st.st_mode)) { r = extract_image_basename( fname, @@ -856,6 +884,7 @@ int image_discover( /* Refresh the stat data for the discovered target */ st = result.st; + fd = safe_close(fd); _cleanup_free_ char *bn = NULL; r = path_extract_filename(result.path, &bn); @@ -901,7 +930,7 @@ int image_discover( if (hashmap_contains(h, pretty)) continue; - r = image_make(class, pretty, dirfd(d), resolved, fname, &st, &image); + r = image_make(class, pretty, dirfd(d), resolved, fname, fd, &st, &image); if (IN_SET(r, -ENOENT, -EMEDIUMTYPE)) continue; if (r < 0) @@ -920,7 +949,14 @@ int image_discover( if (class == IMAGE_MACHINE && !hashmap_contains(h, ".host")) { _cleanup_(image_unrefp) Image *image = NULL; - r = image_make(IMAGE_MACHINE, ".host", AT_FDCWD, NULL, empty_to_root("/"), NULL, &image); + r = image_make(IMAGE_MACHINE, + ".host", + /* dir_fd= */ AT_FDCWD, + /* dir_path= */ NULL, + empty_to_root(root), + /* fd= */ -EBADF, + /* st= */ NULL, + &image); if (r < 0) return r; -- 2.47.3