tree-wide: address post-merge review of #43692 (#44616)

Address Lennart's post-merge review of #43692:

* Check for open errors once in sync-util.
* Use the ERRNO_IS_NEG_*() helpers on returned errors.
* Add is_symlink_at() and propagate errors from the dangling symlink
  checks.
* Propagate unexpected F_CREATED_QUERY errors.
* Try MSG_CMSG_CLOEXEC first in recvmsg_safe() and retry without it
  instead of checking the socket family.
* Refuse FORK_PACK_FDS without FORK_CLOEXEC_OFF.
* Explain why fd_reopen() tries O_EMPTYPATH before ".".
* Add argument comments and a test for is_symlink_at().
This commit is contained in:
Lennart Poettering authored and GitHub committed 2026-11-03 15:58:53 +01:00
commit a4ab89b5d5
23 files changed
+95 -60

No files matched your search

+2 -1
View File
@@ -824,7 +824,8 @@ int fd_reopen(int fd, int flags) {
if (fd == AT_FDCWD)
return RET_NERRNO(openat(AT_FDCWD, ".", flags | O_DIRECTORY));
/* Kernels since 7.2 reopen the fd itself when passed an empty path with O_EMPTYPATH. */
/* Kernels since 7.2 reopen the fd itself when passed an empty path with O_EMPTYPATH. Unlike "."
* below, that needs no search permission on directories, just like /proc/self/fd/. */
static thread_local int have_emptypath = -1;
if (have_emptypath != 0) {
int new_fd = openat(fd, "", flags | O_EMPTYPATH);
+33 -14
View File
@@ -1081,14 +1081,8 @@ int open_mkdir_at_full_label(int dirfd, const char *path, int flags, XOpenFlags
return TAKE_FD(fd);
}
static bool is_symlink_at(int dir_fd, const char *path) {
struct stat st;
return fstatat(dir_fd, path, &st, AT_SYMLINK_NOFOLLOW) >= 0 && S_ISLNK(st.st_mode);
}
int openat_report_new(int dirfd, const char *pathname, int flags, mode_t mode, bool *ret_newly_created) {
int fd;
int fd, r;
/* Just like openat(), but adds one thing: optionally returns whether we created the file anew or if
* it already existed before. This is only relevant if O_CREAT is set without O_EXCL, and thus will
@@ -1129,10 +1123,20 @@ int openat_report_new(int dirfd, const char *pathname, int flags, mode_t mode, b
* without O_CREAT. */
fd = openat(dirfd, pathname, flags | O_NOFOLLOW, mode);
if (fd >= 0) {
/* Unless it went away in the meantime and we created it after all. F_CREATED_QUERY
* tells us since Linux 6.12, with older kernels we assume it still existed. */
if (ret_newly_created)
*ret_newly_created = fcntl(fd, F_CREATED_QUERY) > 0;
if (ret_newly_created) {
/* Unless it went away in the meantime and we created it after all.
* F_CREATED_QUERY tells us since Linux 6.12, older kernels refuse it with
* EINVAL and we assume it still existed. */
r = RET_NERRNO(fcntl(fd, F_CREATED_QUERY));
if (r == -EINVAL || ERRNO_IS_NEG_NOT_SUPPORTED(r))
r = false;
else if (r < 0) {
safe_close(fd);
return r;
}
*ret_newly_created = r;
}
return fd;
}
if (errno == ELOOP && !FLAGS_SET(flags, O_NOFOLLOW)) {
@@ -1147,8 +1151,11 @@ int openat_report_new(int dirfd, const char *pathname, int flags, mode_t mode, b
return -errno;
/* If this is a dangling symlink don't needlessly retry. Just report it like O_NOFOLLOW would. */
if (is_symlink_at(dirfd, pathname))
r = is_symlink_at(dirfd, pathname);
if (r > 0)
return -ELOOP;
if (r < 0 && r != -ENOENT)
return r;
/* Hmm, so now we got ENOENT? Then someone might have removed the symlink in the meantime.
* Let's try again but with a limit so we don't spin forever. */
@@ -1398,7 +1405,10 @@ int xopenat_full_label(int dir_fd, const char *path, int open_flags, XOpenFlags
}
/* Don't create through a dangling symlink, report it like openat_report_new() */
if (is_symlink_at(dir_fd, path)) {
r = is_symlink_at(dir_fd, path);
if (r < 0 && r != -ENOENT)
goto error;
if (r > 0) {
r = -ELOOP;
goto error;
}
@@ -1474,8 +1484,17 @@ int xopenat_full_label(int dir_fd, const char *path, int open_flags, XOpenFlags
if (fd < 0) {
fd = openat_report_new(dir_fd, path, open_flags, mode, &made_file);
if (fd < 0) {
r = fd;
/* Report a dangling symlink in the way of mkdirat() like O_NOFOLLOW would */
r = fd == -ENOENT && dir_exists && is_symlink_at(dir_fd, path) ? -ELOOP : fd;
if (r == -ENOENT && dir_exists) {
int q = is_symlink_at(dir_fd, path);
if (q > 0)
r = -ELOOP;
else if (q < 0 && q != -ENOENT)
r = q;
}
goto error;
}
}
+2
View File
@@ -1449,6 +1449,8 @@ int pidref_safe_fork_full(
assert(!FLAGS_SET(flags, FORK_WAIT|FORK_FREEZE));
assert(!FLAGS_SET(flags, FORK_DETACH) ||
(flags & (FORK_WAIT|FORK_DEATHSIG_SIGTERM|FORK_DEATHSIG_SIGINT|FORK_DEATHSIG_SIGKILL)) == 0);
/* pack_fds() moves the fds with O_CLOEXEC set */
assert(!FLAGS_SET(flags, FORK_PACK_FDS) || FLAGS_SET(flags, FORK_CLOEXEC_OFF));
/* A wrapper around fork(), that does a couple of important initializations in addition to mere
* forking. If provided, ret is initialized in both the parent and the child process, both times
+5 -11
View File
@@ -1475,17 +1475,11 @@ ssize_t recvmsg_safe(int sockfd, struct msghdr *msg, int flags) {
assert(sockfd >= 0);
assert(msg);
/* Only AF_UNIX sockets can pass fds, and only into a control buffer. Other families might refuse
* MSG_CMSG_CLOEXEC, AF_PACKET does with EINVAL. */
if (msg->msg_controllen > 0) {
int af = socket_get_family(sockfd);
if (af < 0)
return af;
if (af == AF_UNIX)
flags |= MSG_CMSG_CLOEXEC;
}
n = recvmsg(sockfd, msg, flags);
/* Fds can only be received into a control buffer. Protocols that can't pass fds might refuse
* MSG_CMSG_CLOEXEC, AF_PACKET does with EINVAL, MCTP with EOPNOTSUPP. Retry without it then. */
n = recvmsg(sockfd, msg, flags | (msg->msg_controllen > 0 ? MSG_CMSG_CLOEXEC : 0));
if (n < 0 && msg->msg_controllen > 0 && IN_SET(errno, EINVAL, EOPNOTSUPP))
n = recvmsg(sockfd, msg, flags);
if (n < 0)
return -errno;
+5 -1
View File
@@ -151,9 +151,13 @@ int fd_verify_symlink(int fd) {
return verify_stat_at(fd, /* path= */ NULL, /* follow= */ false, stat_verify_symlink, /* verify= */ true);
}
int is_symlink_at(int fd, const char *path) {
return verify_stat_at(fd, path, /* follow= */ false, stat_verify_symlink, /* verify= */ false);
}
int is_symlink(const char *path) {
assert(!isempty(path));
return verify_stat_at(AT_FDCWD, path, /* follow= */ false, stat_verify_symlink, /* verify= */ false);
return is_symlink_at(AT_FDCWD, path);
}
static int mode_verify_socket(mode_t mode) {
+1
View File
@@ -19,6 +19,7 @@ int is_dir(const char *path, bool follow);
int stat_verify_symlink(const struct stat *st);
int fd_verify_symlink(int fd);
int is_symlink_at(int fd, const char *path);
int is_symlink(const char *path);
int stat_verify_socket(const struct stat *st);
+6 -12
View File
@@ -139,13 +139,10 @@ int fsync_path_and_parent_at(int at_fd, const char *path) {
return fsync_full(at_fd);
opened_fd = fd_reopen(at_fd, O_RDONLY|O_DIRECTORY);
if (opened_fd < 0)
return opened_fd;
} else {
} else
opened_fd = xopenat(at_fd, path, O_RDONLY|O_NOFOLLOW|O_NONBLOCK);
if (opened_fd < 0)
return opened_fd;
}
if (opened_fd < 0)
return opened_fd;
return fsync_full(opened_fd);
}
@@ -158,13 +155,10 @@ int syncfs_path(int at_fd, const char *path) {
return RET_NERRNO(syncfs(at_fd));
fd = fd_reopen(at_fd, O_RDONLY|O_DIRECTORY);
if (fd < 0)
return fd;
} else {
} else
fd = xopenat(at_fd, path, O_RDONLY|O_NONBLOCK);
if (fd < 0)
return fd;
}
if (fd < 0)
return fd;
return RET_NERRNO(syncfs(fd));
}
+1 -1
View File
@@ -696,7 +696,7 @@ static int smbios_generate(const MetricFamily *mf, sd_varlink *link, void *userd
_cleanup_close_ int dir_fd = xopenat(AT_FDCWD, "/sys/class/dmi/id", O_RDONLY|O_DIRECTORY);
if (dir_fd < 0) {
log_full_errno(ERRNO_IS_DEVICE_ABSENT(dir_fd) ? LOG_DEBUG : LOG_WARNING, dir_fd,
log_full_errno(ERRNO_IS_NEG_DEVICE_ABSENT(dir_fd) ? LOG_DEBUG : LOG_WARNING, dir_fd,
"Failed to open /sys/class/dmi/id/, ignoring: %m");
return 0;
}
+2 -2
View File
@@ -3757,7 +3757,7 @@ static int verity_partition(
/* First, check if the device already exists. */
fd = xopenat(AT_FDCWD, node, O_RDONLY|O_NONBLOCK|O_NOCTTY);
if (fd < 0 && !ERRNO_IS_DEVICE_ABSENT(fd))
if (fd < 0 && !ERRNO_IS_NEG_DEVICE_ABSENT(fd))
return log_debug_errno(fd, "Failed to open verity device %s: %m", node);
if (fd >= 0)
goto check; /* The device already exists. Let's check it. */
@@ -3829,7 +3829,7 @@ static int verity_partition(
/* Now, the device is activated and devlink is created. Let's open it. */
fd = xopenat(AT_FDCWD, node, O_RDONLY|O_NONBLOCK|O_NOCTTY);
if (fd < 0) {
if (!ERRNO_IS_DEVICE_ABSENT(fd))
if (!ERRNO_IS_NEG_DEVICE_ABSENT(fd))
return log_debug_errno(fd, "Failed to open verity device %s: %m", node);
/* The device has already been removed?? */
+1 -1
View File
@@ -108,7 +108,7 @@ static int delete_loopback(const char *device) {
fd = xopenat(AT_FDCWD, device, O_RDONLY);
if (fd < 0) {
if (ERRNO_IS_DEVICE_ABSENT(fd)) {
if (ERRNO_IS_NEG_DEVICE_ABSENT(fd)) {
log_debug_errno(fd, "Tried to open loopback device '%s', but device disappeared by now, ignoring: %m", device);
return 0;
}
+1 -1
View File
@@ -133,7 +133,7 @@ static int delete_md(RaidDevice *m) {
fd = xopenat(AT_FDCWD, m->path, O_RDONLY|O_EXCL);
if (fd < 0) {
if (ERRNO_IS_DEVICE_ABSENT(fd)) {
if (ERRNO_IS_NEG_DEVICE_ABSENT(fd)) {
log_debug_errno(fd, "Tried to open MD device '%s', but device disappeared by now, ignoring: %m", m->path);
return 0;
}
+2 -2
View File
@@ -995,7 +995,7 @@ TEST(fd_reopen_emptypath) {
_cleanup_(rm_rf_physical_and_freep) char *t = NULL;
_cleanup_close_ int tfd = -EBADF, dfd = -EBADF, fd = -EBADF;
ASSERT_OK(tfd = mkdtemp_open(NULL, 0, &t));
ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, /* flags= */ 0, &t));
/* Older kernels ignore the flag and miss the empty path */
fd = openat(tfd, "", O_PATH|O_CLOEXEC|O_EMPTYPATH);
@@ -1013,7 +1013,7 @@ TEST(fd_reopen_emptypath) {
ASSERT_ERROR(RET_NERRNO(openat(dfd, ".", O_RDONLY|O_DIRECTORY|O_CLOEXEC)), EACCES);
ASSERT_OK(fd = fd_reopen(dfd, O_RDONLY|O_DIRECTORY));
ASSERT_OK_POSITIVE(inode_same_at(dfd, NULL, fd, NULL, AT_EMPTY_PATH));
ASSERT_OK_POSITIVE(inode_same_at(dfd, /* filea= */ NULL, fd, /* fileb= */ NULL, AT_EMPTY_PATH));
}
TEST(fd_reopen_cloexec) {
+20
View File
@@ -168,6 +168,26 @@ TEST(is_symlink) {
ASSERT_FAIL(is_symlink("/a/file/which/does/not/exist/i/guess"));
}
TEST(is_symlink_at) {
_cleanup_(rm_rf_physical_and_freep) char *t = NULL;
_cleanup_close_ int tfd = -EBADF, fd = -EBADF;
ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, /* flags= */ 0, &t));
ASSERT_OK_ERRNO(fd = openat(tfd, "file", O_CREAT|O_EXCL|O_WRONLY|O_CLOEXEC, 0600));
fd = safe_close(fd);
ASSERT_OK_ERRNO(symlinkat("missing", tfd, "dangling"));
ASSERT_OK_ZERO(is_symlink_at(tfd, "file"));
ASSERT_OK_POSITIVE(is_symlink_at(tfd, "dangling"));
ASSERT_ERROR(is_symlink_at(tfd, "missing"), ENOENT);
ASSERT_OK_ZERO(is_symlink_at(tfd, /* path= */ NULL));
ASSERT_OK_ERRNO(fd = openat(tfd, "dangling", O_PATH|O_NOFOLLOW|O_CLOEXEC));
ASSERT_OK_POSITIVE(is_symlink_at(fd, /* path= */ NULL));
ASSERT_OK_POSITIVE(is_symlink_at(fd, ""));
ASSERT_ERROR(is_symlink_at(fd, "file"), ENOTDIR);
}
TEST(path_is_fs_type) {
/* run might not be a mount point in build chroots */
if (path_is_mount_point_full("/run", /* root= */ NULL, AT_SYMLINK_FOLLOW) > 0) {
+1 -1
View File
@@ -418,7 +418,7 @@ static int run(int argc, char *argv[]) {
fd = xopenat(AT_FDCWD, ASSERT_PTR(arg_device), O_RDONLY|O_NONBLOCK|O_NOCTTY);
if (fd < 0) {
bool ignore = ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(fd);
bool ignore = ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(fd);
log_full_errno(ignore ? LOG_DEBUG : LOG_WARNING, fd,
"Failed to open device node '%s'%s: %m",
arg_device, ignore ? ", ignoring" : "");
+1 -1
View File
@@ -51,7 +51,7 @@ static int run(int argc, char** argv) {
mtd_fd = xopenat(AT_FDCWD, device, O_RDONLY|O_NOCTTY);
if (mtd_fd < 0) {
bool ignore = ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(mtd_fd);
bool ignore = ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(mtd_fd);
log_full_errno(ignore ? LOG_DEBUG : LOG_WARNING, mtd_fd,
"Failed to open device node '%s'%s: %m",
device, ignore ? ", ignoring" : "");
+1 -1
View File
@@ -567,7 +567,7 @@ static int builtin_blkid(UdevEvent *event, int argc, char *argv[]) {
fd = device_open(dev, O_RDONLY|O_NONBLOCK|O_NOCTTY);
if (fd < 0) {
bool ignore = ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(fd);
bool ignore = ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(fd);
log_device_debug_errno(dev, fd, "Failed to open block device %s%s: %m",
devnode, ignore ? ", ignoring" : "");
return ignore ? 0 : fd;
+1 -1
View File
@@ -36,7 +36,7 @@ static int builtin_btrfs(UdevEvent *event, int argc, char *argv[]) {
_cleanup_close_ int fd = xopenat(AT_FDCWD, "/dev/btrfs-control", O_RDWR|O_NOCTTY);
if (fd < 0) {
if (ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(fd)) {
if (ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(fd)) {
/* Driver not installed? Then we aren't ready. This is useful in initrds that lack
* btrfs.ko. After the host transition (where btrfs.ko will hopefully become
* available) the device can be retriggered and will then be considered ready. */
+2 -2
View File
@@ -204,7 +204,7 @@ static int builtin_keyboard(UdevEvent *event, int argc, char *argv[]) {
if (fd < 0) {
fd = device_open(dev, O_RDWR|O_NONBLOCK|O_NOCTTY);
if (fd < 0) {
bool ignore = ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(fd);
bool ignore = ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(fd);
log_device_full_errno(dev, ignore ? LOG_DEBUG : LOG_WARNING, fd,
"Failed to open device '%s'%s: %m",
node, ignore ? ", ignoring" : "");
@@ -226,7 +226,7 @@ static int builtin_keyboard(UdevEvent *event, int argc, char *argv[]) {
if (fd < 0) {
fd = device_open(dev, O_RDWR|O_NONBLOCK|O_NOCTTY);
if (fd < 0) {
bool ignore = ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(fd);
bool ignore = ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(fd);
log_device_full_errno(dev, ignore ? LOG_DEBUG : LOG_WARNING, fd,
"Failed to open device '%s'%s: %m",
node, ignore ? ", ignoring" : "");
+1 -1
View File
@@ -35,7 +35,7 @@ static int builtin_uaccess(UdevEvent *event, int argc, char *argv[]) {
_cleanup_close_ int fd = device_open(dev, O_PATH);
if (fd < 0) {
bool ignore = ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(fd);
bool ignore = ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(fd);
log_device_full_errno(dev, ignore ? LOG_DEBUG : LOG_WARNING, fd,
"Failed to open device node%s: %m",
ignore ? ", ignoring" : "");
+2 -2
View File
@@ -707,7 +707,7 @@ int udev_node_apply_permissions(
node_fd = device_open(dev, O_PATH);
if (node_fd < 0) {
if (ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(node_fd)) {
if (ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(node_fd)) {
log_device_debug_errno(dev, node_fd, "Device node %s is missing, skipping handling.", devnode);
return 0; /* This is necessarily racey, so ignore missing the device */
}
@@ -740,7 +740,7 @@ int static_node_apply_permissions(
node_fd = xopenat(AT_FDCWD, devnode, O_PATH);
if (node_fd < 0) {
bool ignore = ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(node_fd);
bool ignore = ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(node_fd);
log_full_errno(ignore ? LOG_DEBUG : LOG_WARNING, node_fd,
"Failed to open device node '%s'%s: %m",
devnode, ignore ? ", ignoring" : "");
+2 -2
View File
@@ -112,7 +112,7 @@ static int worker_lock_whole_disk(UdevWorker *worker, sd_device *dev, int *ret_f
fd = device_open(dev_whole_disk, O_RDONLY|O_NONBLOCK|O_NOCTTY);
if (fd < 0) {
bool ignore = ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(fd);
bool ignore = ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(fd);
log_device_debug_errno(dev, fd, "Failed to open '%s'%s: %m", whole_disk, ignore ? ", ignoring" : "");
if (!ignore)
@@ -176,7 +176,7 @@ static int worker_mark_block_device_read_only(sd_device *dev) {
_cleanup_close_ int fd = device_open(dev, O_RDONLY|O_NONBLOCK|O_NOCTTY);
if (fd < 0) {
bool ignore = ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(fd);
bool ignore = ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(fd);
log_device_full_errno(dev, ignore ? LOG_DEBUG : LOG_WARNING, fd,
"Failed to open device node '%s'%s: %m",
node, ignore ? ", ignoring" : "");
+1 -1
View File
@@ -38,7 +38,7 @@ static int run(int argc, char *argv[]) {
fd = xopenat(AT_FDCWD, device, O_RDONLY|O_NOCTTY);
if (fd < 0) {
bool ignore = ERRNO_IS_DEVICE_ABSENT_OR_EMPTY(fd);
bool ignore = ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(fd);
log_full_errno(ignore ? LOG_DEBUG : LOG_WARNING, fd,
"Failed to open device node '%s'%s: %m",
device, ignore ? ", ignoring" : "");
+2 -2
View File
@@ -136,11 +136,11 @@ int qemu_check_vsock_support(void) {
fd = xopenat(AT_FDCWD, "/dev/vhost-vsock", O_RDWR);
if (fd >= 0)
return true;
if (ERRNO_IS_DEVICE_ABSENT(fd)) {
if (ERRNO_IS_NEG_DEVICE_ABSENT(fd)) {
log_debug_errno(fd, "/dev/vhost-vsock device doesn't exist. Not adding a vsock device to the virtual machine.");
return false;
}
if (ERRNO_IS_PRIVILEGE(fd)) {
if (ERRNO_IS_NEG_PRIVILEGE(fd)) {
log_debug_errno(fd, "Permission denied to access /dev/vhost-vsock. Not adding a vsock device to the virtual machine.");
return false;
}