[PATCH] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[PATCH] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Nandakumar Raghavan]

During journal replay, e2fsck writes the primary superblock back to disk
in multiple I/O operations. The payload lands before the checksum, leaving
a transient window where the on-disk superblock has a bad checksum.

If udevd processes a change uevent during this window, libblkid probes the
primary superblock, finds a checksum mismatch, and concludes the partition
has no recognisable filesystem. udev then fires a remove event, wiping all
symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
that depends on those symlinks will fail.

udevd already serialises its own partition probes against whole-disk device
access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
whole-disk device before opening the filesystem. This forces udevd to defer
all probes on that disk until e2fsck exits and the lock is released, by
which point the filesystem is fully consistent.

The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
so that the lock covers the same device node that udevd locks. If sysfs
resolution fails, a warning is emitted and the lock falls back to the
partition device itself.

Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
---
 e2fsck/unix.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 335ca377..12a9f50a 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -36,6 +36,15 @@ extern int optind;
 #ifdef HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
 #endif
+#ifdef HAVE_SYS_FILE_H
+#include <sys/file.h>
+#endif
+#ifdef HAVE_SYS_STAT_H
+#include <sys/stat.h>
+#endif
+#ifdef HAVE_SYS_SYSMACROS_H
+#include <sys/sysmacros.h>
+#endif
 #ifdef HAVE_MALLOC_H
 #include <malloc.h>
 #endif
@@ -1397,6 +1406,92 @@ err:
 	return retval;
 }
 
+static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
+{
+	struct stat st;
+	char partition_attr[256];  /* sysfs 'partition' attribute path */
+	char parent_dev_path[256]; /* sysfs '../dev' of the parent disk */
+	char parent_devnum[32];
+	FILE *f;
+	unsigned int maj, min;
+	int parent_resolved = 0;
+	char lock_path[256];
+	const char *opened_path;
+	int fd;
+
+	if (stat(dev_name, &st) < 0)
+		return -1;
+
+	if (!S_ISBLK(st.st_mode))
+		return -1;
+
+	maj = major(st.st_rdev);
+	min = minor(st.st_rdev);
+
+	/*
+	 * If dev_name is a partition (sysfs 'partition' attribute exists),
+	 * resolve the parent whole-disk device so the flock covers the same
+	 * device node that udevd locks before probing any partition on it.
+	 */
+	snprintf(partition_attr, sizeof(partition_attr),
+		 "/sys/dev/block/%u:%u/partition", maj, min);
+
+	if (access(partition_attr, F_OK) == 0) {
+		snprintf(parent_dev_path, sizeof(parent_dev_path),
+			 "/sys/dev/block/%u:%u/../dev", maj, min);
+
+		f = fopen(parent_dev_path, "r");
+		if (f) {
+			if (fscanf(f, "%31s", parent_devnum) == 1) {
+				unsigned int pmaj, pmin;
+				if (sscanf(parent_devnum, "%u:%u",
+					   &pmaj, &pmin) == 2) {
+					maj = pmaj;
+					min = pmin;
+					parent_resolved = 1;
+				}
+			}
+			fclose(f);
+		}
+		if (!parent_resolved)
+			log_err(ctx, _("Warning: could not resolve whole-disk "
+				       "device for %s; lock may not prevent "
+				       "udev races\n"), dev_name);
+	}
+
+	snprintf(lock_path, sizeof(lock_path),
+		 "/dev/block/%u:%u", maj, min);
+
+	opened_path = lock_path;
+	fd = open(lock_path, O_RDONLY | O_CLOEXEC, 0);
+	if (fd < 0) {
+		fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
+		if (fd < 0)
+			return -1;
+		opened_path = dev_name;
+		if (parent_resolved)
+			log_err(ctx, _("Warning: %s not found; locking %s "
+				       "instead, lock may not prevent "
+				       "udev races\n"), lock_path, dev_name);
+	}
+
+	while (flock(fd, LOCK_EX) != 0) {
+		if (errno == EINTR) {
+			if (ctx->flags & E2F_FLAG_CANCEL) {
+				close(fd);
+				return -1;
+			}
+			continue;
+		}
+		com_err(ctx->program_name, errno,
+			_("while trying to lock %s"), opened_path);
+		close(fd);
+		return -1;
+	}
+
+	return fd;
+}
+
 int main (int argc, char *argv[])
 {
 	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
@@ -1413,6 +1508,7 @@ int main (int argc, char *argv[])
 	int journal_size;
 	int sysval, sys_page_size = 4096;
 	int old_bitmaps;
+	int lock_fd = -1;
 	__u32 features[3];
 	char *cp;
 	enum quota_type qtype;
@@ -1488,6 +1584,12 @@ int main (int argc, char *argv[])
 
 	check_mount(ctx);
 
+	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
+	if (lock_fd < 0)
+		log_err(ctx, _("Warning: could not lock %s; "
+			       "proceeding without block device lock\n"),
+			ctx->filesystem_name);
+
 	if (!(ctx->options & E2F_OPT_PREEN) &&
 	    !(ctx->options & E2F_OPT_NO) &&
 	    !(ctx->options & E2F_OPT_YES)) {
@@ -2169,6 +2271,9 @@ skip_write:
 	ext2fs_close_free(&ctx->fs);
 	free(ctx->journal_name);
 
+	if (lock_fd >= 0)
+		close(lock_fd);
+
 	if (ctx->logf)
 		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
 	e2fsck_free_context(ctx);
-- 
2.54.0

Re: [PATCH] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Nandakumar Raghavan]

Hi,

Gentle ping on this patch.

I would appreciate any feedback when time permits.

On Mon, Jul 20, 2026 at 02:32:46PM +0000, Nandakumar Raghavan wrote:
> During journal replay, e2fsck writes the primary superblock back to disk
> in multiple I/O operations. The payload lands before the checksum, leaving
> a transient window where the on-disk superblock has a bad checksum.
> 
> If udevd processes a change uevent during this window, libblkid probes the
> primary superblock, finds a checksum mismatch, and concludes the partition
> has no recognisable filesystem. udev then fires a remove event, wiping all
> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> that depends on those symlinks will fail.
> 
> udevd already serialises its own partition probes against whole-disk device
> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> whole-disk device before opening the filesystem. This forces udevd to defer
> all probes on that disk until e2fsck exits and the lock is released, by
> which point the filesystem is fully consistent.
> 
> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> so that the lock covers the same device node that udevd locks. If sysfs
> resolution fails, a warning is emitted and the lock falls back to the
> partition device itself.
> 
> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> ---
>  e2fsck/unix.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 105 insertions(+)
> 
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index 335ca377..12a9f50a 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -36,6 +36,15 @@ extern int optind;
>  #ifdef HAVE_SYS_IOCTL_H
>  #include <sys/ioctl.h>
>  #endif
> +#ifdef HAVE_SYS_FILE_H
> +#include <sys/file.h>
> +#endif
> +#ifdef HAVE_SYS_STAT_H
> +#include <sys/stat.h>
> +#endif
> +#ifdef HAVE_SYS_SYSMACROS_H
> +#include <sys/sysmacros.h>
> +#endif
>  #ifdef HAVE_MALLOC_H
>  #include <malloc.h>
>  #endif
> @@ -1397,6 +1406,92 @@ err:
>  	return retval;
>  }
>  
> +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> +{
> +	struct stat st;
> +	char partition_attr[256];  /* sysfs 'partition' attribute path */
> +	char parent_dev_path[256]; /* sysfs '../dev' of the parent disk */
> +	char parent_devnum[32];
> +	FILE *f;
> +	unsigned int maj, min;
> +	int parent_resolved = 0;
> +	char lock_path[256];
> +	const char *opened_path;
> +	int fd;
> +
> +	if (stat(dev_name, &st) < 0)
> +		return -1;
> +
> +	if (!S_ISBLK(st.st_mode))
> +		return -1;
> +
> +	maj = major(st.st_rdev);
> +	min = minor(st.st_rdev);
> +
> +	/*
> +	 * If dev_name is a partition (sysfs 'partition' attribute exists),
> +	 * resolve the parent whole-disk device so the flock covers the same
> +	 * device node that udevd locks before probing any partition on it.
> +	 */
> +	snprintf(partition_attr, sizeof(partition_attr),
> +		 "/sys/dev/block/%u:%u/partition", maj, min);
> +
> +	if (access(partition_attr, F_OK) == 0) {
> +		snprintf(parent_dev_path, sizeof(parent_dev_path),
> +			 "/sys/dev/block/%u:%u/../dev", maj, min);
> +
> +		f = fopen(parent_dev_path, "r");
> +		if (f) {
> +			if (fscanf(f, "%31s", parent_devnum) == 1) {
> +				unsigned int pmaj, pmin;
> +				if (sscanf(parent_devnum, "%u:%u",
> +					   &pmaj, &pmin) == 2) {
> +					maj = pmaj;
> +					min = pmin;
> +					parent_resolved = 1;
> +				}
> +			}
> +			fclose(f);
> +		}
> +		if (!parent_resolved)
> +			log_err(ctx, _("Warning: could not resolve whole-disk "
> +				       "device for %s; lock may not prevent "
> +				       "udev races\n"), dev_name);
> +	}
> +
> +	snprintf(lock_path, sizeof(lock_path),
> +		 "/dev/block/%u:%u", maj, min);
> +
> +	opened_path = lock_path;
> +	fd = open(lock_path, O_RDONLY | O_CLOEXEC, 0);
> +	if (fd < 0) {
> +		fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
> +		if (fd < 0)
> +			return -1;
> +		opened_path = dev_name;
> +		if (parent_resolved)
> +			log_err(ctx, _("Warning: %s not found; locking %s "
> +				       "instead, lock may not prevent "
> +				       "udev races\n"), lock_path, dev_name);
> +	}
> +
> +	while (flock(fd, LOCK_EX) != 0) {
> +		if (errno == EINTR) {
> +			if (ctx->flags & E2F_FLAG_CANCEL) {
> +				close(fd);
> +				return -1;
> +			}
> +			continue;
> +		}
> +		com_err(ctx->program_name, errno,
> +			_("while trying to lock %s"), opened_path);
> +		close(fd);
> +		return -1;
> +	}
> +
> +	return fd;
> +}
> +
>  int main (int argc, char *argv[])
>  {
>  	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
> @@ -1413,6 +1508,7 @@ int main (int argc, char *argv[])
>  	int journal_size;
>  	int sysval, sys_page_size = 4096;
>  	int old_bitmaps;
> +	int lock_fd = -1;
>  	__u32 features[3];
>  	char *cp;
>  	enum quota_type qtype;
> @@ -1488,6 +1584,12 @@ int main (int argc, char *argv[])
>  
>  	check_mount(ctx);
>  
> +	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> +	if (lock_fd < 0)
> +		log_err(ctx, _("Warning: could not lock %s; "
> +			       "proceeding without block device lock\n"),
> +			ctx->filesystem_name);
> +
>  	if (!(ctx->options & E2F_OPT_PREEN) &&
>  	    !(ctx->options & E2F_OPT_NO) &&
>  	    !(ctx->options & E2F_OPT_YES)) {
> @@ -2169,6 +2271,9 @@ skip_write:
>  	ext2fs_close_free(&ctx->fs);
>  	free(ctx->journal_name);
>  
> +	if (lock_fd >= 0)
> +		close(lock_fd);
> +
>  	if (ctx->logf)
>  		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
>  	e2fsck_free_context(ctx);
> -- 
> 2.54.0

Re: [PATCH] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Andreas Dilger]

On Aug 14, 2026, at 23:29, Nandakumar Raghavan <naraghavan@linux.microsoft.com> wrote:
> 
> Hi,
> 
> Gentle ping on this patch.
> 
> I would appreciate any feedback when time permits.
> 
> On Mon, Jul 20, 2026 at 02:32:46PM +0000, Nandakumar Raghavan wrote:
>> During journal replay, e2fsck writes the primary superblock back to disk
>> in multiple I/O operations. The payload lands before the checksum, leaving
>> a transient window where the on-disk superblock has a bad checksum.
>> 
>> If udevd processes a change uevent during this window, libblkid probes the
>> primary superblock, finds a checksum mismatch, and concludes the partition
>> has no recognisable filesystem. udev then fires a remove event, wiping all
>> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
>> that depends on those symlinks will fail.
>> 
>> udevd already serialises its own partition probes against whole-disk device
>> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
>> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
>> whole-disk device before opening the filesystem. This forces udevd to defer
>> all probes on that disk until e2fsck exits and the lock is released, by
>> which point the filesystem is fully consistent.
>> 
>> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
>> so that the lock covers the same device node that udevd locks. If sysfs
>> resolution fails, a warning is emitted and the lock falls back to the
>> partition device itself.
>> 
>> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>

This looks like it would unnecessarily emit an error message if run on any
non-Linux platform (e.g. MacOS, BSD), so it needs to be conditionally built.

>> ---
>> e2fsck/unix.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 105 insertions(+)
>> 
>> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
>> index 335ca377..12a9f50a 100644
>> --- a/e2fsck/unix.c
>> +++ b/e2fsck/unix.c
>> @@ -36,6 +36,15 @@ extern int optind;

Presumably this code is only useful on Linux, so the conditional headers are not needed,
only a single `#ifdef __linux__`?  Not really critical either way.

>> #ifdef HAVE_SYS_IOCTL_H
>> #include <sys/ioctl.h>
>> #endif
>> +#ifdef HAVE_SYS_FILE_H
>> +#include <sys/file.h>
>> +#endif
>> +#ifdef HAVE_SYS_STAT_H
>> +#include <sys/stat.h>
>> +#endif
>> +#ifdef HAVE_SYS_SYSMACROS_H
>> +#include <sys/sysmacros.h>
>> +#endif
>> #ifdef HAVE_MALLOC_H
>> #include <malloc.h>
>> #endif
>> @@ -1397,6 +1406,92 @@ err:
>> +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)

This function should similarly be #ifdef'd out if not building on Linux.

>> +{
>> + 	struct stat st;
>> + 	char partition_attr[256];  /* sysfs 'partition' attribute path */
>> + 	char parent_dev_path[256]; /* sysfs '../dev' of the parent disk */
>> + 	char parent_devnum[32];
>> + 	FILE *f;
>> + 	unsigned int maj, min;
>> + 	int parent_resolved = 0;
>> + 	char lock_path[256];
>> + 	const char *opened_path;
>> + 	int fd;
>> +
>> + 	if (stat(dev_name, &st) < 0)
>> + 		return -1;
>> +
>> + 	if (!S_ISBLK(st.st_mode))
>> + 		return -1;
>> +
>> + 	maj = major(st.st_rdev);
>> + 	min = minor(st.st_rdev);
>> +
>> + 	/*
>> +	 * If dev_name is a partition (sysfs 'partition' attribute exists),
>> +	 * resolve the parent whole-disk device so the flock covers the same
>> +	 * device node that udevd locks before probing any partition on it.
>> +	 */
>> + 	snprintf(partition_attr, sizeof(partition_attr),
>> + 		 "/sys/dev/block/%u:%u/partition", maj, min);
>> +
>> + 	if (access(partition_attr, F_OK) == 0) {
>> + 		snprintf(parent_dev_path, sizeof(parent_dev_path),
>> + 			 "/sys/dev/block/%u:%u/../dev", maj, min);

(minor) this could reuse `partition_attr` here?

>> +		f = fopen(parent_dev_path, "r");
>> +		if (f) {
>> + 			if (fscanf(f, "%31s", parent_devnum) == 1) {
>> +				 unsigned int pmaj, pmin;
>> +				 if (sscanf(parent_devnum, "%u:%u",
>> +					    &pmaj, &pmin) == 2) {
>> + 					maj = pmaj;
>> +					min = pmin;
>> + 					parent_resolved = 1;
>> + 				}
>> +			}
>> + 			fclose(f);
>> + 		}
>> +		if (!parent_resolved)
>> +			 log_err(ctx, _("Warning: could not resolve whole-disk "
>> +				        "device for %s; lock may not prevent "
>> +				        "udev races\n"), dev_name);
>> +	}
>> +
>> + 	snprintf(lock_path, sizeof(lock_path),
>> + 		 "/dev/block/%u:%u", maj, min);

(style) could fit on a single line?

(style) could re-use `partition_attr` here (maybe renamed to `dev_path` or similar)
to avoid having 3 single-use pathnames on the stack.

>> +
>> +	opened_path = lock_path;
>> + 	fd = open(lock_path, O_RDONLY | O_CLOEXEC, 0);
>> + 	if (fd < 0) {
>> + 		fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
>> + 		if (fd < 0)
>> + 			return -1;
>> + 		opened_path = dev_name;
>> + 		if (parent_resolved)
>> + 			log_err(ctx, _("Warning: %s not found; locking %s "
>> +				       "instead, lock may not prevent "
>> +				       "udev races\n"), lock_path, dev_name);

(style) shouldn't split error messages across lines, even if > 80 columns

>> +	}
>> +
>> + 	while (flock(fd, LOCK_EX) != 0) {
>> + 		if (errno == EINTR) {
>> + 			if (ctx->flags & E2F_FLAG_CANCEL) {
>> + 				close(fd);
>> + 				return -1;
>> + 			}
>> + 			continue;

Does this loop/hang forever if you try to kill it with CTRL-C?

>> + 		}
>> + 		com_err(ctx->program_name, errno,
>> + 			_("while trying to lock %s"), opened_path);
>> + 		close(fd);
>> +		return -1;
>> +	}
>> +
>> + 	return fd;
>> +}
>> +
>> int main (int argc, char *argv[])
>> {
>> errcode_t retval = 0, retval2 = 0, orig_retval = 0;
>> @@ -1413,6 +1508,7 @@ int main (int argc, char *argv[])
>> int journal_size;
>> int sysval, sys_page_size = 4096;
>> int old_bitmaps;
>> + int lock_fd = -1;
>> __u32 features[3];
>> char *cp;
>> enum quota_type qtype;
>> @@ -1488,6 +1584,12 @@ int main (int argc, char *argv[])
>> 
>> check_mount(ctx);
>> 
>> + lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
>> + if (lock_fd < 0)
>> + log_err(ctx, _("Warning: could not lock %s; "
>> +       "proceeding without block device lock\n"),
>> + ctx->filesystem_name);
>> +
>> if (!(ctx->options & E2F_OPT_PREEN) &&
>>    !(ctx->options & E2F_OPT_NO) &&
>>    !(ctx->options & E2F_OPT_YES)) {
>> @@ -2169,6 +2271,9 @@ skip_write:
>> ext2fs_close_free(&ctx->fs);
>> free(ctx->journal_name);
>> 
>> + if (lock_fd >= 0)
>> + close(lock_fd);
>> +
>> if (ctx->logf)
>> fprintf(ctx->logf, "Exit status: %d\n", exit_value);
>> e2fsck_free_context(ctx);
>> -- 
>> 2.54.0
> 


Cheers, Andreas

Re: [PATCH] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Nandakumar Raghavan]

On Tue, Aug 18, 2026 at 04:44:26PM -0600, Andreas Dilger wrote:
> On Aug 14, 2026, at 23:29, Nandakumar Raghavan <naraghavan@linux.microsoft.com> wrote:
> > 
> > Hi,
> > 
> > Gentle ping on this patch.
> > 
> > I would appreciate any feedback when time permits.
> > 
> > On Mon, Jul 20, 2026 at 02:32:46PM +0000, Nandakumar Raghavan wrote:
> >> During journal replay, e2fsck writes the primary superblock back to disk
> >> in multiple I/O operations. The payload lands before the checksum, leaving
> >> a transient window where the on-disk superblock has a bad checksum.
> >> 
> >> If udevd processes a change uevent during this window, libblkid probes the
> >> primary superblock, finds a checksum mismatch, and concludes the partition
> >> has no recognisable filesystem. udev then fires a remove event, wiping all
> >> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> >> that depends on those symlinks will fail.
> >> 
> >> udevd already serialises its own partition probes against whole-disk device
> >> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> >> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> >> whole-disk device before opening the filesystem. This forces udevd to defer
> >> all probes on that disk until e2fsck exits and the lock is released, by
> >> which point the filesystem is fully consistent.
> >> 
> >> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> >> so that the lock covers the same device node that udevd locks. If sysfs
> >> resolution fails, a warning is emitted and the lock falls back to the
> >> partition device itself.
> >> 
> >> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
>
Thanks for the review, Andreas. Replies inline, v2 coming with all of these
addressed.
 
> This looks like it would unnecessarily emit an error message if run on any
> non-Linux platform (e.g. MacOS, BSD), so it needs to be conditionally built.

Agreed, fixed in v2 — lock_whole_disk(), its call site, the lock_fd
variable, and the close() cleanup are all now wrapped in #ifdef __linux__.
Nothing from this feature is compiled at all on non-Linux.
> 
> >> ---
> >> e2fsck/unix.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 105 insertions(+)
> >> 
> >> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> >> index 335ca377..12a9f50a 100644
> >> --- a/e2fsck/unix.c
> >> +++ b/e2fsck/unix.c
> >> @@ -36,6 +36,15 @@ extern int optind;
> 
> Presumably this code is only useful on Linux, so the conditional headers are not needed,
> only a single `#ifdef __linux__`?  Not really critical either way.

Fixed. collapsed the three HAVE_SYS_*_H guards into one #ifdef __linux__
block around sys/file.h, sys/stat.h, sys/sysmacros.h.
> 
> >> #ifdef HAVE_SYS_IOCTL_H
> >> #include <sys/ioctl.h>
> >> #endif
> >> +#ifdef HAVE_SYS_FILE_H
> >> +#include <sys/file.h>
> >> +#endif
> >> +#ifdef HAVE_SYS_STAT_H
> >> +#include <sys/stat.h>
> >> +#endif
> >> +#ifdef HAVE_SYS_SYSMACROS_H
> >> +#include <sys/sysmacros.h>
> >> +#endif
> >> #ifdef HAVE_MALLOC_H
> >> #include <malloc.h>
> >> #endif
> >> @@ -1397,6 +1406,92 @@ err:
> >> +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> 
> This function should similarly be #ifdef'd out if not building on Linux.

Fixed. I have went with wrapping the whole function rather than guarding
just the body, so it's fully absent from non-Linux builds.
> 
> >> +{
> >> + 	struct stat st;
> >> + 	char partition_attr[256];  /* sysfs 'partition' attribute path */
> >> + 	char parent_dev_path[256]; /* sysfs '../dev' of the parent disk */
> >> + 	char parent_devnum[32];
> >> + 	FILE *f;
> >> + 	unsigned int maj, min;
> >> + 	int parent_resolved = 0;
> >> + 	char lock_path[256];
> >> + 	const char *opened_path;
> >> + 	int fd;
> >> +
> >> + 	if (stat(dev_name, &st) < 0)
> >> + 		return -1;
> >> +
> >> + 	if (!S_ISBLK(st.st_mode))
> >> + 		return -1;
> >> +
> >> + 	maj = major(st.st_rdev);
> >> + 	min = minor(st.st_rdev);
> >> +
> >> + 	/*
> >> +	 * If dev_name is a partition (sysfs 'partition' attribute exists),
> >> +	 * resolve the parent whole-disk device so the flock covers the same
> >> +	 * device node that udevd locks before probing any partition on it.
> >> +	 */
> >> + 	snprintf(partition_attr, sizeof(partition_attr),
> >> + 		 "/sys/dev/block/%u:%u/partition", maj, min);
> >> +
> >> + 	if (access(partition_attr, F_OK) == 0) {
> >> + 		snprintf(parent_dev_path, sizeof(parent_dev_path),
> >> + 			 "/sys/dev/block/%u:%u/../dev", maj, min);
> 
> (minor) this could reuse `partition_attr` here?
> 
> >> +		f = fopen(parent_dev_path, "r");
> >> +		if (f) {
> >> + 			if (fscanf(f, "%31s", parent_devnum) == 1) {
> >> +				 unsigned int pmaj, pmin;
> >> +				 if (sscanf(parent_devnum, "%u:%u",
> >> +					    &pmaj, &pmin) == 2) {
> >> + 					maj = pmaj;
> >> +					min = pmin;
> >> + 					parent_resolved = 1;
> >> + 				}
> >> +			}
> >> + 			fclose(f);
> >> + 		}
> >> +		if (!parent_resolved)
> >> +			 log_err(ctx, _("Warning: could not resolve whole-disk "
> >> +				        "device for %s; lock may not prevent "
> >> +				        "udev races\n"), dev_name);
> >> +	}
> >> +
> >> + 	snprintf(lock_path, sizeof(lock_path),
> >> + 		 "/dev/block/%u:%u", maj, min);
> 
> (style) could fit on a single line?
> 
> (style) could re-use `partition_attr` here (maybe renamed to `dev_path` or similar)
> to avoid having 3 single-use pathnames on the stack.
Fixed — same dev_path buffer reused for all three paths, and the /dev/block/%u:%u
snprintf is now a single line
> 
> >> +
> >> +	opened_path = lock_path;
> >> + 	fd = open(lock_path, O_RDONLY | O_CLOEXEC, 0);
> >> + 	if (fd < 0) {
> >> + 		fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
> >> + 		if (fd < 0)
> >> + 			return -1;
> >> + 		opened_path = dev_name;
> >> + 		if (parent_resolved)
> >> + 			log_err(ctx, _("Warning: %s not found; locking %s "
> >> +				       "instead, lock may not prevent "
> >> +				       "udev races\n"), lock_path, dev_name);
> 
> (style) shouldn't split error messages across lines, even if > 80 columns
Done — both warning strings are now single-line literals.
> 
> >> +	}
> >> +
> >> + 	while (flock(fd, LOCK_EX) != 0) {
> >> + 		if (errno == EINTR) {
> >> + 			if (ctx->flags & E2F_FLAG_CANCEL) {
> >> + 				close(fd);
> >> + 				return -1;
> >> + 			}
> >> + 			continue;
> 
> Does this loop/hang forever if you try to kill it with CTRL-C?

No. I traced it. The SIGINT/SIGTERM handler is installed earlier in main(),
before lock_whole_disk() runs, and isn't SA_RESTART (that's only applied
to SIGUSR1/SIGUSR2 afterward). So a blocked flock() gets EINTR on Ctrl-C,
E2F_FLAG_CANCEL is already set by the handler, and the existing EINTR
branch in the retry loop breaks out and returns -1 cleanly. So there is 
no hang.

One more thing found while testing v2: the original patch printed "Warning:
could not lock %s; proceeding without block device lock" unconditionally
whenever lock_whole_disk() returned -1 — including the very common case of
running e2fsck against a regular file rather than a block device (which is
how virtually every e2fsprogs test image is checked). This caused issues
with test since the warning message went into stderr. v2 drops that blanket message
entirely; lock_whole_disk() now only warns/errors at the point of a genuine
failure
> 
> >> + 		}
> >> + 		com_err(ctx->program_name, errno,
> >> + 			_("while trying to lock %s"), opened_path);
> >> + 		close(fd);
> >> +		return -1;
> >> +	}
> >> +
> >> + 	return fd;
> >> +}
> >> +
> >> int main (int argc, char *argv[])
> >> {
> >> errcode_t retval = 0, retval2 = 0, orig_retval = 0;
> >> @@ -1413,6 +1508,7 @@ int main (int argc, char *argv[])
> >> int journal_size;
> >> int sysval, sys_page_size = 4096;
> >> int old_bitmaps;
> >> + int lock_fd = -1;
> >> __u32 features[3];
> >> char *cp;
> >> enum quota_type qtype;
> >> @@ -1488,6 +1584,12 @@ int main (int argc, char *argv[])
> >> 
> >> check_mount(ctx);
> >> 
> >> + lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> >> + if (lock_fd < 0)
> >> + log_err(ctx, _("Warning: could not lock %s; "
> >> +       "proceeding without block device lock\n"),
> >> + ctx->filesystem_name);
> >> +
> >> if (!(ctx->options & E2F_OPT_PREEN) &&
> >>    !(ctx->options & E2F_OPT_NO) &&
> >>    !(ctx->options & E2F_OPT_YES)) {
> >> @@ -2169,6 +2271,9 @@ skip_write:
> >> ext2fs_close_free(&ctx->fs);
> >> free(ctx->journal_name);
> >> 
> >> + if (lock_fd >= 0)
> >> + close(lock_fd);
> >> +
> >> if (ctx->logf)
> >> fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> >> e2fsck_free_context(ctx);
> >> -- 
> >> 2.54.0
> > 
> 
> 
> Cheers, Andreas
> 
> 
> 
> 

[PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Nandakumar Raghavan]

During journal replay, e2fsck writes the primary superblock back to disk
in multiple I/O operations. The payload lands before the checksum, leaving
a transient window where the on-disk superblock has a bad checksum.

If udevd processes a change uevent during this window, libblkid probes the
primary superblock, finds a checksum mismatch, and concludes the partition
has no recognisable filesystem. udev then fires a remove event, wiping all
symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
that depends on those symlinks will fail.

udevd already serialises its own partition probes against whole-disk device
access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
whole-disk device before opening the filesystem. This forces udevd to defer
all probes on that disk until e2fsck exits and the lock is released, by
which point the filesystem is fully consistent.

The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
so that the lock covers the same device node that udevd locks. If sysfs
resolution fails, a warning is emitted and the lock falls back to the
partition device itself.

This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
on block devices) and is compiled out entirely on non-Linux platforms.

Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
---

Changes in v2:
- Guard the whole feature (function, call site, lock_fd, cleanup) with
  #ifdef __linux__ instead of just the function body, so it's fully
  absent from non-Linux builds 
- Collapse HAVE_SYS_*_H header guards into a single #ifdef __linux__
- Reuse a single dev_path[] buffer instead of three separate stack
  buffers
- Keep translatable warning strings on one line each, even past 80
  columns 
- Drop the blanket "could not lock" warning that fired even when the
  target wasn't a block device at all (e.g. a regular file), which was
  causing issues with tests; lock_whole_disk() now only warns/errors
  at the point of a genuine failure
- No change needed for the Ctrl-C/EINTR concern — traced and confirmed
  the existing signal handling already unwinds cleanly

 e2fsck/unix.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 335ca377..cf21752a 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -36,6 +36,11 @@ extern int optind;
 #ifdef HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
 #endif
+#ifdef __linux__
+#include <sys/file.h>
+#include <sys/stat.h>
+#include <sys/sysmacros.h>
+#endif
 #ifdef HAVE_MALLOC_H
 #include <malloc.h>
 #endif
@@ -1397,6 +1402,90 @@ err:
 	return retval;
 }
 
