Mountd should use separate lockfiles
authorBen Myers <bpm@sgi.com>
Fri, 3 Apr 2009 19:13:10 +0000 (15:13 -0400)
committerSteve Dickson <steved@redhat.com>
Fri, 3 Apr 2009 19:13:10 +0000 (15:13 -0400)
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>
support/export/rmtab.c
support/export/xtab.c
support/include/nfslib.h
support/nfs/xio.c
utils/mountd/mountd.c
utils/mountd/rmtab.c

index e11a22a..b49e1aa 100644 (file)
@@ -57,7 +57,7 @@ rmtab_read(void)
                   file. */
                int     lockid;
                FILE    *fp;
                   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"))) {
                        return -1;
                rewindrmtabent();
                if (!(fp = fsetrmtabent(_PATH_RMTABTMP, "w"))) {
index 510765a..3b1dcce 100644 (file)
@@ -23,7 +23,7 @@
 static void cond_rename(char *newfile, char *oldfile);
 
 static int
 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
 {
     /* 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;
 
        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) {
                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);
        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);
        } 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
        } else
-               return xtab_read(_PATH_XTAB, 2);
+               return xtab_read(_PATH_XTAB, _PATH_XTABLCK, 2);
 }
 
 int
 xtab_export_read(void)
 {
 }
 
 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
  * 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;
 
 {
        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;
        }
                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()
 {
 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()
 {
 }
 
 int
 xtab_mount_write()
 {
-       return xtab_write(_PATH_XTAB, _PATH_XTABTMP, 0);
+       return xtab_write(_PATH_XTAB, _PATH_XTABTMP, _PATH_XTABLCK, 0);
 }
 
 void
 }
 
 void
@@ -139,7 +141,7 @@ xtab_append(nfs_export *exp)
        struct exportent xe;
        int             lockid;
 
        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;
                return;
        setexportent(_PATH_XTAB, "a");
        xe = exp->m_export;
index a51d79d..9d0d39d 100644 (file)
 #ifndef _PATH_XTABTMP
 #define _PATH_XTABTMP          NFS_STATEDIR "/xtab.tmp"
 #endif
 #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_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_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"
 #ifndef _PATH_PROC_EXPORTS
 #define        _PATH_PROC_EXPORTS      "/proc/fs/nfs/exports"
 #define        _PATH_PROC_EXPORTS_ALT  "/proc/fs/nfsd/exports"
index f21f5f0..5e2e1e9 100644 (file)
@@ -17,6 +17,7 @@
 #include <ctype.h>
 #include <signal.h>
 #include <unistd.h>
 #include <ctype.h>
 #include <signal.h>
 #include <unistd.h>
+#include <errno.h>
 #include "xmalloc.h"
 #include "xlog.h"
 #include "xio.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");
 {
        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)
        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
        else
-               fd = open(fname, (O_RDWR|O_CREAT), mode);
+               fd = open(fname, (O_RDWR|O_CREAT), 0600);
        if (fd < 0) {
        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;
        }
 
                return -1;
        }
 
@@ -74,7 +75,8 @@ xflock(char *fname, char *type)
        alarm(10);
        if (fcntl(fd, F_SETLKW, &fl) < 0) {
                alarm(0);
        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 {
                close(fd);
                fd = 0;
        } else {
index 8084359..25d292b 100644 (file)
@@ -88,6 +88,14 @@ unregister_services (void)
                pmap_unset (MOUNTPROG, MOUNTVERS_NFSV3);
 }
 
                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)
 /* 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();
        /* in parent */
        wait_for_workers();
        unregister_services();
+       cleanup_lockfiles();
        xlog(L_NOTICE, "mountd: no more workers, exiting\n");
        exit(0);
 }
        xlog(L_NOTICE, "mountd: no more workers, exiting\n");
        exit(0);
 }
@@ -170,6 +179,7 @@ killer (int sig)
                kill(0, SIGTERM);
                wait_for_workers();
        }
                kill(0, SIGTERM);
                wait_for_workers();
        }
+       cleanup_lockfiles();
        xlog (L_FATAL, "Caught signal %d, un-registering and exiting.", sig);
 }
 
        xlog (L_FATAL, "Caught signal %d, un-registering and exiting.", sig);
 }
 
index 5787ed6..c371f8d 100644 (file)
@@ -58,7 +58,7 @@ mountlist_add(char *host, const char *path)
        int             lockid;
        long            pos;
 
        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) {
                return;
        setrmtabent("r+");
        while ((rep = getrmtabent(1, &pos)) != NULL) {
@@ -98,7 +98,7 @@ mountlist_del(char *hname, const char *path)
        int             lockid;
        int             match;
 
        int             lockid;
        int             match;
 
-       if ((lockid = xflock(_PATH_RMTAB, "w")) < 0)
+       if ((lockid = xflock(_PATH_RMTABLCK, "w")) < 0)
                return;
        if (!setrmtabent("r")) {
                xfunlock(lockid);
                return;
        if (!setrmtabent("r")) {
                xfunlock(lockid);
@@ -139,7 +139,7 @@ mountlist_del_all(struct sockaddr_in *sin)
        FILE            *fp;
        int             lockid;
 
        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));
                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;
 
        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);
                return NULL;
        if (stat(_PATH_RMTAB, &stb) < 0) {
                xlog(L_ERROR, "can't stat %s", _PATH_RMTAB);