changing lock mechanism

Systems can suffer power interruptions whilst .lock files are in /etc,
preventing scripts and other automation tools from updating shadow's
files which persist across boots.

This commit replaces that mechanism with file locking to avoid problems
of power interruption/crashing.

Minor tweak to groupmems man page, requested by 'xx' on IRC.

Signed-off-by: ed neville <ed@s5h.net>

diff --git a/lib/commonio.c b/lib/commonio.c
index 119d506e799337df586e74eac81d0b386dfa00fa..0e5360611125a5912d82e2c99397c5ae6f4dcfab 100644 (file)
--- a/lib/commonio.c
+++ b/lib/commonio.c
@@ -32,8 +32,6 @@
 
 /* local function prototypes */
 static int lrename (const char *, const char *);
-static int check_link_count (const char *file, bool log);
-static int do_lock_file (const char *file, const char *lock, bool log);
 static /*@null@*/ /*@dependent@*/FILE *fopen_set_perms (
        const char *name,
        const char *mode,
@@ -91,150 +89,6 @@ int lrename (const char *old, const char *new)
        return res;
 }
 
-static int check_link_count (const char *file, bool log)
-{
-       struct stat sb;
-
-       if (stat (file, &sb) != 0) {
-               if (log) {
-                       (void) fprintf (shadow_logfd,
-                                       "%s: %s file stat error: %s\n",
-                                       shadow_progname, file, strerror (errno));
-               }
-               return 0;
-       }
-
-       if (sb.st_nlink != 2) {
-               if (log) {
-                       (void) fprintf (shadow_logfd,
-                                       "%s: %s: lock file already used (nlink: %u)\n",
-                                       shadow_progname, file, sb.st_nlink);
-               }
-               return 0;
-       }
-
-       return 1;
-}
-
-
-static int do_lock_file (const char *file, const char *lock, bool log)
-{
-       int fd;
-       pid_t pid;
-       ssize_t len;
-       int retval;
-       char buf[32];
-
-       fd = open (file, O_CREAT | O_TRUNC | O_WRONLY, 0600);
-       if (-1 == fd) {
-               if (log) {
-                       (void) fprintf (shadow_logfd,
-                                       "%s: %s: %s\n",
-                                       shadow_progname, file, strerror (errno));
-               }
-               return 0;
-       }
-
-       pid = getpid ();
-       snprintf (buf, sizeof buf, "%lu", (unsigned long) pid);
-       len = (ssize_t) strlen (buf) + 1;
-       if (write (fd, buf, (size_t) len) != len) {
-               if (log) {
-                       (void) fprintf (shadow_logfd,
-                                       "%s: %s file write error: %s\n",
-                                       shadow_progname, file, strerror (errno));
-               }
-               (void) close (fd);
-               unlink (file);
-               return 0;
-       }
-       if (fdatasync (fd) == -1) {
-               if (log) {
-                       (void) fprintf (shadow_logfd,
-                                       "%s: %s file sync error: %s\n",
-                                       shadow_progname, file, strerror (errno));
-               }
-               (void) close (fd);
-               unlink (file);
-               return 0;
-       }
-       close (fd);
-
-       if (link (file, lock) == 0) {
-               retval = check_link_count (file, log);
-               unlink (file);
-               return retval;
-       }
-
-       fd = open (lock, O_RDWR);
-       if (-1 == fd) {
-               if (log) {
-                       (void) fprintf (shadow_logfd,
-                                       "%s: %s: %s\n",
-                                       shadow_progname, lock, strerror (errno));
-               }
-               unlink (file);
-               errno = EINVAL;
-               return 0;
-       }
-       len = read (fd, buf, sizeof (buf) - 1);
-       close (fd);
-       if (len <= 0) {
-               if (log) {
-                       (void) fprintf (shadow_logfd,
-                                       "%s: existing lock file %s without a PID\n",
-                                       shadow_progname, lock);
-               }
-               unlink (file);
-               errno = EINVAL;
-               return 0;
-       }
-       buf[len] = '\0';
-       if (get_pid (buf, &pid) == 0) {
-               if (log) {
-                       (void) fprintf (shadow_logfd,
-                                       "%s: existing lock file %s with an invalid PID '%s'\n",
-                                       shadow_progname, lock, buf);
-               }
-               unlink (file);
-               errno = EINVAL;
-               return 0;
-       }
-       if (kill (pid, 0) == 0) {
-               if (log) {
-                       (void) fprintf (shadow_logfd,
-                                       "%s: lock %s already used by PID %lu\n",
-                                       shadow_progname, lock, (unsigned long) pid);
-               }
-               unlink (file);
-               errno = EEXIST;
-               return 0;
-       }
-       if (unlink (lock) != 0) {
-               if (log) {
-                       (void) fprintf (shadow_logfd,
-                                       "%s: cannot get lock %s: %s\n",
-                                       shadow_progname, lock, strerror (errno));
-               }
-               unlink (file);
-               return 0;
-       }
-
-       retval = 0;
-       if (link (file, lock) == 0) {
-               retval = check_link_count (file, log);
-       } else {
-               if (log) {
-                       (void) fprintf (shadow_logfd,
-                                       "%s: cannot get lock %s: %s\n",
-                                       shadow_progname, lock, strerror (errno));
-               }
-       }
-
-       unlink (file);
-       return retval;
-}
-
 
 static /*@null@*/ /*@dependent@*/FILE *fopen_set_perms (
        const char *name,
@@ -349,6 +203,30 @@ bool commonio_present (const struct commonio_db *db)
 }
 
 
