Merge pull request #14261 from keszybz/loop-utils-and-efivars

Fixes for networkd, shared/loop-util, basic/efivars

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)
+                assert(n <= st.st_size - 4);
-                        return -EIO;
 
                 /* Always NUL terminate (2 bytes, to protect UTF-16) */
-                ((char*) buf)[st.st_size - 4] = 0;
+                ((char*) buf)[n - 4] = 0;
-                ((char*) buf)[st.st_size - 4 + 1] = 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;
 }

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 */

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;

src/shared/loop-util.c

@@ -11,6 +11,7 @@
 #include <linux/loop.h>
 #include <sys/file.h>
 #include <sys/ioctl.h>
+#include <unistd.h>
 
 #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)
+                if (loop < 0) {
-                        return -errno;
+                        /* Somebody might've gotten the same number from the kernel, used the device,
-                if (ioctl(loop, LOOP_SET_FD, fd) < 0) {
+                         * 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 */
+                if (++n_attempts >= 64) /* Give up eventually */
-                                return -EBUSY;
+                        return -EBUSY;
-                } else
-                        break;
 
                 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) {
+        if (ioctl(loop_with_fd, LOOP_SET_STATUS64, &info) < 0)
-                r = -errno;
+                return -errno;
-                goto fail;
-        }
 
         d = new(LoopDevice, 1);
-        if (!d) {
+        if (!d)
-                r = -ENOMEM;
+                return -ENOMEM;
-                goto fail;
-        }
-
         *d = (LoopDevice) {
-                .fd = TAKE_FD(loop),
+                .fd = TAKE_FD(loop_with_fd),
                 .node = TAKE_PTR(loopdev),
                 .nr = nr,
         };
 
         *ret = d;
-        return d->fd;
+        return 0;
-
-fail:
-        if (fd >= 0)
-                (void) ioctl(fd, LOOP_CLR_FD);
-        if (d && d->fd >= 0)
-                (void) ioctl(d->fd, LOOP_CLR_FD);
-
-        return r;
 }
 
 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");
+                        log_warning_errno(errno,
-                else {
+                                          "Failed to open loop control device, cannot remove loop device %s: %m",
-                        if (ioctl(control, LOOP_CTL_REMOVE, d->nr) < 0)
+                                          strna(d->node));
-                                log_debug_errno(errno, "Failed to remove loop device: %m");
+                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);

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);