locking.c, locking.1: Make the protocol safe if the lockfile is (re)moved.

Get the device/inode pair from the lockfile descriptor after we've
opened it (so this is the file we're actually going to lock).  Once
we've acquired the lock, check that the file /name/ still has the same
device/inode pair.  If nobody is allowed to move or unlink the lockfile
while we've got the lock, then we know (assuming this check passes) that
we've actually locked the right file and not some deleted thing.

diff --git a/locking.1 b/locking.1
index 51837902af5c222e763919baf6687853b8be363a..a9a37aa52fee905b7df2e02803955744b81f8e35 100644 (file)
--- a/locking.1
+++ b/locking.1
@@ -97,6 +97,21 @@ for days, hours, minutes or seconds, respectively) for the lock to
 become available, and then give up.  This only makes sense with the
 .B \-\-wait
 option, so that is turned on automatically.
 become available, and then give up.  This only makes sense with the
 .B \-\-wait
 option, so that is turned on automatically.

+.PP
+It is safe to unlink or atomically replace the lockfile while holding
+the lock, though these actions will release the lock immediately.  To
+safely delete the lockfile, for example, run
+.IP
+.B "locking lock rm lock"
+.PP
+Similarly, a file can be updated safely by
+.IP
+.nf
+.ft B
+locking file sh -c \e
+\h'8m'"update-file file >file.new && mv file.new file"
+.fi
+.ft R

 .SH "BUGS"
 The
 .B locking
 .SH "BUGS"
 The
 .B locking

diff --git a/locking.c b/locking.c
index 68cbc749a56f59ad0f9c44b38460e5d503165490..bfc5c771927e13d04703108224992ce92aef6933 100644 (file)
--- a/locking.c
+++ b/locking.c
@@ -39,6 +39,7 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <sys/wait.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <sys/wait.h>

+#include <sys/stat.h>

 
 #include <mLib/mdwopt.h>
 #include <mLib/quis.h>
 
 #include <mLib/mdwopt.h>
 #include <mLib/quis.h>


@@ -95,6 +96,7 @@ int main(int argc, char *argv[])
   int t = -1;
   int oflag;
   unsigned int ot = 0;
   int t = -1;
   int oflag;
   unsigned int ot = 0;

+  struct stat st, nst;

   time_t nt;
   pid_t kid;
   int rc;
   time_t nt;
   pid_t kid;
   int rc;


@@ -184,9 +186,18 @@ doneopts:
   ot = alarm(0);
   oalrm = signal(SIGALRM, alrm);
   if (t >= 0) alarm(t);
   ot = alarm(0);
   oalrm = signal(SIGALRM, alrm);
   if (t >= 0) alarm(t);

+again:

   if ((fd = open(file, oflag, 0666)) < 0)
     die(111, "error opening `%s': %s", file, strerror(errno));
   if ((fd = open(file, oflag, 0666)) < 0)
     die(111, "error opening `%s': %s", file, strerror(errno));

+  if (fstat(fd, &st))
+    die(111, "error from fstat on `%s': %s", file, strerror(errno));

   err = fcntl(fd, f & f_wait ? F_SETLKW : F_SETLK, &l) >= 0 ? 0 : errno;
   err = fcntl(fd, f & f_wait ? F_SETLKW : F_SETLK, &l) >= 0 ? 0 : errno;

+  if (stat(file, &nst)) {
+    if (errno == ENOENT) { close(fd); goto again; }
+    else die(111, "error from stat on `%s': %s", file, strerror(errno));
+  }
+  if (st.st_dev != nst.st_dev || st.st_ino != nst.st_ino)
+    { close(fd); goto again; }

 done:
   signal(SIGALRM, oalrm);
   if (!ot)
 done:
   signal(SIGALRM, oalrm);
   if (!ot)