+#ifdef __linux__
+static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
+{
+	struct stat st;
+	char dev_path[256]; /* reused for each sysfs/device path we build */
+	char parent_devnum[32];
+	FILE *f;
+	unsigned int maj, min;
+	int parent_resolved = 0;
+	const char *opened_path;
+	int fd;
+
+	if (stat(dev_name, &st) < 0)
+		return -1;
+
+	if (!S_ISBLK(st.st_mode))
+		return -1;
+
+	maj = major(st.st_rdev);
+	min = minor(st.st_rdev);
+
+	/*
+	 * If dev_name is a partition (sysfs 'partition' attribute exists),
+	 * resolve the parent whole-disk device so the flock covers the same
+	 * device node that udevd locks before probing any partition on it.
+	 */
+	snprintf(dev_path, sizeof(dev_path),
+		 "/sys/dev/block/%u:%u/partition", maj, min);
+
+	if (access(dev_path, F_OK) == 0) {
+		snprintf(dev_path, sizeof(dev_path),
+			 "/sys/dev/block/%u:%u/../dev", maj, min);
+
+		f = fopen(dev_path, "r");
+		if (f) {
+			if (fscanf(f, "%31s", parent_devnum) == 1) {
+				unsigned int pmaj, pmin;
+				if (sscanf(parent_devnum, "%u:%u",
+					   &pmaj, &pmin) == 2) {
+					maj = pmaj;
+					min = pmin;
+					parent_resolved = 1;
+				}
+			}
+			fclose(f);
+		}
+		if (!parent_resolved)
+			log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
+	}
+
+	snprintf(dev_path, sizeof(dev_path), "/dev/block/%u:%u", maj, min);
+
+	opened_path = dev_path;
+	fd = open(dev_path, O_RDONLY | O_CLOEXEC, 0);
+	if (fd < 0) {
+		fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
+		if (fd < 0) {
+			com_err(ctx->program_name, errno,
+				_("while trying to open %s for locking"), dev_name);
+			return -1;
+		}
+		opened_path = dev_name;
+		if (parent_resolved)
+			log_err(ctx, _("Warning: %s not found; locking %s instead, lock may not prevent udev races\n"), dev_path, dev_name);
+	}
+
+	while (flock(fd, LOCK_EX) != 0) {
+		if (errno == EINTR) {
+			if (ctx->flags & E2F_FLAG_CANCEL) {
+				close(fd);
+				return -1;
+			}
+			continue;
+		}
+		com_err(ctx->program_name, errno,
+			_("while trying to lock %s"), opened_path);
+		close(fd);
+		return -1;
+	}
+
+	return fd;
+}
+#endif /* __linux__ */
+
 int main (int argc, char *argv[])
 {
 	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
@@ -1413,6 +1502,9 @@ int main (int argc, char *argv[])
 	int journal_size;
 	int sysval, sys_page_size = 4096;
 	int old_bitmaps;
+#ifdef __linux__
+	int lock_fd = -1;
+#endif
 	__u32 features[3];
 	char *cp;
 	enum quota_type qtype;
@@ -1488,6 +1580,10 @@ int main (int argc, char *argv[])
 
 	check_mount(ctx);
 
+#ifdef __linux__
+	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
+#endif
+
 	if (!(ctx->options & E2F_OPT_PREEN) &&
 	    !(ctx->options & E2F_OPT_NO) &&
 	    !(ctx->options & E2F_OPT_YES)) {
@@ -2169,6 +2265,11 @@ skip_write:
 	ext2fs_close_free(&ctx->fs);
 	free(ctx->journal_name);
 
+#ifdef __linux__
+	if (lock_fd >= 0)
+		close(lock_fd);
+#endif
+
 	if (ctx->logf)
 		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
 	e2fsck_free_context(ctx);
-- 
2.54.0

Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Andreas Dilger]

On Aug 24, 2026, at 10:15, Nandakumar Raghavan <naraghavan@linux.microsoft.com> wrote:
> 
> During journal replay, e2fsck writes the primary superblock back to disk
> in multiple I/O operations. The payload lands before the checksum, leaving
> a transient window where the on-disk superblock has a bad checksum.
> 
> If udevd processes a change uevent during this window, libblkid probes the
> primary superblock, finds a checksum mismatch, and concludes the partition
> has no recognisable filesystem. udev then fires a remove event, wiping all
> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> that depends on those symlinks will fail.
> 
> udevd already serialises its own partition probes against whole-disk device
> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> whole-disk device before opening the filesystem. This forces udevd to defer
> all probes on that disk until e2fsck exits and the lock is released, by
> which point the filesystem is fully consistent.
> 
> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> so that the lock covers the same device node that udevd locks. If sysfs
> resolution fails, a warning is emitted and the lock falls back to the
> partition device itself.
> 
> This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
> on block devices) and is compiled out entirely on non-Linux platforms.
> 
> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>

Looks much better, thanks.
Reviewed-by: Andreas Dilger <adilger@dilger.ca <mailto:adilger@dilger.ca>>

Cheers, Andreas
> ---
> 
> Changes in v2:
> - Guard the whole feature (function, call site, lock_fd, cleanup) with
>  #ifdef __linux__ instead of just the function body, so it's fully
>  absent from non-Linux builds 
> - Collapse HAVE_SYS_*_H header guards into a single #ifdef __linux__
> - Reuse a single dev_path[] buffer instead of three separate stack
>  buffers
> - Keep translatable warning strings on one line each, even past 80
>  columns 
> - Drop the blanket "could not lock" warning that fired even when the
>  target wasn't a block device at all (e.g. a regular file), which was
>  causing issues with tests; lock_whole_disk() now only warns/errors
>  at the point of a genuine failure
> - No change needed for the Ctrl-C/EINTR concern — traced and confirmed
>  the existing signal handling already unwinds cleanly

Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Darrick J. Wong]

On Mon, Aug 24, 2026 at 09:15:11AM -0700, Nandakumar Raghavan wrote:
> During journal replay, e2fsck writes the primary superblock back to disk
> in multiple I/O operations. The payload lands before the checksum, leaving
> a transient window where the on-disk superblock has a bad checksum.
> 
> If udevd processes a change uevent during this window, libblkid probes the
> primary superblock, finds a checksum mismatch, and concludes the partition
> has no recognisable filesystem. udev then fires a remove event, wiping all
> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> that depends on those symlinks will fail.
> 
> udevd already serialises its own partition probes against whole-disk device
> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> whole-disk device before opening the filesystem. This forces udevd to defer
> all probes on that disk until e2fsck exits and the lock is released, by
> which point the filesystem is fully consistent.
> 
> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> so that the lock covers the same device node that udevd locks. If sysfs
> resolution fails, a warning is emitted and the lock falls back to the
> partition device itself.
> 
> This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
> on block devices) and is compiled out entirely on non-Linux platforms.

You're clearly implementing the "Locking Block Device Access" protocol
as documented by systemd:
https://systemd.io/BLOCK_DEVICE_LOCKING/

So why not use libsystemd like the example in that posting provides?
sd_device_get_parent_with_subsystem_devtype is much less gross than
open-coding the same logic here.

> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> ---
> 
> Changes in v2:
> - Guard the whole feature (function, call site, lock_fd, cleanup) with
>   #ifdef __linux__ instead of just the function body, so it's fully
>   absent from non-Linux builds 
> - Collapse HAVE_SYS_*_H header guards into a single #ifdef __linux__
> - Reuse a single dev_path[] buffer instead of three separate stack
>   buffers
> - Keep translatable warning strings on one line each, even past 80
>   columns 
> - Drop the blanket "could not lock" warning that fired even when the
>   target wasn't a block device at all (e.g. a regular file), which was
>   causing issues with tests; lock_whole_disk() now only warns/errors
>   at the point of a genuine failure
> - No change needed for the Ctrl-C/EINTR concern — traced and confirmed
>   the existing signal handling already unwinds cleanly
> 
>  e2fsck/unix.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
> 
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index 335ca377..cf21752a 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -36,6 +36,11 @@ extern int optind;
>  #ifdef HAVE_SYS_IOCTL_H
>  #include <sys/ioctl.h>
>  #endif
> +#ifdef __linux__
> +#include <sys/file.h>
> +#include <sys/stat.h>
> +#include <sys/sysmacros.h>
> +#endif
>  #ifdef HAVE_MALLOC_H
>  #include <malloc.h>
>  #endif
> @@ -1397,6 +1402,90 @@ err:
>  	return retval;
>  }
>  
> +#ifdef __linux__
> +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> +{
> +	struct stat st;
> +	char dev_path[256]; /* reused for each sysfs/device path we build */
> +	char parent_devnum[32];
> +	FILE *f;
> +	unsigned int maj, min;
> +	int parent_resolved = 0;
> +	const char *opened_path;
> +	int fd;
> +
> +	if (stat(dev_name, &st) < 0)
> +		return -1;
> +
> +	if (!S_ISBLK(st.st_mode))
> +		return -1;
> +
> +	maj = major(st.st_rdev);
> +	min = minor(st.st_rdev);
> +
> +	/*
> +	 * If dev_name is a partition (sysfs 'partition' attribute exists),
> +	 * resolve the parent whole-disk device so the flock covers the same
> +	 * device node that udevd locks before probing any partition on it.
> +	 */
> +	snprintf(dev_path, sizeof(dev_path),
> +		 "/sys/dev/block/%u:%u/partition", maj, min);
> +
> +	if (access(dev_path, F_OK) == 0) {
> +		snprintf(dev_path, sizeof(dev_path),
> +			 "/sys/dev/block/%u:%u/../dev", maj, min);
> +
> +		f = fopen(dev_path, "r");
> +		if (f) {
> +			if (fscanf(f, "%31s", parent_devnum) == 1) {
> +				unsigned int pmaj, pmin;
> +				if (sscanf(parent_devnum, "%u:%u",
> +					   &pmaj, &pmin) == 2) {
> +					maj = pmaj;
> +					min = pmin;
> +					parent_resolved = 1;
> +				}
> +			}
> +			fclose(f);
> +		}
> +		if (!parent_resolved)
> +			log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
> +	}
> +
> +	snprintf(dev_path, sizeof(dev_path), "/dev/block/%u:%u", maj, min);
> +
> +	opened_path = dev_path;
> +	fd = open(dev_path, O_RDONLY | O_CLOEXEC, 0);
> +	if (fd < 0) {
> +		fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
> +		if (fd < 0) {
> +			com_err(ctx->program_name, errno,
> +				_("while trying to open %s for locking"), dev_name);
> +			return -1;
> +		}
> +		opened_path = dev_name;
> +		if (parent_resolved)
> +			log_err(ctx, _("Warning: %s not found; locking %s instead, lock may not prevent udev races\n"), dev_path, dev_name);
> +	}
> +
> +	while (flock(fd, LOCK_EX) != 0) {
> +		if (errno == EINTR) {
> +			if (ctx->flags & E2F_FLAG_CANCEL) {
> +				close(fd);
> +				return -1;
> +			}
> +			continue;
> +		}
> +		com_err(ctx->program_name, errno,
> +			_("while trying to lock %s"), opened_path);
> +		close(fd);
> +		return -1;
> +	}
> +
> +	return fd;
> +}
> +#endif /* __linux__ */
> +
>  int main (int argc, char *argv[])
>  {
>  	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
> @@ -1413,6 +1502,9 @@ int main (int argc, char *argv[])
>  	int journal_size;
>  	int sysval, sys_page_size = 4096;
>  	int old_bitmaps;
> +#ifdef __linux__
> +	int lock_fd = -1;
> +#endif
>  	__u32 features[3];
>  	char *cp;
>  	enum quota_type qtype;
> @@ -1488,6 +1580,10 @@ int main (int argc, char *argv[])
>  
>  	check_mount(ctx);
>  
> +#ifdef __linux__
> +	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);

If the filesystem has an external journal device, doesn't that also need
locking?

Why isn't this implemented as part of the unixio manager?

--D

> +#endif
> +
>  	if (!(ctx->options & E2F_OPT_PREEN) &&
>  	    !(ctx->options & E2F_OPT_NO) &&
>  	    !(ctx->options & E2F_OPT_YES)) {
> @@ -2169,6 +2265,11 @@ skip_write:
>  	ext2fs_close_free(&ctx->fs);
>  	free(ctx->journal_name);
>  
> +#ifdef __linux__
> +	if (lock_fd >= 0)
> +		close(lock_fd);
> +#endif
> +
>  	if (ctx->logf)
>  		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
>  	e2fsck_free_context(ctx);
> -- 
> 2.54.0
> 
> 

Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Nandakumar Raghavan]

On Mon, Aug 24, 2026 at 03:40:06PM -0700, Darrick J. Wong wrote:
> On Mon, Aug 24, 2026 at 09:15:11AM -0700, Nandakumar Raghavan wrote:
> > During journal replay, e2fsck writes the primary superblock back to disk
> > in multiple I/O operations. The payload lands before the checksum, leaving
> > a transient window where the on-disk superblock has a bad checksum.
> > 
> > If udevd processes a change uevent during this window, libblkid probes the
> > primary superblock, finds a checksum mismatch, and concludes the partition
> > has no recognisable filesystem. udev then fires a remove event, wiping all
> > symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> > that depends on those symlinks will fail.
> > 
> > udevd already serialises its own partition probes against whole-disk device
> > access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> > event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> > whole-disk device before opening the filesystem. This forces udevd to defer
> > all probes on that disk until e2fsck exits and the lock is released, by
> > which point the filesystem is fully consistent.
> > 
> > The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> > so that the lock covers the same device node that udevd locks. If sysfs
> > resolution fails, a warning is emitted and the lock falls back to the
> > partition device itself.
> > 
> > This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
> > on block devices) and is compiled out entirely on non-Linux platforms.
> 
> You're clearly implementing the "Locking Block Device Access" protocol
> as documented by systemd:
> https://systemd.io/BLOCK_DEVICE_LOCKING/
> 
> So why not use libsystemd like the example in that posting provides?
> sd_device_get_parent_with_subsystem_devtype is much less gross than
> open-coding the same logic here.
This was on purpose and not something I missed. The sd_device_get_parent_with_subsystem_devtype
requires libsystemd v251+, and e2fsck needs to keep working in contexts where
that dependency isn't available or isn't guaranteed to be new enough - non-systemd distros, older
LTS systems doing root-fs repair before userspace is fully up, embedded/recovery images, etc.
Taking a hard runtime dependency on a versioned libsystemd API for a tool this fundamental felt
like a much bigger cost than by parsing (/sys/dev/block/MAJ:MIN/partition, .../dev), which only
relies on a stable kernel ABI with no library and no version floor. Happy to reconsider if
the project wants to take the dependency, but wanted to flag the tradeoff explicitly rather
than silently pick one side.
> 
> > Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> > ---
> > 
> > Changes in v2:
> > - Guard the whole feature (function, call site, lock_fd, cleanup) with
> >   #ifdef __linux__ instead of just the function body, so it's fully
> >   absent from non-Linux builds 
> > - Collapse HAVE_SYS_*_H header guards into a single #ifdef __linux__
> > - Reuse a single dev_path[] buffer instead of three separate stack
> >   buffers
> > - Keep translatable warning strings on one line each, even past 80
> >   columns 
> > - Drop the blanket "could not lock" warning that fired even when the
> >   target wasn't a block device at all (e.g. a regular file), which was
> >   causing issues with tests; lock_whole_disk() now only warns/errors
> >   at the point of a genuine failure
> > - No change needed for the Ctrl-C/EINTR concern — traced and confirmed
> >   the existing signal handling already unwinds cleanly
> > 
> >  e2fsck/unix.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 101 insertions(+)
> > 
> > diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> > index 335ca377..cf21752a 100644
> > --- a/e2fsck/unix.c
> > +++ b/e2fsck/unix.c
> > @@ -36,6 +36,11 @@ extern int optind;
> >  #ifdef HAVE_SYS_IOCTL_H
> >  #include <sys/ioctl.h>
> >  #endif
> > +#ifdef __linux__
> > +#include <sys/file.h>
> > +#include <sys/stat.h>
> > +#include <sys/sysmacros.h>
> > +#endif
> >  #ifdef HAVE_MALLOC_H
> >  #include <malloc.h>
> >  #endif
> > @@ -1397,6 +1402,90 @@ err:
> >  	return retval;
> >  }
> >  
> > +#ifdef __linux__
> > +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> > +{
> > +	struct stat st;
> > +	char dev_path[256]; /* reused for each sysfs/device path we build */
> > +	char parent_devnum[32];
> > +	FILE *f;
> > +	unsigned int maj, min;
> > +	int parent_resolved = 0;
> > +	const char *opened_path;
> > +	int fd;
> > +
> > +	if (stat(dev_name, &st) < 0)
> > +		return -1;
> > +
> > +	if (!S_ISBLK(st.st_mode))
> > +		return -1;
> > +
> > +	maj = major(st.st_rdev);
> > +	min = minor(st.st_rdev);
> > +
> > +	/*
> > +	 * If dev_name is a partition (sysfs 'partition' attribute exists),
> > +	 * resolve the parent whole-disk device so the flock covers the same
> > +	 * device node that udevd locks before probing any partition on it.
> > +	 */
> > +	snprintf(dev_path, sizeof(dev_path),
> > +		 "/sys/dev/block/%u:%u/partition", maj, min);
> > +
> > +	if (access(dev_path, F_OK) == 0) {
> > +		snprintf(dev_path, sizeof(dev_path),
> > +			 "/sys/dev/block/%u:%u/../dev", maj, min);
> > +
> > +		f = fopen(dev_path, "r");
> > +		if (f) {
> > +			if (fscanf(f, "%31s", parent_devnum) == 1) {
> > +				unsigned int pmaj, pmin;
> > +				if (sscanf(parent_devnum, "%u:%u",
> > +					   &pmaj, &pmin) == 2) {
> > +					maj = pmaj;
> > +					min = pmin;
> > +					parent_resolved = 1;
> > +				}
> > +			}
> > +			fclose(f);
> > +		}
> > +		if (!parent_resolved)
> > +			log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
> > +	}
> > +
> > +	snprintf(dev_path, sizeof(dev_path), "/dev/block/%u:%u", maj, min);
> > +
> > +	opened_path = dev_path;
> > +	fd = open(dev_path, O_RDONLY | O_CLOEXEC, 0);
> > +	if (fd < 0) {
> > +		fd = open(dev_name, O_RDONLY | O_CLOEXEC, 0);
> > +		if (fd < 0) {
> > +			com_err(ctx->program_name, errno,
> > +				_("while trying to open %s for locking"), dev_name);
> > +			return -1;
> > +		}
> > +		opened_path = dev_name;
> > +		if (parent_resolved)
> > +			log_err(ctx, _("Warning: %s not found; locking %s instead, lock may not prevent udev races\n"), dev_path, dev_name);
> > +	}
> > +
> > +	while (flock(fd, LOCK_EX) != 0) {
> > +		if (errno == EINTR) {
> > +			if (ctx->flags & E2F_FLAG_CANCEL) {
> > +				close(fd);
> > +				return -1;
> > +			}
> > +			continue;
> > +		}
> > +		com_err(ctx->program_name, errno,
> > +			_("while trying to lock %s"), opened_path);
> > +		close(fd);
> > +		return -1;
> > +	}
> > +
> > +	return fd;
> > +}
> > +#endif /* __linux__ */
> > +
> >  int main (int argc, char *argv[])
> >  {
> >  	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
> > @@ -1413,6 +1502,9 @@ int main (int argc, char *argv[])
> >  	int journal_size;
> >  	int sysval, sys_page_size = 4096;
> >  	int old_bitmaps;
> > +#ifdef __linux__
> > +	int lock_fd = -1;
> > +#endif
> >  	__u32 features[3];
> >  	char *cp;
> >  	enum quota_type qtype;
> > @@ -1488,6 +1580,10 @@ int main (int argc, char *argv[])
> >  
> >  	check_mount(ctx);
> >  
> > +#ifdef __linux__
> > +	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> 
> If the filesystem has an external journal device, doesn't that also need
> locking?
Good point, and no, it doesn't currently - lock_whole_disk() is only called on
ctx->filesystem_name. An external journal device (-O journal_dev) is a separate
block device with its own by-uuid symlinks, and I don't have a strong argument
that it's immune to the same class of race during replay. I'd like to treat
this as a follow-up rather than block on it, since it needs a bit more digging
into whether the external journal's own superblock write pattern actually
creates the same transient-bad-checksum window - but I don't want to just wave
it away either.
> 
> Why isn't this implemented as part of the unixio manager?
Agreed this is architecturally the more correct home for it - mke2fs, tune2fs,
and resize2fs go through the same unix_io manager and do similar multi-step
writes to live block devices, so they're exposed to the same race and get none
of this protection today. I scoped this patch to e2fsck specifically because
that's the most common/severe case in practice and wanted a narrow, reviewable
first fix rather than a bigger refactor touching every tool at once.
I can look at moving this into unix_io_open()/unix_io_close() as a follow-up
so mke2fs/tune2fs/resize2fs pick it up automatically via ext2fs_open()/close() 

> 
> --D
> 
> > +#endif
> > +
> >  	if (!(ctx->options & E2F_OPT_PREEN) &&
> >  	    !(ctx->options & E2F_OPT_NO) &&
> >  	    !(ctx->options & E2F_OPT_YES)) {
> > @@ -2169,6 +2265,11 @@ skip_write:
> >  	ext2fs_close_free(&ctx->fs);
> >  	free(ctx->journal_name);
> >  
> > +#ifdef __linux__
> > +	if (lock_fd >= 0)
> > +		close(lock_fd);
> > +#endif
> > +
> >  	if (ctx->logf)
> >  		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> >  	e2fsck_free_context(ctx);
> > -- 
> > 2.54.0
> > 
> > 

Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Andreas Dilger]

On Aug 26, 2026, at 08:04, Nandakumar Raghavan <naraghavan@linux.microsoft.com> wrote:
> 
> On Mon, Aug 24, 2026 at 03:40:06PM -0700, Darrick J. Wong wrote:
>> On Mon, Aug 24, 2026 at 09:15:11AM -0700, Nandakumar Raghavan wrote:
>>> During journal replay, e2fsck writes the primary superblock back to disk
>>> in multiple I/O operations. The payload lands before the checksum, leaving
>>> a transient window where the on-disk superblock has a bad checksum.
>>> 
>>> If udevd processes a change uevent during this window, libblkid probes the
>>> primary superblock, finds a checksum mismatch, and concludes the partition
>>> has no recognisable filesystem. udev then fires a remove event, wiping all
>>> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
>>> that depends on those symlinks will fail.
>>> 
>>> udevd already serialises its own partition probes against whole-disk device
>>> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
>>> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
>>> whole-disk device before opening the filesystem. This forces udevd to defer
>>> all probes on that disk until e2fsck exits and the lock is released, by
>>> which point the filesystem is fully consistent.
>>> 
>>> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
>>> so that the lock covers the same device node that udevd locks. If sysfs
>>> resolution fails, a warning is emitted and the lock falls back to the
>>> partition device itself.
>>> 
>>> This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
>>> on block devices) and is compiled out entirely on non-Linux platforms.
>> 
>> You're clearly implementing the "Locking Block Device Access" protocol
>> as documented by systemd:
>> https://systemd.io/BLOCK_DEVICE_LOCKING/
>> 
>> So why not use libsystemd like the example in that posting provides?
>> sd_device_get_parent_with_subsystem_devtype is much less gross than
>> open-coding the same logic here.
> This was on purpose and not something I missed. The sd_device_get_parent_with_subsystem_devtype
> requires libsystemd v251+, and e2fsck needs to keep working in contexts where
> that dependency isn't available or isn't guaranteed to be new enough -
> non-systemd distros, older LTS systems doing root-fs repair before userspace
> is fully up, embedded/recovery images, etc.
> Taking a hard runtime dependency on a versioned libsystemd API for a tool
> this fundamental felt like a much bigger cost than by parsing (/sys/dev/block/MAJ:MIN/partition, .../dev), which only relies on a stable
> kernel ABI with no library and no version floor. Happy to reconsider if
> the project wants to take the dependency, but wanted to flag the tradeoff
> explicitly rather than silently pick one side.

I for one am not eager to have e2fsprogs depend on libsystemd to build.

>> 
>>> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
>>> ---
>>> 
>>> @@ -1488,6 +1580,10 @@ int main (int argc, char *argv[])
>>> 
>>> check_mount(ctx);
>>> 
>>> +#ifdef __linux__
>>> + lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
>> 
>> If the filesystem has an external journal device, doesn't that also need
>> locking?
> 
> Good point, and no, it doesn't currently - lock_whole_disk() is only called on
> ctx->filesystem_name. An external journal device (-O journal_dev) is a separate
> block device with its own by-uuid symlinks, and I don't have a strong argument
> that it's immune to the same class of race during replay. I'd like to treat
> this as a follow-up rather than block on it, since it needs a bit more digging
> into whether the external journal's own superblock write pattern actually
> creates the same transient-bad-checksum window - but I don't want to just wave
> it away either.

At one point I was working on having jbd2 "mount" the journal device, and then
ext4 would connect to the running journal as a "service", rather than having
the journal device be mounted exclusively as subpart of the ext4 filesystem.
I might have even posted patches for this to the list (maybe 15 years ago?)

This was intended to solve a number of issues:
- ensuring that the jbd2 block device was visibly "in use" to userspace
- allow a single jbd2 device (e.g. on NVMe) to be shared among multiple ext4
  filesystems (on HDDs)

That shared device would have been much more useful when HDDs were prevalent
and an NVMe device was harder to share (e.g. before LVM/DM).  It had some other
issues like what to do if there are journaled blocks to recover but one of the
using filesystems does not mount and recover them? Maybe copy those blocks to
the end of the journal and shrink the usable journal size?

It still has some appeal from the visibility POV, and for sharing it avoids
stranding dedicated NVMe journal space on one inactive filesystem when other
busy filesystems could be sharing the space.

Cheers, Andreas

Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Nandakumar Raghavan]

