Mountd should use separate lockfiles

Mountd keeps file descriptors used for locks separate from
those used for io and seems to assume that the lock will
only be released on close of the file descriptor that was used
with fcntl.  Actually the lock is released when any file
descriptor for that file is closed.  When setexportent() is called
after xflock() he closes and reopens the io file descriptor and defeats the
lock.

This patch fixes that by using a separate file for locking, cleaning
them up when finished.

Signed-off-by: Ben Myers <bpm@sgi.com>
Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/support/export/rmtab.c b/support/export/rmtab.c
index e11a22a8dd0d96fd18247c5375443c6c4b95eea0..b49e1aaa9c121922d943a4b91cad496b9f9c4924 100644 (file)
--- a/support/export/rmtab.c
+++ b/support/export/rmtab.c
@@ -57,7 +57,7 @@ rmtab_read(void)
                   file. */
                int     lockid;
                FILE    *fp;
-               if ((lockid = xflock(_PATH_RMTAB, "w")) < 0)
+               if ((lockid = xflock(_PATH_RMTABLCK, "w")) < 0)
                        return -1;
                rewindrmtabent();
                if (!(fp = fsetrmtabent(_PATH_RMTABTMP, "w"))) {

diff --git a/support/export/xtab.c b/support/export/xtab.c
index 510765a3f0d59b5517e06b0542f1eb8d0d702f03..3b1dcce325493fec61fbb5385b9a89aadff8da84 100644 (file)
--- a/support/export/xtab.c
+++ b/support/export/xtab.c
@@ -23,7 +23,7 @@
 static void cond_rename(char *newfile, char *oldfile);
 
 static int
-xtab_read(char *xtab, int is_export)
+xtab_read(char *xtab, char *lockfn, int is_export)
 {
     /* is_export == 0  => reading /proc/fs/nfs/exports - we know these things are exported to kernel
      * is_export == 1  => reading /var/lib/nfs/etab - these things are allowed to be exported
@@ -33,7 +33,7 @@ xtab_read(char *xtab, int is_export)
        nfs_export              *exp;
        int                     lockid;
 
-       if ((lockid = xflock(xtab, "r")) < 0)
+       if ((lockid = xflock(lockfn, "r")) < 0)
                return 0;
        setexportent(xtab, "r");
        while ((xp = getexportent(is_export==0, 0)) != NULL) {
@@ -66,18 +66,20 @@ xtab_mount_read(void)
        int fd;
        if ((fd=open(_PATH_PROC_EXPORTS, O_RDONLY))>=0) {
                close(fd);
-               return xtab_read(_PATH_PROC_EXPORTS, 0);
+               return xtab_read(_PATH_PROC_EXPORTS,
+                                _PATH_PROC_EXPORTS, 0);
        } else if ((fd=open(_PATH_PROC_EXPORTS_ALT, O_RDONLY) >= 0)) {
                close(fd);
-               return xtab_read(_PATH_PROC_EXPORTS_ALT, 0);
+               return xtab_read(_PATH_PROC_EXPORTS_ALT,
+                                _PATH_PROC_EXPORTS_ALT, 0);
        } else
-               return xtab_read(_PATH_XTAB, 2);
+               return xtab_read(_PATH_XTAB, _PATH_XTABLCK, 2);
 }
 
 int
 xtab_export_read(void)
 {
-       return xtab_read(_PATH_ETAB, 1);
+       return xtab_read(_PATH_ETAB, _PATH_ETABLCK, 1);
 }
 
 /*
@@ -87,13 +89,13 @@ xtab_export_read(void)
  * fix the auth_reload logic as well...
  */
 static int
-xtab_write(char *xtab, char *xtabtmp, int is_export)
+xtab_write(char *xtab, char *xtabtmp, char *lockfn, int is_export)
 {
        struct exportent        xe;
        nfs_export              *exp;
        int                     lockid, i;
 
-       if ((lockid = xflock(xtab, "w")) < 0) {
+       if ((lockid = xflock(lockfn, "w")) < 0) {
                xlog(L_ERROR, "can't lock %s for writing", xtab);
                return 0;
        }
@@ -124,13 +126,13 @@ xtab_write(char *xtab, char *xtabtmp, int is_export)
 int
 xtab_export_write()
 {
-       return xtab_write(_PATH_ETAB, _PATH_ETABTMP, 1);
+       return xtab_write(_PATH_ETAB, _PATH_ETABTMP, _PATH_ETABLCK, 1);
 }
 
 int
 xtab_mount_write()
 {
-       return xtab_write(_PATH_XTAB, _PATH_XTABTMP, 0);
+       return xtab_write(_PATH_XTAB, _PATH_XTABTMP, _PATH_XTABLCK, 0);
 }
 
 void
@@ -139,7 +141,7 @@ xtab_append(nfs_export *exp)
        struct exportent xe;
        int             lockid;
 
-       if ((lockid = xflock(_PATH_XTAB, "w")) < 0)
+       if ((lockid = xflock(_PATH_XTABLCK, "w")) < 0)
                return;
        setexportent(_PATH_XTAB, "a");
        xe = exp->m_export;

diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index a51d79d60eb9ee1d92e498c3f5c6b8d046921c49..9d0d39d438d2c19872c313d59529a06ebd639109 100644 (file)
--- a/support/include/nfslib.h
+++ b/support/include/nfslib.h
@@ -34,18 +34,27 @@
 #ifndef _PATH_XTABTMP
 #define _PATH_XTABTMP          NFS_STATEDIR "/xtab.tmp"
 #endif
+#ifndef _PATH_XTABLCK
+#define _PATH_XTABLCK          NFS_STATEDIR "/.xtab.lock"
+#endif
 #ifndef _PATH_ETAB
 #define _PATH_ETAB             NFS_STATEDIR "/etab"
 #endif
 #ifndef _PATH_ETABTMP
 #define _PATH_ETABTMP          NFS_STATEDIR "/etab.tmp"
 #endif
+#ifndef _PATH_ETABLCK
+#define _PATH_ETABLCK          NFS_STATEDIR "/.etab.lock"
+#endif
 #ifndef _PATH_RMTAB
 #define _PATH_RMTAB            NFS_STATEDIR "/rmtab"
 #endif
 #ifndef _PATH_RMTABTMP
 #define _PATH_RMTABTMP         _PATH_RMTAB ".tmp"
 #endif
+#ifndef _PATH_RMTABLCK
+#define _PATH_RMTABLCK         NFS_STATEDIR "/.rmtab.lock"
+#endif
 #ifndef _PATH_PROC_EXPORTS
 #define        _PATH_PROC_EXPORTS      "/proc/fs/nfs/exports"
 #define        _PATH_PROC_EXPORTS_ALT  "/proc/fs/nfsd/exports"

diff --git a/support/nfs/xio.c b/support/nfs/xio.c
index f21f5f058f9d15c5fdccb601cc68e7a44fab4a7f..5e2e1e95e7228d42ef81ed8b1f44bc7cc2db2d13 100644 (file)
--- a/support/nfs/xio.c
+++ b/support/nfs/xio.c
@@ -17,6 +17,7 @@
 #include <ctype.h>
 #include <signal.h>
 #include <unistd.h>
+#include <errno.h>
 #include "xmalloc.h"
 #include "xlog.h"
 #include "xio.h"
@@ -54,16 +55,16 @@ xflock(char *fname, char *type)
 {
        struct sigaction sa, oldsa;
        int             readonly = !strcmp(type, "r");
-       mode_t  mode = (S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH);
        struct flock    fl = { readonly? F_RDLCK : F_WRLCK, SEEK_SET, 0, 0, 0 };
        int             fd;
 
        if (readonly)
-               fd = open(fname, O_RDONLY);
+               fd = open(fname, (O_RDONLY|O_CREAT), 0600);
        else
-               fd = open(fname, (O_RDWR|O_CREAT), mode);
+               fd = open(fname, (O_RDWR|O_CREAT), 0600);
        if (fd < 0) {
-               xlog(L_WARNING, "could not open %s for locking", fname);
+               xlog(L_WARNING, "could not open %s for locking: errno %d (%s)",
+                               fname, errno, strerror(errno));
                return -1;
        }
 
@@ -74,7 +75,8 @@ xflock(char *fname, char *type)
        alarm(10);
        if (fcntl(fd, F_SETLKW, &fl) < 0) {
                alarm(0);
-               xlog(L_WARNING, "failed to lock %s", fname);
+               xlog(L_WARNING, "failed to lock %s: errno %d (%s)",
+                               fname, errno, strerror(errno));
                close(fd);
                fd = 0;
        } else {

diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index 8084359e290edd34faf489d56ff7a268b649d1f1..25d292bdc5efeafc8f82362343723986018dd64d 100644 (file)
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -88,6 +88,14 @@ unregister_services (void)
                pmap_unset (MOUNTPROG, MOUNTVERS_NFSV3);
 }
 
+static void
+cleanup_lockfiles (void)
+{
+       unlink(_PATH_XTABLCK);
+       unlink(_PATH_ETABLCK);
+       unlink(_PATH_RMTABLCK);
+}
+
 /* Wait for all worker child processes to exit and reap them */
 static void
 wait_for_workers (void)
@@ -154,6 +162,7 @@ fork_workers(void)
        /* in parent */
        wait_for_workers();
        unregister_services();
+       cleanup_lockfiles();
        xlog(L_NOTICE, "mountd: no more workers, exiting\n");
        exit(0);
 }
@@ -170,6 +179,7 @@ killer (int sig)
                kill(0, SIGTERM);
                wait_for_workers();
        }
+       cleanup_lockfiles();
        xlog (L_FATAL, "Caught signal %d, un-registering and exiting.", sig);
 }
 

diff --git a/utils/mountd/rmtab.c b/utils/mountd/rmtab.c
index 5787ed6a0bebacdc9f4bb270d933833e28f425bf..c371f8db8bef27bd8d9dd5e98807d026e754122d 100644 (file)
--- a/utils/mountd/rmtab.c
+++ b/utils/mountd/rmtab.c
@@ -58,7 +58,7 @@ mountlist_add(char *host, const char *path)
        int             lockid;
        long            pos;
 
-       if ((lockid = xflock(_PATH_RMTAB, "a")) < 0)
+       if ((lockid = xflock(_PATH_RMTABLCK, "a")) < 0)
                return;
        setrmtabent("r+");
        while ((rep = getrmtabent(1, &pos)) != NULL) {
@@ -98,7 +98,7 @@ mountlist_del(char *hname, const char *path)
        int             lockid;
        int             match;
 
-       if ((lockid = xflock(_PATH_RMTAB, "w")) < 0)
+       if ((lockid = xflock(_PATH_RMTABLCK, "w")) < 0)
                return;
        if (!setrmtabent("r")) {
                xfunlock(lockid);
@@ -139,7 +139,7 @@ mountlist_del_all(struct sockaddr_in *sin)
        FILE            *fp;
        int             lockid;
 
-       if ((lockid = xflock(_PATH_RMTAB, "w")) < 0)
+       if ((lockid = xflock(_PATH_RMTABLCK, "w")) < 0)
                return;
        if (!(hp = gethostbyaddr((char *)&addr, sizeof(addr), AF_INET))) {
                xlog(L_ERROR, "can't get hostname of %s", inet_ntoa(addr));
@@ -188,7 +188,7 @@ mountlist_list(void)
        struct in_addr          addr;
        struct hostent          *he;
 
-       if ((lockid = xflock(_PATH_RMTAB, "r")) < 0)
+       if ((lockid = xflock(_PATH_RMTABLCK, "r")) < 0)
                return NULL;
        if (stat(_PATH_RMTAB, &stb) < 0) {
                xlog(L_ERROR, "can't stat %s", _PATH_RMTAB);