+int do_fcntl_lock (const char *file, bool log, short type)
+{
+       int fd;
+       struct flock lck = {
+               .l_type = type,
+               .l_whence = SEEK_SET,
+               .l_start = 0,
+               .l_len = 0,
+       };
+
+       fd = open (file, O_WRONLY, 0600);
+       if (-1 == fd) {
+               if (log) {
+                               (void) fprintf (shadow_logfd, "%s: %s: %s\n",
+                                               shadow_progname, file, strerror (errno));
+               }
+               return 0;
+       }
+
+       fcntl (fd, F_OFD_SETLKW, &lck);
+       close(fd);
+       return(1);
+}
+
 int commonio_lock_nowait (struct commonio_db *db, bool log)
 {
        char* file = NULL;
@@ -372,8 +250,8 @@ int commonio_lock_nowait (struct commonio_db *db, bool log)
        }
        snprintf (file, file_len, "%s.%lu",
                  db->filename, (unsigned long) getpid ());
-       snprintf (lock, lock_file_len, "%s.lock", db->filename);
-       if (do_lock_file (file, lock, log) != 0) {
+
+       if (do_fcntl_lock (db->filename, log, F_WRLCK | F_RDLCK) != 0) {
                db->locked = true;
                lock_count++;
                err = 1;
@@ -471,8 +349,6 @@ static void dec_lock_count (void)
 
 int commonio_unlock (struct commonio_db *db)
 {
-       char lock[1024];
-
        if (db->isopen) {
                db->readonly = true;
                if (commonio_close (db) == 0) {
@@ -483,13 +359,9 @@ int commonio_unlock (struct commonio_db *db)
                }
        }
        if (db->locked) {
-               /*
-                * Unlock in reverse order: remove the lock file,
-                * then call ulckpwdf() (if used) on last unlock.
-                */
                db->locked = false;
-               snprintf (lock, sizeof lock, "%s.lock", db->filename);
-               unlink (lock);
+
+               do_fcntl_lock (db->filename, false, F_UNLCK);
                dec_lock_count ();
                return 1;
        }

diff --git a/lib/commonio.h b/lib/commonio.h
index e63c5b4d1507f88f655e5f6f26988034115ecdc7..d5354bfbec41b3b78e380d760c42c7bca906cdd8 100644 (file)
--- a/lib/commonio.h
+++ b/lib/commonio.h
@@ -123,6 +123,7 @@ extern int commonio_setname (struct commonio_db *, const char *);
 extern bool commonio_present (const struct commonio_db *db);
 extern int commonio_lock (struct commonio_db *);
 extern int commonio_lock_nowait (struct commonio_db *, bool log);
+extern int do_fcntl_lock (const char *file, bool log, short type);
 extern int commonio_open (struct commonio_db *, int);
 extern /*@observer@*/ /*@null@*/const void *commonio_locate (struct commonio_db *, const char *);
 extern int commonio_update (struct commonio_db *, const void *);

diff --git a/man/groupmems.8.xml b/man/groupmems.8.xml
index 7d06c8b89fb310c7a56405258ebd6901da2ef3be..cbc846228d20fada574fe73cea90e7ec8afaf04e 100644 (file)
--- a/man/groupmems.8.xml
+++ b/man/groupmems.8.xml
@@ -167,7 +167,7 @@
     <programlisting>
        $ groupadd -r groups
        $ chmod 2710 groupmems
-       $ chown root.groups groupmems
+       $ chown root:groups groupmems
        $ groupmems -g groups -a gk4
     </programlisting>
   </refsect1>