On Thu, Aug 27, 2026 at 01:33:45AM -0600, Andreas Dilger wrote:
> On Aug 26, 2026, at 08:04, Nandakumar Raghavan <naraghavan@linux.microsoft.com> wrote:
> > 
> > On Mon, Aug 24, 2026 at 03:40:06PM -0700, Darrick J. Wong wrote:
> >> On Mon, Aug 24, 2026 at 09:15:11AM -0700, Nandakumar Raghavan wrote:
> >>> During journal replay, e2fsck writes the primary superblock back to disk
> >>> in multiple I/O operations. The payload lands before the checksum, leaving
> >>> a transient window where the on-disk superblock has a bad checksum.
> >>> 
> >>> If udevd processes a change uevent during this window, libblkid probes the
> >>> primary superblock, finds a checksum mismatch, and concludes the partition
> >>> has no recognisable filesystem. udev then fires a remove event, wiping all
> >>> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> >>> that depends on those symlinks will fail.
> >>> 
> >>> udevd already serialises its own partition probes against whole-disk device
> >>> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> >>> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> >>> whole-disk device before opening the filesystem. This forces udevd to defer
> >>> all probes on that disk until e2fsck exits and the lock is released, by
> >>> which point the filesystem is fully consistent.
> >>> 
> >>> The whole-disk device is resolved via sysfs (/sys/dev/block/MAJ:MIN/partition)
> >>> so that the lock covers the same device node that udevd locks. If sysfs
> >>> resolution fails, a warning is emitted and the lock falls back to the
> >>> partition device itself.
> >>> 
> >>> This mechanism relies on Linux-specific interfaces (sysfs, flock() semantics
> >>> on block devices) and is compiled out entirely on non-Linux platforms.
> >> 
> >> You're clearly implementing the "Locking Block Device Access" protocol
> >> as documented by systemd:
> >> https://systemd.io/BLOCK_DEVICE_LOCKING/
> >> 
> >> So why not use libsystemd like the example in that posting provides?
> >> sd_device_get_parent_with_subsystem_devtype is much less gross than
> >> open-coding the same logic here.
> > This was on purpose and not something I missed. The sd_device_get_parent_with_subsystem_devtype
> > requires libsystemd v251+, and e2fsck needs to keep working in contexts where
> > that dependency isn't available or isn't guaranteed to be new enough -
> > non-systemd distros, older LTS systems doing root-fs repair before userspace
> > is fully up, embedded/recovery images, etc.
> > Taking a hard runtime dependency on a versioned libsystemd API for a tool
> > this fundamental felt like a much bigger cost than by parsing (/sys/dev/block/MAJ:MIN/partition, .../dev), which only relies on a stable
> > kernel ABI with no library and no version floor. Happy to reconsider if
> > the project wants to take the dependency, but wanted to flag the tradeoff
> > explicitly rather than silently pick one side.
> 
> I for one am not eager to have e2fsprogs depend on libsystemd to build.
> 
> >> 
> >>> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> >>> ---
> >>> 
> >>> @@ -1488,6 +1580,10 @@ int main (int argc, char *argv[])
> >>> 
> >>> check_mount(ctx);
> >>> 
> >>> +#ifdef __linux__
> >>> + lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> >> 
> >> If the filesystem has an external journal device, doesn't that also need
> >> locking?
> > 
> > Good point, and no, it doesn't currently - lock_whole_disk() is only called on
> > ctx->filesystem_name. An external journal device (-O journal_dev) is a separate
> > block device with its own by-uuid symlinks, and I don't have a strong argument
> > that it's immune to the same class of race during replay. I'd like to treat
> > this as a follow-up rather than block on it, since it needs a bit more digging
> > into whether the external journal's own superblock write pattern actually
> > creates the same transient-bad-checksum window - but I don't want to just wave
> > it away either.
> 
> At one point I was working on having jbd2 "mount" the journal device, and then
> ext4 would connect to the running journal as a "service", rather than having
> the journal device be mounted exclusively as subpart of the ext4 filesystem.
> I might have even posted patches for this to the list (maybe 15 years ago?)
> 
> This was intended to solve a number of issues:
> - ensuring that the jbd2 block device was visibly "in use" to userspace
> - allow a single jbd2 device (e.g. on NVMe) to be shared among multiple ext4
>   filesystems (on HDDs)
> 
> That shared device would have been much more useful when HDDs were prevalent
> and an NVMe device was harder to share (e.g. before LVM/DM).  It had some other
> issues like what to do if there are journaled blocks to recover but one of the
> using filesystems does not mount and recover them? Maybe copy those blocks to
> the end of the journal and shrink the usable journal size?
> 
> It still has some appeal from the visibility POV, and for sharing it avoids
> stranding dedicated NVMe journal space on one inactive filesystem when other
> busy filesystems could be sharing the space.
> 
> Cheers, Andreas
Thanks for the history, Andreas and I can see how it would solve the visibility
problem more fundamentally than one-off locking external journal devices case by case.

That said, it sounds like a much bigger architectural change than what I'm
trying to do here. I'll keep it in mind as background, but for this patch
I'd still like to treat "should the external journal device also get
locked" as a narrower, separate follow-up rather than pull in a jbd2
redesign.
> 
> 
> 
> 

Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Theodore Tso]

On Wed, Aug 26, 2026 at 07:04:48AM -0500, Nandakumar Raghavan wrote:
> This was on purpose and not something I missed. The sd_device_get_parent_with_subsystem_devtype
> requires libsystemd v251+, and e2fsck needs to keep working in contexts where
> that dependency isn't available or isn't guaranteed to be new enough - non-systemd distros, older
> LTS systems doing root-fs repair before userspace is fully up, embedded/recovery images, etc.

Cqn you name any other OS that has implemented

	https://systemd.io/BLOCK_DEVICE_LOCKING/

*other* than systemd?  So for non-systemd distros, who cares if we
don't have the functionality?  So what we could do is to use autoconf
with something like this:

AC_CHECK_LIB(libsystemd, sd_device_get_parent_with_subsystem_devtype,
	AC_DEFINE(HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE, 1,
		[Define to 1 if libsystemd has sd_device_get_parent_with_subsystem_devtype]))

We would only do the flock(LOCK_EX) if and only if the function is
present.  If it isn't, say on MacOS, FreeBSD, or RHEL 7 we won't play
the systemd locking game --- which is fine, because it doesn't matter
on those platforms.

If you care about running e2fsck built on RHEL 8 on RHEL 7, that's an
easy problem to solve.  A executable linked using shared libraries
won't work already since the shared libraries on RHEL 8 are too new.
But we have a solution already, which is to use e2fsck.static, which
is statically linked.

> Taking a hard runtime dependency on a versioned libsystemd API for a tool this fundamental felt
> like a much bigger cost than by parsing (/sys/dev/block/MAJ:MIN/partition, .../dev), which only
> relies on a stable kernel ABI with no library and no version floor. Happy to reconsider if
> the project wants to take the dependency, but wanted to flag the tradeoff explicitly rather
> than silently pick one side.

If we really cared about taking a binary built on a system with
libsystemd. and running on a system without libsystemd, another
solution that we've used is dlopen.

For an example of that's done, see misc/create_inode_libarchve.c and
the associated autoconf tests in configure.ac in the e2fsprogs
sources.  This is how "mke2fs -d embed_rootfs.tar.gz -t ext4 embed.img
1G" is handled without needing a hard dependency on libarchive being
installed.  If it's not installed then you'll just get the error
message "you need libarchive to be able to process tarballs".

I don't think the dlopen() approach is needed here, though.

Cheers,

					- Ted

Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Nandakumar Raghavan]

On Thu, Aug 27, 2026 at 05:01:09PM -0400, Theodore Tso wrote:
> On Wed, Aug 26, 2026 at 07:04:48AM -0500, Nandakumar Raghavan wrote:
> > This was on purpose and not something I missed. The sd_device_get_parent_with_subsystem_devtype
> > requires libsystemd v251+, and e2fsck needs to keep working in contexts where
> > that dependency isn't available or isn't guaranteed to be new enough - non-systemd distros, older
> > LTS systems doing root-fs repair before userspace is fully up, embedded/recovery images, etc.
> 
> Cqn you name any other OS that has implemented
> 
> 	https://systemd.io/BLOCK_DEVICE_LOCKING/
> 
> *other* than systemd?  So for non-systemd distros, who cares if we
> don't have the functionality?  So what we could do is to use autoconf
> with something like this:
> 
> AC_CHECK_LIB(libsystemd, sd_device_get_parent_with_subsystem_devtype,
> 	AC_DEFINE(HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE, 1,
> 		[Define to 1 if libsystemd has sd_device_get_parent_with_subsystem_devtype]))
> 
> We would only do the flock(LOCK_EX) if and only if the function is
> present.  If it isn't, say on MacOS, FreeBSD, or RHEL 7 we won't play
> the systemd locking game --- which is fine, because it doesn't matter
> on those platforms.
> 
> If you care about running e2fsck built on RHEL 8 on RHEL 7, that's an
> easy problem to solve.  A executable linked using shared libraries
> won't work already since the shared libraries on RHEL 8 are too new.
> But we have a solution already, which is to use e2fsck.static, which
> is statically linked.
> 
> > Taking a hard runtime dependency on a versioned libsystemd API for a tool this fundamental felt
> > like a much bigger cost than by parsing (/sys/dev/block/MAJ:MIN/partition, .../dev), which only
> > relies on a stable kernel ABI with no library and no version floor. Happy to reconsider if
> > the project wants to take the dependency, but wanted to flag the tradeoff explicitly rather
> > than silently pick one side.
> 
> If we really cared about taking a binary built on a system with
> libsystemd. and running on a system without libsystemd, another
> solution that we've used is dlopen.
> 
> For an example of that's done, see misc/create_inode_libarchve.c and
> the associated autoconf tests in configure.ac in the e2fsprogs
> sources.  This is how "mke2fs -d embed_rootfs.tar.gz -t ext4 embed.img
> 1G" is handled without needing a hard dependency on libarchive being
> installed.  If it's not installed then you'll just get the error
> message "you need libarchive to be able to process tarballs".
> 
> I don't think the dlopen() approach is needed here, though.
> 
> Cheers,
> 
> 					- Ted
Thank you, Ted for the comments. I agree on all points. I will rework this to use AC_CHECK_LIB as
you described, switch to the sd-device API when libsystemd is present, and
drop the whole locking feature cleanly when it isn't. 
Patch v3 to follow.

[PATCH v3] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Nandakumar Raghavan]

During journal replay, e2fsck writes the primary superblock back to disk
in multiple I/O operations. The payload lands before the checksum, leaving
a transient window where the on-disk superblock has a bad checksum.

If udevd processes a change uevent during this window, libblkid probes the
primary superblock, finds a checksum mismatch, and concludes the partition
has no recognisable filesystem. udev then fires a remove event, wiping all
symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
that depends on those symlinks will fail.

udevd already serialises its own partition probes against whole-disk device
access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
whole-disk device before opening the filesystem. This forces udevd to defer
all probes on that disk until e2fsck exits and the lock is released, by
which point the filesystem is fully consistent.

The whole-disk device is resolved using libsystemd's sd-device API
(sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype())
rather than parsing sysfs, so the lock covers the same device node
that udevd itself resolves and locks.

This locking is entirely optional at build time where configure checks for
sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB, and
e2fsck.c only compiles in the locking code when it is found. On platforms
without libsystemd there is no udevd to race against in the first place,
so nothing is lost by skipping it there.

Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
---

Changes in v3:
    - Resolve the parent whole-disk device using libsystemd's sd-device API
      (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype(),
      sd_device_get_devname()) instead of parsing sysfs paths
    - Gate the feature on autoconf detecting
      sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB; compile it
      out entirely when libsystemd is absent, rather than falling back to another
      implementation
    - Use sd_device_new_from_devnum() rather than the sd_device_new_from_devname()
      used by the systemd.io example, since the former has been available in
      libsystemd for much longer, keeping the version floor as low as possible
    - Keep lock_whole_disk()'s signature unconditional and provide a stub returning
      -1 on the #else branch, so main()'s primary code path has no #ifdefs in it
    
Changes in v2:
    - Guard the whole feature (function, call site, lock_fd, cleanup) rather than
      just the function body, so it is fully absent from non-Linux builds 
    - Collapse the three HAVE_SYS_*_H header guards into a single conditional
    - Reuse a single dev_path[] buffer instead of three single-use stack buffers
    - Keep translatable warning strings on one line each, even past 80 columns
    - Drop the blanket "could not lock" warning that fired even when the target was
      not a block device at all (e.g. a regular file), which broke most of
      "make check"; lock_whole_disk() now only warns at the point of a genuine failure
    - No change needed for the Ctrl-C/EINTR concern: the SIGINT handler is installed
      before lock_whole_disk() runs and is not SA_RESTART, so a blocked flock() is
      interrupted and the existing E2F_FLAG_CANCEL check unwinds cleanly

 MCONFIG.in         |  1 +
 configure          | 71 +++++++++++++++++++++++++++++++++++
 configure.ac       | 14 +++++++
 e2fsck/Makefile.in |  6 +--
 e2fsck/unix.c      | 93 ++++++++++++++++++++++++++++++++++++++++++++++
 lib/config.h.in    |  6 +++
 6 files changed, 188 insertions(+), 3 deletions(-)

diff --git a/MCONFIG.in b/MCONFIG.in
index d66e2f3b..d3055955 100644
--- a/MCONFIG.in
+++ b/MCONFIG.in
@@ -137,6 +137,7 @@ LIBE2P = $(LIB)/libe2p@LIB_EXT@
 LIBEXT2FS = $(LIB)/libext2fs@LIB_EXT@
 LIBUUID = @LIBUUID@ @SOCKET_LIB@
 LIBMAGIC = @MAGIC_LIB@
+LIBSYSTEMD = @SYSTEMD_LIB@
 LIBFUSE = @FUSE_LIB@
 LIBSUPPORT = $(LIBINTL) $(LIB)/libsupport@STATIC_LIB_EXT@
 LIBBLKID = @LIBBLKID@ @PRIVATE_LIBS_CMT@ $(LIBUUID)
diff --git a/configure b/configure
index b04b31af..696dd07c 100755
--- a/configure
+++ b/configure
@@ -702,6 +702,7 @@ fuse3_CFLAGS
 CLOCK_GETTIME_LIB
 ARCHIVE_LIBS
 ARCHIVE_CFLAGS
+SYSTEMD_LIB
 MAGIC_LIB
 SOCKET_LIB
 SIZEOF_TIME_T
@@ -14130,6 +14131,76 @@ if test "$ac_cv_func_dlopen" = yes ; then
    MAGIC_LIB=$DLOPEN_LIB
 fi
 
+SYSTEMD_LIB=
+{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd" >&5
+printf %s "checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd... " >&6; }
+if test ${ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype+y}
+then :
+  printf %s "(cached) " >&6
+else case e in #(
+  e) ac_check_lib_save_LIBS=$LIBS
+LIBS="-lsystemd  $LIBS"
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.
+   The 'extern "C"' is for builds by C++ compilers;
+   although this is not generally supported in C code supporting it here
+   has little cost and some practical benefit (sr 110532).  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char sd_device_get_parent_with_subsystem_devtype (void);
+int
+main (void)
+{
+return sd_device_get_parent_with_subsystem_devtype ();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"
+then :
+  ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=yes
+else case e in #(
+  e) ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=no ;;
+esac
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.beam \
+    conftest$ac_exeext conftest.$ac_ext
+LIBS=$ac_check_lib_save_LIBS ;;
+esac
+fi
+{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&5
+printf "%s\n" "$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&6; }
+if test "x$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" = xyes
+then :
+  SYSTEMD_LIB=-lsystemd
+fi
+
+if test -n "$SYSTEMD_LIB"; then
+	       for ac_header in systemd/sd-device.h
+do :
+  ac_fn_c_check_header_compile "$LINENO" "systemd/sd-device.h" "ac_cv_header_systemd_sd_device_h" "$ac_includes_default"
+if test "x$ac_cv_header_systemd_sd_device_h" = xyes
+then :
+  printf "%s\n" "#define HAVE_SYSTEMD_SD_DEVICE_H 1" >>confdefs.h
+
+else case e in #(
+  e) SYSTEMD_LIB= ;;
+esac
+fi
+
+done
+fi
+if test -n "$SYSTEMD_LIB"; then
+
+printf "%s\n" "#define HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE 1" >>confdefs.h
+
+fi
+
 
 # Check whether --with-libarchive was given.
 if test ${with_libarchive+y}
diff --git a/configure.ac b/configure.ac
index 4921f81f..9f585aae 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1309,6 +1309,20 @@ if test "$ac_cv_func_dlopen" = yes ; then
 fi
 AC_SUBST(MAGIC_LIB)
 dnl
