From db44ebdc74658103c7b6a7d2c557f14ab27814f0 Mon Sep 17 00:00:00 2001 From: Alexey Sheplyakov Date: Wed, 11 Aug 2021 21:15:40 +0400 Subject: [PATCH] Avoid races between init mounting loopback device and udev probing it LOOP_SET_FD, LOOP_SET_STATUS ioctls trigger a `change` event with loopback device in question. udev handles those events with (builtin) blkid command. Probing a device with blkid takes a while, so init might try to mount the loopback device in question while `blkid` is still running. As a result init and udev block each other. Eventually (after 3 minutes or whatever udev event timeout is) `blkid` gets killed and boot proceeds. However such long delays are very annoying. Therefore run `udev_settle` after each loop related ioctl to avoid the concurrent access to the same loopback device. Closes: #40687 --- lomount.c | 21 +++++++++++++++++++++ tools.c | 21 +++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/lomount.c b/lomount.c index aea8ed2..b39caf3 100644 --- a/lomount.c +++ b/lomount.c @@ -31,6 +31,7 @@ #include "log.h" #include "mount.h" #include "modules.h" +#include "udev.h" #include "lomount.h" @@ -98,6 +99,13 @@ set_loop (const char *device, const char *file) close(ffd); return 1; } + /* Note: LOOP_SET_FD triggers change event. While processing it + * udev reads the loop device with builtin blkid. This can race + * with subsequent access by kernel due to LOOP_SET_STATUS (or + * mounting the loop device). Therefore give udev a chance to + * process the event without concurrent access to the loop device + */ + udev_settle(); if (ioctl(fd, LOOP_SET_STATUS, &loopinfo) < 0) { (void) ioctl (fd, LOOP_CLR_FD, 0); @@ -106,8 +114,21 @@ set_loop (const char *device, const char *file) return 1; } + /* Same here: LOOP_SET_STATUS triggers change event, give udev + * a chance to process it without concurrent access to the loop + * device. In other words, prevent the caller of this function + * from mounting the device while udev still running blkid on it + */ + udev_settle(); + close(fd); close(ffd); + /* udev might be monitoring loopback device (with fanotify) and + * will synthesize a change event on close. Give udev some time + * to process without racing with the caller of this function + * (which is going to mount the newly configured loopback device) + */ + udev_settle(); return 0; } diff --git a/tools.c b/tools.c index bbe6289..31e4ed9 100644 --- a/tools.c +++ b/tools.c @@ -47,6 +47,7 @@ #ifdef SPAWN_SPLASH #include "init.h" #endif +#include "udev.h" /* If we have more than that amount of memory (in bytes), we assume we can load the second stage as a ramdisk */ #define MEM_LIMIT_RAMDISK (256 * 1024 * 1024) @@ -650,6 +651,13 @@ int do_losetup(char * device, char * target) close( targfd ); return -1; } + /* Note: LOOP_SET_FD triggers change event. While processing it + * udev reads the loop device with builtin blkid. This can race + * with subsequent access by kernel due to LOOP_SET_STATUS (or + * mounting the loop device). Therefore give udev a chance to + * process the event without concurrent access to the loop device + */ + udev_settle(); memset(&loopInfo, 0, sizeof(loopInfo)); strcpy(loopInfo.lo_name, target); @@ -657,7 +665,20 @@ int do_losetup(char * device, char * target) if ( ioctl(loopfd, LOOP_SET_STATUS, &loopInfo) < 0 ) log_message( "losetup: error setting up loopback device: %s", strerror(errno) ); + /* Same here: LOOP_SET_STATUS triggers change event, give udev + * a chance to process it without concurrent access to the loop + * device. In other words, prevent the caller of this function + * from mounting the device while udev still running blkid on it + */ + udev_settle(); + close( loopfd ); + /* udev might be monitoring loopback device (with fanotify) and + * will synthesize a change event on close. Give udev some time + * to process without racing with the caller of this function + * (which is going to mount the newly configured loopback device) + */ + udev_settle(); close( targfd ); return 0; }