From a156eb89c827206ee5b51d53016ba63be0c90449 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 17:06:21 +0200 Subject: [PATCH 01/11] sd-device: use right type for usec_initialized --- src/libsystemd/sd-device/device-internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-device/device-internal.h b/src/libsystemd/sd-device/device-internal.h index c1a81e3b41..c7f9b59087 100644 --- a/src/libsystemd/sd-device/device-internal.h +++ b/src/libsystemd/sd-device/device-internal.h @@ -69,7 +69,7 @@ struct sd_device { char *id_filename; - uint64_t usec_initialized; + usec_t usec_initialized; mode_t devmode; uid_t devuid; From 02ef01ade3b16c05dfc8698181d7f562f4934ef3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 15:56:03 +0200 Subject: [PATCH 02/11] sd-device: use right clock when comparing initialization usec we actually use CLOCK_MONOTONIC for the timestamp, hence when comparing/subtracting it from the current time, also use CLOCK_MONOTONIC. --- src/libsystemd/sd-device/sd-device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 927a4f1d52..200e2077ee 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -1441,10 +1441,10 @@ _public_ int sd_device_get_usec_since_initialized(sd_device *device, uint64_t *u if (!device->is_initialized) return -EBUSY; - if (!device->usec_initialized) + if (device->usec_initialized == 0) return -ENODATA; - now_ts = now(clock_boottime_or_monotonic()); + now_ts = now(CLOCK_MONOTONIC); if (now_ts < device->usec_initialized) return -EIO; From 38bd449f9617228e92dacf7fc0415d2f13b97af2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 10:46:34 +0200 Subject: [PATCH 03/11] loop-util: make loop_device_make() return fd in all code paths Previously, loop_device_make() would return the device fd in one success code path, but not the other (where' we'd just return 0). loop_device_open() returns it in all cases. Hence, let's clean this up, and make sure in all success code paths of both functions we return it (even though it strictly speaking is redundant, since we return it in LoopDevice anyway, and currently noone actually relies on this). --- src/shared/loop-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index 2aa4936965..9ec2f0c1ab 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -441,7 +441,7 @@ int loop_device_make( }; *ret = d; - return 0; + return d->fd; } int loop_device_make_by_path(const char *path, int open_flags, uint32_t loop_flags, LoopDevice **ret) { From b0dbffd86856d60e0c558505978ef56d087960f9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 10:48:22 +0200 Subject: [PATCH 04/11] loop-util: port to random_u64_range() Doesn't matter, but it's a bit easier to read I'd claim. --- src/shared/loop-util.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index 9ec2f0c1ab..b8f7d0cc7b 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -252,7 +252,7 @@ static int loop_configure( /* Sleep some random time, but at least 10ms, at most 250ms. Increase the delay the more * failed attempts we see */ (void) usleep(UINT64_C(10) * USEC_PER_MSEC + - random_u64() % (UINT64_C(240) * USEC_PER_MSEC * n_attempts/64)); + random_u64_range(UINT64_C(240) * USEC_PER_MSEC * n_attempts/64)); } return 0; @@ -422,8 +422,8 @@ int loop_device_make( /* Wait some random time, to make collision less likely. Let's pick a random time in the * range 0ms…250ms, linearly scaled by the number of failed attempts. */ - (void) usleep(random_u64() % (UINT64_C(10) * USEC_PER_MSEC + - UINT64_C(240) * USEC_PER_MSEC * n_attempts/64)); + (void) usleep(random_u64_range(UINT64_C(10) * USEC_PER_MSEC + + UINT64_C(240) * USEC_PER_MSEC * n_attempts/64)); } if (fstat(loop_with_fd, &st) < 0) From 79e8393a6acb0115c6b8995db6ddbf4215d0851c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 17:10:45 +0200 Subject: [PATCH 05/11] loop-util: initialize .devno in loop_device_open() too --- src/shared/loop-util.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index b8f7d0cc7b..ce4a72a24a 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -444,7 +444,12 @@ int loop_device_make( return d->fd; } -int loop_device_make_by_path(const char *path, int open_flags, uint32_t loop_flags, LoopDevice **ret) { +int loop_device_make_by_path( + const char *path, + int open_flags, + uint32_t loop_flags, + LoopDevice **ret) { + _cleanup_close_ int fd = -1; int r; @@ -567,6 +572,7 @@ int loop_device_open(const char *loop_path, int open_flags, LoopDevice **ret) { .nr = nr, .node = TAKE_PTR(p), .relinquished = true, /* It's not ours, don't try to destroy it when this object is freed */ + .devno = st.st_dev, }; *ret = d; From 31c75fcc415056e7ded80136547fd4658fa554f7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 10:56:38 +0200 Subject: [PATCH 06/11] loop-util: read kernel's uevent seqnum right before attaching a loopback device Later, this will allow us to ignore uevents from earlier attachments a bit better, as we can compare uevent seqnums with this boundary. It's not a full fix for the race though, since we cannot atomically determine the uevent and attach the device, but it at least shortens the window a bit. --- src/shared/loop-util.c | 48 ++++++++++++++++++++++++++++++++++++++++-- src/shared/loop-util.h | 1 + 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index ce4a72a24a..3615995de0 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -53,6 +53,23 @@ static int loop_is_bound(int fd) { return true; /* bound! */ } +static int get_current_uevent_seqnum(uint64_t *ret) { + _cleanup_free_ char *p = NULL; + int r; + + r = read_full_virtual_file("/sys/kernel/uevent_seqnum", &p, NULL); + if (r < 0) + return log_debug_errno(r, "Failed to read current uevent sequence number: %m"); + + truncate_nl(p); + + r = safe_atou64(p, ret); + if (r < 0) + return log_debug_errno(r, "Failed to parse current uevent sequence number: %s", p); + + return 0; +} + static int device_has_block_children(sd_device *d) { _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; const char *main_sn, *main_ss; @@ -114,11 +131,13 @@ static int loop_configure( int fd, int nr, const struct loop_config *c, - bool *try_loop_configure) { + bool *try_loop_configure, + uint64_t *ret_seqnum_not_before) { _cleanup_(sd_device_unrefp) sd_device *d = NULL; _cleanup_free_ char *sysname = NULL; _cleanup_close_ int lock_fd = -1; + uint64_t seqnum; int r; assert(fd >= 0); @@ -167,6 +186,16 @@ static int loop_configure( } if (*try_loop_configure) { + /* Acquire uevent seqnum immediately before attaching the loopback device. This allows + * callers to ignore all uevents with a seqnum before this one, if they need to associate + * uevent with this attachment. Doing so isn't race-free though, as uevents that happen in + * the window between this reading of the seqnum, and the LOOP_CONFIGURE call might still be + * mistaken as originating from our attachment, even though might be caused by an earlier + * use. But doing this at least shortens the race window a bit. */ + r = get_current_uevent_seqnum(&seqnum); + if (r < 0) + return r; + if (ioctl(fd, LOOP_CONFIGURE, c) < 0) { /* Do fallback only if LOOP_CONFIGURE is not supported, propagate all other * errors. Note that the kernel is weird: non-existing ioctls currently return EINVAL @@ -224,10 +253,18 @@ static int loop_configure( goto fail; } + if (ret_seqnum_not_before) + *ret_seqnum_not_before = seqnum; + return 0; } } + /* Let's read the seqnum again, to shorten the window. */ + r = get_current_uevent_seqnum(&seqnum); + if (r < 0) + return r; + /* Since kernel commit 5db470e229e22b7eda6e23b5566e532c96fb5bc3 (kernel v5.0) the LOOP_SET_STATUS64 * ioctl can return EAGAIN in case we change the lo_offset field, if someone else is accessing the * block device while we try to reconfigure it. This is a pretty common case, since udev might @@ -255,6 +292,9 @@ static int loop_configure( random_u64_range(UINT64_C(240) * USEC_PER_MSEC * n_attempts/64)); } + if (ret_seqnum_not_before) + *ret_seqnum_not_before = seqnum; + return 0; fail: @@ -312,6 +352,7 @@ int loop_device_make( bool try_loop_configure = true; struct loop_config config; LoopDevice *d = NULL; + uint64_t seqnum = UINT64_MAX; struct stat st; int nr = -1, r; @@ -354,6 +395,7 @@ int loop_device_make( .node = TAKE_PTR(loopdev), .relinquished = true, /* It's not allocated by us, don't destroy it when this object is freed */ .devno = st.st_rdev, + .uevent_seqnum_not_before = UINT64_MAX, }; *ret = d; @@ -401,7 +443,7 @@ int loop_device_make( if (!IN_SET(errno, ENOENT, ENXIO)) return -errno; } else { - r = loop_configure(loop, nr, &config, &try_loop_configure); + r = loop_configure(loop, nr, &config, &try_loop_configure, &seqnum); if (r >= 0) { loop_with_fd = TAKE_FD(loop); break; @@ -438,6 +480,7 @@ int loop_device_make( .node = TAKE_PTR(loopdev), .nr = nr, .devno = st.st_rdev, + .uevent_seqnum_not_before = seqnum, }; *ret = d; @@ -573,6 +616,7 @@ int loop_device_open(const char *loop_path, int open_flags, LoopDevice **ret) { .node = TAKE_PTR(p), .relinquished = true, /* It's not ours, don't try to destroy it when this object is freed */ .devno = st.st_dev, + .uevent_seqnum_not_before = UINT64_MAX, }; *ret = d; diff --git a/src/shared/loop-util.h b/src/shared/loop-util.h index 619b34716b..6df4f91c22 100644 --- a/src/shared/loop-util.h +++ b/src/shared/loop-util.h @@ -13,6 +13,7 @@ struct LoopDevice { dev_t devno; char *node; bool relinquished; + uint64_t uevent_seqnum_not_before; /* uevent sequm right before we attached the loopback device, or UINT64_MAX if we don't know */ }; int loop_device_make(int fd, int open_flags, uint64_t offset, uint64_t size, uint32_t loop_flags, LoopDevice **ret); From 75dc190d39bb71428ab0a0e6185af42aef9451a1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 15:57:29 +0200 Subject: [PATCH 07/11] dissect: ignore old uevents when waiting for loopback partition scan Let's drop all monitor uevent that were enqueued before we actually started setting up the device. This doesn't fix the race, but it makes the race window smaller: since we cannot determine the uevent seqnum and the loopback attachment atomically, there's a tiny window where uevents might be generated by the device which we mistake for being associated with out use of the loopback device. --- src/core/namespace.c | 1 + src/dissect/dissect.c | 1 + src/gpt-auto-generator/gpt-auto-generator.c | 1 + src/nspawn/nspawn.c | 1 + src/portable/portable.c | 1 + src/shared/discover-image.c | 4 ++- src/shared/dissect-image.c | 29 ++++++++++++++++++--- src/shared/dissect-image.h | 4 +-- src/sysext/sysext.c | 1 + src/test/test-loop-block.c | 6 ++--- 10 files changed, 39 insertions(+), 10 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index 9f9d47d34a..bffcb5ac86 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -1863,6 +1863,7 @@ int setup_namespace( loop_device->fd, &verity, root_image_options, + loop_device->uevent_seqnum_not_before, dissect_image_flags, &dissected_image); if (r < 0) diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index c21d3e47e4..d70f1e791f 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -781,6 +781,7 @@ static int run(int argc, char *argv[]) { arg_image, &arg_verity_settings, NULL, + d->uevent_seqnum_not_before, arg_flags, &m); if (r < 0) diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index 42549a2cd8..eb80165bbe 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -672,6 +672,7 @@ static int enumerate_partitions(dev_t devnum) { r = dissect_image( fd, NULL, NULL, + UINT64_MAX, DISSECT_IMAGE_GPT_ONLY| DISSECT_IMAGE_NO_UDEV| DISSECT_IMAGE_USR_NO_ROOT, diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 164a330207..0701602418 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -5483,6 +5483,7 @@ static int run(int argc, char *argv[]) { arg_image, &arg_verity_settings, NULL, + loop->uevent_seqnum_not_before, dissect_image_flags, &dissected_image); if (r == -ENOPKG) { diff --git a/src/portable/portable.c b/src/portable/portable.c index e2bce27947..28d884a951 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -395,6 +395,7 @@ static int portable_extract_by_path( r = dissect_image( d->fd, NULL, NULL, + d->uevent_seqnum_not_before, DISSECT_IMAGE_READ_ONLY | DISSECT_IMAGE_GENERIC_ROOT | DISSECT_IMAGE_REQUIRE_ROOT | diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index 43138e0a6f..c343d08931 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -1201,10 +1201,12 @@ int image_read_metadata(Image *i) { r = dissect_image( d->fd, NULL, NULL, + d->uevent_seqnum_not_before, DISSECT_IMAGE_GENERIC_ROOT | DISSECT_IMAGE_REQUIRE_ROOT | DISSECT_IMAGE_RELAX_VAR_CHECK | - DISSECT_IMAGE_USR_NO_ROOT, &m); + DISSECT_IMAGE_USR_NO_ROOT, + &m); if (r < 0) return r; diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index f6971038de..345e1db9a7 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -260,6 +260,7 @@ struct wait_data { sd_device *parent_device; blkid_partition blkidp; sd_device *found; + uint64_t uevent_seqnum_not_before; }; static inline void wait_data_done(struct wait_data *d) { @@ -275,6 +276,20 @@ static int device_monitor_handler(sd_device_monitor *monitor, sd_device *device, if (device_for_action(device, SD_DEVICE_REMOVE)) return 0; + if (w->uevent_seqnum_not_before != UINT64_MAX) { + uint64_t seqnum; + + r = sd_device_get_seqnum(device, &seqnum); + if (r < 0) + goto finish; + + if (seqnum <= w->uevent_seqnum_not_before) { /* From an older use of this loop device */ + log_debug("Dropping event because seqnum too old (%" PRIu64 " <= %" PRIu64 ")", + seqnum, w->uevent_seqnum_not_before); + return 0; + } + } + r = device_is_partition(device, w->parent_device, w->blkidp); if (r < 0) goto finish; @@ -294,6 +309,7 @@ static int wait_for_partition_device( sd_device *parent, blkid_partition pp, usec_t deadline, + uint64_t uevent_seqnum_not_before, sd_device **ret) { _cleanup_(sd_event_source_unrefp) sd_event_source *timeout_source = NULL; @@ -336,6 +352,7 @@ static int wait_for_partition_device( _cleanup_(wait_data_done) struct wait_data w = { .parent_device = parent, .blkidp = pp, + .uevent_seqnum_not_before = uevent_seqnum_not_before, }; r = sd_device_monitor_start(monitor, device_monitor_handler, &w); @@ -492,6 +509,7 @@ int dissect_image( int fd, const VeritySettings *verity, const MountOptions *mount_options, + uint64_t uevent_seqnum_not_before, DissectImageFlags flags, DissectedImage **ret) { @@ -744,7 +762,7 @@ int dissect_image( if (!pp) return errno_or_else(EIO); - r = wait_for_partition_device(d, pp, deadline, &q); + r = wait_for_partition_device(d, pp, deadline, uevent_seqnum_not_before, &q); if (r < 0) return r; @@ -2579,6 +2597,7 @@ int dissect_image_and_warn( const char *name, const VeritySettings *verity, const MountOptions *mount_options, + uint64_t uevent_seqnum_not_before, DissectImageFlags flags, DissectedImage **ret) { @@ -2593,7 +2612,7 @@ int dissect_image_and_warn( name = buffer; } - r = dissect_image(fd, verity, mount_options, flags, ret); + r = dissect_image(fd, verity, mount_options, uevent_seqnum_not_before, flags, ret); switch (r) { case -EOPNOTSUPP: @@ -2701,7 +2720,7 @@ int mount_image_privately_interactively( if (r < 0) return log_error_errno(r, "Failed to set up loopback device: %m"); - r = dissect_image_and_warn(d->fd, image, &verity, NULL, flags, &dissected_image); + r = dissect_image_and_warn(d->fd, image, &verity, NULL, d->uevent_seqnum_not_before, flags, &dissected_image); if (r < 0) return r; @@ -2792,6 +2811,7 @@ int verity_dissect_and_mount( loop_device->fd, &verity, options, + loop_device->uevent_seqnum_not_before, dissect_image_flags, &dissected_image); /* No partition table? Might be a single-filesystem image, try again */ @@ -2800,7 +2820,8 @@ int verity_dissect_and_mount( loop_device->fd, &verity, options, - dissect_image_flags|DISSECT_IMAGE_NO_PARTITION_TABLE, + loop_device->uevent_seqnum_not_before, + dissect_image_flags | DISSECT_IMAGE_NO_PARTITION_TABLE, &dissected_image); if (r < 0) return log_debug_errno(r, "Failed to dissect image: %m"); diff --git a/src/shared/dissect-image.h b/src/shared/dissect-image.h index d51049e78a..5d0b1d5e65 100644 --- a/src/shared/dissect-image.h +++ b/src/shared/dissect-image.h @@ -159,8 +159,8 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(MountOptions*, mount_options_free_all); const char* mount_options_from_designator(const MountOptions *options, PartitionDesignator designator); int probe_filesystem(const char *node, char **ret_fstype); -int dissect_image(int fd, const VeritySettings *verity, const MountOptions *mount_options, DissectImageFlags flags, DissectedImage **ret); -int dissect_image_and_warn(int fd, const char *name, const VeritySettings *verity, const MountOptions *mount_options, DissectImageFlags flags, DissectedImage **ret); +int dissect_image(int fd, const VeritySettings *verity, const MountOptions *mount_options, uint64_t uevent_seqnum_not_before, DissectImageFlags flags, DissectedImage **ret); +int dissect_image_and_warn(int fd, const char *name, const VeritySettings *verity, const MountOptions *mount_options, uint64_t uevent_seqnum_not_before, DissectImageFlags flags, DissectedImage **ret); DissectedImage* dissected_image_unref(DissectedImage *m); DEFINE_TRIVIAL_CLEANUP_FUNC(DissectedImage*, dissected_image_unref); diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index 49ec23d934..c305a30fae 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -532,6 +532,7 @@ static int merge_subprocess(Hashmap *images, const char *workspace) { img->path, &verity_settings, NULL, + d->uevent_seqnum_not_before, flags, &m); if (r < 0) diff --git a/src/test/test-loop-block.c b/src/test/test-loop-block.c index 93f2da70e7..cfa999eff7 100644 --- a/src/test/test-loop-block.c +++ b/src/test/test-loop-block.c @@ -51,7 +51,7 @@ static void* thread_func(void *ptr) { log_notice("Acquired loop device %s, will mount on %s", loop->node, mounted); - r = dissect_image(loop->fd, NULL, NULL, DISSECT_IMAGE_READ_ONLY, &dissected); + r = dissect_image(loop->fd, NULL, NULL, loop->uevent_seqnum_not_before, DISSECT_IMAGE_READ_ONLY, &dissected); if (r < 0) log_error_errno(r, "Failed dissect loopback device %s: %m", loop->node); assert_se(r >= 0); @@ -188,7 +188,7 @@ int main(int argc, char *argv[]) { sfdisk = NULL; assert_se(loop_device_make(fd, O_RDWR, 0, UINT64_MAX, LO_FLAGS_PARTSCAN, &loop) >= 0); - assert_se(dissect_image(loop->fd, NULL, NULL, 0, &dissected) >= 0); + assert_se(dissect_image(loop->fd, NULL, NULL, loop->uevent_seqnum_not_before, 0, &dissected) >= 0); assert_se(dissected->partitions[PARTITION_ESP].found); assert_se(dissected->partitions[PARTITION_ESP].node); @@ -212,7 +212,7 @@ int main(int argc, char *argv[]) { assert_se(make_filesystem(dissected->partitions[PARTITION_HOME].node, "ext4", "home", id, true) >= 0); dissected = dissected_image_unref(dissected); - assert_se(dissect_image(loop->fd, NULL, NULL, 0, &dissected) >= 0); + assert_se(dissect_image(loop->fd, NULL, NULL, loop->uevent_seqnum_not_before, 0, &dissected) >= 0); assert_se(mkdtemp_malloc(NULL, &mounted) >= 0); From 8626b43be42b9db065f8fdf2b057990ef511150e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 16:01:53 +0200 Subject: [PATCH 08/11] sd-device: add API to query from when a udev database entry is We already store a CLOCK_MONOTONIC timestamp for each device appearance, let' make this queriable. This is useful to determine whether a udev device database entry is from a current appearance of the device or a previous one, by comparing it with appropriately taken timestamps. --- src/libsystemd/libsystemd.sym | 1 + src/libsystemd/sd-device/sd-device.c | 21 +++++++++++++++++++++ src/systemd/sd-device.h | 1 + 3 files changed, 23 insertions(+) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index b025b55026..996bf25d6c 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -756,4 +756,5 @@ LIBSYSTEMD_249 { global: sd_device_monitor_filter_add_match_sysattr; sd_device_monitor_filter_add_match_parent; + sd_device_get_usec_initialized; } LIBSYSTEMD_248; diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 200e2077ee..4b7f22e207 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -1428,6 +1428,27 @@ _public_ int sd_device_get_is_initialized(sd_device *device) { return device->is_initialized; } +_public_ int sd_device_get_usec_initialized(sd_device *device, uint64_t *ret) { + int r; + + assert_return(device, -EINVAL); + + r = device_read_db(device); + if (r < 0) + return r; + + if (!device->is_initialized) + return -EBUSY; + + if (device->usec_initialized == 0) + return -ENODATA; + + if (ret) + *ret = device->usec_initialized; + + return 0; +} + _public_ int sd_device_get_usec_since_initialized(sd_device *device, uint64_t *usec) { usec_t now_ts; int r; diff --git a/src/systemd/sd-device.h b/src/systemd/sd-device.h index e760e03f35..8b7b361295 100644 --- a/src/systemd/sd-device.h +++ b/src/systemd/sd-device.h @@ -79,6 +79,7 @@ int sd_device_get_action(sd_device *device, sd_device_action_t *ret); int sd_device_get_seqnum(sd_device *device, uint64_t *ret); int sd_device_get_is_initialized(sd_device *device); +int sd_device_get_usec_initialized(sd_device *device, uint64_t *usec); int sd_device_get_usec_since_initialized(sd_device *device, uint64_t *usec); const char *sd_device_get_tag_first(sd_device *device); From 8ede1e86b2cfdbe7bb1e1913a08e068a82d4532c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 16:08:43 +0200 Subject: [PATCH 09/11] loop-util: track CLOCK_MONOTONIC timestamp immediately before attaching a loopback device This is similar to the preceding work to store the uevent seqnum, but this stores the CLOCK_MONOTONIC timestamp. Why? This allows to validate udev database entries, to determine if they were created *after* we attached the device. The uevent seqnum logic allows us to validate uevent, and the timestamp database entries, hence together we should be able to validate both sources of truth for us. (note that this is all racy, just a bit less racy, since we cannot atomically attach loopback devices and get the timestamp for it, the same way we can't get the uevent seqnum. Thus is shortens the race window, but doesn#t close it). --- src/shared/loop-util.c | 16 ++++++++++++++-- src/shared/loop-util.h | 2 ++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c index 3615995de0..c038e7aae6 100644 --- a/src/shared/loop-util.c +++ b/src/shared/loop-util.c @@ -132,12 +132,14 @@ static int loop_configure( int nr, const struct loop_config *c, bool *try_loop_configure, - uint64_t *ret_seqnum_not_before) { + uint64_t *ret_seqnum_not_before, + usec_t *ret_timestamp_not_before) { _cleanup_(sd_device_unrefp) sd_device *d = NULL; _cleanup_free_ char *sysname = NULL; _cleanup_close_ int lock_fd = -1; uint64_t seqnum; + usec_t timestamp; int r; assert(fd >= 0); @@ -195,6 +197,7 @@ static int loop_configure( r = get_current_uevent_seqnum(&seqnum); if (r < 0) return r; + timestamp = now(CLOCK_MONOTONIC); if (ioctl(fd, LOOP_CONFIGURE, c) < 0) { /* Do fallback only if LOOP_CONFIGURE is not supported, propagate all other @@ -255,6 +258,8 @@ static int loop_configure( if (ret_seqnum_not_before) *ret_seqnum_not_before = seqnum; + if (ret_timestamp_not_before) + *ret_timestamp_not_before = timestamp; return 0; } @@ -264,6 +269,7 @@ static int loop_configure( r = get_current_uevent_seqnum(&seqnum); if (r < 0) return r; + timestamp = now(CLOCK_MONOTONIC); /* Since kernel commit 5db470e229e22b7eda6e23b5566e532c96fb5bc3 (kernel v5.0) the LOOP_SET_STATUS64 * ioctl can return EAGAIN in case we change the lo_offset field, if someone else is accessing the @@ -294,6 +300,8 @@ static int loop_configure( if (ret_seqnum_not_before) *ret_seqnum_not_before = seqnum; + if (ret_timestamp_not_before) + *ret_timestamp_not_before = timestamp; return 0; @@ -353,6 +361,7 @@ int loop_device_make( struct loop_config config; LoopDevice *d = NULL; uint64_t seqnum = UINT64_MAX; + usec_t timestamp = USEC_INFINITY; struct stat st; int nr = -1, r; @@ -396,6 +405,7 @@ int loop_device_make( .relinquished = true, /* It's not allocated by us, don't destroy it when this object is freed */ .devno = st.st_rdev, .uevent_seqnum_not_before = UINT64_MAX, + .timestamp_not_before = USEC_INFINITY, }; *ret = d; @@ -443,7 +453,7 @@ int loop_device_make( if (!IN_SET(errno, ENOENT, ENXIO)) return -errno; } else { - r = loop_configure(loop, nr, &config, &try_loop_configure, &seqnum); + r = loop_configure(loop, nr, &config, &try_loop_configure, &seqnum, ×tamp); if (r >= 0) { loop_with_fd = TAKE_FD(loop); break; @@ -481,6 +491,7 @@ int loop_device_make( .nr = nr, .devno = st.st_rdev, .uevent_seqnum_not_before = seqnum, + .timestamp_not_before = timestamp, }; *ret = d; @@ -617,6 +628,7 @@ int loop_device_open(const char *loop_path, int open_flags, LoopDevice **ret) { .relinquished = true, /* It's not ours, don't try to destroy it when this object is freed */ .devno = st.st_dev, .uevent_seqnum_not_before = UINT64_MAX, + .timestamp_not_before = USEC_INFINITY, }; *ret = d; diff --git a/src/shared/loop-util.h b/src/shared/loop-util.h index 6df4f91c22..e06dfebb7c 100644 --- a/src/shared/loop-util.h +++ b/src/shared/loop-util.h @@ -2,6 +2,7 @@ #pragma once #include "macro.h" +#include "time-util.h" typedef struct LoopDevice LoopDevice; @@ -14,6 +15,7 @@ struct LoopDevice { char *node; bool relinquished; uint64_t uevent_seqnum_not_before; /* uevent sequm right before we attached the loopback device, or UINT64_MAX if we don't know */ + usec_t timestamp_not_before; /* CLOCK_MONOTONIC timestamp taken immediately before attaching the loopback device, or USEC_INFINITY if we don't know */ }; int loop_device_make(int fd, int open_flags, uint64_t offset, uint64_t size, uint32_t loop_flags, LoopDevice **ret); From 4a62257d6837fe33c7fb93a417156b94a6c8c83a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 16:19:27 +0200 Subject: [PATCH 10/11] dissect: ignore udev database entries from before the loopback attachment This tries to shorten the race of device reuse a bit more: let's ignore udev database entries that are older than the time where we started to use a loopback device. This doesn't fix the whole loopback device raciness mess, but it makes the race window a bit shorter. --- src/core/namespace.c | 1 + src/dissect/dissect.c | 1 + src/gpt-auto-generator/gpt-auto-generator.c | 1 + src/nspawn/nspawn.c | 1 + src/portable/portable.c | 1 + src/shared/discover-image.c | 1 + src/shared/dissect-image.c | 32 +++++++++++++++------ src/shared/dissect-image.h | 4 +-- src/sysext/sysext.c | 1 + src/test/test-loop-block.c | 6 ++-- 10 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index bffcb5ac86..77fb0d4394 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -1864,6 +1864,7 @@ int setup_namespace( &verity, root_image_options, loop_device->uevent_seqnum_not_before, + loop_device->timestamp_not_before, dissect_image_flags, &dissected_image); if (r < 0) diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index d70f1e791f..f1288b41a7 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -782,6 +782,7 @@ static int run(int argc, char *argv[]) { &arg_verity_settings, NULL, d->uevent_seqnum_not_before, + d->timestamp_not_before, arg_flags, &m); if (r < 0) diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index eb80165bbe..d3af814136 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -673,6 +673,7 @@ static int enumerate_partitions(dev_t devnum) { fd, NULL, NULL, UINT64_MAX, + USEC_INFINITY, DISSECT_IMAGE_GPT_ONLY| DISSECT_IMAGE_NO_UDEV| DISSECT_IMAGE_USR_NO_ROOT, diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 0701602418..ed9b31e63b 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -5484,6 +5484,7 @@ static int run(int argc, char *argv[]) { &arg_verity_settings, NULL, loop->uevent_seqnum_not_before, + loop->timestamp_not_before, dissect_image_flags, &dissected_image); if (r == -ENOPKG) { diff --git a/src/portable/portable.c b/src/portable/portable.c index 28d884a951..0799bff53d 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -396,6 +396,7 @@ static int portable_extract_by_path( d->fd, NULL, NULL, d->uevent_seqnum_not_before, + d->timestamp_not_before, DISSECT_IMAGE_READ_ONLY | DISSECT_IMAGE_GENERIC_ROOT | DISSECT_IMAGE_REQUIRE_ROOT | diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index c343d08931..8d04ba04fc 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -1202,6 +1202,7 @@ int image_read_metadata(Image *i) { d->fd, NULL, NULL, d->uevent_seqnum_not_before, + d->timestamp_not_before, DISSECT_IMAGE_GENERIC_ROOT | DISSECT_IMAGE_REQUIRE_ROOT | DISSECT_IMAGE_RELAX_VAR_CHECK | diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 345e1db9a7..53b613702e 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -123,10 +123,6 @@ static int enumerator_for_parent(sd_device *d, sd_device_enumerator **ret) { if (r < 0) return r; - r = sd_device_enumerator_allow_uninitialized(e); - if (r < 0) - return r; - r = sd_device_enumerator_add_match_subsystem(e, "block", true); if (r < 0) return r; @@ -229,6 +225,7 @@ static int device_is_partition(sd_device *d, sd_device *expected_parent, blkid_p static int find_partition( sd_device *parent, blkid_partition pp, + usec_t timestamp_not_before, sd_device **ret) { _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; @@ -244,6 +241,18 @@ static int find_partition( return r; FOREACH_DEVICE(e, q) { + uint64_t usec; + + r = sd_device_get_usec_initialized(q, &usec); + if (r == -EBUSY) /* Not initialized yet */ + continue; + if (r < 0) + return r; + + if (timestamp_not_before != USEC_INFINITY && + usec < timestamp_not_before) /* udev database entry older than our attachment? Then it's not ours */ + continue; + r = device_is_partition(q, parent, pp); if (r < 0) return r; @@ -310,6 +319,7 @@ static int wait_for_partition_device( blkid_partition pp, usec_t deadline, uint64_t uevent_seqnum_not_before, + usec_t timestamp_not_before, sd_device **ret) { _cleanup_(sd_event_source_unrefp) sd_event_source *timeout_source = NULL; @@ -321,7 +331,7 @@ static int wait_for_partition_device( assert(pp); assert(ret); - r = find_partition(parent, pp, ret); + r = find_partition(parent, pp, timestamp_not_before, ret); if (r != -ENXIO) return r; @@ -360,7 +370,7 @@ static int wait_for_partition_device( return r; /* Check again, the partition might have appeared in the meantime */ - r = find_partition(parent, pp, ret); + r = find_partition(parent, pp, timestamp_not_before, ret); if (r != -ENXIO) return r; @@ -510,6 +520,7 @@ int dissect_image( const VeritySettings *verity, const MountOptions *mount_options, uint64_t uevent_seqnum_not_before, + usec_t timestamp_not_before, DissectImageFlags flags, DissectedImage **ret) { @@ -762,7 +773,7 @@ int dissect_image( if (!pp) return errno_or_else(EIO); - r = wait_for_partition_device(d, pp, deadline, uevent_seqnum_not_before, &q); + r = wait_for_partition_device(d, pp, deadline, uevent_seqnum_not_before, timestamp_not_before, &q); if (r < 0) return r; @@ -2598,6 +2609,7 @@ int dissect_image_and_warn( const VeritySettings *verity, const MountOptions *mount_options, uint64_t uevent_seqnum_not_before, + usec_t timestamp_not_before, DissectImageFlags flags, DissectedImage **ret) { @@ -2612,7 +2624,7 @@ int dissect_image_and_warn( name = buffer; } - r = dissect_image(fd, verity, mount_options, uevent_seqnum_not_before, flags, ret); + r = dissect_image(fd, verity, mount_options, uevent_seqnum_not_before, timestamp_not_before, flags, ret); switch (r) { case -EOPNOTSUPP: @@ -2720,7 +2732,7 @@ int mount_image_privately_interactively( if (r < 0) return log_error_errno(r, "Failed to set up loopback device: %m"); - r = dissect_image_and_warn(d->fd, image, &verity, NULL, d->uevent_seqnum_not_before, flags, &dissected_image); + r = dissect_image_and_warn(d->fd, image, &verity, NULL, d->uevent_seqnum_not_before, d->timestamp_not_before, flags, &dissected_image); if (r < 0) return r; @@ -2812,6 +2824,7 @@ int verity_dissect_and_mount( &verity, options, loop_device->uevent_seqnum_not_before, + loop_device->timestamp_not_before, dissect_image_flags, &dissected_image); /* No partition table? Might be a single-filesystem image, try again */ @@ -2821,6 +2834,7 @@ int verity_dissect_and_mount( &verity, options, loop_device->uevent_seqnum_not_before, + loop_device->timestamp_not_before, dissect_image_flags | DISSECT_IMAGE_NO_PARTITION_TABLE, &dissected_image); if (r < 0) diff --git a/src/shared/dissect-image.h b/src/shared/dissect-image.h index 5d0b1d5e65..88106ee4f3 100644 --- a/src/shared/dissect-image.h +++ b/src/shared/dissect-image.h @@ -159,8 +159,8 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(MountOptions*, mount_options_free_all); const char* mount_options_from_designator(const MountOptions *options, PartitionDesignator designator); int probe_filesystem(const char *node, char **ret_fstype); -int dissect_image(int fd, const VeritySettings *verity, const MountOptions *mount_options, uint64_t uevent_seqnum_not_before, DissectImageFlags flags, DissectedImage **ret); -int dissect_image_and_warn(int fd, const char *name, const VeritySettings *verity, const MountOptions *mount_options, uint64_t uevent_seqnum_not_before, DissectImageFlags flags, DissectedImage **ret); +int dissect_image(int fd, const VeritySettings *verity, const MountOptions *mount_options, uint64_t uevent_seqnum_not_before, usec_t timestamp_not_before, DissectImageFlags flags, DissectedImage **ret); +int dissect_image_and_warn(int fd, const char *name, const VeritySettings *verity, const MountOptions *mount_options, uint64_t uevent_seqnum_not_before, usec_t timestamp_not_before, DissectImageFlags flags, DissectedImage **ret); DissectedImage* dissected_image_unref(DissectedImage *m); DEFINE_TRIVIAL_CLEANUP_FUNC(DissectedImage*, dissected_image_unref); diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index c305a30fae..c5fdf99aa7 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -533,6 +533,7 @@ static int merge_subprocess(Hashmap *images, const char *workspace) { &verity_settings, NULL, d->uevent_seqnum_not_before, + d->timestamp_not_before, flags, &m); if (r < 0) diff --git a/src/test/test-loop-block.c b/src/test/test-loop-block.c index cfa999eff7..ba44b5f3f9 100644 --- a/src/test/test-loop-block.c +++ b/src/test/test-loop-block.c @@ -51,7 +51,7 @@ static void* thread_func(void *ptr) { log_notice("Acquired loop device %s, will mount on %s", loop->node, mounted); - r = dissect_image(loop->fd, NULL, NULL, loop->uevent_seqnum_not_before, DISSECT_IMAGE_READ_ONLY, &dissected); + r = dissect_image(loop->fd, NULL, NULL, loop->uevent_seqnum_not_before, loop->timestamp_not_before, DISSECT_IMAGE_READ_ONLY, &dissected); if (r < 0) log_error_errno(r, "Failed dissect loopback device %s: %m", loop->node); assert_se(r >= 0); @@ -188,7 +188,7 @@ int main(int argc, char *argv[]) { sfdisk = NULL; assert_se(loop_device_make(fd, O_RDWR, 0, UINT64_MAX, LO_FLAGS_PARTSCAN, &loop) >= 0); - assert_se(dissect_image(loop->fd, NULL, NULL, loop->uevent_seqnum_not_before, 0, &dissected) >= 0); + assert_se(dissect_image(loop->fd, NULL, NULL, loop->uevent_seqnum_not_before, loop->timestamp_not_before, 0, &dissected) >= 0); assert_se(dissected->partitions[PARTITION_ESP].found); assert_se(dissected->partitions[PARTITION_ESP].node); @@ -212,7 +212,7 @@ int main(int argc, char *argv[]) { assert_se(make_filesystem(dissected->partitions[PARTITION_HOME].node, "ext4", "home", id, true) >= 0); dissected = dissected_image_unref(dissected); - assert_se(dissect_image(loop->fd, NULL, NULL, loop->uevent_seqnum_not_before, 0, &dissected) >= 0); + assert_se(dissect_image(loop->fd, NULL, NULL, loop->uevent_seqnum_not_before, loop->timestamp_not_before, 0, &dissected) >= 0); assert_se(mkdtemp_malloc(NULL, &mounted) >= 0); From 6e1454b4b904ac33416da4d1ce841c2a574d9f62 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Apr 2021 17:21:22 +0200 Subject: [PATCH 11/11] ci: drop test/TEST-50-DISSECT/deny-list-ubuntu-ci Let's see if this makes the test stable on the CI. --- test/TEST-50-DISSECT/deny-list-ubuntu-ci | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 test/TEST-50-DISSECT/deny-list-ubuntu-ci diff --git a/test/TEST-50-DISSECT/deny-list-ubuntu-ci b/test/TEST-50-DISSECT/deny-list-ubuntu-ci deleted file mode 100644 index e7585d747e..0000000000 --- a/test/TEST-50-DISSECT/deny-list-ubuntu-ci +++ /dev/null @@ -1,2 +0,0 @@ -Skip this test due to issue #17469 -https://github.com/systemd/systemd/issues/17469