+dnl See if libsystemd has sd-device APIs for whole-disk BSD locking
+dnl
+SYSTEMD_LIB=
+AC_CHECK_LIB(systemd, sd_device_get_parent_with_subsystem_devtype,
+	[SYSTEMD_LIB=-lsystemd])
+if test -n "$SYSTEMD_LIB"; then
+	AC_CHECK_HEADERS([systemd/sd-device.h], , [SYSTEMD_LIB=])
+fi
+if test -n "$SYSTEMD_LIB"; then
+	AC_DEFINE(HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE, 1,
+		[Define to 1 if libsystemd has sd-device APIs for whole-disk locking])
+fi
+AC_SUBST(SYSTEMD_LIB)
+dnl
 dnl libarchive
 dnl
 AC_ARG_WITH([libarchive],
diff --git a/e2fsck/Makefile.in b/e2fsck/Makefile.in
index fbb7b156..a050eafe 100644
--- a/e2fsck/Makefile.in
+++ b/e2fsck/Makefile.in
@@ -17,20 +17,20 @@ MANPAGES=	e2fsck.8
 FMANPAGES=	e2fsck.conf.5
 
 LIBS= $(LIBSUPPORT) $(LIBEXT2FS) $(LIBCOM_ERR) $(LIBBLKID) $(LIBUUID) \
-	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(SYSLIBS)
+	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
 DEPLIBS= $(DEPLIBSUPPORT) $(LIBEXT2FS) $(DEPLIBCOM_ERR) $(DEPLIBBLKID) \
 	 $(DEPLIBUUID) $(DEPLIBE2P)
 
 STATIC_LIBS= $(STATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) $(STATIC_LIBCOM_ERR) \
 	     $(STATIC_LIBBLKID) $(STATIC_LIBUUID) $(LIBINTL) $(STATIC_LIBE2P) \
-	     $(LIBMAGIC) $(SYSLIBS)
+	     $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
 STATIC_DEPLIBS= $(DEPSTATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) \
 		$(DEPSTATIC_LIBCOM_ERR) $(DEPSTATIC_LIBBLKID) \
 		$(DEPSTATIC_LIBUUID) $(DEPSTATIC_LIBE2P)
 
 PROFILED_LIBS= $(PROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
 	       $(PROFILED_LIBCOM_ERR) $(PROFILED_LIBBLKID) $(PROFILED_LIBUUID) \
-	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(SYSLIBS)
+	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
 PROFILED_DEPLIBS= $(DEPPROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
 		  $(DEPPROFILED_LIBCOM_ERR) $(DEPPROFILED_LIBBLKID) \
 		  $(DEPPROFILED_LIBUUID) $(DEPPROFILED_LIBE2P)
diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 335ca377..51bc41eb 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -36,6 +36,11 @@ extern int optind;
 #ifdef HAVE_SYS_IOCTL_H
 #include <sys/ioctl.h>
 #endif
+#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
+#include <sys/file.h>
+#include <sys/stat.h>
+#include <systemd/sd-device.h>
+#endif
 #ifdef HAVE_MALLOC_H
 #include <malloc.h>
 #endif
@@ -1397,6 +1402,88 @@ err:
 	return retval;
 }
 
+static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
+{
+#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
+	struct stat st;
+	sd_device *dev = NULL, *whole_dev;
+	const char *subsystem, *devtype, *whole_disk_devname;
+	int r, fd;
+
+	if (stat(dev_name, &st) < 0)
+		return -1;
+
+	if (!S_ISBLK(st.st_mode))
+		return -1;
+
+	r = sd_device_new_from_devnum(&dev, 'b', st.st_rdev);
+	if (r < 0)
+		return -1;
+
+	r = sd_device_get_subsystem(dev, &subsystem);
+	if (r < 0 || strcmp(subsystem, "block") != 0) {
+		sd_device_unref(dev);
+		return -1;
+	}
+
+	r = sd_device_get_devtype(dev, &devtype);
+	if (r < 0) {
+		sd_device_unref(dev);
+		return -1;
+	}
+
+	/*
+	 * If dev_name is a partition, resolve the parent whole-disk device
+	 * so the flock covers the same device node that udevd locks before
+	 * probing any partition on it.
+	 */
+	if (strcmp(devtype, "disk") == 0)
+		whole_dev = dev;
+	else {
+		r = sd_device_get_parent_with_subsystem_devtype(dev, "block",
+								 "disk", &whole_dev);
+		if (r < 0) {
+			log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
+			sd_device_unref(dev);
+			return -1;
+		}
+	}
+
+	r = sd_device_get_devname(whole_dev, &whole_disk_devname);
+	if (r < 0) {
+		sd_device_unref(dev);
+		return -1;
+	}
+
+	fd = open(whole_disk_devname, O_RDONLY | O_CLOEXEC, 0);
+	if (fd < 0) {
+		com_err(ctx->program_name, errno,
+			_("while trying to open %s for locking"), whole_disk_devname);
+		sd_device_unref(dev);
+		return -1;
+	}
+	sd_device_unref(dev);
+
+	while (flock(fd, LOCK_EX) != 0) {
+		if (errno == EINTR) {
+			if (ctx->flags & E2F_FLAG_CANCEL) {
+				close(fd);
+				return -1;
+			}
+			continue;
+		}
+		com_err(ctx->program_name, errno,
+			_("while trying to lock %s"), whole_disk_devname);
+		close(fd);
+		return -1;
+	}
+
+	return fd;
+#else
+	return -1;
+#endif /* HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE */
+}
+
 int main (int argc, char *argv[])
 {
 	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
@@ -1413,6 +1500,7 @@ int main (int argc, char *argv[])
 	int journal_size;
 	int sysval, sys_page_size = 4096;
 	int old_bitmaps;
+	int lock_fd = -1;
 	__u32 features[3];
 	char *cp;
 	enum quota_type qtype;
@@ -1488,6 +1576,8 @@ int main (int argc, char *argv[])
 
 	check_mount(ctx);
 
+	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
+
 	if (!(ctx->options & E2F_OPT_PREEN) &&
 	    !(ctx->options & E2F_OPT_NO) &&
 	    !(ctx->options & E2F_OPT_YES)) {
@@ -2169,6 +2259,9 @@ skip_write:
 	ext2fs_close_free(&ctx->fs);
 	free(ctx->journal_name);
 
+	if (lock_fd >= 0)
+		close(lock_fd);
+
 	if (ctx->logf)
 		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
 	e2fsck_free_context(ctx);
diff --git a/lib/config.h.in b/lib/config.h.in
index f129abfe..bc3ae4aa 100644
--- a/lib/config.h.in
+++ b/lib/config.h.in
@@ -352,6 +352,9 @@
 /* Define to 1 if if struct sockaddr contains sa_len */
 #undef HAVE_SA_LEN
 
+/* Define to 1 if libsystemd has sd-device APIs for whole-disk locking */
+#undef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
+
 /* Define to 1 if you have the 'secure_getenv' function. */
 #undef HAVE_SECURE_GETENV
 
@@ -430,6 +433,9 @@
 /* Define to 1 if you have the 'sysconf' function. */
 #undef HAVE_SYSCONF
 
+/* Define to 1 if you have the <systemd/sd-device.h> header file. */
+#undef HAVE_SYSTEMD_SD_DEVICE_H
+
 /* Define to 1 if you have the <sys/acl.h> header file. */
 #undef HAVE_SYS_ACL_H
 
-- 
2.55.0

Re: [PATCH v3] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Nandakumar Raghavan]

On Thu, Sep 10, 2026 at 05:04:41AM -0700, Nandakumar Raghavan wrote:
> During journal replay, e2fsck writes the primary superblock back to disk
> in multiple I/O operations. The payload lands before the checksum, leaving
> a transient window where the on-disk superblock has a bad checksum.
> 
> If udevd processes a change uevent during this window, libblkid probes the
> primary superblock, finds a checksum mismatch, and concludes the partition
> has no recognisable filesystem. udev then fires a remove event, wiping all
> symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> that depends on those symlinks will fail.
> 
> udevd already serialises its own partition probes against whole-disk device
> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> whole-disk device before opening the filesystem. This forces udevd to defer
> all probes on that disk until e2fsck exits and the lock is released, by
> which point the filesystem is fully consistent.
> 
> The whole-disk device is resolved using libsystemd's sd-device API
> (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype())
> rather than parsing sysfs, so the lock covers the same device node
> that udevd itself resolves and locks.
> 
> This locking is entirely optional at build time where configure checks for
> sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB, and
> e2fsck.c only compiles in the locking code when it is found. On platforms
> without libsystemd there is no udevd to race against in the first place,
> so nothing is lost by skipping it there.
> 
> Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> ---
> 
> Changes in v3:
>     - Resolve the parent whole-disk device using libsystemd's sd-device API
>       (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype(),
>       sd_device_get_devname()) instead of parsing sysfs paths
>     - Gate the feature on autoconf detecting
>       sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB; compile it
>       out entirely when libsystemd is absent, rather than falling back to another
>       implementation
>     - Use sd_device_new_from_devnum() rather than the sd_device_new_from_devname()
>       used by the systemd.io example, since the former has been available in
>       libsystemd for much longer, keeping the version floor as low as possible
>     - Keep lock_whole_disk()'s signature unconditional and provide a stub returning
>       -1 on the #else branch, so main()'s primary code path has no #ifdefs in it
>     
> Changes in v2:
>     - Guard the whole feature (function, call site, lock_fd, cleanup) rather than
>       just the function body, so it is fully absent from non-Linux builds 
>     - Collapse the three HAVE_SYS_*_H header guards into a single conditional
>     - Reuse a single dev_path[] buffer instead of three single-use stack buffers
>     - Keep translatable warning strings on one line each, even past 80 columns
>     - Drop the blanket "could not lock" warning that fired even when the target was
>       not a block device at all (e.g. a regular file), which broke most of
>       "make check"; lock_whole_disk() now only warns at the point of a genuine failure
>     - No change needed for the Ctrl-C/EINTR concern: the SIGINT handler is installed
>       before lock_whole_disk() runs and is not SA_RESTART, so a blocked flock() is
>       interrupted and the existing E2F_FLAG_CANCEL check unwinds cleanly
> 
>  MCONFIG.in         |  1 +
>  configure          | 71 +++++++++++++++++++++++++++++++++++
>  configure.ac       | 14 +++++++
>  e2fsck/Makefile.in |  6 +--
>  e2fsck/unix.c      | 93 ++++++++++++++++++++++++++++++++++++++++++++++
>  lib/config.h.in    |  6 +++
>  6 files changed, 188 insertions(+), 3 deletions(-)
> 
> diff --git a/MCONFIG.in b/MCONFIG.in
> index d66e2f3b..d3055955 100644
> --- a/MCONFIG.in
> +++ b/MCONFIG.in
> @@ -137,6 +137,7 @@ LIBE2P = $(LIB)/libe2p@LIB_EXT@
>  LIBEXT2FS = $(LIB)/libext2fs@LIB_EXT@
>  LIBUUID = @LIBUUID@ @SOCKET_LIB@
>  LIBMAGIC = @MAGIC_LIB@
> +LIBSYSTEMD = @SYSTEMD_LIB@
>  LIBFUSE = @FUSE_LIB@
>  LIBSUPPORT = $(LIBINTL) $(LIB)/libsupport@STATIC_LIB_EXT@
>  LIBBLKID = @LIBBLKID@ @PRIVATE_LIBS_CMT@ $(LIBUUID)
> diff --git a/configure b/configure
> index b04b31af..696dd07c 100755
> --- a/configure
> +++ b/configure
> @@ -702,6 +702,7 @@ fuse3_CFLAGS
>  CLOCK_GETTIME_LIB
>  ARCHIVE_LIBS
>  ARCHIVE_CFLAGS
> +SYSTEMD_LIB
>  MAGIC_LIB
>  SOCKET_LIB
>  SIZEOF_TIME_T
> @@ -14130,6 +14131,76 @@ if test "$ac_cv_func_dlopen" = yes ; then
>     MAGIC_LIB=$DLOPEN_LIB
>  fi
>  
> +SYSTEMD_LIB=
> +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd" >&5
> +printf %s "checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd... " >&6; }
> +if test ${ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype+y}
> +then :
> +  printf %s "(cached) " >&6
> +else case e in #(
> +  e) ac_check_lib_save_LIBS=$LIBS
> +LIBS="-lsystemd  $LIBS"
> +cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h.  */
> +
> +/* Override any GCC internal prototype to avoid an error.
> +   Use char because int might match the return type of a GCC
> +   builtin and then its argument prototype would still apply.
> +   The 'extern "C"' is for builds by C++ compilers;
> +   although this is not generally supported in C code supporting it here
> +   has little cost and some practical benefit (sr 110532).  */
> +#ifdef __cplusplus
> +extern "C"
> +#endif
> +char sd_device_get_parent_with_subsystem_devtype (void);
> +int
> +main (void)
> +{
> +return sd_device_get_parent_with_subsystem_devtype ();
> +  ;
> +  return 0;
> +}
> +_ACEOF
> +if ac_fn_c_try_link "$LINENO"
> +then :
> +  ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=yes
> +else case e in #(
> +  e) ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=no ;;
> +esac
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.beam \
> +    conftest$ac_exeext conftest.$ac_ext
> +LIBS=$ac_check_lib_save_LIBS ;;
> +esac
> +fi
> +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&5
> +printf "%s\n" "$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&6; }
> +if test "x$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" = xyes
> +then :
> +  SYSTEMD_LIB=-lsystemd
> +fi
> +
> +if test -n "$SYSTEMD_LIB"; then
> +	       for ac_header in systemd/sd-device.h
> +do :
> +  ac_fn_c_check_header_compile "$LINENO" "systemd/sd-device.h" "ac_cv_header_systemd_sd_device_h" "$ac_includes_default"
> +if test "x$ac_cv_header_systemd_sd_device_h" = xyes
> +then :
> +  printf "%s\n" "#define HAVE_SYSTEMD_SD_DEVICE_H 1" >>confdefs.h
> +
> +else case e in #(
> +  e) SYSTEMD_LIB= ;;
> +esac
> +fi
> +
> +done
> +fi
> +if test -n "$SYSTEMD_LIB"; then
> +
> +printf "%s\n" "#define HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE 1" >>confdefs.h
> +
> +fi
> +
>  
>  # Check whether --with-libarchive was given.
>  if test ${with_libarchive+y}
> diff --git a/configure.ac b/configure.ac
> index 4921f81f..9f585aae 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1309,6 +1309,20 @@ if test "$ac_cv_func_dlopen" = yes ; then
>  fi
>  AC_SUBST(MAGIC_LIB)
>  dnl
> +dnl See if libsystemd has sd-device APIs for whole-disk BSD locking
> +dnl
> +SYSTEMD_LIB=
> +AC_CHECK_LIB(systemd, sd_device_get_parent_with_subsystem_devtype,
> +	[SYSTEMD_LIB=-lsystemd])
> +if test -n "$SYSTEMD_LIB"; then
> +	AC_CHECK_HEADERS([systemd/sd-device.h], , [SYSTEMD_LIB=])
> +fi
> +if test -n "$SYSTEMD_LIB"; then
> +	AC_DEFINE(HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE, 1,
> +		[Define to 1 if libsystemd has sd-device APIs for whole-disk locking])
> +fi
> +AC_SUBST(SYSTEMD_LIB)
> +dnl
>  dnl libarchive
>  dnl
>  AC_ARG_WITH([libarchive],
> diff --git a/e2fsck/Makefile.in b/e2fsck/Makefile.in
> index fbb7b156..a050eafe 100644
> --- a/e2fsck/Makefile.in
> +++ b/e2fsck/Makefile.in
> @@ -17,20 +17,20 @@ MANPAGES=	e2fsck.8
>  FMANPAGES=	e2fsck.conf.5
>  
>  LIBS= $(LIBSUPPORT) $(LIBEXT2FS) $(LIBCOM_ERR) $(LIBBLKID) $(LIBUUID) \
> -	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(SYSLIBS)
> +	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
>  DEPLIBS= $(DEPLIBSUPPORT) $(LIBEXT2FS) $(DEPLIBCOM_ERR) $(DEPLIBBLKID) \
>  	 $(DEPLIBUUID) $(DEPLIBE2P)
>  
>  STATIC_LIBS= $(STATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) $(STATIC_LIBCOM_ERR) \
>  	     $(STATIC_LIBBLKID) $(STATIC_LIBUUID) $(LIBINTL) $(STATIC_LIBE2P) \
> -	     $(LIBMAGIC) $(SYSLIBS)
> +	     $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
>  STATIC_DEPLIBS= $(DEPSTATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) \
>  		$(DEPSTATIC_LIBCOM_ERR) $(DEPSTATIC_LIBBLKID) \
>  		$(DEPSTATIC_LIBUUID) $(DEPSTATIC_LIBE2P)
>  
>  PROFILED_LIBS= $(PROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
>  	       $(PROFILED_LIBCOM_ERR) $(PROFILED_LIBBLKID) $(PROFILED_LIBUUID) \
> -	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(SYSLIBS)
> +	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
>  PROFILED_DEPLIBS= $(DEPPROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
>  		  $(DEPPROFILED_LIBCOM_ERR) $(DEPPROFILED_LIBBLKID) \
>  		  $(DEPPROFILED_LIBUUID) $(DEPPROFILED_LIBE2P)
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index 335ca377..51bc41eb 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -36,6 +36,11 @@ extern int optind;
>  #ifdef HAVE_SYS_IOCTL_H
>  #include <sys/ioctl.h>
>  #endif
> +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> +#include <sys/file.h>
> +#include <sys/stat.h>
> +#include <systemd/sd-device.h>
> +#endif
>  #ifdef HAVE_MALLOC_H
>  #include <malloc.h>
>  #endif
> @@ -1397,6 +1402,88 @@ err:
>  	return retval;
>  }
>  
> +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> +{
> +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> +	struct stat st;
> +	sd_device *dev = NULL, *whole_dev;
> +	const char *subsystem, *devtype, *whole_disk_devname;
> +	int r, fd;
> +
> +	if (stat(dev_name, &st) < 0)
> +		return -1;
> +
> +	if (!S_ISBLK(st.st_mode))
> +		return -1;
> +
> +	r = sd_device_new_from_devnum(&dev, 'b', st.st_rdev);
> +	if (r < 0)
> +		return -1;
> +
> +	r = sd_device_get_subsystem(dev, &subsystem);
> +	if (r < 0 || strcmp(subsystem, "block") != 0) {
> +		sd_device_unref(dev);
> +		return -1;
> +	}
> +
> +	r = sd_device_get_devtype(dev, &devtype);
> +	if (r < 0) {
> +		sd_device_unref(dev);
> +		return -1;
> +	}
> +
> +	/*
> +	 * If dev_name is a partition, resolve the parent whole-disk device
> +	 * so the flock covers the same device node that udevd locks before
> +	 * probing any partition on it.
> +	 */
> +	if (strcmp(devtype, "disk") == 0)
> +		whole_dev = dev;
> +	else {
> +		r = sd_device_get_parent_with_subsystem_devtype(dev, "block",
> +								 "disk", &whole_dev);
> +		if (r < 0) {
> +			log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
> +			sd_device_unref(dev);
> +			return -1;
> +		}
> +	}
> +
> +	r = sd_device_get_devname(whole_dev, &whole_disk_devname);
> +	if (r < 0) {
> +		sd_device_unref(dev);
> +		return -1;
> +	}
> +
> +	fd = open(whole_disk_devname, O_RDONLY | O_CLOEXEC, 0);
> +	if (fd < 0) {
> +		com_err(ctx->program_name, errno,
> +			_("while trying to open %s for locking"), whole_disk_devname);
> +		sd_device_unref(dev);
> +		return -1;
> +	}
> +	sd_device_unref(dev);
> +
> +	while (flock(fd, LOCK_EX) != 0) {
> +		if (errno == EINTR) {
> +			if (ctx->flags & E2F_FLAG_CANCEL) {
> +				close(fd);
> +				return -1;
> +			}
> +			continue;
> +		}
> +		com_err(ctx->program_name, errno,
> +			_("while trying to lock %s"), whole_disk_devname);
> +		close(fd);
> +		return -1;
> +	}
> +
> +	return fd;
> +#else
> +	return -1;
> +#endif /* HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE */
> +}
> +
>  int main (int argc, char *argv[])
>  {
>  	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
> @@ -1413,6 +1500,7 @@ int main (int argc, char *argv[])
>  	int journal_size;
>  	int sysval, sys_page_size = 4096;
>  	int old_bitmaps;
> +	int lock_fd = -1;
>  	__u32 features[3];
>  	char *cp;
>  	enum quota_type qtype;
> @@ -1488,6 +1576,8 @@ int main (int argc, char *argv[])
>  
>  	check_mount(ctx);
>  
> +	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> +
>  	if (!(ctx->options & E2F_OPT_PREEN) &&
>  	    !(ctx->options & E2F_OPT_NO) &&
>  	    !(ctx->options & E2F_OPT_YES)) {
> @@ -2169,6 +2259,9 @@ skip_write:
>  	ext2fs_close_free(&ctx->fs);
>  	free(ctx->journal_name);
>  
> +	if (lock_fd >= 0)
> +		close(lock_fd);
> +
>  	if (ctx->logf)
>  		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
>  	e2fsck_free_context(ctx);
> diff --git a/lib/config.h.in b/lib/config.h.in
> index f129abfe..bc3ae4aa 100644
> --- a/lib/config.h.in
> +++ b/lib/config.h.in
> @@ -352,6 +352,9 @@
>  /* Define to 1 if if struct sockaddr contains sa_len */
>  #undef HAVE_SA_LEN
>  
> +/* Define to 1 if libsystemd has sd-device APIs for whole-disk locking */
> +#undef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> +
>  /* Define to 1 if you have the 'secure_getenv' function. */
>  #undef HAVE_SECURE_GETENV
>  
> @@ -430,6 +433,9 @@
>  /* Define to 1 if you have the 'sysconf' function. */
>  #undef HAVE_SYSCONF
>  
> +/* Define to 1 if you have the <systemd/sd-device.h> header file. */
> +#undef HAVE_SYSTEMD_SD_DEVICE_H
> +
>  /* Define to 1 if you have the <sys/acl.h> header file. */
>  #undef HAVE_SYS_ACL_H
>  
> -- 
> 2.55.0

Hi,

Gentle ping on this patch.

I would appreciate any feedback.

Re: [PATCH v3] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Dan Streetman]

On Sun, 20 Sep 2026, Nandakumar Raghavan wrote:

> On Thu, Sep 10, 2026 at 05:04:41AM -0700, Nandakumar Raghavan wrote:
> > During journal replay, e2fsck writes the primary superblock back to disk
> > in multiple I/O operations. The payload lands before the checksum, leaving
> > a transient window where the on-disk superblock has a bad checksum.
> > 
> > If udevd processes a change uevent during this window, libblkid probes the
> > primary superblock, finds a checksum mismatch, and concludes the partition
> > has no recognisable filesystem. udev then fires a remove event, wiping all
> > symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> > that depends on those symlinks will fail.
> > 
> > udevd already serialises its own partition probes against whole-disk device
> > access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> > event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> > whole-disk device before opening the filesystem. This forces udevd to defer
> > all probes on that disk until e2fsck exits and the lock is released, by
> > which point the filesystem is fully consistent.
> > 
> > The whole-disk device is resolved using libsystemd's sd-device API
> > (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype())
> > rather than parsing sysfs, so the lock covers the same device node
> > that udevd itself resolves and locks.
> > 
> > This locking is entirely optional at build time where configure checks for
> > sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB, and
> > e2fsck.c only compiles in the locking code when it is found. On platforms
> > without libsystemd there is no udevd to race against in the first place,
> > so nothing is lost by skipping it there.
> > 
> > Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> > ---
> > 
> > Changes in v3:
> >     - Resolve the parent whole-disk device using libsystemd's sd-device API
> >       (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype(),
> >       sd_device_get_devname()) instead of parsing sysfs paths
> >     - Gate the feature on autoconf detecting
> >       sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB; compile it
> >       out entirely when libsystemd is absent, rather than falling back to another
> >       implementation
> >     - Use sd_device_new_from_devnum() rather than the sd_device_new_from_devname()
> >       used by the systemd.io example, since the former has been available in
> >       libsystemd for much longer, keeping the version floor as low as possible
> >     - Keep lock_whole_disk()'s signature unconditional and provide a stub returning
> >       -1 on the #else branch, so main()'s primary code path has no #ifdefs in it
> >     
> > Changes in v2:
> >     - Guard the whole feature (function, call site, lock_fd, cleanup) rather than
> >       just the function body, so it is fully absent from non-Linux builds 
> >     - Collapse the three HAVE_SYS_*_H header guards into a single conditional
> >     - Reuse a single dev_path[] buffer instead of three single-use stack buffers
> >     - Keep translatable warning strings on one line each, even past 80 columns
> >     - Drop the blanket "could not lock" warning that fired even when the target was
> >       not a block device at all (e.g. a regular file), which broke most of
> >       "make check"; lock_whole_disk() now only warns at the point of a genuine failure
> >     - No change needed for the Ctrl-C/EINTR concern: the SIGINT handler is installed
> >       before lock_whole_disk() runs and is not SA_RESTART, so a blocked flock() is
> >       interrupted and the existing E2F_FLAG_CANCEL check unwinds cleanly
> > 
> >  MCONFIG.in         |  1 +
> >  configure          | 71 +++++++++++++++++++++++++++++++++++
> >  configure.ac       | 14 +++++++
> >  e2fsck/Makefile.in |  6 +--
> >  e2fsck/unix.c      | 93 ++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/config.h.in    |  6 +++
> >  6 files changed, 188 insertions(+), 3 deletions(-)
> > 
> > diff --git a/MCONFIG.in b/MCONFIG.in
> > index d66e2f3b..d3055955 100644
> > --- a/MCONFIG.in
> > +++ b/MCONFIG.in
> > @@ -137,6 +137,7 @@ LIBE2P = $(LIB)/libe2p@LIB_EXT@
> >  LIBEXT2FS = $(LIB)/libext2fs@LIB_EXT@
> >  LIBUUID = @LIBUUID@ @SOCKET_LIB@
> >  LIBMAGIC = @MAGIC_LIB@
> > +LIBSYSTEMD = @SYSTEMD_LIB@
> >  LIBFUSE = @FUSE_LIB@
> >  LIBSUPPORT = $(LIBINTL) $(LIB)/libsupport@STATIC_LIB_EXT@
> >  LIBBLKID = @LIBBLKID@ @PRIVATE_LIBS_CMT@ $(LIBUUID)
> > diff --git a/configure b/configure
> > index b04b31af..696dd07c 100755
> > --- a/configure
> > +++ b/configure
> > @@ -702,6 +702,7 @@ fuse3_CFLAGS
> >  CLOCK_GETTIME_LIB
> >  ARCHIVE_LIBS
> >  ARCHIVE_CFLAGS
> > +SYSTEMD_LIB
> >  MAGIC_LIB
> >  SOCKET_LIB
> >  SIZEOF_TIME_T
> > @@ -14130,6 +14131,76 @@ if test "$ac_cv_func_dlopen" = yes ; then
> >     MAGIC_LIB=$DLOPEN_LIB
> >  fi
> >  
> > +SYSTEMD_LIB=
> > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd" >&5
> > +printf %s "checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd... " >&6; }
> > +if test ${ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype+y}
> > +then :
> > +  printf %s "(cached) " >&6
> > +else case e in #(
> > +  e) ac_check_lib_save_LIBS=$LIBS
> > +LIBS="-lsystemd  $LIBS"
> > +cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> > +/* end confdefs.h.  */
> > +
> > +/* Override any GCC internal prototype to avoid an error.
> > +   Use char because int might match the return type of a GCC
> > +   builtin and then its argument prototype would still apply.
> > +   The 'extern "C"' is for builds by C++ compilers;
> > +   although this is not generally supported in C code supporting it here
> > +   has little cost and some practical benefit (sr 110532).  */
> > +#ifdef __cplusplus
> > +extern "C"
> > +#endif
> > +char sd_device_get_parent_with_subsystem_devtype (void);
> > +int
> > +main (void)
> > +{
> > +return sd_device_get_parent_with_subsystem_devtype ();
> > +  ;
> > +  return 0;
> > +}
> > +_ACEOF
> > +if ac_fn_c_try_link "$LINENO"
> > +then :
> > +  ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=yes
> > +else case e in #(
> > +  e) ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=no ;;
> > +esac
> > +fi
> > +rm -f core conftest.err conftest.$ac_objext conftest.beam \
> > +    conftest$ac_exeext conftest.$ac_ext
> > +LIBS=$ac_check_lib_save_LIBS ;;
> > +esac
> > +fi
> > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&5
> > +printf "%s\n" "$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&6; }
> > +if test "x$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" = xyes
> > +then :
> > +  SYSTEMD_LIB=-lsystemd
> > +fi
> > +
> > +if test -n "$SYSTEMD_LIB"; then
> > +	       for ac_header in systemd/sd-device.h
> > +do :
> > +  ac_fn_c_check_header_compile "$LINENO" "systemd/sd-device.h" "ac_cv_header_systemd_sd_device_h" "$ac_includes_default"
> > +if test "x$ac_cv_header_systemd_sd_device_h" = xyes
> > +then :
> > +  printf "%s\n" "#define HAVE_SYSTEMD_SD_DEVICE_H 1" >>confdefs.h
> > +
> > +else case e in #(
> > +  e) SYSTEMD_LIB= ;;
> > +esac
> > +fi
> > +
> > +done
> > +fi
> > +if test -n "$SYSTEMD_LIB"; then
> > +
> > +printf "%s\n" "#define HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE 1" >>confdefs.h
> > +
> > +fi
> > +
> >  
> >  # Check whether --with-libarchive was given.
> >  if test ${with_libarchive+y}
> > diff --git a/configure.ac b/configure.ac
> > index 4921f81f..9f585aae 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -1309,6 +1309,20 @@ if test "$ac_cv_func_dlopen" = yes ; then
> >  fi
> >  AC_SUBST(MAGIC_LIB)
> >  dnl
> > +dnl See if libsystemd has sd-device APIs for whole-disk BSD locking
> > +dnl
> > +SYSTEMD_LIB=
> > +AC_CHECK_LIB(systemd, sd_device_get_parent_with_subsystem_devtype,
> > +	[SYSTEMD_LIB=-lsystemd])
> > +if test -n "$SYSTEMD_LIB"; then
> > +	AC_CHECK_HEADERS([systemd/sd-device.h], , [SYSTEMD_LIB=])
> > +fi
> > +if test -n "$SYSTEMD_LIB"; then
> > +	AC_DEFINE(HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE, 1,
> > +		[Define to 1 if libsystemd has sd-device APIs for whole-disk locking])
> > +fi
> > +AC_SUBST(SYSTEMD_LIB)
> > +dnl
> >  dnl libarchive
> >  dnl
> >  AC_ARG_WITH([libarchive],
> > diff --git a/e2fsck/Makefile.in b/e2fsck/Makefile.in
> > index fbb7b156..a050eafe 100644
> > --- a/e2fsck/Makefile.in
> > +++ b/e2fsck/Makefile.in
> > @@ -17,20 +17,20 @@ MANPAGES=	e2fsck.8
> >  FMANPAGES=	e2fsck.conf.5
> >  
> >  LIBS= $(LIBSUPPORT) $(LIBEXT2FS) $(LIBCOM_ERR) $(LIBBLKID) $(LIBUUID) \
> > -	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(SYSLIBS)
> > +	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> >  DEPLIBS= $(DEPLIBSUPPORT) $(LIBEXT2FS) $(DEPLIBCOM_ERR) $(DEPLIBBLKID) \
> >  	 $(DEPLIBUUID) $(DEPLIBE2P)
> >  
> >  STATIC_LIBS= $(STATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) $(STATIC_LIBCOM_ERR) \
> >  	     $(STATIC_LIBBLKID) $(STATIC_LIBUUID) $(LIBINTL) $(STATIC_LIBE2P) \
> > -	     $(LIBMAGIC) $(SYSLIBS)
> > +	     $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> >  STATIC_DEPLIBS= $(DEPSTATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) \
> >  		$(DEPSTATIC_LIBCOM_ERR) $(DEPSTATIC_LIBBLKID) \
> >  		$(DEPSTATIC_LIBUUID) $(DEPSTATIC_LIBE2P)
> >  
> >  PROFILED_LIBS= $(PROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> >  	       $(PROFILED_LIBCOM_ERR) $(PROFILED_LIBBLKID) $(PROFILED_LIBUUID) \
> > -	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(SYSLIBS)
> > +	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> >  PROFILED_DEPLIBS= $(DEPPROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> >  		  $(DEPPROFILED_LIBCOM_ERR) $(DEPPROFILED_LIBBLKID) \
> >  		  $(DEPPROFILED_LIBUUID) $(DEPPROFILED_LIBE2P)
> > diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> > index 335ca377..51bc41eb 100644
> > --- a/e2fsck/unix.c
> > +++ b/e2fsck/unix.c
> > @@ -36,6 +36,11 @@ extern int optind;
> >  #ifdef HAVE_SYS_IOCTL_H
> >  #include <sys/ioctl.h>
> >  #endif
> > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > +#include <sys/file.h>
> > +#include <sys/stat.h>
> > +#include <systemd/sd-device.h>
> > +#endif
> >  #ifdef HAVE_MALLOC_H
> >  #include <malloc.h>
> >  #endif
> > @@ -1397,6 +1402,88 @@ err:
> >  	return retval;
> >  }
> >  
> > +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> > +{
> > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > +	struct stat st;
> > +	sd_device *dev = NULL, *whole_dev;
> > +	const char *subsystem, *devtype, *whole_disk_devname;
> > +	int r, fd;
> > +
> > +	if (stat(dev_name, &st) < 0)
> > +		return -1;
> > +
> > +	if (!S_ISBLK(st.st_mode))
> > +		return -1;
> > +
> > +	r = sd_device_new_from_devnum(&dev, 'b', st.st_rdev);
> > +	if (r < 0)
> > +		return -1;
> > +
> > +	r = sd_device_get_subsystem(dev, &subsystem);
> > +	if (r < 0 || strcmp(subsystem, "block") != 0) {
> > +		sd_device_unref(dev);
> > +		return -1;
> > +	}
> > +
> > +	r = sd_device_get_devtype(dev, &devtype);
> > +	if (r < 0) {
> > +		sd_device_unref(dev);
> > +		return -1;
> > +	}
> > +
> > +	/*
> > +	 * If dev_name is a partition, resolve the parent whole-disk device
> > +	 * so the flock covers the same device node that udevd locks before
> > +	 * probing any partition on it.
> > +	 */
> > +	if (strcmp(devtype, "disk") == 0)
> > +		whole_dev = dev;
> > +	else {
> > +		r = sd_device_get_parent_with_subsystem_devtype(dev, "block",
> > +								 "disk", &whole_dev);
> > +		if (r < 0) {
> > +			log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
> > +			sd_device_unref(dev);
> > +			return -1;
> > +		}
> > +	}
> > +
> > +	r = sd_device_get_devname(whole_dev, &whole_disk_devname);
> > +	if (r < 0) {
> > +		sd_device_unref(dev);
> > +		return -1;
> > +	}
> > +
> > +	fd = open(whole_disk_devname, O_RDONLY | O_CLOEXEC, 0);
> > +	if (fd < 0) {
> > +		com_err(ctx->program_name, errno,
> > +			_("while trying to open %s for locking"), whole_disk_devname);
> > +		sd_device_unref(dev);
> > +		return -1;
> > +	}
> > +	sd_device_unref(dev);
> > +
> > +	while (flock(fd, LOCK_EX) != 0) {
> > +		if (errno == EINTR) {
> > +			if (ctx->flags & E2F_FLAG_CANCEL) {
> > +				close(fd);
> > +				return -1;
> > +			}
> > +			continue;
> > +		}
> > +		com_err(ctx->program_name, errno,
> > +			_("while trying to lock %s"), whole_disk_devname);
> > +		close(fd);
> > +		return -1;
> > +	}
> > +
> > +	return fd;
> > +#else
> > +	return -1;
> > +#endif /* HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE */
> > +}
> > +
> >  int main (int argc, char *argv[])
> >  {
> >  	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
> > @@ -1413,6 +1500,7 @@ int main (int argc, char *argv[])
> >  	int journal_size;
> >  	int sysval, sys_page_size = 4096;
> >  	int old_bitmaps;
> > +	int lock_fd = -1;
> >  	__u32 features[3];
> >  	char *cp;
> >  	enum quota_type qtype;
> > @@ -1488,6 +1576,8 @@ int main (int argc, char *argv[])
> >  
> >  	check_mount(ctx);
> >  
> > +	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> > +
> >  	if (!(ctx->options & E2F_OPT_PREEN) &&
> >  	    !(ctx->options & E2F_OPT_NO) &&
> >  	    !(ctx->options & E2F_OPT_YES)) {
> > @@ -2169,6 +2259,9 @@ skip_write:
> >  	ext2fs_close_free(&ctx->fs);
> >  	free(ctx->journal_name);
> >  
> > +	if (lock_fd >= 0)
> > +		close(lock_fd);
> > +
> >  	if (ctx->logf)
> >  		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> >  	e2fsck_free_context(ctx);
> > diff --git a/lib/config.h.in b/lib/config.h.in
> > index f129abfe..bc3ae4aa 100644
> > --- a/lib/config.h.in
> > +++ b/lib/config.h.in
> > @@ -352,6 +352,9 @@
> >  /* Define to 1 if if struct sockaddr contains sa_len */
> >  #undef HAVE_SA_LEN
> >  
> > +/* Define to 1 if libsystemd has sd-device APIs for whole-disk locking */
> > +#undef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > +
> >  /* Define to 1 if you have the 'secure_getenv' function. */
> >  #undef HAVE_SECURE_GETENV
> >  
> > @@ -430,6 +433,9 @@
> >  /* Define to 1 if you have the 'sysconf' function. */
> >  #undef HAVE_SYSCONF
> >  
> > +/* Define to 1 if you have the <systemd/sd-device.h> header file. */
> > +#undef HAVE_SYSTEMD_SD_DEVICE_H
> > +
> >  /* Define to 1 if you have the <sys/acl.h> header file. */
> >  #undef HAVE_SYS_ACL_H
> >  
> > -- 
> > 2.55.0
> 
> Hi,
> 
> Gentle ping on this patch.
> 
> I would appreciate any feedback.
> 

Can you clarify why this should go into only fsck.ext4? Doesn't this
problem exist for other filesystems too?

I sent an earlier email as well with links to:

1) fsck used to lock the device, but it surfaced a bug in udevd
https://bugs.freedesktop.org/show_bug.cgi?id=79576

2) because of the bug, fsck stopped locking the device
https://github.com/util-linux/util-linux/commit/3bbdae633f4a1dda5f95ee6c61f18a1c8ef12250

3) the systemd-udevd bug was fixed
https://github.com/systemd/systemd/commit/5d354e525a5

To me, it makes more sense for the locking that already exists in fsck
to get updated (or reverted) to lock the entire device, using the
existing -l param (or maybe a new param like --lock-device,
--udevd-lock, etc., if util-linux maintainers don't want to change -l
behavior).

Do you see an issue with doing the locking there instead of here in
fsck.ext4?

Re: [PATCH v3] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Darrick J. Wong]

