discover-image: modernize image discovery around O_PATH (#35513)

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.
This commit is contained in:
Lennart Poettering
2024-12-17 17:39:59 +01:00
committed by GitHub
3 changed files with 117 additions and 67 deletions

View File

@@ -145,6 +145,11 @@ int read_attr_fd(int fd, unsigned *ret) {
if (!S_ISDIR(st.st_mode) && !S_ISREG(st.st_mode))
return -ENOTTY;
_cleanup_close_ int fd_close = -EBADF;
fd = fd_reopen_condition(fd, O_RDONLY|O_CLOEXEC|O_NOCTTY, O_PATH, &fd_close); /* drop O_PATH if it is set */
if (fd < 0)
return fd;
return RET_NERRNO(ioctl(fd, FS_IOC_GETFLAGS, ret));
}
@@ -155,11 +160,9 @@ int read_attr_at(int dir_fd, const char *path, unsigned *ret) {
assert(dir_fd >= 0 || dir_fd == AT_FDCWD);
assert(ret);
if (isempty(path)) {
fd = fd_reopen_condition(dir_fd, O_RDONLY|O_CLOEXEC|O_NOCTTY, O_PATH, &fd_close); /* drop O_PATH if it is set */
if (fd < 0)
return fd;
} else {
if (isempty(path) && dir_fd != AT_FDCWD)
fd = dir_fd;
else {
fd_close = xopenat(dir_fd, path, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
if (fd_close < 0)
return fd_close;

View File

@@ -329,6 +329,12 @@ int btrfs_subvol_get_info_fd(int fd, uint64_t subvol_id, BtrfsSubvolInfo *ret) {
assert(fd >= 0);
assert(ret);
/* Make sure this works on O_PATH fds */
_cleanup_close_ int fd_close = -EBADF;
fd = fd_reopen_condition(fd, O_CLOEXEC|O_RDONLY|O_DIRECTORY, O_PATH, &fd_close);
if (fd < 0)
return fd;
if (subvol_id == 0) {
r = btrfs_subvol_get_id_fd(fd, &subvol_id);
if (r < 0)

View File

@@ -295,10 +295,15 @@ static int image_update_quota(Image *i, int fd) {
return -EOPNOTSUPP;
if (fd < 0) {
fd_close = open(i->path, O_CLOEXEC|O_NOCTTY|O_DIRECTORY);
fd_close = open(i->path, O_CLOEXEC|O_DIRECTORY);
if (fd_close < 0)
return -errno;
fd = fd_close;
} else {
/* Convert from O_PATH to proper fd, if needed */
fd = fd_reopen_condition(fd, O_CLOEXEC|O_DIRECTORY, O_PATH, &fd_close);
if (fd < 0)
return fd;
}
r = btrfs_quota_scan_ongoing(fd);
@@ -323,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
@@ -344,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;
@@ -380,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);
@@ -401,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,
@@ -426,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,
@@ -445,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(
@@ -463,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,
@@ -478,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 */
@@ -499,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);
}
@@ -530,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,
@@ -592,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);
@@ -609,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)
@@ -618,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);
@@ -683,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);
@@ -702,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)
@@ -716,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;
@@ -736,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_MACHINE,
".host",
/* dir_fd= */ AT_FDCWD,
/* dir_path= */ NULL,
/* filename= */ "/",
/* fd= */ -EBADF,
/* st= */ NULL,
ret);
return image_make(_IMAGE_CLASS_INVALID, NULL, AT_FDCWD, NULL, path, 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) {
@@ -753,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);
@@ -773,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,
@@ -851,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);
@@ -896,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)
@@ -915,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;