Fix long standing race condition associated with how lockf uses open(2)

for mutual exclusion:

A brief description of the problem:

	1) Proc A picks up non-blocking lock on file X
	2) Proc B attempts to pickup lock, fails then waits
	3) Proc C attempts to pickup lock, fails then waits
	4) Proc A releases lock
	5) Proc B acquires lock, release it to pickup a non-blocking version
	6) Proc C acquires lock, release it to pickup a non-blocking version
	7) Both process B and C race each other to pickup lock again

This occurs mainly because the processes do not keep the lock after they have
been waiting on it. They drop it, attempt to re-acquire it. (They use the wait
to notify when the lock has become available then race to pick it up). This
results in additional CPU utilization during the race, and can also result
in processes picking locks up out of order.

This change attempts to correct this problem by eliminating the test/acquire
race and having the operating system handle it.

Reported by:	kris
Tested by:	kris
MFC after:	1 week

1 files changed, 8 insertions, 30 deletions

diff --git a/usr.bin/lockf/lockf.c b/usr.bin/lockf/lockf.c
index 6995d0abfba40..de4aac9ac0be7 100644
--- a/usr.bin/lockf/lockf.c
+++ b/usr.bin/lockf/lockf.c
@@ -38,15 +38,14 @@ __FBSDID("$FreeBSD$");
 #include <sysexits.h>
 #include <unistd.h>
 
-static int acquire_lock(const char *name);
 static void cleanup(void);
 static void killed(int sig);
 static void timeout(int sig);
 static void usage(void);
-static void wait_for_lock(const char *name);
+static int wait_for_lock(const char *name);
 
 static const char *lockname;
-static int lockfd;
+static int lockfd = -1;
 static int keep;
 static volatile sig_atomic_t timed_out;
 
@@ -106,11 +105,8 @@ main(int argc, char **argv)
 	alarm(waitsec);
     }
 
-    lockfd = acquire_lock(lockname);
-    while (lockfd == -1 && !timed_out && waitsec != 0) {
-	wait_for_lock(lockname);
-	lockfd = acquire_lock(lockname);
-    }
+    while (lockfd == -1 && !timed_out && waitsec != 0)
+	lockfd = wait_for_lock(lockname);
 
     if (waitsec > 0)
 	alarm(0);
@@ -149,23 +145,6 @@ main(int argc, char **argv)
 }
 
 /*
- * Try to acquire a lock on the given file, but don't wait for it.  Returns
- * an open file descriptor on success, or -1 on failure.
- */
-static int
-acquire_lock(const char *name)
-{
-    int fd;
-
-    if ((fd = open(name, O_RDONLY|O_CREAT|O_EXLOCK|O_NONBLOCK, 0666)) == -1) {
-	if (errno == EAGAIN || errno == EINTR)
-	    return -1;
-	err(EX_CANTCREAT, "cannot open %s", name);
-    }
-    return fd;
-}
-
-/*
  * Remove the lock file.
  */
 static void
@@ -210,16 +189,15 @@ usage(void)
 /*
  * Wait until it might be possible to acquire a lock on the given file.
  */
-static void
+static int
 wait_for_lock(const char *name)
 {
     int fd;
 
-    if ((fd = open(name, O_RDONLY|O_EXLOCK)) == -1) {
+    if ((fd = open(name, O_CREAT|O_RDONLY|O_EXLOCK, 0666)) == -1) {
 	if (errno == ENOENT || errno == EINTR)
-	    return;
+	    return (-1);
 	err(EX_CANTCREAT, "cannot open %s", name);
     }
-    close(fd);
-    return;
+    return (fd);
 }