svn commit: r238952 - head/sys/kern

[John Baldwin]

On Tuesday, July 31, 2012 2:25:01 pm John Baldwin wrote:
> Author: jhb
> Date: Tue Jul 31 18:25:00 2012
> New Revision: 238952
> URL: http://svn.freebsd.org/changeset/base/238952
> 
> Log:
>   Reorder the managament of advisory locks on open files so that the advisory
>   lock is obtained before the write count is increased during open() and the
>   lock is released after the write count is decreased during close().
>   
>   The first change closes a race where an open() that will block with O_SHLOCK
>   or O_EXLOCK can increase the write count while it waits.  If the process
>   holding the current lock on the file then tries to call exec() on the file
>   it has locked, it can fail with ETXTBUSY even though the advisory lock is
>   preventing other threads from succesfully completeing a writable open().
>   
>   The second change closes a race where a read-only open() with O_SHLOCK or
>   O_EXLOCK may return successfully while the write count is non-zero due to
>   another descriptor that had the advisory lock and was blocking the open()
>   still being in the process of closing.  If the process that completed the
>   open() then attempts to call exec() on the file it locked, it can fail with
>   ETXTBUSY even though the other process that held a write lock has closed
>   the file and released the lock.
>   
>   Reviewed by:	kib
>   MFC after:	1 month

Oops, should have included:

Tested by:	pho

I have a regression test (of sorts) for this.  If you run it on an unpatched
system it should start emitting errors within a few seconds.

#include <sys/types.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <err.h>
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

static void
usage(void)
{
	fprintf(stderr, "Usage: flock_close_race <binary> [args]\n");
	exit(1);
}

static void
child(const char *binary)
{
	int fd;

	/* Exit as soon as our parent exits. */
	while (getppid() != 1) {
		fd = open(binary, O_RDWR | O_EXLOCK);
		if (fd < 0) {
			/*
			 * This may get ETXTBSY since exit() will
			 * close its open fd's (thus releasing the
			 * lock), before it releases the vmspace (and
			 * mapping of the binary).
			 */
			if (errno == ETXTBSY)
				continue;
			err(1, "can't open %s", binary);
		}
		close(fd);
	}
	exit(0);
}

static void
exec_child(char **av)
{
	int fd;

	fd = open(av[0], O_RDONLY | O_SHLOCK);
	execv(av[0], av);
	err(127, "execv");
}

int
main(int ac, char **av)
{
	struct stat sb;
	pid_t pid;

	if (ac < 2)
		usage();
	if (stat(av[1], &sb) != 0)
		err(1, "stat(%s)", av[1]);
	if (!S_ISREG(sb.st_mode))
		errx(1, "%s not an executable", av[1]);

	pid = fork();
	if (pid < 0)
		err(1, "fork");
	if (pid == 0)
		child(av[1]);
	for (;;) {
		pid = fork();
		if (pid < 0)
			err(1, "fork");
		if (pid == 0)
			exec_child(av + 1);
		wait(NULL);
	}
	return (0);
}

-- 
John Baldwin