linux-ext4 - Re: [PATCH 10/10] libext2fs: add posix advisory locking to the unix IO manager

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251008220940.GB6170@frogsfrogsfrogs>
Date: Wed, 8 Oct 2025 15:09:40 -0700
From: "Darrick J. Wong" <djwong@...nel.org>
To: tytso@....edu
Cc: miklos@...redi.hu, neal@...pa.dev, linux-fsdevel@...r.kernel.org,
	linux-ext4@...r.kernel.org, John@...ves.net, bernd@...ernd.com,
	joannelkoong@...il.com
Subject: Re: [PATCH 10/10] libext2fs: add posix advisory locking to the unix
 IO manager

On Mon, Sep 15, 2025 at 05:58:43PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@...nel.org>
> 
> Add support for using flock() to protect the files opened by the Unix IO
> manager so that we can't mount the same fs multiple times.  This also
> prevents systemd and udev from accessing the device while e2fsprogs is
> doing something with the device.
> 
> Link: https://systemd.io/BLOCK_DEVICE_LOCKING/
> Signed-off-by: "Darrick J. Wong" <djwong@...nel.org>

This actually causes a lot of problems with fstests -- if fuse2fs
flock()s the block device, then udevd will spin in a slow trylock loop
until the bdev can be locked.  Meanwhile, any scripts calling udevadm
settle will block until fuse2fs exits (or it gives up after 2 minutes go
by), because udev still has a uevent that it cannot settle.  This causes
any test that uses udevadm settle to take forever to run.

In general, we don't want to block udev from reading the block device
while fuse2fs has it mounted.  For block devices this is unnecessary
anyway because we have O_EXCL.

However, the advisory locking is still useful for coordinating access to
filesystem images in regular files, so I'll rework this to only do it
for regular files.

--D

> ---
>  lib/ext2fs/unix_io.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> 
> diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
> index 068be689326443..55007ad7d2ae15 100644
> --- a/lib/ext2fs/unix_io.c
> +++ b/lib/ext2fs/unix_io.c
> @@ -65,6 +65,12 @@
>  #include <pthread.h>
>  #endif
>  
> +#if defined(HAVE_SYS_FILE_H) && defined(HAVE_SIGNAL_H)
> +# include <sys/file.h>
> +# include <signal.h>
> +# define WANT_LOCK_UNIX_FD
> +#endif
> +
>  #if defined(__linux__) && defined(_IO) && !defined(BLKROGET)
>  #define BLKROGET   _IO(0x12, 94) /* Get read-only status (0 = read_write).  */
>  #endif
> @@ -149,6 +155,9 @@ struct unix_private_data {
>  	pthread_mutex_t bounce_mutex;
>  	pthread_mutex_t stats_mutex;
>  #endif
> +#ifdef WANT_LOCK_UNIX_FD
> +	int	lock_flags;
> +#endif
>  };
>  
>  #define IS_ALIGNED(n, align) ((((uintptr_t) n) & \
> @@ -897,6 +906,47 @@ int ext2fs_fstat(int fd, ext2fs_struct_stat *buf)
>  #endif
>  }
>  
> +#ifdef WANT_LOCK_UNIX_FD
> +static void unix_lock_alarm_handler(int signal, siginfo_t *data, void *p)
> +{
> +	/* do nothing, the signal will abort the flock operation */
> +}
> +
> +static int unix_lock_fd(int fd, int flags)
> +{
> +	struct sigaction newsa = {
> +		.sa_flags = SA_SIGINFO,
> +		.sa_sigaction = unix_lock_alarm_handler,
> +	};
> +	struct sigaction oldsa;
> +	const int operation = (flags & IO_FLAG_EXCLUSIVE) ? LOCK_EX : LOCK_SH;
> +	int ret;
> +
> +	/* wait five seconds for the lock */
> +	ret = sigaction(SIGALRM, &newsa, &oldsa);
> +	if (ret)
> +		return ret;
> +
> +	alarm(5);
> +
> +	ret = flock(fd, operation);
> +	if (ret == 0)
> +		ret = operation;
> +	else if (errno == EINTR) {
> +		errno = EWOULDBLOCK;
> +		ret = -1;
> +	}
> +
> +	alarm(0);
> +	sigaction(SIGALRM, &oldsa, NULL);
> +	return ret;
> +}
> +
> +static void unix_unlock_fd(int fd)
> +{
> +	flock(fd, LOCK_UN);
> +}
> +#endif
>  
>  static errcode_t unix_open_channel(const char *name, int fd,
>  				   int flags, io_channel *channel,
> @@ -935,6 +985,16 @@ static errcode_t unix_open_channel(const char *name, int fd,
>  	if (retval)
>  		goto cleanup;
>  
> +#ifdef WANT_LOCK_UNIX_FD
> +	if (flags & IO_FLAG_RW) {
> +		data->lock_flags = unix_lock_fd(fd, flags);
> +		if (data->lock_flags < 0) {
> +			retval = errno;
> +			goto cleanup;
> +		}
> +	}
> +#endif
> +
>  	strcpy(io->name, name);
>  	io->private_data = data;
>  	io->block_size = 1024;
> @@ -1200,6 +1260,10 @@ static errcode_t unix_close(io_channel channel)
>  	if (retval2 && !retval)
>  		retval = retval2;
>  
> +#ifdef WANT_LOCK_UNIX_FD
> +	if (data->lock_flags)
> +		unix_unlock_fd(data->dev);
> +#endif
>  	if (close(data->dev) < 0 && !retval)
>  		retval = errno;
>  	free_cache(data);
> 
>