On Wed, Sep 23, 2026 at 03:55:23PM -0400, Dan Streetman wrote:
> 
> 
> On Sun, 20 Sep 2026, Nandakumar Raghavan wrote:
> 
> > On Thu, Sep 10, 2026 at 05:04:41AM -0700, Nandakumar Raghavan wrote:
> > > During journal replay, e2fsck writes the primary superblock back to disk
> > > in multiple I/O operations. The payload lands before the checksum, leaving
> > > a transient window where the on-disk superblock has a bad checksum.
> > > 
> > > If udevd processes a change uevent during this window, libblkid probes the
> > > primary superblock, finds a checksum mismatch, and concludes the partition
> > > has no recognisable filesystem. udev then fires a remove event, wiping all
> > > symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> > > that depends on those symlinks will fail.
> > > 
> > > udevd already serialises its own partition probes against whole-disk device
> > > access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> > > event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> > > whole-disk device before opening the filesystem. This forces udevd to defer
> > > all probes on that disk until e2fsck exits and the lock is released, by
> > > which point the filesystem is fully consistent.
> > > 
> > > The whole-disk device is resolved using libsystemd's sd-device API
> > > (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype())
> > > rather than parsing sysfs, so the lock covers the same device node
> > > that udevd itself resolves and locks.
> > > 
> > > This locking is entirely optional at build time where configure checks for
> > > sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB, and
> > > e2fsck.c only compiles in the locking code when it is found. On platforms
> > > without libsystemd there is no udevd to race against in the first place,
> > > so nothing is lost by skipping it there.
> > > 
> > > Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> > > ---
> > > 
> > > Changes in v3:
> > >     - Resolve the parent whole-disk device using libsystemd's sd-device API
> > >       (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype(),
> > >       sd_device_get_devname()) instead of parsing sysfs paths
> > >     - Gate the feature on autoconf detecting
> > >       sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB; compile it
> > >       out entirely when libsystemd is absent, rather than falling back to another
> > >       implementation
> > >     - Use sd_device_new_from_devnum() rather than the sd_device_new_from_devname()
> > >       used by the systemd.io example, since the former has been available in
> > >       libsystemd for much longer, keeping the version floor as low as possible
> > >     - Keep lock_whole_disk()'s signature unconditional and provide a stub returning
> > >       -1 on the #else branch, so main()'s primary code path has no #ifdefs in it
> > >     
> > > Changes in v2:
> > >     - Guard the whole feature (function, call site, lock_fd, cleanup) rather than
> > >       just the function body, so it is fully absent from non-Linux builds 
> > >     - Collapse the three HAVE_SYS_*_H header guards into a single conditional
> > >     - Reuse a single dev_path[] buffer instead of three single-use stack buffers
> > >     - Keep translatable warning strings on one line each, even past 80 columns
> > >     - Drop the blanket "could not lock" warning that fired even when the target was
> > >       not a block device at all (e.g. a regular file), which broke most of
> > >       "make check"; lock_whole_disk() now only warns at the point of a genuine failure
> > >     - No change needed for the Ctrl-C/EINTR concern: the SIGINT handler is installed
> > >       before lock_whole_disk() runs and is not SA_RESTART, so a blocked flock() is
> > >       interrupted and the existing E2F_FLAG_CANCEL check unwinds cleanly
> > > 
> > >  MCONFIG.in         |  1 +
> > >  configure          | 71 +++++++++++++++++++++++++++++++++++
> > >  configure.ac       | 14 +++++++
> > >  e2fsck/Makefile.in |  6 +--
> > >  e2fsck/unix.c      | 93 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  lib/config.h.in    |  6 +++
> > >  6 files changed, 188 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/MCONFIG.in b/MCONFIG.in
> > > index d66e2f3b..d3055955 100644
> > > --- a/MCONFIG.in
> > > +++ b/MCONFIG.in
> > > @@ -137,6 +137,7 @@ LIBE2P = $(LIB)/libe2p@LIB_EXT@
> > >  LIBEXT2FS = $(LIB)/libext2fs@LIB_EXT@
> > >  LIBUUID = @LIBUUID@ @SOCKET_LIB@
> > >  LIBMAGIC = @MAGIC_LIB@
> > > +LIBSYSTEMD = @SYSTEMD_LIB@
> > >  LIBFUSE = @FUSE_LIB@
> > >  LIBSUPPORT = $(LIBINTL) $(LIB)/libsupport@STATIC_LIB_EXT@
> > >  LIBBLKID = @LIBBLKID@ @PRIVATE_LIBS_CMT@ $(LIBUUID)
> > > diff --git a/configure b/configure
> > > index b04b31af..696dd07c 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -702,6 +702,7 @@ fuse3_CFLAGS
> > >  CLOCK_GETTIME_LIB
> > >  ARCHIVE_LIBS
> > >  ARCHIVE_CFLAGS
> > > +SYSTEMD_LIB
> > >  MAGIC_LIB
> > >  SOCKET_LIB
> > >  SIZEOF_TIME_T
> > > @@ -14130,6 +14131,76 @@ if test "$ac_cv_func_dlopen" = yes ; then
> > >     MAGIC_LIB=$DLOPEN_LIB
> > >  fi
> > >  
> > > +SYSTEMD_LIB=
> > > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd" >&5
> > > +printf %s "checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd... " >&6; }
> > > +if test ${ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype+y}
> > > +then :
> > > +  printf %s "(cached) " >&6
> > > +else case e in #(
> > > +  e) ac_check_lib_save_LIBS=$LIBS
> > > +LIBS="-lsystemd  $LIBS"
> > > +cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> > > +/* end confdefs.h.  */
> > > +
> > > +/* Override any GCC internal prototype to avoid an error.
> > > +   Use char because int might match the return type of a GCC
> > > +   builtin and then its argument prototype would still apply.
> > > +   The 'extern "C"' is for builds by C++ compilers;
> > > +   although this is not generally supported in C code supporting it here
> > > +   has little cost and some practical benefit (sr 110532).  */
> > > +#ifdef __cplusplus
> > > +extern "C"
> > > +#endif
> > > +char sd_device_get_parent_with_subsystem_devtype (void);
> > > +int
> > > +main (void)
> > > +{
> > > +return sd_device_get_parent_with_subsystem_devtype ();
> > > +  ;
> > > +  return 0;
> > > +}
> > > +_ACEOF
> > > +if ac_fn_c_try_link "$LINENO"
> > > +then :
> > > +  ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=yes
> > > +else case e in #(
> > > +  e) ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=no ;;
> > > +esac
> > > +fi
> > > +rm -f core conftest.err conftest.$ac_objext conftest.beam \
> > > +    conftest$ac_exeext conftest.$ac_ext
> > > +LIBS=$ac_check_lib_save_LIBS ;;
> > > +esac
> > > +fi
> > > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&5
> > > +printf "%s\n" "$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&6; }
> > > +if test "x$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" = xyes
> > > +then :
> > > +  SYSTEMD_LIB=-lsystemd
> > > +fi
> > > +
> > > +if test -n "$SYSTEMD_LIB"; then
> > > +	       for ac_header in systemd/sd-device.h
> > > +do :
> > > +  ac_fn_c_check_header_compile "$LINENO" "systemd/sd-device.h" "ac_cv_header_systemd_sd_device_h" "$ac_includes_default"
> > > +if test "x$ac_cv_header_systemd_sd_device_h" = xyes
> > > +then :
> > > +  printf "%s\n" "#define HAVE_SYSTEMD_SD_DEVICE_H 1" >>confdefs.h
> > > +
> > > +else case e in #(
> > > +  e) SYSTEMD_LIB= ;;
> > > +esac
> > > +fi
> > > +
> > > +done
> > > +fi
> > > +if test -n "$SYSTEMD_LIB"; then
> > > +
> > > +printf "%s\n" "#define HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE 1" >>confdefs.h
> > > +
> > > +fi
> > > +
> > >  
> > >  # Check whether --with-libarchive was given.
> > >  if test ${with_libarchive+y}
> > > diff --git a/configure.ac b/configure.ac
> > > index 4921f81f..9f585aae 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -1309,6 +1309,20 @@ if test "$ac_cv_func_dlopen" = yes ; then
> > >  fi
> > >  AC_SUBST(MAGIC_LIB)
> > >  dnl
> > > +dnl See if libsystemd has sd-device APIs for whole-disk BSD locking
> > > +dnl
> > > +SYSTEMD_LIB=
> > > +AC_CHECK_LIB(systemd, sd_device_get_parent_with_subsystem_devtype,
> > > +	[SYSTEMD_LIB=-lsystemd])
> > > +if test -n "$SYSTEMD_LIB"; then
> > > +	AC_CHECK_HEADERS([systemd/sd-device.h], , [SYSTEMD_LIB=])
> > > +fi
> > > +if test -n "$SYSTEMD_LIB"; then
> > > +	AC_DEFINE(HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE, 1,
> > > +		[Define to 1 if libsystemd has sd-device APIs for whole-disk locking])
> > > +fi
> > > +AC_SUBST(SYSTEMD_LIB)
> > > +dnl
> > >  dnl libarchive
> > >  dnl
> > >  AC_ARG_WITH([libarchive],
> > > diff --git a/e2fsck/Makefile.in b/e2fsck/Makefile.in
> > > index fbb7b156..a050eafe 100644
> > > --- a/e2fsck/Makefile.in
> > > +++ b/e2fsck/Makefile.in
> > > @@ -17,20 +17,20 @@ MANPAGES=	e2fsck.8
> > >  FMANPAGES=	e2fsck.conf.5
> > >  
> > >  LIBS= $(LIBSUPPORT) $(LIBEXT2FS) $(LIBCOM_ERR) $(LIBBLKID) $(LIBUUID) \
> > > -	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(SYSLIBS)
> > > +	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > >  DEPLIBS= $(DEPLIBSUPPORT) $(LIBEXT2FS) $(DEPLIBCOM_ERR) $(DEPLIBBLKID) \
> > >  	 $(DEPLIBUUID) $(DEPLIBE2P)
> > >  
> > >  STATIC_LIBS= $(STATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) $(STATIC_LIBCOM_ERR) \
> > >  	     $(STATIC_LIBBLKID) $(STATIC_LIBUUID) $(LIBINTL) $(STATIC_LIBE2P) \
> > > -	     $(LIBMAGIC) $(SYSLIBS)
> > > +	     $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > >  STATIC_DEPLIBS= $(DEPSTATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) \
> > >  		$(DEPSTATIC_LIBCOM_ERR) $(DEPSTATIC_LIBBLKID) \
> > >  		$(DEPSTATIC_LIBUUID) $(DEPSTATIC_LIBE2P)
> > >  
> > >  PROFILED_LIBS= $(PROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> > >  	       $(PROFILED_LIBCOM_ERR) $(PROFILED_LIBBLKID) $(PROFILED_LIBUUID) \
> > > -	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(SYSLIBS)
> > > +	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > >  PROFILED_DEPLIBS= $(DEPPROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> > >  		  $(DEPPROFILED_LIBCOM_ERR) $(DEPPROFILED_LIBBLKID) \
> > >  		  $(DEPPROFILED_LIBUUID) $(DEPPROFILED_LIBE2P)
> > > diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> > > index 335ca377..51bc41eb 100644
> > > --- a/e2fsck/unix.c
> > > +++ b/e2fsck/unix.c
> > > @@ -36,6 +36,11 @@ extern int optind;
> > >  #ifdef HAVE_SYS_IOCTL_H
> > >  #include <sys/ioctl.h>
> > >  #endif
> > > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > +#include <sys/file.h>
> > > +#include <sys/stat.h>
> > > +#include <systemd/sd-device.h>
> > > +#endif
> > >  #ifdef HAVE_MALLOC_H
> > >  #include <malloc.h>
> > >  #endif
> > > @@ -1397,6 +1402,88 @@ err:
> > >  	return retval;
> > >  }
> > >  
> > > +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> > > +{
> > > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > +	struct stat st;
> > > +	sd_device *dev = NULL, *whole_dev;
> > > +	const char *subsystem, *devtype, *whole_disk_devname;
> > > +	int r, fd;
> > > +
> > > +	if (stat(dev_name, &st) < 0)
> > > +		return -1;
> > > +
> > > +	if (!S_ISBLK(st.st_mode))
> > > +		return -1;
> > > +
> > > +	r = sd_device_new_from_devnum(&dev, 'b', st.st_rdev);
> > > +	if (r < 0)
> > > +		return -1;
> > > +
> > > +	r = sd_device_get_subsystem(dev, &subsystem);
> > > +	if (r < 0 || strcmp(subsystem, "block") != 0) {
> > > +		sd_device_unref(dev);
> > > +		return -1;
> > > +	}
> > > +
> > > +	r = sd_device_get_devtype(dev, &devtype);
> > > +	if (r < 0) {
> > > +		sd_device_unref(dev);
> > > +		return -1;
> > > +	}
> > > +
> > > +	/*
> > > +	 * If dev_name is a partition, resolve the parent whole-disk device
> > > +	 * so the flock covers the same device node that udevd locks before
> > > +	 * probing any partition on it.
> > > +	 */
> > > +	if (strcmp(devtype, "disk") == 0)
> > > +		whole_dev = dev;
> > > +	else {
> > > +		r = sd_device_get_parent_with_subsystem_devtype(dev, "block",
> > > +								 "disk", &whole_dev);
> > > +		if (r < 0) {
> > > +			log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
> > > +			sd_device_unref(dev);
> > > +			return -1;
> > > +		}
> > > +	}
> > > +
> > > +	r = sd_device_get_devname(whole_dev, &whole_disk_devname);
> > > +	if (r < 0) {
> > > +		sd_device_unref(dev);
> > > +		return -1;
> > > +	}
> > > +
> > > +	fd = open(whole_disk_devname, O_RDONLY | O_CLOEXEC, 0);
> > > +	if (fd < 0) {
> > > +		com_err(ctx->program_name, errno,
> > > +			_("while trying to open %s for locking"), whole_disk_devname);
> > > +		sd_device_unref(dev);
> > > +		return -1;
> > > +	}
> > > +	sd_device_unref(dev);
> > > +
> > > +	while (flock(fd, LOCK_EX) != 0) {
> > > +		if (errno == EINTR) {
> > > +			if (ctx->flags & E2F_FLAG_CANCEL) {
> > > +				close(fd);
> > > +				return -1;
> > > +			}
> > > +			continue;
> > > +		}
> > > +		com_err(ctx->program_name, errno,
> > > +			_("while trying to lock %s"), whole_disk_devname);
> > > +		close(fd);
> > > +		return -1;
> > > +	}
> > > +
> > > +	return fd;
> > > +#else
> > > +	return -1;
> > > +#endif /* HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE */
> > > +}
> > > +
> > >  int main (int argc, char *argv[])
> > >  {
> > >  	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
> > > @@ -1413,6 +1500,7 @@ int main (int argc, char *argv[])
> > >  	int journal_size;
> > >  	int sysval, sys_page_size = 4096;
> > >  	int old_bitmaps;
> > > +	int lock_fd = -1;
> > >  	__u32 features[3];
> > >  	char *cp;
> > >  	enum quota_type qtype;
> > > @@ -1488,6 +1576,8 @@ int main (int argc, char *argv[])
> > >  
> > >  	check_mount(ctx);
> > >  
> > > +	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> > > +
> > >  	if (!(ctx->options & E2F_OPT_PREEN) &&
> > >  	    !(ctx->options & E2F_OPT_NO) &&
> > >  	    !(ctx->options & E2F_OPT_YES)) {
> > > @@ -2169,6 +2259,9 @@ skip_write:
> > >  	ext2fs_close_free(&ctx->fs);
> > >  	free(ctx->journal_name);
> > >  
> > > +	if (lock_fd >= 0)
> > > +		close(lock_fd);
> > > +
> > >  	if (ctx->logf)
> > >  		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> > >  	e2fsck_free_context(ctx);
> > > diff --git a/lib/config.h.in b/lib/config.h.in
> > > index f129abfe..bc3ae4aa 100644
> > > --- a/lib/config.h.in
> > > +++ b/lib/config.h.in
> > > @@ -352,6 +352,9 @@
> > >  /* Define to 1 if if struct sockaddr contains sa_len */
> > >  #undef HAVE_SA_LEN
> > >  
> > > +/* Define to 1 if libsystemd has sd-device APIs for whole-disk locking */
> > > +#undef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > +
> > >  /* Define to 1 if you have the 'secure_getenv' function. */
> > >  #undef HAVE_SECURE_GETENV
> > >  
> > > @@ -430,6 +433,9 @@
> > >  /* Define to 1 if you have the 'sysconf' function. */
> > >  #undef HAVE_SYSCONF
> > >  
> > > +/* Define to 1 if you have the <systemd/sd-device.h> header file. */
> > > +#undef HAVE_SYSTEMD_SD_DEVICE_H
> > > +
> > >  /* Define to 1 if you have the <sys/acl.h> header file. */
> > >  #undef HAVE_SYS_ACL_H
> > >  
> > > -- 
> > > 2.55.0
> > 
> > Hi,
> > 
> > Gentle ping on this patch.
> > 
> > I would appreciate any feedback.
> > 
> 
> Can you clarify why this should go into only fsck.ext4? Doesn't this
> problem exist for other filesystems too?
> 
> I sent an earlier email as well with links to:
> 
> 1) fsck used to lock the device, but it surfaced a bug in udevd
> https://bugs.freedesktop.org/show_bug.cgi?id=79576
> 
> 2) because of the bug, fsck stopped locking the device
> https://github.com/util-linux/util-linux/commit/3bbdae633f4a1dda5f95ee6c61f18a1c8ef12250
> 
> 3) the systemd-udevd bug was fixed
> https://github.com/systemd/systemd/commit/5d354e525a5
> 
> To me, it makes more sense for the locking that already exists in fsck
> to get updated (or reverted) to lock the entire device, using the
> existing -l param (or maybe a new param like --lock-device,
> --udevd-lock, etc., if util-linux maintainers don't want to change -l
> behavior).
> 
> Do you see an issue with doing the locking there instead of here in
> fsck.ext4?

/sbin/fsck (aka the dispatch wrapper program) doesn't necessarily know
which block device(s) are going to be opened by a the fsck.$FSTYP
program that it creates.  It might be able to infer that by opening any
parameter and performing the udev locking protocol after checking if
what it opened is a block device, but that wouldn't work for (say) a
fsck.XXX program for a multi-device filesystem wherein you only need to
specify one device and it will find the others.

That said, this patchset also doesn't handle multi-device ext4
filesystems (i.e. external jbd2 journal device) because the author
doesn't want to do that.  In their defense, the udev flock()ing protocol
requires one to determine if an opened block device is a partition; if
it is, then it requires opening and locking the parent bdev (e.g. sdf1
-> sdf) instead of locking the original device.  This makes it way more
complicated for multi-device filesystems because now the client has to
detect multiple partitions coming from the same underlying device and
handle that appropriately.  I don't know why the protocol designers made
that choice.

I can run "trace-cmd record -e 'flock*'" to observe the locking
interactions with scsi disk partitions, but for whatever reason I don't
see any flocking going on if I use kpartx to create the partitions with
device-mapper.  No idea why that is.

--D

Re: [PATCH v3] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Dan Streetman]

On Wed, 23 Sep 2026, Darrick J. Wong wrote:

