diff --git a/src/basic/efivars.c b/src/basic/efivars.c index 7b14c062df..bfde67a883 100644 --- a/src/basic/efivars.c +++ b/src/basic/efivars.c @@ -90,13 +90,14 @@ int efi_get_variable( n = read(fd, buf, (size_t) st.st_size - 4); if (n < 0) return -errno; - if (n != st.st_size - 4) - return -EIO; + assert(n <= st.st_size - 4); /* Always NUL terminate (2 bytes, to protect UTF-16) */ - ((char*) buf)[st.st_size - 4] = 0; - ((char*) buf)[st.st_size - 4 + 1] = 0; - } + ((char*) buf)[n - 4] = 0; + ((char*) buf)[n - 4 + 1] = 0; + } else + /* Assume that the reported size is accurate */ + n = st.st_size - 4; /* Note that efivarfs interestingly doesn't require ftruncate() to update an existing EFI variable * with a smaller value. */ @@ -108,7 +109,7 @@ int efi_get_variable( *ret_value = TAKE_PTR(buf); if (ret_size) - *ret_size = (size_t) st.st_size - 4; + *ret_size = n; return 0; } diff --git a/src/network/networkd-brvlan.c b/src/network/networkd-brvlan.c index 970065d32a..41f09287f2 100644 --- a/src/network/networkd-brvlan.c +++ b/src/network/networkd-brvlan.c @@ -180,7 +180,9 @@ int br_vlan_configure(Link *link, uint16_t pvid, uint32_t *br_vid_bitmap, uint32 /* master needs flag self */ if (!link->network->bridge) { flags = BRIDGE_FLAGS_SELF; - sd_netlink_message_append_data(req, IFLA_BRIDGE_FLAGS, &flags, sizeof(uint16_t)); + r = sd_netlink_message_append_data(req, IFLA_BRIDGE_FLAGS, &flags, sizeof(uint16_t)); + if (r < 0) + return log_link_error_errno(link, r, "Could not open IFLA_BRIDGE_FLAGS: %m"); } /* add vlan info */ diff --git a/src/shared/efi-loader.c b/src/shared/efi-loader.c index 3d1df09907..108f31d502 100644 --- a/src/shared/efi-loader.c +++ b/src/shared/efi-loader.c @@ -562,17 +562,20 @@ int efi_loader_get_boot_usec(usec_t *firmware, usec_t *loader) { r = read_usec(EFI_VENDOR_LOADER, "LoaderTimeInitUSec", &x); if (r < 0) - return r; + return log_debug_errno(r, "Failed to read LoaderTimeInitUSec: %m"); r = read_usec(EFI_VENDOR_LOADER, "LoaderTimeExecUSec", &y); if (r < 0) - return r; + return log_debug_errno(r, "Failed to read LoaderTimeExecUSec: %m"); if (y == 0 || y < x) - return -EIO; + return log_debug_errno(SYNTHETIC_ERRNO(EIO), + "Bad LoaderTimeInitUSec=%"PRIu64", LoaderTimeExecUSec=%" PRIu64"; refusing.", + x, y); if (y > USEC_PER_HOUR) - return -EIO; + return log_debug_errno(SYNTHETIC_ERRNO(EIO), + "LoaderTimeExecUSec=%"PRIu64" too large, refusing.", x); *firmware = x; *loader = y; diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index acf3eae2d7..6cb45f1f1b 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "alloc-util.h" #include "fd-util.h" @@ -19,6 +20,14 @@ #include "parse-util.h" #include "stat-util.h" #include "stdio-util.h" +#include "string-util.h" + +static void cleanup_clear_loop_close(int *fd) { + if (*fd >= 0) { + (void) ioctl(*fd, LOOP_CLR_FD); + (void) safe_close(*fd); + } +} int loop_device_make_full( int fd, @@ -28,9 +37,7 @@ int loop_device_make_full( uint32_t loop_flags, LoopDevice **ret) { - _cleanup_close_ int control = -1, loop = -1; _cleanup_free_ char *loopdev = NULL; - unsigned n_attempts = 0; struct loop_info64 info; LoopDevice *d = NULL; struct stat st; @@ -44,7 +51,7 @@ int loop_device_make_full( return -errno; if (S_ISBLK(st.st_mode)) { - if (ioctl(loop, LOOP_GET_STATUS64, &info) >= 0) { + if (ioctl(fd, LOOP_GET_STATUS64, &info) >= 0) { /* Oh! This is a loopback device? That's interesting! */ #if HAVE_VALGRIND_MEMCHECK_H @@ -58,7 +65,7 @@ int loop_device_make_full( } if (offset == 0 && IN_SET(size, 0, UINT64_MAX)) { - int copy; + _cleanup_close_ int copy = -1; /* If this is already a block device, store a copy of the fd as it is */ @@ -69,9 +76,8 @@ int loop_device_make_full( d = new(LoopDevice, 1); if (!d) return -ENOMEM; - *d = (LoopDevice) { - .fd = copy, + .fd = TAKE_FD(copy), .nr = nr, .node = TAKE_PTR(loopdev), .relinquished = true, /* It's not allocated by us, don't destroy it when this object is freed */ @@ -86,13 +92,18 @@ int loop_device_make_full( return r; } + _cleanup_close_ int control = -1; + _cleanup_(cleanup_clear_loop_close) int loop_with_fd = -1; + control = open("/dev/loop-control", O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK); if (control < 0) return -errno; /* Loop around LOOP_CTL_GET_FREE, since at the moment we attempt to open the returned device it might * be gone already, taken by somebody else racing against us. */ - for (;;) { + for (unsigned n_attempts = 0;;) { + _cleanup_close_ int loop = -1; + nr = ioctl(control, LOOP_CTL_GET_FREE); if (nr < 0) return -errno; @@ -101,19 +112,24 @@ int loop_device_make_full( return -ENOMEM; loop = open(loopdev, O_CLOEXEC|O_NONBLOCK|O_NOCTTY|open_flags); - if (loop < 0) - return -errno; - if (ioctl(loop, LOOP_SET_FD, fd) < 0) { + if (loop < 0) { + /* Somebody might've gotten the same number from the kernel, used the device, + * and called LOOP_CTL_REMOVE on it. Let's retry with a new number. */ + if (errno != ENOENT) + return -errno; + } else { + if (ioctl(loop, LOOP_SET_FD, fd) >= 0) { + loop_with_fd = TAKE_FD(loop); + break; + } if (errno != EBUSY) return -errno; + } - if (++n_attempts >= 64) /* Give up eventually */ - return -EBUSY; - } else - break; + if (++n_attempts >= 64) /* Give up eventually */ + return -EBUSY; loopdev = mfree(loopdev); - loop = safe_close(loop); } info = (struct loop_info64) { @@ -123,33 +139,20 @@ int loop_device_make_full( .lo_sizelimit = size == UINT64_MAX ? 0 : size, }; - if (ioctl(loop, LOOP_SET_STATUS64, &info) < 0) { - r = -errno; - goto fail; - } + if (ioctl(loop_with_fd, LOOP_SET_STATUS64, &info) < 0) + return -errno; d = new(LoopDevice, 1); - if (!d) { - r = -ENOMEM; - goto fail; - } - + if (!d) + return -ENOMEM; *d = (LoopDevice) { - .fd = TAKE_FD(loop), + .fd = TAKE_FD(loop_with_fd), .node = TAKE_PTR(loopdev), .nr = nr, }; *ret = d; - return d->fd; - -fail: - if (fd >= 0) - (void) ioctl(fd, LOOP_CLR_FD); - if (d && d->fd >= 0) - (void) ioctl(d->fd, LOOP_CLR_FD); - - return r; + return 0; } int loop_device_make_by_path(const char *path, int open_flags, uint32_t loop_flags, LoopDevice **ret) { @@ -163,7 +166,7 @@ int loop_device_make_by_path(const char *path, int open_flags, uint32_t loop_fla if (fd < 0) return -errno; - return loop_device_make(fd, open_flags, loop_flags, ret); + return loop_device_make_full(fd, open_flags, 0, 0, loop_flags, ret); } LoopDevice* loop_device_unref(LoopDevice *d) { @@ -171,7 +174,6 @@ LoopDevice* loop_device_unref(LoopDevice *d) { return NULL; if (d->fd >= 0) { - if (d->nr >= 0 && !d->relinquished) { if (ioctl(d->fd, LOOP_CLR_FD) < 0) log_debug_errno(errno, "Failed to clear loop device: %m"); @@ -186,11 +188,19 @@ LoopDevice* loop_device_unref(LoopDevice *d) { control = open("/dev/loop-control", O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK); if (control < 0) - log_debug_errno(errno, "Failed to open loop control device: %m"); - else { - if (ioctl(control, LOOP_CTL_REMOVE, d->nr) < 0) - log_debug_errno(errno, "Failed to remove loop device: %m"); - } + log_warning_errno(errno, + "Failed to open loop control device, cannot remove loop device %s: %m", + strna(d->node)); + else + for (unsigned n_attempts = 0;;) { + if (ioctl(control, LOOP_CTL_REMOVE, d->nr) >= 0) + break; + if (errno != EBUSY || ++n_attempts >= 64) { + log_warning_errno(errno, "Failed to remove device %s: %m", strna(d->node)); + break; + } + usleep(50 * USEC_PER_MSEC); + } } free(d->node); diff --git a/src/shared/loop-util.h b/src/shared/loop-util.h index 5156b46ad6..a9c9a647a5 100644 --- a/src/shared/loop-util.h +++ b/src/shared/loop-util.h @@ -15,10 +15,6 @@ struct LoopDevice { }; int loop_device_make_full(int fd, int open_flags, uint64_t offset, uint64_t size, uint32_t loop_flags, LoopDevice **ret); -static inline int loop_device_make(int fd, int open_flags, uint32_t loop_flags, LoopDevice **ret) { - return loop_device_make_full(fd, open_flags, 0, 0, loop_flags, ret); -} - int loop_device_make_by_path(const char *path, int open_flags, uint32_t loop_flags, LoopDevice **ret); int loop_device_open(const char *loop_path, int open_flags, LoopDevice **ret);