> On Wed, Sep 23, 2026 at 03:55:23PM -0400, Dan Streetman wrote:
> > 
> > 
> > On Sun, 20 Sep 2026, Nandakumar Raghavan wrote:
> > 
> > > On Thu, Sep 10, 2026 at 05:04:41AM -0700, Nandakumar Raghavan wrote:
> > > > During journal replay, e2fsck writes the primary superblock back to disk
> > > > in multiple I/O operations. The payload lands before the checksum, leaving
> > > > a transient window where the on-disk superblock has a bad checksum.
> > > > 
> > > > If udevd processes a change uevent during this window, libblkid probes the
> > > > primary superblock, finds a checksum mismatch, and concludes the partition
> > > > has no recognisable filesystem. udev then fires a remove event, wiping all
> > > > symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> > > > that depends on those symlinks will fail.
> > > > 
> > > > udevd already serialises its own partition probes against whole-disk device
> > > > access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> > > > event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> > > > whole-disk device before opening the filesystem. This forces udevd to defer
> > > > all probes on that disk until e2fsck exits and the lock is released, by
> > > > which point the filesystem is fully consistent.
> > > > 
> > > > The whole-disk device is resolved using libsystemd's sd-device API
> > > > (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype())
> > > > rather than parsing sysfs, so the lock covers the same device node
> > > > that udevd itself resolves and locks.
> > > > 
> > > > This locking is entirely optional at build time where configure checks for
> > > > sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB, and
> > > > e2fsck.c only compiles in the locking code when it is found. On platforms
> > > > without libsystemd there is no udevd to race against in the first place,
> > > > so nothing is lost by skipping it there.
> > > > 
> > > > Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> > > > ---
> > > > 
> > > > Changes in v3:
> > > >     - Resolve the parent whole-disk device using libsystemd's sd-device API
> > > >       (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype(),
> > > >       sd_device_get_devname()) instead of parsing sysfs paths
> > > >     - Gate the feature on autoconf detecting
> > > >       sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB; compile it
> > > >       out entirely when libsystemd is absent, rather than falling back to another
> > > >       implementation
> > > >     - Use sd_device_new_from_devnum() rather than the sd_device_new_from_devname()
> > > >       used by the systemd.io example, since the former has been available in
> > > >       libsystemd for much longer, keeping the version floor as low as possible
> > > >     - Keep lock_whole_disk()'s signature unconditional and provide a stub returning
> > > >       -1 on the #else branch, so main()'s primary code path has no #ifdefs in it
> > > >     
> > > > Changes in v2:
> > > >     - Guard the whole feature (function, call site, lock_fd, cleanup) rather than
> > > >       just the function body, so it is fully absent from non-Linux builds 
> > > >     - Collapse the three HAVE_SYS_*_H header guards into a single conditional
> > > >     - Reuse a single dev_path[] buffer instead of three single-use stack buffers
> > > >     - Keep translatable warning strings on one line each, even past 80 columns
> > > >     - Drop the blanket "could not lock" warning that fired even when the target was
> > > >       not a block device at all (e.g. a regular file), which broke most of
> > > >       "make check"; lock_whole_disk() now only warns at the point of a genuine failure
> > > >     - No change needed for the Ctrl-C/EINTR concern: the SIGINT handler is installed
> > > >       before lock_whole_disk() runs and is not SA_RESTART, so a blocked flock() is
> > > >       interrupted and the existing E2F_FLAG_CANCEL check unwinds cleanly
> > > > 
> > > >  MCONFIG.in         |  1 +
> > > >  configure          | 71 +++++++++++++++++++++++++++++++++++
> > > >  configure.ac       | 14 +++++++
> > > >  e2fsck/Makefile.in |  6 +--
> > > >  e2fsck/unix.c      | 93 ++++++++++++++++++++++++++++++++++++++++++++++
> > > >  lib/config.h.in    |  6 +++
> > > >  6 files changed, 188 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/MCONFIG.in b/MCONFIG.in
> > > > index d66e2f3b..d3055955 100644
> > > > --- a/MCONFIG.in
> > > > +++ b/MCONFIG.in
> > > > @@ -137,6 +137,7 @@ LIBE2P = $(LIB)/libe2p@LIB_EXT@
> > > >  LIBEXT2FS = $(LIB)/libext2fs@LIB_EXT@
> > > >  LIBUUID = @LIBUUID@ @SOCKET_LIB@
> > > >  LIBMAGIC = @MAGIC_LIB@
> > > > +LIBSYSTEMD = @SYSTEMD_LIB@
> > > >  LIBFUSE = @FUSE_LIB@
> > > >  LIBSUPPORT = $(LIBINTL) $(LIB)/libsupport@STATIC_LIB_EXT@
> > > >  LIBBLKID = @LIBBLKID@ @PRIVATE_LIBS_CMT@ $(LIBUUID)
> > > > diff --git a/configure b/configure
> > > > index b04b31af..696dd07c 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -702,6 +702,7 @@ fuse3_CFLAGS
> > > >  CLOCK_GETTIME_LIB
> > > >  ARCHIVE_LIBS
> > > >  ARCHIVE_CFLAGS
> > > > +SYSTEMD_LIB
> > > >  MAGIC_LIB
> > > >  SOCKET_LIB
> > > >  SIZEOF_TIME_T
> > > > @@ -14130,6 +14131,76 @@ if test "$ac_cv_func_dlopen" = yes ; then
> > > >     MAGIC_LIB=$DLOPEN_LIB
> > > >  fi
> > > >  
> > > > +SYSTEMD_LIB=
> > > > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd" >&5
> > > > +printf %s "checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd... " >&6; }
> > > > +if test ${ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype+y}
> > > > +then :
> > > > +  printf %s "(cached) " >&6
> > > > +else case e in #(
> > > > +  e) ac_check_lib_save_LIBS=$LIBS
> > > > +LIBS="-lsystemd  $LIBS"
> > > > +cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> > > > +/* end confdefs.h.  */
> > > > +
> > > > +/* Override any GCC internal prototype to avoid an error.
> > > > +   Use char because int might match the return type of a GCC
> > > > +   builtin and then its argument prototype would still apply.
> > > > +   The 'extern "C"' is for builds by C++ compilers;
> > > > +   although this is not generally supported in C code supporting it here
> > > > +   has little cost and some practical benefit (sr 110532).  */
> > > > +#ifdef __cplusplus
> > > > +extern "C"
> > > > +#endif
> > > > +char sd_device_get_parent_with_subsystem_devtype (void);
> > > > +int
> > > > +main (void)
> > > > +{
> > > > +return sd_device_get_parent_with_subsystem_devtype ();
> > > > +  ;
> > > > +  return 0;
> > > > +}
> > > > +_ACEOF
> > > > +if ac_fn_c_try_link "$LINENO"
> > > > +then :
> > > > +  ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=yes
> > > > +else case e in #(
> > > > +  e) ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=no ;;
> > > > +esac
> > > > +fi
> > > > +rm -f core conftest.err conftest.$ac_objext conftest.beam \
> > > > +    conftest$ac_exeext conftest.$ac_ext
> > > > +LIBS=$ac_check_lib_save_LIBS ;;
> > > > +esac
> > > > +fi
> > > > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&5
> > > > +printf "%s\n" "$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&6; }
> > > > +if test "x$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" = xyes
> > > > +then :
> > > > +  SYSTEMD_LIB=-lsystemd
> > > > +fi
> > > > +
> > > > +if test -n "$SYSTEMD_LIB"; then
> > > > +	       for ac_header in systemd/sd-device.h
> > > > +do :
> > > > +  ac_fn_c_check_header_compile "$LINENO" "systemd/sd-device.h" "ac_cv_header_systemd_sd_device_h" "$ac_includes_default"
> > > > +if test "x$ac_cv_header_systemd_sd_device_h" = xyes
> > > > +then :
> > > > +  printf "%s\n" "#define HAVE_SYSTEMD_SD_DEVICE_H 1" >>confdefs.h
> > > > +
> > > > +else case e in #(
> > > > +  e) SYSTEMD_LIB= ;;
> > > > +esac
> > > > +fi
> > > > +
> > > > +done
> > > > +fi
> > > > +if test -n "$SYSTEMD_LIB"; then
> > > > +
> > > > +printf "%s\n" "#define HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE 1" >>confdefs.h
> > > > +
> > > > +fi
> > > > +
> > > >  
> > > >  # Check whether --with-libarchive was given.
> > > >  if test ${with_libarchive+y}
> > > > diff --git a/configure.ac b/configure.ac
> > > > index 4921f81f..9f585aae 100644
> > > > --- a/configure.ac
> > > > +++ b/configure.ac
> > > > @@ -1309,6 +1309,20 @@ if test "$ac_cv_func_dlopen" = yes ; then
> > > >  fi
> > > >  AC_SUBST(MAGIC_LIB)
> > > >  dnl
> > > > +dnl See if libsystemd has sd-device APIs for whole-disk BSD locking
> > > > +dnl
> > > > +SYSTEMD_LIB=
> > > > +AC_CHECK_LIB(systemd, sd_device_get_parent_with_subsystem_devtype,
> > > > +	[SYSTEMD_LIB=-lsystemd])
> > > > +if test -n "$SYSTEMD_LIB"; then
> > > > +	AC_CHECK_HEADERS([systemd/sd-device.h], , [SYSTEMD_LIB=])
> > > > +fi
> > > > +if test -n "$SYSTEMD_LIB"; then
> > > > +	AC_DEFINE(HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE, 1,
> > > > +		[Define to 1 if libsystemd has sd-device APIs for whole-disk locking])
> > > > +fi
> > > > +AC_SUBST(SYSTEMD_LIB)
> > > > +dnl
> > > >  dnl libarchive
> > > >  dnl
> > > >  AC_ARG_WITH([libarchive],
> > > > diff --git a/e2fsck/Makefile.in b/e2fsck/Makefile.in
> > > > index fbb7b156..a050eafe 100644
> > > > --- a/e2fsck/Makefile.in
> > > > +++ b/e2fsck/Makefile.in
> > > > @@ -17,20 +17,20 @@ MANPAGES=	e2fsck.8
> > > >  FMANPAGES=	e2fsck.conf.5
> > > >  
> > > >  LIBS= $(LIBSUPPORT) $(LIBEXT2FS) $(LIBCOM_ERR) $(LIBBLKID) $(LIBUUID) \
> > > > -	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(SYSLIBS)
> > > > +	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > > >  DEPLIBS= $(DEPLIBSUPPORT) $(LIBEXT2FS) $(DEPLIBCOM_ERR) $(DEPLIBBLKID) \
> > > >  	 $(DEPLIBUUID) $(DEPLIBE2P)
> > > >  
> > > >  STATIC_LIBS= $(STATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) $(STATIC_LIBCOM_ERR) \
> > > >  	     $(STATIC_LIBBLKID) $(STATIC_LIBUUID) $(LIBINTL) $(STATIC_LIBE2P) \
> > > > -	     $(LIBMAGIC) $(SYSLIBS)
> > > > +	     $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > > >  STATIC_DEPLIBS= $(DEPSTATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) \
> > > >  		$(DEPSTATIC_LIBCOM_ERR) $(DEPSTATIC_LIBBLKID) \
> > > >  		$(DEPSTATIC_LIBUUID) $(DEPSTATIC_LIBE2P)
> > > >  
> > > >  PROFILED_LIBS= $(PROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> > > >  	       $(PROFILED_LIBCOM_ERR) $(PROFILED_LIBBLKID) $(PROFILED_LIBUUID) \
> > > > -	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(SYSLIBS)
> > > > +	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > > >  PROFILED_DEPLIBS= $(DEPPROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> > > >  		  $(DEPPROFILED_LIBCOM_ERR) $(DEPPROFILED_LIBBLKID) \
> > > >  		  $(DEPPROFILED_LIBUUID) $(DEPPROFILED_LIBE2P)
> > > > diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> > > > index 335ca377..51bc41eb 100644
> > > > --- a/e2fsck/unix.c
> > > > +++ b/e2fsck/unix.c
> > > > @@ -36,6 +36,11 @@ extern int optind;
> > > >  #ifdef HAVE_SYS_IOCTL_H
> > > >  #include <sys/ioctl.h>
> > > >  #endif
> > > > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > > +#include <sys/file.h>
> > > > +#include <sys/stat.h>
> > > > +#include <systemd/sd-device.h>
> > > > +#endif
> > > >  #ifdef HAVE_MALLOC_H
> > > >  #include <malloc.h>
> > > >  #endif
> > > > @@ -1397,6 +1402,88 @@ err:
> > > >  	return retval;
> > > >  }
> > > >  
> > > > +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> > > > +{
> > > > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > > +	struct stat st;
> > > > +	sd_device *dev = NULL, *whole_dev;
> > > > +	const char *subsystem, *devtype, *whole_disk_devname;
> > > > +	int r, fd;
> > > > +
> > > > +	if (stat(dev_name, &st) < 0)
> > > > +		return -1;
> > > > +
> > > > +	if (!S_ISBLK(st.st_mode))
> > > > +		return -1;
> > > > +
> > > > +	r = sd_device_new_from_devnum(&dev, 'b', st.st_rdev);
> > > > +	if (r < 0)
> > > > +		return -1;
> > > > +
> > > > +	r = sd_device_get_subsystem(dev, &subsystem);
> > > > +	if (r < 0 || strcmp(subsystem, "block") != 0) {
> > > > +		sd_device_unref(dev);
> > > > +		return -1;
> > > > +	}
> > > > +
> > > > +	r = sd_device_get_devtype(dev, &devtype);
> > > > +	if (r < 0) {
> > > > +		sd_device_unref(dev);
> > > > +		return -1;
> > > > +	}
> > > > +
> > > > +	/*
> > > > +	 * If dev_name is a partition, resolve the parent whole-disk device
> > > > +	 * so the flock covers the same device node that udevd locks before
> > > > +	 * probing any partition on it.
> > > > +	 */
> > > > +	if (strcmp(devtype, "disk") == 0)
> > > > +		whole_dev = dev;
> > > > +	else {
> > > > +		r = sd_device_get_parent_with_subsystem_devtype(dev, "block",
> > > > +								 "disk", &whole_dev);
> > > > +		if (r < 0) {
> > > > +			log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
> > > > +			sd_device_unref(dev);
> > > > +			return -1;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	r = sd_device_get_devname(whole_dev, &whole_disk_devname);
> > > > +	if (r < 0) {
> > > > +		sd_device_unref(dev);
> > > > +		return -1;
> > > > +	}
> > > > +
> > > > +	fd = open(whole_disk_devname, O_RDONLY | O_CLOEXEC, 0);
> > > > +	if (fd < 0) {
> > > > +		com_err(ctx->program_name, errno,
> > > > +			_("while trying to open %s for locking"), whole_disk_devname);
> > > > +		sd_device_unref(dev);
> > > > +		return -1;
> > > > +	}
> > > > +	sd_device_unref(dev);
> > > > +
> > > > +	while (flock(fd, LOCK_EX) != 0) {
> > > > +		if (errno == EINTR) {
> > > > +			if (ctx->flags & E2F_FLAG_CANCEL) {
> > > > +				close(fd);
> > > > +				return -1;
> > > > +			}
> > > > +			continue;
> > > > +		}
> > > > +		com_err(ctx->program_name, errno,
> > > > +			_("while trying to lock %s"), whole_disk_devname);
> > > > +		close(fd);
> > > > +		return -1;
> > > > +	}
> > > > +
> > > > +	return fd;
> > > > +#else
> > > > +	return -1;
> > > > +#endif /* HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE */
> > > > +}
> > > > +
> > > >  int main (int argc, char *argv[])
> > > >  {
> > > >  	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
> > > > @@ -1413,6 +1500,7 @@ int main (int argc, char *argv[])
> > > >  	int journal_size;
> > > >  	int sysval, sys_page_size = 4096;
> > > >  	int old_bitmaps;
> > > > +	int lock_fd = -1;
> > > >  	__u32 features[3];
> > > >  	char *cp;
> > > >  	enum quota_type qtype;
> > > > @@ -1488,6 +1576,8 @@ int main (int argc, char *argv[])
> > > >  
> > > >  	check_mount(ctx);
> > > >  
> > > > +	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> > > > +
> > > >  	if (!(ctx->options & E2F_OPT_PREEN) &&
> > > >  	    !(ctx->options & E2F_OPT_NO) &&
> > > >  	    !(ctx->options & E2F_OPT_YES)) {
> > > > @@ -2169,6 +2259,9 @@ skip_write:
> > > >  	ext2fs_close_free(&ctx->fs);
> > > >  	free(ctx->journal_name);
> > > >  
> > > > +	if (lock_fd >= 0)
> > > > +		close(lock_fd);
> > > > +
> > > >  	if (ctx->logf)
> > > >  		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> > > >  	e2fsck_free_context(ctx);
> > > > diff --git a/lib/config.h.in b/lib/config.h.in
> > > > index f129abfe..bc3ae4aa 100644
> > > > --- a/lib/config.h.in
> > > > +++ b/lib/config.h.in
> > > > @@ -352,6 +352,9 @@
> > > >  /* Define to 1 if if struct sockaddr contains sa_len */
> > > >  #undef HAVE_SA_LEN
> > > >  
> > > > +/* Define to 1 if libsystemd has sd-device APIs for whole-disk locking */
> > > > +#undef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > > +
> > > >  /* Define to 1 if you have the 'secure_getenv' function. */
> > > >  #undef HAVE_SECURE_GETENV
> > > >  
> > > > @@ -430,6 +433,9 @@
> > > >  /* Define to 1 if you have the 'sysconf' function. */
> > > >  #undef HAVE_SYSCONF
> > > >  
> > > > +/* Define to 1 if you have the <systemd/sd-device.h> header file. */
> > > > +#undef HAVE_SYSTEMD_SD_DEVICE_H
> > > > +
> > > >  /* Define to 1 if you have the <sys/acl.h> header file. */
> > > >  #undef HAVE_SYS_ACL_H
> > > >  
> > > > -- 
> > > > 2.55.0
> > > 
> > > Hi,
> > > 
> > > Gentle ping on this patch.
> > > 
> > > I would appreciate any feedback.
> > > 
> > 
> > Can you clarify why this should go into only fsck.ext4? Doesn't this
> > problem exist for other filesystems too?
> > 
> > I sent an earlier email as well with links to:
> > 
> > 1) fsck used to lock the device, but it surfaced a bug in udevd
> > https://bugs.freedesktop.org/show_bug.cgi?id=79576
> > 
> > 2) because of the bug, fsck stopped locking the device
> > https://github.com/util-linux/util-linux/commit/3bbdae633f4a1dda5f95ee6c61f18a1c8ef12250
> > 
> > 3) the systemd-udevd bug was fixed
> > https://github.com/systemd/systemd/commit/5d354e525a5
> > 
> > To me, it makes more sense for the locking that already exists in fsck
> > to get updated (or reverted) to lock the entire device, using the
> > existing -l param (or maybe a new param like --lock-device,
> > --udevd-lock, etc., if util-linux maintainers don't want to change -l
> > behavior).
> > 
> > Do you see an issue with doing the locking there instead of here in
> > fsck.ext4?
> 
> /sbin/fsck (aka the dispatch wrapper program) doesn't necessarily know
> which block device(s) are going to be opened by a the fsck.$FSTYP
> program that it creates.  It might be able to infer that by opening any
> parameter and performing the udev locking protocol after checking if
> what it opened is a block device, but that wouldn't work for (say) a
> fsck.XXX program for a multi-device filesystem wherein you only need to
> specify one device and it will find the others.

I'm not following how you would have an ext4 filesystem on multiple
devices/partitions, unless you are talking about md or dm, in which
case it's irrelevant for the purposes of systemd-udevd because md/dm
create a single logical device (e.g. /dev/md127) that is what udevd
(and fsck.ext4) actually cares about.

If I'm completely missing what you mean, can you give an example of
what you're talking about?

> 
> That said, this patchset also doesn't handle multi-device ext4
> filesystems (i.e. external jbd2 journal device) because the author

I don't think an external journal is relevant for the purposes of udevd
locking, is it?

> doesn't want to do that.  In their defense, the udev flock()ing protocol
> requires one to determine if an opened block device is a partition; if
> it is, then it requires opening and locking the parent bdev (e.g. sdf1
> -> sdf) instead of locking the original device.  This makes it way more
> complicated for multi-device filesystems because now the client has to
> detect multiple partitions coming from the same underlying device and
> handle that appropriately.  I don't know why the protocol designers made
> that choice.
> 
> I can run "trace-cmd record -e 'flock*'" to observe the locking
> interactions with scsi disk partitions, but for whatever reason I don't
> see any flocking going on if I use kpartx to create the partitions with
> device-mapper.  No idea why that is.
> 
> --D
> 

Re: [PATCH v3] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Darrick J. Wong]

On Thu, Sep 24, 2026 at 04:19:00PM -0400, Dan Streetman wrote:
> 
> 
> On Wed, 23 Sep 2026, Darrick J. Wong wrote:
> 
> > On Wed, Sep 23, 2026 at 03:55:23PM -0400, Dan Streetman wrote:
> > > 
> > > 
> > > On Sun, 20 Sep 2026, Nandakumar Raghavan wrote:
> > > 
> > > > On Thu, Sep 10, 2026 at 05:04:41AM -0700, Nandakumar Raghavan wrote:
> > > > > During journal replay, e2fsck writes the primary superblock back to disk
> > > > > in multiple I/O operations. The payload lands before the checksum, leaving
> > > > > a transient window where the on-disk superblock has a bad checksum.
> > > > > 
> > > > > If udevd processes a change uevent during this window, libblkid probes the
> > > > > primary superblock, finds a checksum mismatch, and concludes the partition
> > > > > has no recognisable filesystem. udev then fires a remove event, wiping all
> > > > > symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> > > > > that depends on those symlinks will fail.
> > > > > 
> > > > > udevd already serialises its own partition probes against whole-disk device
> > > > > access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> > > > > event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> > > > > whole-disk device before opening the filesystem. This forces udevd to defer
> > > > > all probes on that disk until e2fsck exits and the lock is released, by
> > > > > which point the filesystem is fully consistent.
> > > > > 
> > > > > The whole-disk device is resolved using libsystemd's sd-device API
> > > > > (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype())
> > > > > rather than parsing sysfs, so the lock covers the same device node
> > > > > that udevd itself resolves and locks.
> > > > > 
> > > > > This locking is entirely optional at build time where configure checks for
> > > > > sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB, and
> > > > > e2fsck.c only compiles in the locking code when it is found. On platforms
> > > > > without libsystemd there is no udevd to race against in the first place,
> > > > > so nothing is lost by skipping it there.
> > > > > 
> > > > > Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> > > > > ---
> > > > > 
> > > > > Changes in v3:
> > > > >     - Resolve the parent whole-disk device using libsystemd's sd-device API
> > > > >       (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype(),
> > > > >       sd_device_get_devname()) instead of parsing sysfs paths
> > > > >     - Gate the feature on autoconf detecting
> > > > >       sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB; compile it
> > > > >       out entirely when libsystemd is absent, rather than falling back to another
> > > > >       implementation
> > > > >     - Use sd_device_new_from_devnum() rather than the sd_device_new_from_devname()
> > > > >       used by the systemd.io example, since the former has been available in
> > > > >       libsystemd for much longer, keeping the version floor as low as possible
> > > > >     - Keep lock_whole_disk()'s signature unconditional and provide a stub returning
> > > > >       -1 on the #else branch, so main()'s primary code path has no #ifdefs in it
> > > > >     
> > > > > Changes in v2:
> > > > >     - Guard the whole feature (function, call site, lock_fd, cleanup) rather than
> > > > >       just the function body, so it is fully absent from non-Linux builds 
> > > > >     - Collapse the three HAVE_SYS_*_H header guards into a single conditional
> > > > >     - Reuse a single dev_path[] buffer instead of three single-use stack buffers
> > > > >     - Keep translatable warning strings on one line each, even past 80 columns
> > > > >     - Drop the blanket "could not lock" warning that fired even when the target was
> > > > >       not a block device at all (e.g. a regular file), which broke most of
> > > > >       "make check"; lock_whole_disk() now only warns at the point of a genuine failure
> > > > >     - No change needed for the Ctrl-C/EINTR concern: the SIGINT handler is installed
> > > > >       before lock_whole_disk() runs and is not SA_RESTART, so a blocked flock() is
> > > > >       interrupted and the existing E2F_FLAG_CANCEL check unwinds cleanly
> > > > > 
> > > > >  MCONFIG.in         |  1 +
> > > > >  configure          | 71 +++++++++++++++++++++++++++++++++++
> > > > >  configure.ac       | 14 +++++++
> > > > >  e2fsck/Makefile.in |  6 +--
> > > > >  e2fsck/unix.c      | 93 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  lib/config.h.in    |  6 +++
> > > > >  6 files changed, 188 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/MCONFIG.in b/MCONFIG.in
> > > > > index d66e2f3b..d3055955 100644
> > > > > --- a/MCONFIG.in
> > > > > +++ b/MCONFIG.in
> > > > > @@ -137,6 +137,7 @@ LIBE2P = $(LIB)/libe2p@LIB_EXT@
> > > > >  LIBEXT2FS = $(LIB)/libext2fs@LIB_EXT@
> > > > >  LIBUUID = @LIBUUID@ @SOCKET_LIB@
> > > > >  LIBMAGIC = @MAGIC_LIB@
> > > > > +LIBSYSTEMD = @SYSTEMD_LIB@
> > > > >  LIBFUSE = @FUSE_LIB@
> > > > >  LIBSUPPORT = $(LIBINTL) $(LIB)/libsupport@STATIC_LIB_EXT@
> > > > >  LIBBLKID = @LIBBLKID@ @PRIVATE_LIBS_CMT@ $(LIBUUID)
> > > > > diff --git a/configure b/configure
> > > > > index b04b31af..696dd07c 100755
> > > > > --- a/configure
> > > > > +++ b/configure
> > > > > @@ -702,6 +702,7 @@ fuse3_CFLAGS
> > > > >  CLOCK_GETTIME_LIB
> > > > >  ARCHIVE_LIBS
> > > > >  ARCHIVE_CFLAGS
> > > > > +SYSTEMD_LIB
> > > > >  MAGIC_LIB
> > > > >  SOCKET_LIB
> > > > >  SIZEOF_TIME_T
> > > > > @@ -14130,6 +14131,76 @@ if test "$ac_cv_func_dlopen" = yes ; then
> > > > >     MAGIC_LIB=$DLOPEN_LIB
> > > > >  fi
> > > > >  
> > > > > +SYSTEMD_LIB=
> > > > > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd" >&5
> > > > > +printf %s "checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd... " >&6; }
> > > > > +if test ${ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype+y}
> > > > > +then :
> > > > > +  printf %s "(cached) " >&6
> > > > > +else case e in #(
> > > > > +  e) ac_check_lib_save_LIBS=$LIBS
> > > > > +LIBS="-lsystemd  $LIBS"
> > > > > +cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> > > > > +/* end confdefs.h.  */
> > > > > +
> > > > > +/* Override any GCC internal prototype to avoid an error.
> > > > > +   Use char because int might match the return type of a GCC
> > > > > +   builtin and then its argument prototype would still apply.
> > > > > +   The 'extern "C"' is for builds by C++ compilers;
> > > > > +   although this is not generally supported in C code supporting it here
> > > > > +   has little cost and some practical benefit (sr 110532).  */
> > > > > +#ifdef __cplusplus
> > > > > +extern "C"
> > > > > +#endif
> > > > > +char sd_device_get_parent_with_subsystem_devtype (void);
> > > > > +int
> > > > > +main (void)
> > > > > +{
> > > > > +return sd_device_get_parent_with_subsystem_devtype ();
> > > > > +  ;
> > > > > +  return 0;
> > > > > +}
> > > > > +_ACEOF
> > > > > +if ac_fn_c_try_link "$LINENO"
> > > > > +then :
> > > > > +  ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=yes
> > > > > +else case e in #(
> > > > > +  e) ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=no ;;
> > > > > +esac
> > > > > +fi
> > > > > +rm -f core conftest.err conftest.$ac_objext conftest.beam \
> > > > > +    conftest$ac_exeext conftest.$ac_ext
> > > > > +LIBS=$ac_check_lib_save_LIBS ;;
> > > > > +esac
> > > > > +fi
> > > > > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&5
> > > > > +printf "%s\n" "$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&6; }
> > > > > +if test "x$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" = xyes
> > > > > +then :
> > > > > +  SYSTEMD_LIB=-lsystemd
> > > > > +fi
> > > > > +
> > > > > +if test -n "$SYSTEMD_LIB"; then
> > > > > +	       for ac_header in systemd/sd-device.h
> > > > > +do :
> > > > > +  ac_fn_c_check_header_compile "$LINENO" "systemd/sd-device.h" "ac_cv_header_systemd_sd_device_h" "$ac_includes_default"
> > > > > +if test "x$ac_cv_header_systemd_sd_device_h" = xyes
> > > > > +then :
> > > > > +  printf "%s\n" "#define HAVE_SYSTEMD_SD_DEVICE_H 1" >>confdefs.h
> > > > > +
> > > > > +else case e in #(
> > > > > +  e) SYSTEMD_LIB= ;;
> > > > > +esac
> > > > > +fi
> > > > > +
> > > > > +done
> > > > > +fi
> > > > > +if test -n "$SYSTEMD_LIB"; then
> > > > > +
> > > > > +printf "%s\n" "#define HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE 1" >>confdefs.h
> > > > > +
> > > > > +fi
> > > > > +
> > > > >  
> > > > >  # Check whether --with-libarchive was given.
> > > > >  if test ${with_libarchive+y}
> > > > > diff --git a/configure.ac b/configure.ac
> > > > > index 4921f81f..9f585aae 100644
> > > > > --- a/configure.ac
> > > > > +++ b/configure.ac
> > > > > @@ -1309,6 +1309,20 @@ if test "$ac_cv_func_dlopen" = yes ; then
> > > > >  fi
> > > > >  AC_SUBST(MAGIC_LIB)
> > > > >  dnl
> > > > > +dnl See if libsystemd has sd-device APIs for whole-disk BSD locking
> > > > > +dnl
> > > > > +SYSTEMD_LIB=
> > > > > +AC_CHECK_LIB(systemd, sd_device_get_parent_with_subsystem_devtype,
> > > > > +	[SYSTEMD_LIB=-lsystemd])
> > > > > +if test -n "$SYSTEMD_LIB"; then
> > > > > +	AC_CHECK_HEADERS([systemd/sd-device.h], , [SYSTEMD_LIB=])
> > > > > +fi
> > > > > +if test -n "$SYSTEMD_LIB"; then
> > > > > +	AC_DEFINE(HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE, 1,
> > > > > +		[Define to 1 if libsystemd has sd-device APIs for whole-disk locking])
> > > > > +fi
> > > > > +AC_SUBST(SYSTEMD_LIB)
> > > > > +dnl
> > > > >  dnl libarchive
> > > > >  dnl
> > > > >  AC_ARG_WITH([libarchive],
> > > > > diff --git a/e2fsck/Makefile.in b/e2fsck/Makefile.in
> > > > > index fbb7b156..a050eafe 100644
> > > > > --- a/e2fsck/Makefile.in
> > > > > +++ b/e2fsck/Makefile.in
> > > > > @@ -17,20 +17,20 @@ MANPAGES=	e2fsck.8
> > > > >  FMANPAGES=	e2fsck.conf.5
> > > > >  
> > > > >  LIBS= $(LIBSUPPORT) $(LIBEXT2FS) $(LIBCOM_ERR) $(LIBBLKID) $(LIBUUID) \
> > > > > -	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(SYSLIBS)
> > > > > +	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > > > >  DEPLIBS= $(DEPLIBSUPPORT) $(LIBEXT2FS) $(DEPLIBCOM_ERR) $(DEPLIBBLKID) \
> > > > >  	 $(DEPLIBUUID) $(DEPLIBE2P)
> > > > >  
> > > > >  STATIC_LIBS= $(STATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) $(STATIC_LIBCOM_ERR) \
> > > > >  	     $(STATIC_LIBBLKID) $(STATIC_LIBUUID) $(LIBINTL) $(STATIC_LIBE2P) \
> > > > > -	     $(LIBMAGIC) $(SYSLIBS)
> > > > > +	     $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > > > >  STATIC_DEPLIBS= $(DEPSTATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) \
> > > > >  		$(DEPSTATIC_LIBCOM_ERR) $(DEPSTATIC_LIBBLKID) \
> > > > >  		$(DEPSTATIC_LIBUUID) $(DEPSTATIC_LIBE2P)
> > > > >  
> > > > >  PROFILED_LIBS= $(PROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> > > > >  	       $(PROFILED_LIBCOM_ERR) $(PROFILED_LIBBLKID) $(PROFILED_LIBUUID) \
> > > > > -	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(SYSLIBS)
> > > > > +	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > > > >  PROFILED_DEPLIBS= $(DEPPROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> > > > >  		  $(DEPPROFILED_LIBCOM_ERR) $(DEPPROFILED_LIBBLKID) \
> > > > >  		  $(DEPPROFILED_LIBUUID) $(DEPPROFILED_LIBE2P)
> > > > > diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> > > > > index 335ca377..51bc41eb 100644
> > > > > --- a/e2fsck/unix.c
> > > > > +++ b/e2fsck/unix.c
> > > > > @@ -36,6 +36,11 @@ extern int optind;
> > > > >  #ifdef HAVE_SYS_IOCTL_H
> > > > >  #include <sys/ioctl.h>
> > > > >  #endif
> > > > > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > > > +#include <sys/file.h>
> > > > > +#include <sys/stat.h>
> > > > > +#include <systemd/sd-device.h>
> > > > > +#endif
> > > > >  #ifdef HAVE_MALLOC_H
> > > > >  #include <malloc.h>
> > > > >  #endif
> > > > > @@ -1397,6 +1402,88 @@ err:
> > > > >  	return retval;
> > > > >  }
> > > > >  
> > > > > +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> > > > > +{
> > > > > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > > > +	struct stat st;
> > > > > +	sd_device *dev = NULL, *whole_dev;
> > > > > +	const char *subsystem, *devtype, *whole_disk_devname;
> > > > > +	int r, fd;
> > > > > +
> > > > > +	if (stat(dev_name, &st) < 0)
> > > > > +		return -1;
> > > > > +
> > > > > +	if (!S_ISBLK(st.st_mode))
> > > > > +		return -1;
> > > > > +
> > > > > +	r = sd_device_new_from_devnum(&dev, 'b', st.st_rdev);
> > > > > +	if (r < 0)
> > > > > +		return -1;
> > > > > +
> > > > > +	r = sd_device_get_subsystem(dev, &subsystem);
> > > > > +	if (r < 0 || strcmp(subsystem, "block") != 0) {
> > > > > +		sd_device_unref(dev);
> > > > > +		return -1;
> > > > > +	}
> > > > > +
> > > > > +	r = sd_device_get_devtype(dev, &devtype);
> > > > > +	if (r < 0) {
> > > > > +		sd_device_unref(dev);
> > > > > +		return -1;
> > > > > +	}
> > > > > +
> > > > > +	/*
> > > > > +	 * If dev_name is a partition, resolve the parent whole-disk device
> > > > > +	 * so the flock covers the same device node that udevd locks before
> > > > > +	 * probing any partition on it.
> > > > > +	 */
> > > > > +	if (strcmp(devtype, "disk") == 0)
> > > > > +		whole_dev = dev;
> > > > > +	else {
> > > > > +		r = sd_device_get_parent_with_subsystem_devtype(dev, "block",
> > > > > +								 "disk", &whole_dev);
> > > > > +		if (r < 0) {
> > > > > +			log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
> > > > > +			sd_device_unref(dev);
> > > > > +			return -1;
> > > > > +		}
> > > > > +	}
> > > > > +
> > > > > +	r = sd_device_get_devname(whole_dev, &whole_disk_devname);
> > > > > +	if (r < 0) {
> > > > > +		sd_device_unref(dev);
> > > > > +		return -1;
> > > > > +	}
> > > > > +
> > > > > +	fd = open(whole_disk_devname, O_RDONLY | O_CLOEXEC, 0);
> > > > > +	if (fd < 0) {
> > > > > +		com_err(ctx->program_name, errno,
> > > > > +			_("while trying to open %s for locking"), whole_disk_devname);
> > > > > +		sd_device_unref(dev);
> > > > > +		return -1;
> > > > > +	}
> > > > > +	sd_device_unref(dev);
> > > > > +
> > > > > +	while (flock(fd, LOCK_EX) != 0) {
> > > > > +		if (errno == EINTR) {
> > > > > +			if (ctx->flags & E2F_FLAG_CANCEL) {
> > > > > +				close(fd);
> > > > > +				return -1;
> > > > > +			}
> > > > > +			continue;
> > > > > +		}
> > > > > +		com_err(ctx->program_name, errno,
> > > > > +			_("while trying to lock %s"), whole_disk_devname);
> > > > > +		close(fd);
> > > > > +		return -1;
> > > > > +	}
> > > > > +
> > > > > +	return fd;
> > > > > +#else
> > > > > +	return -1;
> > > > > +#endif /* HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE */
> > > > > +}
> > > > > +
> > > > >  int main (int argc, char *argv[])
> > > > >  {
> > > > >  	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
> > > > > @@ -1413,6 +1500,7 @@ int main (int argc, char *argv[])
> > > > >  	int journal_size;
> > > > >  	int sysval, sys_page_size = 4096;
> > > > >  	int old_bitmaps;
> > > > > +	int lock_fd = -1;
> > > > >  	__u32 features[3];
> > > > >  	char *cp;
> > > > >  	enum quota_type qtype;
> > > > > @@ -1488,6 +1576,8 @@ int main (int argc, char *argv[])
> > > > >  
> > > > >  	check_mount(ctx);
> > > > >  
> > > > > +	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> > > > > +
> > > > >  	if (!(ctx->options & E2F_OPT_PREEN) &&
> > > > >  	    !(ctx->options & E2F_OPT_NO) &&
> > > > >  	    !(ctx->options & E2F_OPT_YES)) {
> > > > > @@ -2169,6 +2259,9 @@ skip_write:
> > > > >  	ext2fs_close_free(&ctx->fs);
> > > > >  	free(ctx->journal_name);
> > > > >  
> > > > > +	if (lock_fd >= 0)
> > > > > +		close(lock_fd);
> > > > > +
> > > > >  	if (ctx->logf)
> > > > >  		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> > > > >  	e2fsck_free_context(ctx);
> > > > > diff --git a/lib/config.h.in b/lib/config.h.in
> > > > > index f129abfe..bc3ae4aa 100644
> > > > > --- a/lib/config.h.in
> > > > > +++ b/lib/config.h.in
> > > > > @@ -352,6 +352,9 @@
> > > > >  /* Define to 1 if if struct sockaddr contains sa_len */
> > > > >  #undef HAVE_SA_LEN
> > > > >  
> > > > > +/* Define to 1 if libsystemd has sd-device APIs for whole-disk locking */
> > > > > +#undef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > > > +
> > > > >  /* Define to 1 if you have the 'secure_getenv' function. */
> > > > >  #undef HAVE_SECURE_GETENV
> > > > >  
> > > > > @@ -430,6 +433,9 @@
> > > > >  /* Define to 1 if you have the 'sysconf' function. */
> > > > >  #undef HAVE_SYSCONF
> > > > >  
> > > > > +/* Define to 1 if you have the <systemd/sd-device.h> header file. */
> > > > > +#undef HAVE_SYSTEMD_SD_DEVICE_H
> > > > > +
> > > > >  /* Define to 1 if you have the <sys/acl.h> header file. */
> > > > >  #undef HAVE_SYS_ACL_H
> > > > >  
> > > > > -- 
> > > > > 2.55.0
> > > > 
> > > > Hi,
> > > > 
> > > > Gentle ping on this patch.
> > > > 
> > > > I would appreciate any feedback.
> > > > 
> > > 
> > > Can you clarify why this should go into only fsck.ext4? Doesn't this
> > > problem exist for other filesystems too?
> > > 
> > > I sent an earlier email as well with links to:
> > > 
> > > 1) fsck used to lock the device, but it surfaced a bug in udevd
> > > https://bugs.freedesktop.org/show_bug.cgi?id=79576
> > > 
> > > 2) because of the bug, fsck stopped locking the device
> > > https://github.com/util-linux/util-linux/commit/3bbdae633f4a1dda5f95ee6c61f18a1c8ef12250
> > > 
> > > 3) the systemd-udevd bug was fixed
> > > https://github.com/systemd/systemd/commit/5d354e525a5
> > > 
> > > To me, it makes more sense for the locking that already exists in fsck
> > > to get updated (or reverted) to lock the entire device, using the
> > > existing -l param (or maybe a new param like --lock-device,
> > > --udevd-lock, etc., if util-linux maintainers don't want to change -l
> > > behavior).
> > > 
> > > Do you see an issue with doing the locking there instead of here in
> > > fsck.ext4?
> > 
> > /sbin/fsck (aka the dispatch wrapper program) doesn't necessarily know
> > which block device(s) are going to be opened by a the fsck.$FSTYP
> > program that it creates.  It might be able to infer that by opening any
> > parameter and performing the udev locking protocol after checking if
> > what it opened is a block device, but that wouldn't work for (say) a
> > fsck.XXX program for a multi-device filesystem wherein you only need to
> > specify one device and it will find the others.
> 
> I'm not following how you would have an ext4 filesystem on multiple
> devices/partitions, unless you are talking about md or dm, in which
> case it's irrelevant for the purposes of systemd-udevd because md/dm
> create a single logical device (e.g. /dev/md127) that is what udevd
> (and fsck.ext4) actually cares about.

Many Linux filesystems can attach to multiple block devices and store
data on them: XFS, btrfs, and bcachefs.  ext4 doesn't store any of its
own data on the journal device, but it still has to open the journal
device.

> If I'm completely missing what you mean, can you give an example of
> what you're talking about?

ext4 supports having a separate blockdev for a journal:

# mkfs.ext4 -O journal_dev /dev/sdb
mke2fs 1.47.2 (1-Jan-2025)
Discarding device blocks: done                            
Creating filesystem with 2579968 4k blocks and 0 inodes
Filesystem UUID: e11a1211-fdd7-408a-b342-a131692c2356
Superblock backups stored on blocks: 

Zeroing journal device:                
# mkfs.ext4 -J device=/dev/sdb -F /dev/sda
mke2fs 1.47.2 (1-Jan-2025)
Using journal device's blocksize: 4096
Discarding device blocks: done                            
Creating filesystem with 2579968 4k blocks and 645904 inodes
Filesystem UUID: a1789ae9-76e7-47e3-9972-5be0b8505517
Superblock backups stored on blocks: 
        32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632

Allocating group tables: done                            
Writing inode tables: done                            
Adding journal to device /dev/sdb: done
Writing superblocks and filesystem accounting information: done 

# blkid /dev/sd[ab]
/dev/sda: UUID="a1789ae9-76e7-47e3-9972-5be0b8505517" EXT_JOURNAL="e11a1211-fdd7-408a-b342-a131692c2356" BLOCK_SIZE="4096" TYPE="ext4"
/dev/sdb: UUID="e11a1211-fdd7-408a-b342-a131692c2356" BLOCK_SIZE="4096" LOGUUID="e11a1211-fdd7-408a-b342-a131692c2356" TYPE="jbd"

Note that you don't have to pass the journal to e2fsck:

# e2fsck -fn /dev/sda
e2fsck 1.47.5~WIP-2026-03-08 (8-Mar-2026)
Pass 1: Checking inodes, blocks, and sizes
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information
/dev/sda: 12/645904 files (0.0% non-contiguous), 50288/2579968 blocks

> > That said, this patchset also doesn't handle multi-device ext4
> > filesystems (i.e. external jbd2 journal device) because the author
> 
> I don't think an external journal is relevant for the purposes of udevd
> locking, is it?

Yes it is, e2fsck could need to replay the journal contents, and
presumably you want the same protection for the secondary devices as the
primary.

--D

> > doesn't want to do that.  In their defense, the udev flock()ing protocol
> > requires one to determine if an opened block device is a partition; if
> > it is, then it requires opening and locking the parent bdev (e.g. sdf1
> > -> sdf) instead of locking the original device.  This makes it way more
> > complicated for multi-device filesystems because now the client has to
> > detect multiple partitions coming from the same underlying device and
> > handle that appropriately.  I don't know why the protocol designers made
> > that choice.
> > 
> > I can run "trace-cmd record -e 'flock*'" to observe the locking
> > interactions with scsi disk partitions, but for whatever reason I don't
> > see any flocking going on if I use kpartx to create the partitions with
> > device-mapper.  No idea why that is.
> > 
> > --D
> > 
> 

Re: [PATCH v3] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Nandakumar Raghavan]

On Thu, Sep 24, 2026 at 02:03:20PM -0700, Darrick J. Wong wrote:
> On Thu, Sep 24, 2026 at 04:19:00PM -0400, Dan Streetman wrote:
> > 
> > 
> > On Wed, 23 Sep 2026, Darrick J. Wong wrote:
> > 
> > > On Wed, Sep 23, 2026 at 03:55:23PM -0400, Dan Streetman wrote:
> > > > 
> > > > 
> > > > On Sun, 20 Sep 2026, Nandakumar Raghavan wrote:
> > > > 
> > > > > On Thu, Sep 10, 2026 at 05:04:41AM -0700, Nandakumar Raghavan wrote:
> > > > > > During journal replay, e2fsck writes the primary superblock back to disk
> > > > > > in multiple I/O operations. The payload lands before the checksum, leaving
> > > > > > a transient window where the on-disk superblock has a bad checksum.
> > > > > > 
> > > > > > If udevd processes a change uevent during this window, libblkid probes the
> > > > > > primary superblock, finds a checksum mismatch, and concludes the partition
> > > > > > has no recognisable filesystem. udev then fires a remove event, wiping all
> > > > > > symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> > > > > > that depends on those symlinks will fail.
> > > > > > 
> > > > > > udevd already serialises its own partition probes against whole-disk device
> > > > > > access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> > > > > > event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> > > > > > whole-disk device before opening the filesystem. This forces udevd to defer
> > > > > > all probes on that disk until e2fsck exits and the lock is released, by
> > > > > > which point the filesystem is fully consistent.
> > > > > > 
> > > > > > The whole-disk device is resolved using libsystemd's sd-device API
> > > > > > (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype())
> > > > > > rather than parsing sysfs, so the lock covers the same device node
> > > > > > that udevd itself resolves and locks.
> > > > > > 
> > > > > > This locking is entirely optional at build time where configure checks for
> > > > > > sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB, and
> > > > > > e2fsck.c only compiles in the locking code when it is found. On platforms
> > > > > > without libsystemd there is no udevd to race against in the first place,
> > > > > > so nothing is lost by skipping it there.
> > > > > > 
> > > > > > Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> > > > > > ---
> > > > > > 
> > > > > > Changes in v3:
> > > > > >     - Resolve the parent whole-disk device using libsystemd's sd-device API
> > > > > >       (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype(),
> > > > > >       sd_device_get_devname()) instead of parsing sysfs paths
> > > > > >     - Gate the feature on autoconf detecting
> > > > > >       sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB; compile it
> > > > > >       out entirely when libsystemd is absent, rather than falling back to another
> > > > > >       implementation
> > > > > >     - Use sd_device_new_from_devnum() rather than the sd_device_new_from_devname()
> > > > > >       used by the systemd.io example, since the former has been available in
> > > > > >       libsystemd for much longer, keeping the version floor as low as possible
> > > > > >     - Keep lock_whole_disk()'s signature unconditional and provide a stub returning
> > > > > >       -1 on the #else branch, so main()'s primary code path has no #ifdefs in it
> > > > > >     
> > > > > > Changes in v2:
> > > > > >     - Guard the whole feature (function, call site, lock_fd, cleanup) rather than
> > > > > >       just the function body, so it is fully absent from non-Linux builds 
> > > > > >     - Collapse the three HAVE_SYS_*_H header guards into a single conditional
> > > > > >     - Reuse a single dev_path[] buffer instead of three single-use stack buffers
> > > > > >     - Keep translatable warning strings on one line each, even past 80 columns
> > > > > >     - Drop the blanket "could not lock" warning that fired even when the target was
> > > > > >       not a block device at all (e.g. a regular file), which broke most of
> > > > > >       "make check"; lock_whole_disk() now only warns at the point of a genuine failure
> > > > > >     - No change needed for the Ctrl-C/EINTR concern: the SIGINT handler is installed
> > > > > >       before lock_whole_disk() runs and is not SA_RESTART, so a blocked flock() is
> > > > > >       interrupted and the existing E2F_FLAG_CANCEL check unwinds cleanly
> > > > > > 
> > > > > >  MCONFIG.in         |  1 +
> > > > > >  configure          | 71 +++++++++++++++++++++++++++++++++++
> > > > > >  configure.ac       | 14 +++++++
> > > > > >  e2fsck/Makefile.in |  6 +--
> > > > > >  e2fsck/unix.c      | 93 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  lib/config.h.in    |  6 +++
> > > > > >  6 files changed, 188 insertions(+), 3 deletions(-)
> > > > > > 
> > > > > > diff --git a/MCONFIG.in b/MCONFIG.in
> > > > > > index d66e2f3b..d3055955 100644
> > > > > > --- a/MCONFIG.in
> > > > > > +++ b/MCONFIG.in
> > > > > > @@ -137,6 +137,7 @@ LIBE2P = $(LIB)/libe2p@LIB_EXT@
> > > > > >  LIBEXT2FS = $(LIB)/libext2fs@LIB_EXT@
> > > > > >  LIBUUID = @LIBUUID@ @SOCKET_LIB@
> > > > > >  LIBMAGIC = @MAGIC_LIB@
> > > > > > +LIBSYSTEMD = @SYSTEMD_LIB@
> > > > > >  LIBFUSE = @FUSE_LIB@
> > > > > >  LIBSUPPORT = $(LIBINTL) $(LIB)/libsupport@STATIC_LIB_EXT@
> > > > > >  LIBBLKID = @LIBBLKID@ @PRIVATE_LIBS_CMT@ $(LIBUUID)
> > > > > > diff --git a/configure b/configure
> > > > > > index b04b31af..696dd07c 100755
> > > > > > --- a/configure
> > > > > > +++ b/configure
> > > > > > @@ -702,6 +702,7 @@ fuse3_CFLAGS
> > > > > >  CLOCK_GETTIME_LIB
> > > > > >  ARCHIVE_LIBS
> > > > > >  ARCHIVE_CFLAGS
> > > > > > +SYSTEMD_LIB
> > > > > >  MAGIC_LIB
> > > > > >  SOCKET_LIB
> > > > > >  SIZEOF_TIME_T
> > > > > > @@ -14130,6 +14131,76 @@ if test "$ac_cv_func_dlopen" = yes ; then
> > > > > >     MAGIC_LIB=$DLOPEN_LIB
> > > > > >  fi
> > > > > >  
> > > > > > +SYSTEMD_LIB=
> > > > > > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd" >&5
> > > > > > +printf %s "checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd... " >&6; }
> > > > > > +if test ${ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype+y}
> > > > > > +then :
> > > > > > +  printf %s "(cached) " >&6
> > > > > > +else case e in #(
> > > > > > +  e) ac_check_lib_save_LIBS=$LIBS
> > > > > > +LIBS="-lsystemd  $LIBS"
> > > > > > +cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> > > > > > +/* end confdefs.h.  */
> > > > > > +
> > > > > > +/* Override any GCC internal prototype to avoid an error.
> > > > > > +   Use char because int might match the return type of a GCC
> > > > > > +   builtin and then its argument prototype would still apply.
> > > > > > +   The 'extern "C"' is for builds by C++ compilers;
> > > > > > +   although this is not generally supported in C code supporting it here
> > > > > > +   has little cost and some practical benefit (sr 110532).  */
> > > > > > +#ifdef __cplusplus
> > > > > > +extern "C"
> > > > > > +#endif
> > > > > > +char sd_device_get_parent_with_subsystem_devtype (void);
> > > > > > +int
> > > > > > +main (void)
> > > > > > +{
> > > > > > +return sd_device_get_parent_with_subsystem_devtype ();
> > > > > > +  ;
> > > > > > +  return 0;
> > > > > > +}
> > > > > > +_ACEOF
> > > > > > +if ac_fn_c_try_link "$LINENO"
> > > > > > +then :
> > > > > > +  ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=yes
> > > > > > +else case e in #(
> > > > > > +  e) ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=no ;;
> > > > > > +esac
> > > > > > +fi
> > > > > > +rm -f core conftest.err conftest.$ac_objext conftest.beam \
> > > > > > +    conftest$ac_exeext conftest.$ac_ext
> > > > > > +LIBS=$ac_check_lib_save_LIBS ;;
> > > > > > +esac
> > > > > > +fi
> > > > > > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&5
> > > > > > +printf "%s\n" "$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&6; }
> > > > > > +if test "x$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" = xyes
> > > > > > +then :
> > > > > > +  SYSTEMD_LIB=-lsystemd
> > > > > > +fi
> > > > > > +
> > > > > > +if test -n "$SYSTEMD_LIB"; then
> > > > > > +	       for ac_header in systemd/sd-device.h
> > > > > > +do :
> > > > > > +  ac_fn_c_check_header_compile "$LINENO" "systemd/sd-device.h" "ac_cv_header_systemd_sd_device_h" "$ac_includes_default"
> > > > > > +if test "x$ac_cv_header_systemd_sd_device_h" = xyes
> > > > > > +then :
> > > > > > +  printf "%s\n" "#define HAVE_SYSTEMD_SD_DEVICE_H 1" >>confdefs.h
> > > > > > +
> > > > > > +else case e in #(
> > > > > > +  e) SYSTEMD_LIB= ;;
> > > > > > +esac
> > > > > > +fi
> > > > > > +
> > > > > > +done
> > > > > > +fi
> > > > > > +if test -n "$SYSTEMD_LIB"; then
> > > > > > +
> > > > > > +printf "%s\n" "#define HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE 1" >>confdefs.h
> > > > > > +
> > > > > > +fi
> > > > > > +
> > > > > >  
> > > > > >  # Check whether --with-libarchive was given.
> > > > > >  if test ${with_libarchive+y}
> > > > > > diff --git a/configure.ac b/configure.ac
> > > > > > index 4921f81f..9f585aae 100644
> > > > > > --- a/configure.ac
> > > > > > +++ b/configure.ac
> > > > > > @@ -1309,6 +1309,20 @@ if test "$ac_cv_func_dlopen" = yes ; then
> > > > > >  fi
> > > > > >  AC_SUBST(MAGIC_LIB)
> > > > > >  dnl
> > > > > > +dnl See if libsystemd has sd-device APIs for whole-disk BSD locking
> > > > > > +dnl
> > > > > > +SYSTEMD_LIB=
> > > > > > +AC_CHECK_LIB(systemd, sd_device_get_parent_with_subsystem_devtype,
> > > > > > +	[SYSTEMD_LIB=-lsystemd])
> > > > > > +if test -n "$SYSTEMD_LIB"; then
> > > > > > +	AC_CHECK_HEADERS([systemd/sd-device.h], , [SYSTEMD_LIB=])
> > > > > > +fi
> > > > > > +if test -n "$SYSTEMD_LIB"; then
> > > > > > +	AC_DEFINE(HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE, 1,
> > > > > > +		[Define to 1 if libsystemd has sd-device APIs for whole-disk locking])
> > > > > > +fi
> > > > > > +AC_SUBST(SYSTEMD_LIB)
> > > > > > +dnl
> > > > > >  dnl libarchive
> > > > > >  dnl
> > > > > >  AC_ARG_WITH([libarchive],
> > > > > > diff --git a/e2fsck/Makefile.in b/e2fsck/Makefile.in
> > > > > > index fbb7b156..a050eafe 100644
> > > > > > --- a/e2fsck/Makefile.in
> > > > > > +++ b/e2fsck/Makefile.in
> > > > > > @@ -17,20 +17,20 @@ MANPAGES=	e2fsck.8
> > > > > >  FMANPAGES=	e2fsck.conf.5
> > > > > >  
> > > > > >  LIBS= $(LIBSUPPORT) $(LIBEXT2FS) $(LIBCOM_ERR) $(LIBBLKID) $(LIBUUID) \
> > > > > > -	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(SYSLIBS)
> > > > > > +	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > > > > >  DEPLIBS= $(DEPLIBSUPPORT) $(LIBEXT2FS) $(DEPLIBCOM_ERR) $(DEPLIBBLKID) \
> > > > > >  	 $(DEPLIBUUID) $(DEPLIBE2P)
> > > > > >  
> > > > > >  STATIC_LIBS= $(STATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) $(STATIC_LIBCOM_ERR) \
> > > > > >  	     $(STATIC_LIBBLKID) $(STATIC_LIBUUID) $(LIBINTL) $(STATIC_LIBE2P) \
> > > > > > -	     $(LIBMAGIC) $(SYSLIBS)
> > > > > > +	     $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > > > > >  STATIC_DEPLIBS= $(DEPSTATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) \
> > > > > >  		$(DEPSTATIC_LIBCOM_ERR) $(DEPSTATIC_LIBBLKID) \
> > > > > >  		$(DEPSTATIC_LIBUUID) $(DEPSTATIC_LIBE2P)
> > > > > >  
> > > > > >  PROFILED_LIBS= $(PROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> > > > > >  	       $(PROFILED_LIBCOM_ERR) $(PROFILED_LIBBLKID) $(PROFILED_LIBUUID) \
> > > > > > -	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(SYSLIBS)
> > > > > > +	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > > > > >  PROFILED_DEPLIBS= $(DEPPROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> > > > > >  		  $(DEPPROFILED_LIBCOM_ERR) $(DEPPROFILED_LIBBLKID) \
> > > > > >  		  $(DEPPROFILED_LIBUUID) $(DEPPROFILED_LIBE2P)
> > > > > > diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> > > > > > index 335ca377..51bc41eb 100644
> > > > > > --- a/e2fsck/unix.c
> > > > > > +++ b/e2fsck/unix.c
> > > > > > @@ -36,6 +36,11 @@ extern int optind;
> > > > > >  #ifdef HAVE_SYS_IOCTL_H
> > > > > >  #include <sys/ioctl.h>
> > > > > >  #endif
> > > > > > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > > > > +#include <sys/file.h>
> > > > > > +#include <sys/stat.h>
> > > > > > +#include <systemd/sd-device.h>
> > > > > > +#endif
> > > > > >  #ifdef HAVE_MALLOC_H
> > > > > >  #include <malloc.h>
> > > > > >  #endif
> > > > > > @@ -1397,6 +1402,88 @@ err:
> > > > > >  	return retval;
> > > > > >  }
> > > > > >  
> > > > > > +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> > > > > > +{
> > > > > > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > > > > +	struct stat st;
> > > > > > +	sd_device *dev = NULL, *whole_dev;
> > > > > > +	const char *subsystem, *devtype, *whole_disk_devname;
> > > > > > +	int r, fd;
> > > > > > +
> > > > > > +	if (stat(dev_name, &st) < 0)
> > > > > > +		return -1;
> > > > > > +
> > > > > > +	if (!S_ISBLK(st.st_mode))
> > > > > > +		return -1;
> > > > > > +
> > > > > > +	r = sd_device_new_from_devnum(&dev, 'b', st.st_rdev);
> > > > > > +	if (r < 0)
> > > > > > +		return -1;
> > > > > > +
> > > > > > +	r = sd_device_get_subsystem(dev, &subsystem);
> > > > > > +	if (r < 0 || strcmp(subsystem, "block") != 0) {
> > > > > > +		sd_device_unref(dev);
> > > > > > +		return -1;
> > > > > > +	}
> > > > > > +
> > > > > > +	r = sd_device_get_devtype(dev, &devtype);
> > > > > > +	if (r < 0) {
> > > > > > +		sd_device_unref(dev);
> > > > > > +		return -1;
> > > > > > +	}
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * If dev_name is a partition, resolve the parent whole-disk device
> > > > > > +	 * so the flock covers the same device node that udevd locks before
> > > > > > +	 * probing any partition on it.
> > > > > > +	 */
> > > > > > +	if (strcmp(devtype, "disk") == 0)
> > > > > > +		whole_dev = dev;
> > > > > > +	else {
> > > > > > +		r = sd_device_get_parent_with_subsystem_devtype(dev, "block",
> > > > > > +								 "disk", &whole_dev);
> > > > > > +		if (r < 0) {
> > > > > > +			log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
> > > > > > +			sd_device_unref(dev);
> > > > > > +			return -1;
> > > > > > +		}
> > > > > > +	}
> > > > > > +
> > > > > > +	r = sd_device_get_devname(whole_dev, &whole_disk_devname);
> > > > > > +	if (r < 0) {
> > > > > > +		sd_device_unref(dev);
> > > > > > +		return -1;
> > > > > > +	}
> > > > > > +
> > > > > > +	fd = open(whole_disk_devname, O_RDONLY | O_CLOEXEC, 0);
> > > > > > +	if (fd < 0) {
> > > > > > +		com_err(ctx->program_name, errno,
> > > > > > +			_("while trying to open %s for locking"), whole_disk_devname);
> > > > > > +		sd_device_unref(dev);
> > > > > > +		return -1;
> > > > > > +	}
> > > > > > +	sd_device_unref(dev);
> > > > > > +
> > > > > > +	while (flock(fd, LOCK_EX) != 0) {
> > > > > > +		if (errno == EINTR) {
> > > > > > +			if (ctx->flags & E2F_FLAG_CANCEL) {
> > > > > > +				close(fd);
> > > > > > +				return -1;
> > > > > > +			}
> > > > > > +			continue;
> > > > > > +		}
> > > > > > +		com_err(ctx->program_name, errno,
> > > > > > +			_("while trying to lock %s"), whole_disk_devname);
> > > > > > +		close(fd);
> > > > > > +		return -1;
> > > > > > +	}
> > > > > > +
> > > > > > +	return fd;
> > > > > > +#else
> > > > > > +	return -1;
> > > > > > +#endif /* HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE */
> > > > > > +}
> > > > > > +
> > > > > >  int main (int argc, char *argv[])
> > > > > >  {
> > > > > >  	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
> > > > > > @@ -1413,6 +1500,7 @@ int main (int argc, char *argv[])
> > > > > >  	int journal_size;
> > > > > >  	int sysval, sys_page_size = 4096;
> > > > > >  	int old_bitmaps;
> > > > > > +	int lock_fd = -1;
> > > > > >  	__u32 features[3];
> > > > > >  	char *cp;
> > > > > >  	enum quota_type qtype;
> > > > > > @@ -1488,6 +1576,8 @@ int main (int argc, char *argv[])
> > > > > >  
> > > > > >  	check_mount(ctx);
> > > > > >  
> > > > > > +	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> > > > > > +
> > > > > >  	if (!(ctx->options & E2F_OPT_PREEN) &&
> > > > > >  	    !(ctx->options & E2F_OPT_NO) &&
> > > > > >  	    !(ctx->options & E2F_OPT_YES)) {
> > > > > > @@ -2169,6 +2259,9 @@ skip_write:
> > > > > >  	ext2fs_close_free(&ctx->fs);
> > > > > >  	free(ctx->journal_name);
> > > > > >  
> > > > > > +	if (lock_fd >= 0)
> > > > > > +		close(lock_fd);
> > > > > > +
> > > > > >  	if (ctx->logf)
> > > > > >  		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> > > > > >  	e2fsck_free_context(ctx);
> > > > > > diff --git a/lib/config.h.in b/lib/config.h.in
> > > > > > index f129abfe..bc3ae4aa 100644
> > > > > > --- a/lib/config.h.in
> > > > > > +++ b/lib/config.h.in
> > > > > > @@ -352,6 +352,9 @@
> > > > > >  /* Define to 1 if if struct sockaddr contains sa_len */
> > > > > >  #undef HAVE_SA_LEN
> > > > > >  
> > > > > > +/* Define to 1 if libsystemd has sd-device APIs for whole-disk locking */
> > > > > > +#undef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > > > > +
> > > > > >  /* Define to 1 if you have the 'secure_getenv' function. */
> > > > > >  #undef HAVE_SECURE_GETENV
> > > > > >  
> > > > > > @@ -430,6 +433,9 @@
> > > > > >  /* Define to 1 if you have the 'sysconf' function. */
> > > > > >  #undef HAVE_SYSCONF
> > > > > >  
> > > > > > +/* Define to 1 if you have the <systemd/sd-device.h> header file. */
> > > > > > +#undef HAVE_SYSTEMD_SD_DEVICE_H
> > > > > > +
> > > > > >  /* Define to 1 if you have the <sys/acl.h> header file. */
> > > > > >  #undef HAVE_SYS_ACL_H
> > > > > >  
> > > > > > -- 
> > > > > > 2.55.0
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > Gentle ping on this patch.
> > > > > 
> > > > > I would appreciate any feedback.
> > > > > 
> > > > 
> > > > Can you clarify why this should go into only fsck.ext4? Doesn't this
> > > > problem exist for other filesystems too?
> > > > 
> > > > I sent an earlier email as well with links to:
> > > > 
> > > > 1) fsck used to lock the device, but it surfaced a bug in udevd
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=79576
> > > > 
> > > > 2) because of the bug, fsck stopped locking the device
> > > > https://github.com/util-linux/util-linux/commit/3bbdae633f4a1dda5f95ee6c61f18a1c8ef12250
> > > > 
> > > > 3) the systemd-udevd bug was fixed
> > > > https://github.com/systemd/systemd/commit/5d354e525a5
> > > > 
> > > > To me, it makes more sense for the locking that already exists in fsck
> > > > to get updated (or reverted) to lock the entire device, using the
> > > > existing -l param (or maybe a new param like --lock-device,
> > > > --udevd-lock, etc., if util-linux maintainers don't want to change -l
> > > > behavior).
> > > > 
> > > > Do you see an issue with doing the locking there instead of here in
> > > > fsck.ext4?
> > > 
> > > /sbin/fsck (aka the dispatch wrapper program) doesn't necessarily know
> > > which block device(s) are going to be opened by a the fsck.$FSTYP
> > > program that it creates.  It might be able to infer that by opening any
> > > parameter and performing the udev locking protocol after checking if
> > > what it opened is a block device, but that wouldn't work for (say) a
> > > fsck.XXX program for a multi-device filesystem wherein you only need to
> > > specify one device and it will find the others.
> > 
> > I'm not following how you would have an ext4 filesystem on multiple
> > devices/partitions, unless you are talking about md or dm, in which
> > case it's irrelevant for the purposes of systemd-udevd because md/dm
> > create a single logical device (e.g. /dev/md127) that is what udevd
> > (and fsck.ext4) actually cares about.
> 
> Many Linux filesystems can attach to multiple block devices and store
> data on them: XFS, btrfs, and bcachefs.  ext4 doesn't store any of its
> own data on the journal device, but it still has to open the journal
> device.
> 
> > If I'm completely missing what you mean, can you give an example of
> > what you're talking about?
> 
> ext4 supports having a separate blockdev for a journal:
> 
> # mkfs.ext4 -O journal_dev /dev/sdb
> mke2fs 1.47.2 (1-Jan-2025)
> Discarding device blocks: done                            
> Creating filesystem with 2579968 4k blocks and 0 inodes
> Filesystem UUID: e11a1211-fdd7-408a-b342-a131692c2356
> Superblock backups stored on blocks: 
> 
> Zeroing journal device:                
> # mkfs.ext4 -J device=/dev/sdb -F /dev/sda
> mke2fs 1.47.2 (1-Jan-2025)
> Using journal device's blocksize: 4096
> Discarding device blocks: done                            
> Creating filesystem with 2579968 4k blocks and 645904 inodes
> Filesystem UUID: a1789ae9-76e7-47e3-9972-5be0b8505517
> Superblock backups stored on blocks: 
>         32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632
> 
> Allocating group tables: done                            
> Writing inode tables: done                            
> Adding journal to device /dev/sdb: done
> Writing superblocks and filesystem accounting information: done 
> 
> # blkid /dev/sd[ab]
> /dev/sda: UUID="a1789ae9-76e7-47e3-9972-5be0b8505517" EXT_JOURNAL="e11a1211-fdd7-408a-b342-a131692c2356" BLOCK_SIZE="4096" TYPE="ext4"
> /dev/sdb: UUID="e11a1211-fdd7-408a-b342-a131692c2356" BLOCK_SIZE="4096" LOGUUID="e11a1211-fdd7-408a-b342-a131692c2356" TYPE="jbd"
> 
> Note that you don't have to pass the journal to e2fsck:
> 
> # e2fsck -fn /dev/sda
> e2fsck 1.47.5~WIP-2026-03-08 (8-Mar-2026)
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> Pass 3: Checking directory connectivity
> Pass 4: Checking reference counts
> Pass 5: Checking group summary information
> /dev/sda: 12/645904 files (0.0% non-contiguous), 50288/2579968 blocks
> 
> > > That said, this patchset also doesn't handle multi-device ext4
> > > filesystems (i.e. external jbd2 journal device) because the author
> > 
Not unwilling it just does not fit as a second lock_whole_disk() call.
BLOCK_DEVICE_LOCKING.md does specify the multi-device case (lock in
ascending major, then minor order), but if both devices
are partitions of the same disk they resolve to the same parent, and
flock() locks are per open file description, so a second blocking
LOCK_EX on its own fd would block against the lock this process already
holds. It needs resolve, dedup, order plus check superblock first.
I am happy to send it as a follow-up once this lands.

> > I don't think an external journal is relevant for the purposes of udevd
> > locking, is it?
> 
> Yes it is, e2fsck could need to replay the journal contents, and
> presumably you want the same protection for the secondary devices as the
> primary.
Agreed, the journal device is written during replay just like the
primary, so it wants the same lock. And as your example shows, the
journal is not passed on the command line in the normal case
> 
> --D
> 
> > > doesn't want to do that.  In their defense, the udev flock()ing protocol
> > > requires one to determine if an opened block device is a partition; if
> > > it is, then it requires opening and locking the parent bdev (e.g. sdf1
> > > -> sdf) instead of locking the original device.  This makes it way more
> > > complicated for multi-device filesystems because now the client has to
> > > detect multiple partitions coming from the same underlying device and
> > > handle that appropriately.  I don't know why the protocol designers made
> > > that choice.
> > > 
> > > I can run "trace-cmd record -e 'flock*'" to observe the locking
> > > interactions with scsi disk partitions, but for whatever reason I don't
> > > see any flocking going on if I use kpartx to create the partitions with
> > > device-mapper.  No idea why that is.
> > > 
> > > --D
> > > 
> > 

Re: [PATCH v3] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Mike Small]

"Darrick J. Wong" <djwong@kernel.org> writes:

> On Wed, Sep 23, 2026 at 03:55:23PM -0400, Dan Streetman wrote:
>> 
>> 
>> On Sun, 20 Sep 2026, Nandakumar Raghavan wrote:
>> 
>> > On Thu, Sep 10, 2026 at 05:04:41AM -0700, Nandakumar Raghavan wrote:
>> > > During journal replay, e2fsck writes the primary superblock back to disk
>> > > in multiple I/O operations. The payload lands before the checksum, leaving
>> > > a transient window where the on-disk superblock has a bad checksum.
>> > > 
>> > > If udevd processes a change uevent during this window, libblkid probes the
>> > > primary superblock, finds a checksum mismatch, and concludes the partition
>> > > has no recognisable filesystem. udev then fires a remove event, wiping all
>> > > symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
>> > > that depends on those symlinks will fail.
>> > > 
>> > > udevd already serialises its own partition probes against whole-disk device
>> > > access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
>> > > event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
>> > > whole-disk device before opening the filesystem. This forces udevd to defer
>> > > all probes on that disk until e2fsck exits and the lock is released, by
>> > > which point the filesystem is fully consistent.
...
>> > Gentle ping on this patch.
>> > 
>> > I would appreciate any feedback.
>> > 
>> 
>> Can you clarify why this should go into only fsck.ext4? Doesn't this
>> problem exist for other filesystems too?
>> 
>> I sent an earlier email as well with links to:
>> 
>> 1) fsck used to lock the device, but it surfaced a bug in udevd
>> https://bugs.freedesktop.org/show_bug.cgi?id=79576
>> 
>> 2) because of the bug, fsck stopped locking the device
>> https://github.com/util-linux/util-linux/commit/3bbdae633f4a1dda5f95ee6c61f18a1c8ef12250
>> 
>> 3) the systemd-udevd bug was fixed
>> https://github.com/systemd/systemd/commit/5d354e525a5
>> 
>> To me, it makes more sense for the locking that already exists in fsck
>> to get updated (or reverted) to lock the entire device, using the
>> existing -l param (or maybe a new param like --lock-device,
>> --udevd-lock, etc., if util-linux maintainers don't want to change -l
>> behavior).
>> 
>> Do you see an issue with doing the locking there instead of here in
>> fsck.ext4?
>
> /sbin/fsck (aka the dispatch wrapper program) doesn't necessarily know
> which block device(s) are going to be opened by a the fsck.$FSTYP
> program that it creates.  It might be able to infer that by opening any
> parameter and performing the udev locking protocol after checking if
> what it opened is a block device, but that wouldn't work for (say) a
> fsck.XXX program for a multi-device filesystem wherein you only need to
> specify one device and it will find the others.
>
> That said, this patchset also doesn't handle multi-device ext4
> filesystems (i.e. external jbd2 journal device) because the author
> doesn't want to do that.  In their defense, the udev flock()ing protocol
> requires one to determine if an opened block device is a partition; if
> it is, then it requires opening and locking the parent bdev (e.g. sdf1
> -> sdf) instead of locking the original device.  This makes it way more
> complicated for multi-device filesystems because now the client has to
> detect multiple partitions coming from the same underlying device and
> handle that appropriately.  I don't know why the protocol designers made
> that choice.
>
> I can run "trace-cmd record -e 'flock*'" to observe the locking
> interactions with scsi disk partitions, but for whatever reason I don't
> see any flocking going on if I use kpartx to create the partitions with
> device-mapper.  No idea why that is.
>
> --D

Systemd-udevd will not take its shared lock when the device in the
uevent starts with "dm-", "md", or "drbd". See udev_get_whole_disk() in
src/udev/udev-worker.c and how that's used by worker_lock_whole_disk().
Maybe that explains you not seeing the flocks in the second case. Would
the partition device names (or what their symlinks expand to?) look like
/^dm-*/?

Regards,
Mike Small

Re: [PATCH v3] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Darrick J. Wong]

On Mon, Oct 05, 2026 at 03:35:11PM +0000, Mike Small wrote:
> "Darrick J. Wong" <djwong@kernel.org> writes:
> 
> > On Wed, Sep 23, 2026 at 03:55:23PM -0400, Dan Streetman wrote:
> >> 
> >> 
> >> On Sun, 20 Sep 2026, Nandakumar Raghavan wrote:
> >> 
> >> > On Thu, Sep 10, 2026 at 05:04:41AM -0700, Nandakumar Raghavan wrote:
> >> > > During journal replay, e2fsck writes the primary superblock back to disk
> >> > > in multiple I/O operations. The payload lands before the checksum, leaving
> >> > > a transient window where the on-disk superblock has a bad checksum.
> >> > > 
> >> > > If udevd processes a change uevent during this window, libblkid probes the
> >> > > primary superblock, finds a checksum mismatch, and concludes the partition
> >> > > has no recognisable filesystem. udev then fires a remove event, wiping all
> >> > > symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> >> > > that depends on those symlinks will fail.
> >> > > 
> >> > > udevd already serialises its own partition probes against whole-disk device
> >> > > access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> >> > > event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> >> > > whole-disk device before opening the filesystem. This forces udevd to defer
> >> > > all probes on that disk until e2fsck exits and the lock is released, by
> >> > > which point the filesystem is fully consistent.
> ...
> >> > Gentle ping on this patch.
> >> > 
> >> > I would appreciate any feedback.
> >> > 
> >> 
> >> Can you clarify why this should go into only fsck.ext4? Doesn't this
> >> problem exist for other filesystems too?
> >> 
> >> I sent an earlier email as well with links to:
> >> 
> >> 1) fsck used to lock the device, but it surfaced a bug in udevd
> >> https://bugs.freedesktop.org/show_bug.cgi?id=79576
> >> 
> >> 2) because of the bug, fsck stopped locking the device
> >> https://github.com/util-linux/util-linux/commit/3bbdae633f4a1dda5f95ee6c61f18a1c8ef12250
> >> 
> >> 3) the systemd-udevd bug was fixed
> >> https://github.com/systemd/systemd/commit/5d354e525a5
> >> 
> >> To me, it makes more sense for the locking that already exists in fsck
> >> to get updated (or reverted) to lock the entire device, using the
> >> existing -l param (or maybe a new param like --lock-device,
> >> --udevd-lock, etc., if util-linux maintainers don't want to change -l
> >> behavior).
> >> 
> >> Do you see an issue with doing the locking there instead of here in
> >> fsck.ext4?
> >
> > /sbin/fsck (aka the dispatch wrapper program) doesn't necessarily know
> > which block device(s) are going to be opened by a the fsck.$FSTYP
> > program that it creates.  It might be able to infer that by opening any
> > parameter and performing the udev locking protocol after checking if
> > what it opened is a block device, but that wouldn't work for (say) a
> > fsck.XXX program for a multi-device filesystem wherein you only need to
> > specify one device and it will find the others.
> >
> > That said, this patchset also doesn't handle multi-device ext4
> > filesystems (i.e. external jbd2 journal device) because the author
> > doesn't want to do that.  In their defense, the udev flock()ing protocol
> > requires one to determine if an opened block device is a partition; if
> > it is, then it requires opening and locking the parent bdev (e.g. sdf1
> > -> sdf) instead of locking the original device.  This makes it way more
> > complicated for multi-device filesystems because now the client has to
> > detect multiple partitions coming from the same underlying device and
> > handle that appropriately.  I don't know why the protocol designers made
> > that choice.
> >
> > I can run "trace-cmd record -e 'flock*'" to observe the locking
> > interactions with scsi disk partitions, but for whatever reason I don't
> > see any flocking going on if I use kpartx to create the partitions with
> > device-mapper.  No idea why that is.
> >
> > --D
> 
> Systemd-udevd will not take its shared lock when the device in the
> uevent starts with "dm-", "md", or "drbd". See udev_get_whole_disk() in
> src/udev/udev-worker.c and how that's used by worker_lock_whole_disk().

Doesn't that mean that blkid and e2fsck can still stomp on each other if
the device is /dev/dm-0 ?

That's not in the specification, which means that for us, it's an
undocumented implementation detail.

> Maybe that explains you not seeing the flocks in the second case. Would
> the partition device names (or what their symlinks expand to?) look like
> /^dm-*/?

(For kpartx, yes it does since it uses dm to create partition devices)

But the fact that udev doesn't take the flock *at all* on dm/md/drbd
devices makes this whole proposal feel pointless.  Why would we add more
code to e2fsprogs to satisfy a locking protocol that even the supposed
benefactor doesn't follow consistently?

--D

> Regards,
> Mike Small

Re: [PATCH v3] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[Nandakumar Raghavan]

On Wed, Sep 23, 2026 at 03:55:23PM -0400, Dan Streetman wrote:
> 
> 
> On Sun, 20 Sep 2026, Nandakumar Raghavan wrote:
> 
> > On Thu, Sep 10, 2026 at 05:04:41AM -0700, Nandakumar Raghavan wrote:
> > > During journal replay, e2fsck writes the primary superblock back to disk
> > > in multiple I/O operations. The payload lands before the checksum, leaving
> > > a transient window where the on-disk superblock has a bad checksum.
> > > 
> > > If udevd processes a change uevent during this window, libblkid probes the
> > > primary superblock, finds a checksum mismatch, and concludes the partition
> > > has no recognisable filesystem. udev then fires a remove event, wiping all
> > > symlinks in /dev/disk/by-label/ and /dev/disk/by-uuid/. Any mount unit
> > > that depends on those symlinks will fail.
> > > 
> > > udevd already serialises its own partition probes against whole-disk device
> > > access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> > > event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> > > whole-disk device before opening the filesystem. This forces udevd to defer
> > > all probes on that disk until e2fsck exits and the lock is released, by
> > > which point the filesystem is fully consistent.
> > > 
> > > The whole-disk device is resolved using libsystemd's sd-device API
> > > (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype())
> > > rather than parsing sysfs, so the lock covers the same device node
> > > that udevd itself resolves and locks.
> > > 
> > > This locking is entirely optional at build time where configure checks for
> > > sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB, and
> > > e2fsck.c only compiles in the locking code when it is found. On platforms
> > > without libsystemd there is no udevd to race against in the first place,
> > > so nothing is lost by skipping it there.
> > > 
> > > Signed-off-by: Nandakumar Raghavan <naraghavan@linux.microsoft.com>
> > > ---
> > > 
> > > Changes in v3:
> > >     - Resolve the parent whole-disk device using libsystemd's sd-device API
> > >       (sd_device_new_from_devnum(), sd_device_get_parent_with_subsystem_devtype(),
> > >       sd_device_get_devname()) instead of parsing sysfs paths
> > >     - Gate the feature on autoconf detecting
> > >       sd_device_get_parent_with_subsystem_devtype() via AC_CHECK_LIB; compile it
> > >       out entirely when libsystemd is absent, rather than falling back to another
> > >       implementation
> > >     - Use sd_device_new_from_devnum() rather than the sd_device_new_from_devname()
> > >       used by the systemd.io example, since the former has been available in
> > >       libsystemd for much longer, keeping the version floor as low as possible
> > >     - Keep lock_whole_disk()'s signature unconditional and provide a stub returning
> > >       -1 on the #else branch, so main()'s primary code path has no #ifdefs in it
> > >     
> > > Changes in v2:
> > >     - Guard the whole feature (function, call site, lock_fd, cleanup) rather than
> > >       just the function body, so it is fully absent from non-Linux builds 
> > >     - Collapse the three HAVE_SYS_*_H header guards into a single conditional
> > >     - Reuse a single dev_path[] buffer instead of three single-use stack buffers
> > >     - Keep translatable warning strings on one line each, even past 80 columns
> > >     - Drop the blanket "could not lock" warning that fired even when the target was
> > >       not a block device at all (e.g. a regular file), which broke most of
> > >       "make check"; lock_whole_disk() now only warns at the point of a genuine failure
> > >     - No change needed for the Ctrl-C/EINTR concern: the SIGINT handler is installed
> > >       before lock_whole_disk() runs and is not SA_RESTART, so a blocked flock() is
> > >       interrupted and the existing E2F_FLAG_CANCEL check unwinds cleanly
> > > 
> > >  MCONFIG.in         |  1 +
> > >  configure          | 71 +++++++++++++++++++++++++++++++++++
> > >  configure.ac       | 14 +++++++
> > >  e2fsck/Makefile.in |  6 +--
> > >  e2fsck/unix.c      | 93 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  lib/config.h.in    |  6 +++
> > >  6 files changed, 188 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/MCONFIG.in b/MCONFIG.in
> > > index d66e2f3b..d3055955 100644
> > > --- a/MCONFIG.in
> > > +++ b/MCONFIG.in
> > > @@ -137,6 +137,7 @@ LIBE2P = $(LIB)/libe2p@LIB_EXT@
> > >  LIBEXT2FS = $(LIB)/libext2fs@LIB_EXT@
> > >  LIBUUID = @LIBUUID@ @SOCKET_LIB@
> > >  LIBMAGIC = @MAGIC_LIB@
> > > +LIBSYSTEMD = @SYSTEMD_LIB@
> > >  LIBFUSE = @FUSE_LIB@
> > >  LIBSUPPORT = $(LIBINTL) $(LIB)/libsupport@STATIC_LIB_EXT@
> > >  LIBBLKID = @LIBBLKID@ @PRIVATE_LIBS_CMT@ $(LIBUUID)
> > > diff --git a/configure b/configure
> > > index b04b31af..696dd07c 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -702,6 +702,7 @@ fuse3_CFLAGS
> > >  CLOCK_GETTIME_LIB
> > >  ARCHIVE_LIBS
> > >  ARCHIVE_CFLAGS
> > > +SYSTEMD_LIB
> > >  MAGIC_LIB
> > >  SOCKET_LIB
> > >  SIZEOF_TIME_T
> > > @@ -14130,6 +14131,76 @@ if test "$ac_cv_func_dlopen" = yes ; then
> > >     MAGIC_LIB=$DLOPEN_LIB
> > >  fi
> > >  
> > > +SYSTEMD_LIB=
> > > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd" >&5
> > > +printf %s "checking for sd_device_get_parent_with_subsystem_devtype in -lsystemd... " >&6; }
> > > +if test ${ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype+y}
> > > +then :
> > > +  printf %s "(cached) " >&6
> > > +else case e in #(
> > > +  e) ac_check_lib_save_LIBS=$LIBS
> > > +LIBS="-lsystemd  $LIBS"
> > > +cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> > > +/* end confdefs.h.  */
> > > +
> > > +/* Override any GCC internal prototype to avoid an error.
> > > +   Use char because int might match the return type of a GCC
> > > +   builtin and then its argument prototype would still apply.
> > > +   The 'extern "C"' is for builds by C++ compilers;
> > > +   although this is not generally supported in C code supporting it here
> > > +   has little cost and some practical benefit (sr 110532).  */
> > > +#ifdef __cplusplus
> > > +extern "C"
> > > +#endif
> > > +char sd_device_get_parent_with_subsystem_devtype (void);
> > > +int
> > > +main (void)
> > > +{
> > > +return sd_device_get_parent_with_subsystem_devtype ();
> > > +  ;
> > > +  return 0;
> > > +}
> > > +_ACEOF
> > > +if ac_fn_c_try_link "$LINENO"
> > > +then :
> > > +  ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=yes
> > > +else case e in #(
> > > +  e) ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype=no ;;
> > > +esac
> > > +fi
> > > +rm -f core conftest.err conftest.$ac_objext conftest.beam \
> > > +    conftest$ac_exeext conftest.$ac_ext
> > > +LIBS=$ac_check_lib_save_LIBS ;;
> > > +esac
> > > +fi
> > > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&5
> > > +printf "%s\n" "$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" >&6; }
> > > +if test "x$ac_cv_lib_systemd_sd_device_get_parent_with_subsystem_devtype" = xyes
> > > +then :
> > > +  SYSTEMD_LIB=-lsystemd
> > > +fi
> > > +
> > > +if test -n "$SYSTEMD_LIB"; then
> > > +	       for ac_header in systemd/sd-device.h
> > > +do :
> > > +  ac_fn_c_check_header_compile "$LINENO" "systemd/sd-device.h" "ac_cv_header_systemd_sd_device_h" "$ac_includes_default"
> > > +if test "x$ac_cv_header_systemd_sd_device_h" = xyes
> > > +then :
> > > +  printf "%s\n" "#define HAVE_SYSTEMD_SD_DEVICE_H 1" >>confdefs.h
> > > +
> > > +else case e in #(
> > > +  e) SYSTEMD_LIB= ;;
> > > +esac
> > > +fi
> > > +
> > > +done
> > > +fi
> > > +if test -n "$SYSTEMD_LIB"; then
> > > +
> > > +printf "%s\n" "#define HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE 1" >>confdefs.h
> > > +
> > > +fi
> > > +
> > >  
> > >  # Check whether --with-libarchive was given.
> > >  if test ${with_libarchive+y}
> > > diff --git a/configure.ac b/configure.ac
> > > index 4921f81f..9f585aae 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -1309,6 +1309,20 @@ if test "$ac_cv_func_dlopen" = yes ; then
> > >  fi
> > >  AC_SUBST(MAGIC_LIB)
> > >  dnl
> > > +dnl See if libsystemd has sd-device APIs for whole-disk BSD locking
> > > +dnl
> > > +SYSTEMD_LIB=
> > > +AC_CHECK_LIB(systemd, sd_device_get_parent_with_subsystem_devtype,
> > > +	[SYSTEMD_LIB=-lsystemd])
> > > +if test -n "$SYSTEMD_LIB"; then
> > > +	AC_CHECK_HEADERS([systemd/sd-device.h], , [SYSTEMD_LIB=])
> > > +fi
> > > +if test -n "$SYSTEMD_LIB"; then
> > > +	AC_DEFINE(HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE, 1,
> > > +		[Define to 1 if libsystemd has sd-device APIs for whole-disk locking])
> > > +fi
> > > +AC_SUBST(SYSTEMD_LIB)
> > > +dnl
> > >  dnl libarchive
> > >  dnl
> > >  AC_ARG_WITH([libarchive],
> > > diff --git a/e2fsck/Makefile.in b/e2fsck/Makefile.in
> > > index fbb7b156..a050eafe 100644
> > > --- a/e2fsck/Makefile.in
> > > +++ b/e2fsck/Makefile.in
> > > @@ -17,20 +17,20 @@ MANPAGES=	e2fsck.8
> > >  FMANPAGES=	e2fsck.conf.5
> > >  
> > >  LIBS= $(LIBSUPPORT) $(LIBEXT2FS) $(LIBCOM_ERR) $(LIBBLKID) $(LIBUUID) \
> > > -	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(SYSLIBS)
> > > +	$(LIBINTL) $(LIBE2P) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > >  DEPLIBS= $(DEPLIBSUPPORT) $(LIBEXT2FS) $(DEPLIBCOM_ERR) $(DEPLIBBLKID) \
> > >  	 $(DEPLIBUUID) $(DEPLIBE2P)
> > >  
> > >  STATIC_LIBS= $(STATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) $(STATIC_LIBCOM_ERR) \
> > >  	     $(STATIC_LIBBLKID) $(STATIC_LIBUUID) $(LIBINTL) $(STATIC_LIBE2P) \
> > > -	     $(LIBMAGIC) $(SYSLIBS)
> > > +	     $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > >  STATIC_DEPLIBS= $(DEPSTATIC_LIBSUPPORT) $(STATIC_LIBEXT2FS) \
> > >  		$(DEPSTATIC_LIBCOM_ERR) $(DEPSTATIC_LIBBLKID) \
> > >  		$(DEPSTATIC_LIBUUID) $(DEPSTATIC_LIBE2P)
> > >  
> > >  PROFILED_LIBS= $(PROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> > >  	       $(PROFILED_LIBCOM_ERR) $(PROFILED_LIBBLKID) $(PROFILED_LIBUUID) \
> > > -	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(SYSLIBS)
> > > +	       $(PROFILED_LIBE2P) $(LIBINTL) $(LIBMAGIC) $(LIBSYSTEMD) $(SYSLIBS)
> > >  PROFILED_DEPLIBS= $(DEPPROFILED_LIBSUPPORT) $(PROFILED_LIBEXT2FS) \
> > >  		  $(DEPPROFILED_LIBCOM_ERR) $(DEPPROFILED_LIBBLKID) \
> > >  		  $(DEPPROFILED_LIBUUID) $(DEPPROFILED_LIBE2P)
> > > diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> > > index 335ca377..51bc41eb 100644
> > > --- a/e2fsck/unix.c
> > > +++ b/e2fsck/unix.c
> > > @@ -36,6 +36,11 @@ extern int optind;
> > >  #ifdef HAVE_SYS_IOCTL_H
> > >  #include <sys/ioctl.h>
> > >  #endif
> > > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > +#include <sys/file.h>
> > > +#include <sys/stat.h>
> > > +#include <systemd/sd-device.h>
> > > +#endif
> > >  #ifdef HAVE_MALLOC_H
> > >  #include <malloc.h>
> > >  #endif
> > > @@ -1397,6 +1402,88 @@ err:
> > >  	return retval;
> > >  }
> > >  
> > > +static int lock_whole_disk(e2fsck_t ctx, const char *dev_name)
> > > +{
> > > +#ifdef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > +	struct stat st;
> > > +	sd_device *dev = NULL, *whole_dev;
> > > +	const char *subsystem, *devtype, *whole_disk_devname;
> > > +	int r, fd;
> > > +
> > > +	if (stat(dev_name, &st) < 0)
> > > +		return -1;
> > > +
> > > +	if (!S_ISBLK(st.st_mode))
> > > +		return -1;
> > > +
> > > +	r = sd_device_new_from_devnum(&dev, 'b', st.st_rdev);
> > > +	if (r < 0)
> > > +		return -1;
> > > +
> > > +	r = sd_device_get_subsystem(dev, &subsystem);
> > > +	if (r < 0 || strcmp(subsystem, "block") != 0) {
> > > +		sd_device_unref(dev);
> > > +		return -1;
> > > +	}
> > > +
> > > +	r = sd_device_get_devtype(dev, &devtype);
> > > +	if (r < 0) {
> > > +		sd_device_unref(dev);
> > > +		return -1;
> > > +	}
> > > +
> > > +	/*
> > > +	 * If dev_name is a partition, resolve the parent whole-disk device
> > > +	 * so the flock covers the same device node that udevd locks before
> > > +	 * probing any partition on it.
> > > +	 */
> > > +	if (strcmp(devtype, "disk") == 0)
> > > +		whole_dev = dev;
> > > +	else {
> > > +		r = sd_device_get_parent_with_subsystem_devtype(dev, "block",
> > > +								 "disk", &whole_dev);
> > > +		if (r < 0) {
> > > +			log_err(ctx, _("Warning: could not resolve whole-disk device for %s; lock may not prevent udev races\n"), dev_name);
> > > +			sd_device_unref(dev);
> > > +			return -1;
> > > +		}
> > > +	}
> > > +
> > > +	r = sd_device_get_devname(whole_dev, &whole_disk_devname);
> > > +	if (r < 0) {
> > > +		sd_device_unref(dev);
> > > +		return -1;
> > > +	}
> > > +
> > > +	fd = open(whole_disk_devname, O_RDONLY | O_CLOEXEC, 0);
> > > +	if (fd < 0) {
> > > +		com_err(ctx->program_name, errno,
> > > +			_("while trying to open %s for locking"), whole_disk_devname);
> > > +		sd_device_unref(dev);
> > > +		return -1;
> > > +	}
> > > +	sd_device_unref(dev);
> > > +
> > > +	while (flock(fd, LOCK_EX) != 0) {
> > > +		if (errno == EINTR) {
> > > +			if (ctx->flags & E2F_FLAG_CANCEL) {
> > > +				close(fd);
> > > +				return -1;
> > > +			}
> > > +			continue;
> > > +		}
> > > +		com_err(ctx->program_name, errno,
> > > +			_("while trying to lock %s"), whole_disk_devname);
> > > +		close(fd);
> > > +		return -1;
> > > +	}
> > > +
> > > +	return fd;
> > > +#else
> > > +	return -1;
> > > +#endif /* HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE */
> > > +}
> > > +
> > >  int main (int argc, char *argv[])
> > >  {
> > >  	errcode_t	retval = 0, retval2 = 0, orig_retval = 0;
> > > @@ -1413,6 +1500,7 @@ int main (int argc, char *argv[])
> > >  	int journal_size;
> > >  	int sysval, sys_page_size = 4096;
> > >  	int old_bitmaps;
> > > +	int lock_fd = -1;
> > >  	__u32 features[3];
> > >  	char *cp;
> > >  	enum quota_type qtype;
> > > @@ -1488,6 +1576,8 @@ int main (int argc, char *argv[])
> > >  
> > >  	check_mount(ctx);
> > >  
> > > +	lock_fd = lock_whole_disk(ctx, ctx->filesystem_name);
> > > +
> > >  	if (!(ctx->options & E2F_OPT_PREEN) &&
> > >  	    !(ctx->options & E2F_OPT_NO) &&
> > >  	    !(ctx->options & E2F_OPT_YES)) {
> > > @@ -2169,6 +2259,9 @@ skip_write:
> > >  	ext2fs_close_free(&ctx->fs);
> > >  	free(ctx->journal_name);
> > >  
> > > +	if (lock_fd >= 0)
> > > +		close(lock_fd);
> > > +
> > >  	if (ctx->logf)
> > >  		fprintf(ctx->logf, "Exit status: %d\n", exit_value);
> > >  	e2fsck_free_context(ctx);
> > > diff --git a/lib/config.h.in b/lib/config.h.in
> > > index f129abfe..bc3ae4aa 100644
> > > --- a/lib/config.h.in
> > > +++ b/lib/config.h.in
> > > @@ -352,6 +352,9 @@
> > >  /* Define to 1 if if struct sockaddr contains sa_len */
> > >  #undef HAVE_SA_LEN
> > >  
> > > +/* Define to 1 if libsystemd has sd-device APIs for whole-disk locking */
> > > +#undef HAVE_SD_DEVICE_GET_PARENT_WITH_SUBSYSTEM_DEVTYPE
> > > +
> > >  /* Define to 1 if you have the 'secure_getenv' function. */
> > >  #undef HAVE_SECURE_GETENV
> > >  
> > > @@ -430,6 +433,9 @@
> > >  /* Define to 1 if you have the 'sysconf' function. */
> > >  #undef HAVE_SYSCONF
> > >  
> > > +/* Define to 1 if you have the <systemd/sd-device.h> header file. */
> > > +#undef HAVE_SYSTEMD_SD_DEVICE_H
> > > +
> > >  /* Define to 1 if you have the <sys/acl.h> header file. */
> > >  #undef HAVE_SYS_ACL_H
> > >  
> > > -- 
> > > 2.55.0
> > 
> > Hi,
> > 
> > Gentle ping on this patch.
> > 
> > I would appreciate any feedback.
> > 
> 
> Can you clarify why this should go into only fsck.ext4? Doesn't this
> problem exist for other filesystems too?
It can, yes - the race is not ext4-specific. Any fsck that writes
metadata while udevd is probing the same disk can hit it. But there is
no shared code path to fix it in each fsck opens its own devices and
does its own I/O, so each one has to participate in the locking
protocol itself. This patch covers ext2/ext3/ext4, which are all
e2fsck. xfs_repair, btrfs check and so on would need equivalent
changes in their own projects.

> 
> I sent an earlier email as well with links to:
> 
> 1) fsck used to lock the device, but it surfaced a bug in udevd
> https://bugs.freedesktop.org/show_bug.cgi?id=79576
> 
> 2) because of the bug, fsck stopped locking the device
> https://github.com/util-linux/util-linux/commit/3bbdae633f4a1dda5f95ee6c61f18a1c8ef12250
> 
> 3) the systemd-udevd bug was fixed
> https://github.com/systemd/systemd/commit/5d354e525a5
> 
> To me, it makes more sense for the locking that already exists in fsck
> to get updated (or reverted) to lock the entire device, using the
> existing -l param (or maybe a new param like --lock-device,
> --udevd-lock, etc., if util-linux maintainers don't want to change -l
> behavior).

> 
> Do you see an issue with doing the locking there instead of here in
> fsck.ext4?
Darrick covered the main structural problem - /sbin/fsck doesn't know
which devices the fsck.$FSTYP backend will actually open.

On top of that, today -l is not a weaker version of this patch, it is
a different mechanism. From disk-utils/fsck.c on master:

 - lock_disk() flocks /run/fsck/<diskname>.lock, a regular file. udevd
   flocks the device node. The two never interact.

 - it returns early for is_irrotational_disk(), so -l does nothing at
   all.

If util-linux wants a --udevd-lock on the dispatcher later, this patch
doesn't block it. But per systemd.io/BLOCK_DEVICE_LOCKING the lock
is meant to be taken by the tool that actually modifies the device and
it is recommended to take LOCK_EX BSD file locks when manipulating block
devices in all tools that change file system block devices (mkfs, fsck etc)
and doing it in e2fsck is what makes the guarantee hold regardless of who
invoked it.

Re: [PATCH v2] e2fsck: take flock(LOCK_EX) on whole-disk device during filesystem check

[ddstreet]

On Mon, 24 Aug 2026, Nandakumar Raghavan wrote:

> udevd already serialises its own partition probes against whole-disk device
> access using flock(LOCK_SH|LOCK_NB); if EAGAIN is returned it requeues the
> event. Take advantage of this protocol by acquiring flock(LOCK_EX) on the
> whole-disk device before opening the filesystem. This forces udevd to defer
> all probes on that disk until e2fsck exits and the lock is released, by
> which point the filesystem is fully consistent.

I believe this was already part of fsck, and it conflicted with udevd and so was removed:
https://bugs.freedesktop.org/show_bug.cgi?id=79576
https://github.com/util-linux/util-linux/commit/3bbdae633f4a1dda5f95ee6c61f18a1c8ef12250

That particular problem was already fixed in systemd-udevd:
https://github.com/systemd/systemd/commit/5d354e525a5

But in general, does it really make sense for _only_ e2fsck to lock
the device? Wouldn't it make more sense to add the device flock back
into fsck itself? Or if the problem is during boot between
systemd-fsck and systemd-udevd, wouldn't it be better to fix the issue
in systemd-fsck?