linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
@ 2019-05-21 12:46 Trond Myklebust
  2019-05-21 12:46 ` [RFC PATCH v2 1/7] mountd: Ensure we don't share cache file descriptors among processes Trond Myklebust
  2019-05-21 17:40 ` [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf Chuck Lever
  0 siblings, 2 replies; 21+ messages in thread
From: Trond Myklebust @ 2019-05-21 12:46 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

The following patchset adds support for the 'root_dir' configuration
option for nfsd in nfs.conf. If a user sets this option to a valid
directory path, then nfsd will act as if it is confined to a chroot
jail based on that directory. All paths in /etc/exporfs and from
exportfs are then resolved relative to that directory.


Trond Myklebust (7):
  mountd: Ensure we don't share cache file descriptors among processes.
  Add a simple workqueue mechanism
  Add utilities for resolving nfsd paths and stat()ing them
  Add a helper to return the real path given an export entry
  Add helpers to read/write to a file through the chrooted thread
  Add support for the nfsd rootdir configuration option to rpc.mountd
  Add support for the nfsd root directory to exportfs

 aclocal/libpthread.m4       |  13 +-
 configure.ac                |   6 +-
 nfs.conf                    |   1 +
 support/export/export.c     |  24 +++
 support/include/Makefile.am |   2 +
 support/include/exportfs.h  |   1 +
 support/include/nfsd_path.h |  17 ++
 support/include/nfslib.h    |   1 +
 support/include/workqueue.h |  22 +++
 support/misc/Makefile.am    |   3 +-
 support/misc/mountpoint.c   |   5 +-
 support/misc/nfsd_path.c    | 175 +++++++++++++++++++++
 support/misc/workqueue.c    | 306 ++++++++++++++++++++++++++++++++++++
 support/nfs/exports.c       |   4 +
 systemd/nfs.conf.man        |   3 +-
 utils/exportfs/Makefile.am  |   2 +-
 utils/exportfs/exportfs.c   |  32 +++-
 utils/mountd/Makefile.am    |   3 +-
 utils/mountd/cache.c        |  79 +++++++---
 utils/mountd/mountd.c       |  13 +-
 utils/nfsd/nfsd.man         |   6 +
 21 files changed, 676 insertions(+), 42 deletions(-)
 create mode 100644 support/include/nfsd_path.h
 create mode 100644 support/include/workqueue.h
 create mode 100644 support/misc/nfsd_path.c
 create mode 100644 support/misc/workqueue.c

-- 
2.21.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [RFC PATCH v2 1/7] mountd: Ensure we don't share cache file descriptors among processes.
  2019-05-21 12:46 [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf Trond Myklebust
@ 2019-05-21 12:46 ` Trond Myklebust
  2019-05-21 12:46   ` [RFC PATCH v2 2/7] Add a simple workqueue mechanism Trond Myklebust
  2019-05-21 17:40 ` [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf Chuck Lever
  1 sibling, 1 reply; 21+ messages in thread
From: Trond Myklebust @ 2019-05-21 12:46 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

Sharing cache descriptors without using locking can be very bad.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 utils/mountd/mountd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index fb7bba4cd390..88a207b3a85a 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -836,8 +836,6 @@ main(int argc, char **argv)
 	if (!foreground)
 		closeall(3);
 
-	cache_open();
-
 	unregister_services();
 	if (version2()) {
 		listeners += nfs_svc_create("mountd", MOUNTPROG,
@@ -888,6 +886,9 @@ main(int argc, char **argv)
 	if (num_threads > 1)
 		fork_workers();
 
+	/* Open files now to avoid sharing descriptors among forked processes */
+	cache_open();
+
 	xlog(L_NOTICE, "Version " VERSION " starting");
 	my_svc_run();
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v2 2/7] Add a simple workqueue mechanism
  2019-05-21 12:46 ` [RFC PATCH v2 1/7] mountd: Ensure we don't share cache file descriptors among processes Trond Myklebust
@ 2019-05-21 12:46   ` Trond Myklebust
  2019-05-21 12:46     ` [RFC PATCH v2 3/7] Add utilities for resolving nfsd paths and stat()ing them Trond Myklebust
  0 siblings, 1 reply; 21+ messages in thread
From: Trond Myklebust @ 2019-05-21 12:46 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

Add a simple workqueue mechanism to allow us to run threads that are
subject to chroot(), and have them operate on the knfsd kernel daemon.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 aclocal/libpthread.m4       |  13 +-
 configure.ac                |   6 +-
 support/include/Makefile.am |   1 +
 support/include/workqueue.h |  18 +++
 support/misc/Makefile.am    |   2 +-
 support/misc/workqueue.c    | 228 ++++++++++++++++++++++++++++++++++++
 utils/mountd/Makefile.am    |   3 +-
 7 files changed, 262 insertions(+), 9 deletions(-)
 create mode 100644 support/include/workqueue.h
 create mode 100644 support/misc/workqueue.c

diff --git a/aclocal/libpthread.m4 b/aclocal/libpthread.m4
index e87d2a0c2dc5..55e046e38cd1 100644
--- a/aclocal/libpthread.m4
+++ b/aclocal/libpthread.m4
@@ -3,11 +3,12 @@ dnl
 AC_DEFUN([AC_LIBPTHREAD], [
 
     dnl Check for library, but do not add -lpthreads to LIBS
-    AC_CHECK_LIB([pthread], [pthread_create], [LIBPTHREAD=-lpthread],
-                 [AC_MSG_ERROR([libpthread not found.])])
-  AC_SUBST(LIBPTHREAD)
-
-  AC_CHECK_HEADERS([pthread.h], ,
-                   [AC_MSG_ERROR([libpthread headers not found.])])
+    AC_CHECK_LIB([pthread], [pthread_create],
+		[AC_DEFINE([HAVE_LIBPTHREAD], [1],
+				[Define to 1 if you have libpthread.])
+		AC_CHECK_HEADERS([pthread.h], [],
+				[AC_MSG_ERROR([libpthread headers not found.])])
+		 AC_SUBST([LIBPTHREAD],[-lpthread])],
+                 [$1])
 
 ])dnl
diff --git a/configure.ac b/configure.ac
index 4d7096193d0b..c6c2d73b06dd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -320,6 +320,10 @@ AC_CHECK_FUNC([getservbyname], ,
 
 AC_CHECK_LIB([crypt], [crypt], [LIBCRYPT="-lcrypt"])
 
+AC_CHECK_HEADERS([sched.h], [], [])
+AC_CHECK_FUNCS([unshare], [] , [])
+AC_LIBPTHREAD([])
+
 if test "$enable_nfsv4" = yes; then
   dnl check for libevent libraries and headers
   AC_LIBEVENT
@@ -417,7 +421,7 @@ if test "$enable_gss" = yes; then
   AC_KERBEROS_V5
 
   dnl Check for pthreads
-  AC_LIBPTHREAD
+  AC_LIBPTHREAD([AC_MSG_ERROR([libpthread not found.])])
 
   dnl librpcsecgss already has a dependency on libgssapi,
   dnl but we need to make sure we get the right version
diff --git a/support/include/Makefile.am b/support/include/Makefile.am
index 599f500e2b40..df5e47836d29 100644
--- a/support/include/Makefile.am
+++ b/support/include/Makefile.am
@@ -19,6 +19,7 @@ noinst_HEADERS = \
 	sockaddr.h \
 	tcpwrapper.h \
 	v4root.h \
+	workqueue.h \
 	xio.h \
 	xlog.h \
 	xmalloc.h \
diff --git a/support/include/workqueue.h b/support/include/workqueue.h
new file mode 100644
index 000000000000..518be82f1b34
--- /dev/null
+++ b/support/include/workqueue.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2019 Trond Myklebust <trond.myklebust@hammerspace.com>
+ */
+#ifndef WORKQUEUE_H
+#define WORKQUEUE_H
+
+struct xthread_workqueue;
+
+struct xthread_workqueue *xthread_workqueue_alloc(void);
+void xthread_workqueue_shutdown(struct xthread_workqueue *wq);
+
+void xthread_work_run_sync(struct xthread_workqueue *wq,
+		void (*fn)(void *), void *data);
+
+void xthread_workqueue_chroot(struct xthread_workqueue *wq,
+		const char *path);
+
+#endif
diff --git a/support/misc/Makefile.am b/support/misc/Makefile.am
index 8936b0d64e45..d0bff8feb6ae 100644
--- a/support/misc/Makefile.am
+++ b/support/misc/Makefile.am
@@ -1,6 +1,6 @@
 ## Process this file with automake to produce Makefile.in
 
 noinst_LIBRARIES = libmisc.a
-libmisc_a_SOURCES = tcpwrapper.c from_local.c mountpoint.c file.c
+libmisc_a_SOURCES = tcpwrapper.c from_local.c mountpoint.c file.c workqueue.c
 
 MAINTAINERCLEANFILES = Makefile.in
diff --git a/support/misc/workqueue.c b/support/misc/workqueue.c
new file mode 100644
index 000000000000..b8d03446f2c7
--- /dev/null
+++ b/support/misc/workqueue.c
@@ -0,0 +1,228 @@
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "config.h"
+#include "workqueue.h"
+#include "xlog.h"
+
+#if defined(HAVE_SCHED_H) && defined(HAVE_LIBPTHREAD) && defined(HAVE_UNSHARE)
+#include <sched.h>
+#include <pthread.h>
+
+struct xwork_struct {
+	struct xwork_struct *next;
+	void (*fn)(void *);
+	void *data;
+};
+
+struct xwork_queue {
+	struct xwork_struct *head;
+	struct xwork_struct **tail;
+
+	unsigned char shutdown : 1;
+};
+
+static void xwork_queue_init(struct xwork_queue *queue)
+{
+	queue->head = NULL;
+	queue->tail = &queue->head;
+	queue->shutdown = 0;
+}
+
+static void xwork_enqueue(struct xwork_queue *queue,
+		struct xwork_struct *entry)
+{
+	entry->next = NULL;
+	*queue->tail = entry;
+	queue->tail = &entry->next;
+}
+
+static struct xwork_struct *xwork_dequeue(struct xwork_queue *queue)
+{
+	struct xwork_struct *entry = NULL;
+	if (queue->head) {
+		entry = queue->head;
+		queue->head = entry->next;
+		if (!queue->head)
+			queue->tail = &queue->head;
+	}
+	return entry;
+}
+
+struct xthread_work {
+	struct xwork_struct work;
+
+	pthread_cond_t cond;
+};
+
+struct xthread_workqueue {
+	struct xwork_queue queue;
+
+	pthread_mutex_t mutex;
+	pthread_cond_t cond;
+};
+
+static void xthread_workqueue_init(struct xthread_workqueue *wq)
+{
+	xwork_queue_init(&wq->queue);
+	pthread_mutex_init(&wq->mutex, NULL);
+	pthread_cond_init(&wq->cond, NULL);
+}
+
+static void xthread_workqueue_fini(struct xthread_workqueue *wq)
+{
+	pthread_cond_destroy(&wq->cond);
+	pthread_mutex_destroy(&wq->mutex);
+}
+
+static int xthread_work_enqueue(struct xthread_workqueue *wq,
+		struct xthread_work *work)
+{
+	xwork_enqueue(&wq->queue, &work->work);
+	pthread_cond_signal(&wq->cond);
+	return 0;
+}
+
+static struct xthread_work *xthread_work_dequeue(struct xthread_workqueue *wq)
+{
+	return (struct xthread_work *)xwork_dequeue(&wq->queue);
+}
+
+static void xthread_workqueue_do_work(struct xthread_workqueue *wq)
+{
+	struct xthread_work *work;
+
+	pthread_mutex_lock(&wq->mutex);
+	/* Signal the caller that we're up and running */
+	pthread_cond_signal(&wq->cond);
+	for (;;) {
+		work = xthread_work_dequeue(wq);
+		if (work) {
+			work->work.fn(work->work.data);
+			pthread_cond_signal(&work->cond);
+			continue;
+		}
+		if (wq->queue.shutdown)
+			break;
+		pthread_cond_wait(&wq->cond, &wq->mutex);
+	}
+	pthread_mutex_unlock(&wq->mutex);
+}
+
+void xthread_workqueue_shutdown(struct xthread_workqueue *wq)
+{
+	pthread_mutex_lock(&wq->mutex);
+	wq->queue.shutdown = 1;
+	pthread_cond_signal(&wq->cond);
+	pthread_mutex_unlock(&wq->mutex);
+}
+
+static void xthread_workqueue_free(struct xthread_workqueue *wq)
+{
+	xthread_workqueue_fini(wq);
+	free(wq);
+}
+
+static void xthread_workqueue_cleanup(void *data)
+{
+	xthread_workqueue_free(data);
+}
+
+static void *xthread_workqueue_worker(void *data)
+{
+	pthread_cleanup_push(xthread_workqueue_cleanup, data);
+	xthread_workqueue_do_work(data);
+	pthread_cleanup_pop(1);
+	return NULL;
+}
+
+struct xthread_workqueue *xthread_workqueue_alloc(void)
+{
+	struct xthread_workqueue *ret;
+	pthread_t thread;
+
+	ret = malloc(sizeof(*ret));
+	if (ret) {
+		xthread_workqueue_init(ret);
+
+		pthread_mutex_lock(&ret->mutex);
+		if (pthread_create(&thread, NULL,
+					xthread_workqueue_worker,
+					ret) == 0) {
+			/* Wait for thread to start */
+			pthread_cond_wait(&ret->cond, &ret->mutex);
+			pthread_mutex_unlock(&ret->mutex);
+			return ret;
+		}
+		pthread_mutex_unlock(&ret->mutex);
+		xthread_workqueue_free(ret);
+		ret = NULL;
+	}
+	return NULL;
+}
+
+void xthread_work_run_sync(struct xthread_workqueue *wq,
+		void (*fn)(void *), void *data)
+{
+	struct xthread_work work = {
+		{
+			NULL,
+			fn,
+			data
+		},
+		PTHREAD_COND_INITIALIZER,
+	};
+	pthread_mutex_lock(&wq->mutex);
+	xthread_work_enqueue(wq, &work);
+	pthread_cond_wait(&work.cond, &wq->mutex);
+	pthread_mutex_unlock(&wq->mutex);
+	pthread_cond_destroy(&work.cond);
+}
+
+static void xthread_workqueue_do_chroot(void *data)
+{
+	const char *path = data;
+
+	if (unshare(CLONE_FS) != 0) {
+		xlog_err("unshare() failed: %m");
+		return;
+	}
+	if (chroot(path) != 0)
+		xlog_err("chroot() failed: %m");
+}
+
+void xthread_workqueue_chroot(struct xthread_workqueue *wq,
+		const char *path)
+{
+	xthread_work_run_sync(wq, xthread_workqueue_do_chroot, (void *)path);
+}
+
+#else
+
+struct xthread_workqueue {
+};
+
+static struct xthread_workqueue ret;
+
+struct xthread_workqueue *xthread_workqueue_alloc(void)
+{
+	return &ret;
+}
+
+void xthread_workqueue_shutdown(struct xthread_workqueue *wq)
+{
+}
+
+void xthread_work_run_sync(struct xthread_workqueue *wq,
+		void (*fn)(void *), void *data)
+{
+	fn(data);
+}
+
+void xthread_workqueue_chroot(struct xthread_workqueue *wq,
+		const char *path)
+{
+	xlog_err("Unable to run as chroot");
+}
+
+#endif /* defined(HAVE_SCHED_H) && defined(HAVE_LIBPTHREAD) && defined(HAVE_UNSHARE) */
diff --git a/utils/mountd/Makefile.am b/utils/mountd/Makefile.am
index 73eeb3f35070..18610f18238c 100644
--- a/utils/mountd/Makefile.am
+++ b/utils/mountd/Makefile.am
@@ -19,7 +19,8 @@ mountd_LDADD = ../../support/export/libexport.a \
 	       ../../support/nfs/libnfs.la \
 	       ../../support/misc/libmisc.a \
 	       $(OPTLIBS) \
-	       $(LIBBSD) $(LIBWRAP) $(LIBNSL) $(LIBBLKID) $(LIBTIRPC)
+	       $(LIBBSD) $(LIBWRAP) $(LIBNSL) $(LIBBLKID) $(LIBTIRPC) \
+	       $(LIBPTHREAD)
 mountd_CPPFLAGS = $(AM_CPPFLAGS) $(CPPFLAGS) \
 		  -I$(top_builddir)/support/include \
 		  -I$(top_srcdir)/support/export
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v2 3/7] Add utilities for resolving nfsd paths and stat()ing them
  2019-05-21 12:46   ` [RFC PATCH v2 2/7] Add a simple workqueue mechanism Trond Myklebust
@ 2019-05-21 12:46     ` Trond Myklebust
  2019-05-21 12:46       ` [RFC PATCH v2 4/7] Add a helper to return the real path given an export entry Trond Myklebust
  0 siblings, 1 reply; 21+ messages in thread
From: Trond Myklebust @ 2019-05-21 12:46 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

Add helper functions that can resolve nfsd paths by prepending the
necessary prefix if the admin has specified a root path in the
nfs.conf file.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 configure.ac                |   2 +-
 support/include/Makefile.am |   1 +
 support/include/nfsd_path.h |  17 ++++
 support/misc/Makefile.am    |   3 +-
 support/misc/nfsd_path.c    | 173 ++++++++++++++++++++++++++++++++++++
 5 files changed, 194 insertions(+), 2 deletions(-)
 create mode 100644 support/include/nfsd_path.h
 create mode 100644 support/misc/nfsd_path.c

diff --git a/configure.ac b/configure.ac
index c6c2d73b06dd..4793daeb2716 100644
--- a/configure.ac
+++ b/configure.ac
@@ -321,7 +321,7 @@ AC_CHECK_FUNC([getservbyname], ,
 AC_CHECK_LIB([crypt], [crypt], [LIBCRYPT="-lcrypt"])
 
 AC_CHECK_HEADERS([sched.h], [], [])
-AC_CHECK_FUNCS([unshare], [] , [])
+AC_CHECK_FUNCS([unshare openat fstatat], [] , [])
 AC_LIBPTHREAD([])
 
 if test "$enable_nfsv4" = yes; then
diff --git a/support/include/Makefile.am b/support/include/Makefile.am
index df5e47836d29..fbf487eee0e2 100644
--- a/support/include/Makefile.am
+++ b/support/include/Makefile.am
@@ -10,6 +10,7 @@ noinst_HEADERS = \
 	misc.h \
 	nfs_mntent.h \
 	nfs_paths.h \
+	nfsd_path.h \
 	nfslib.h \
 	nfsrpc.h \
 	nls.h \
diff --git a/support/include/nfsd_path.h b/support/include/nfsd_path.h
new file mode 100644
index 000000000000..5936cd5ed666
--- /dev/null
+++ b/support/include/nfsd_path.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) 2019 Trond Myklebust <trond.myklebust@hammerspace.com>
+ */
+#ifndef XPATH_H
+#define XPATH_H
+
+void 		nfsd_path_init(void);
+void 		nfsd_path_nfsd_rootfs_close(void);
+
+const char *	nfsd_path_nfsd_rootdir(void);
+char *		nfsd_path_strip_root(char *pathname);
+char *		nfsd_path_prepend_dir(const char *dir, const char *pathname);
+
+int 		nfsd_path_stat(const char *pathname, struct stat *statbuf);
+int 		nfsd_path_lstat(const char *pathname, struct stat *statbuf);
+
+#endif
diff --git a/support/misc/Makefile.am b/support/misc/Makefile.am
index d0bff8feb6ae..ff1e8ab79ae3 100644
--- a/support/misc/Makefile.am
+++ b/support/misc/Makefile.am
@@ -1,6 +1,7 @@
 ## Process this file with automake to produce Makefile.in
 
 noinst_LIBRARIES = libmisc.a
-libmisc_a_SOURCES = tcpwrapper.c from_local.c mountpoint.c file.c workqueue.c
+libmisc_a_SOURCES = tcpwrapper.c from_local.c mountpoint.c file.c \
+		    nfsd_path.c workqueue.c
 
 MAINTAINERCLEANFILES = Makefile.in
diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c
new file mode 100644
index 000000000000..481ba49a38fd
--- /dev/null
+++ b/support/misc/nfsd_path.c
@@ -0,0 +1,173 @@
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "config.h"
+#include "conffile.h"
+#include "xmalloc.h"
+#include "xlog.h"
+#include "nfsd_path.h"
+
+static int
+nfsd_path_isslash(const char *path)
+{
+	return path[0] == '/' && path[1] == '/';
+}
+
+static int
+nfsd_path_isdot(const char *path)
+{
+	return path[0] == '.' && path[1] == '/';
+}
+
+static const char *
+nfsd_path_strip(const char *path)
+{
+	if (!path || *path == '\0')
+		goto out;
+	for (;;) {
+		if (nfsd_path_isslash(path)) {
+			path++;
+			continue;
+		}
+		if (nfsd_path_isdot(path)) {
+			path += 2;
+			continue;
+		}
+		break;
+	}
+out:
+	return path;
+}
+
+const char *
+nfsd_path_nfsd_rootdir(void)
+{
+	const char *rootdir;
+
+	rootdir = nfsd_path_strip(conf_get_str("nfsd", "root_dir"));
+	if (!rootdir || rootdir[0] ==  '\0')
+		return NULL;
+	if (rootdir[0] == '/' && rootdir[1] == '\0')
+		return NULL;
+	return rootdir;
+}
+
+char *
+nfsd_path_strip_root(char *pathname)
+{
+	const char *dir = nfsd_path_nfsd_rootdir();
+	char *ret;
+
+	ret = strstr(pathname, dir);
+	if (!ret || ret != pathname)
+		return pathname;
+	return pathname + strlen(dir);
+}
+
+char *
+nfsd_path_prepend_dir(const char *dir, const char *pathname)
+{
+	size_t len, dirlen;
+	char *ret;
+
+	dirlen = strlen(dir);
+	while (dirlen > 0 && dir[dirlen - 1] == '/')
+		dirlen--;
+	if (!dirlen)
+		return NULL;
+	len = dirlen + strlen(pathname) + 1;
+	ret = xmalloc(len + 1);
+	snprintf(ret, len, "%.*s/%s", (int)dirlen, dir, pathname);
+	return ret;
+}
+
+#if defined(HAVE_FSTATAT) && defined(HAVE_OPENAT)
+static int nfsd_rootfs = AT_FDCWD;
+
+void nfsd_path_nfsd_rootfs_close(void)
+{
+	if (nfsd_rootfs != AT_FDCWD) {
+		close(nfsd_rootfs);
+		nfsd_rootfs = AT_FDCWD;
+	}
+}
+
+void nfsd_path_init(void)
+{
+	const char *rootdir = nfsd_path_nfsd_rootdir();
+
+	nfsd_path_nfsd_rootfs_close();
+	if (rootdir) {
+		nfsd_rootfs = openat(AT_FDCWD, rootdir, O_PATH);
+		if (nfsd_rootfs == -1)
+			xlog_err("Could not open directory %s: %m", rootdir);
+	}
+}
+
+int nfsd_path_stat(const char *pathname, struct stat *statbuf)
+{
+	if (nfsd_rootfs != AT_FDCWD) {
+		while (pathname[0] == '/')
+			pathname++;
+	}
+	return fstatat(nfsd_rootfs, pathname, statbuf, AT_EMPTY_PATH |
+			AT_NO_AUTOMOUNT);
+}
+
+int nfsd_path_lstat(const char *pathname, struct stat *statbuf)
+{
+	if (nfsd_rootfs != AT_FDCWD) {
+		while (pathname[0] == '/')
+			pathname++;
+	}
+	return fstatat(nfsd_rootfs, pathname, statbuf, AT_EMPTY_PATH |
+			AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW);
+}
+
+#else /* defined(HAVE_FSTATAT) && defined(HAVE_OPENAT) */
+void nfsd_path_init(void)
+{
+}
+
+void nfsd_path_nfsd_rootfs_close(void)
+{
+}
+
+int nfsd_path_stat(const char *pathname, struct stat *statbuf)
+{
+	const char *rootdir = nfsd_path_nfsd_rootdir();
+	char *str;
+	int ret;
+
+	if (!rootdir)
+		goto out_stat;
+	str = nfsd_path_prepend_dir(rootdir, nfsd_path_strip(pathname));
+	if (!str)
+		goto out_stat;
+	ret = stat(str, statbuf);
+	xfree(str);
+	return ret;
+out_stat:
+	return stat(pathname, statbuf);
+}
+
+int nfsd_path_lstat(const char *pathname, struct stat *statbuf)
+{
+	const char *rootdir = nfsd_path_nfsd_rootdir();
+	char *str;
+	int ret;
+
+	if (!rootdir)
+		goto out_lstat;
+	str = nfsd_path_prepend_dir(rootdir, nfsd_path_strip(pathname));
+	if (!str)
+		goto out_lstat;
+	ret = lstat(str, statbuf);
+	xfree(str);
+	return ret;
+out_lstat:
+	return lstat(pathname, statbuf);
+}
+#endif
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v2 4/7] Add a helper to return the real path given an export entry
  2019-05-21 12:46     ` [RFC PATCH v2 3/7] Add utilities for resolving nfsd paths and stat()ing them Trond Myklebust
@ 2019-05-21 12:46       ` Trond Myklebust
  2019-05-21 12:46         ` [RFC PATCH v2 5/7] Add helpers to read/write to a file through the chrooted thread Trond Myklebust
  0 siblings, 1 reply; 21+ messages in thread
From: Trond Myklebust @ 2019-05-21 12:46 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

Add a helper that can prepend the nfsd root directory path in order
to allow mountd to perform its comparisons with mtab etc.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 support/export/export.c    | 24 ++++++++++++++++++++++++
 support/include/exportfs.h |  1 +
 support/include/nfslib.h   |  1 +
 support/misc/nfsd_path.c   |  4 +++-
 support/nfs/exports.c      |  4 ++++
 5 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/support/export/export.c b/support/export/export.c
index fbe68e84e5b3..229b02eb2dd4 100644
--- a/support/export/export.c
+++ b/support/export/export.c
@@ -20,6 +20,7 @@
 #include "xmalloc.h"
 #include "nfslib.h"
 #include "exportfs.h"
+#include "nfsd_path.h"
 
 exp_hash_table exportlist[MCL_MAXTYPES] = {{NULL, {{NULL,NULL}, }}, }; 
 static int export_hash(char *);
@@ -30,6 +31,28 @@ static void	export_add(nfs_export *exp);
 static int	export_check(const nfs_export *exp, const struct addrinfo *ai,
 				const char *path);
 
+/* Return a real path for the export. */
+static void
+exportent_mkrealpath(struct exportent *eep)
+{
+	char *chroot = nfsd_path_nfsd_rootdir();
+	char *ret = NULL;
+
+	if (chroot)
+		ret = nfsd_path_prepend_dir(chroot, eep->e_path);
+	if (!ret)
+		ret = xstrdup(eep->e_path);
+	eep->e_realpath = ret;
+}
+
+char *
+exportent_realpath(struct exportent *eep)
+{
+	if (!eep->e_realpath)
+		exportent_mkrealpath(eep);
+	return eep->e_realpath;
+}
+
 void
 exportent_release(struct exportent *eep)
 {
@@ -39,6 +62,7 @@ exportent_release(struct exportent *eep)
 	free(eep->e_fslocdata);
 	free(eep->e_uuid);
 	xfree(eep->e_hostname);
+	xfree(eep->e_realpath);
 }
 
 static void
diff --git a/support/include/exportfs.h b/support/include/exportfs.h
index 4e0d9d132b4c..daa7e2a06d82 100644
--- a/support/include/exportfs.h
+++ b/support/include/exportfs.h
@@ -171,5 +171,6 @@ struct export_features {
 
 struct export_features *get_export_features(void);
 void fix_pseudoflavor_flags(struct exportent *ep);
+char *exportent_realpath(struct exportent *eep);
 
 #endif /* EXPORTFS_H */
diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index b09fce42e677..84d8270b330f 100644
--- a/support/include/nfslib.h
+++ b/support/include/nfslib.h
@@ -84,6 +84,7 @@ struct exportent {
 	char *		e_uuid;
 	struct sec_entry e_secinfo[SECFLAVOR_COUNT+1];
 	unsigned int	e_ttl;
+	char *		e_realpath;
 };
 
 struct rmtabent {
diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c
index 481ba49a38fd..386b171f26df 100644
--- a/support/misc/nfsd_path.c
+++ b/support/misc/nfsd_path.c
@@ -77,9 +77,11 @@ nfsd_path_prepend_dir(const char *dir, const char *pathname)
 		dirlen--;
 	if (!dirlen)
 		return NULL;
+	while (pathname[0] == '/')
+		pathname++;
 	len = dirlen + strlen(pathname) + 1;
 	ret = xmalloc(len + 1);
-	snprintf(ret, len, "%.*s/%s", (int)dirlen, dir, pathname);
+	snprintf(ret, len+1, "%.*s/%s", (int)dirlen, dir, pathname);
 	return ret;
 }
 
diff --git a/support/nfs/exports.c b/support/nfs/exports.c
index 5f4cb9568814..3ecfde797e3b 100644
--- a/support/nfs/exports.c
+++ b/support/nfs/exports.c
@@ -155,6 +155,7 @@ getexportent(int fromkernel, int fromexports)
 	}
 
 	xfree(ee.e_hostname);
+	xfree(ee.e_realpath);
 	ee = def_ee;
 
 	/* Check for default client */
@@ -358,6 +359,7 @@ dupexportent(struct exportent *dst, struct exportent *src)
 	if (src->e_uuid)
 		dst->e_uuid = strdup(src->e_uuid);
 	dst->e_hostname = NULL;
+	dst->e_realpath = NULL;
 }
 
 struct exportent *
@@ -369,6 +371,8 @@ mkexportent(char *hname, char *path, char *options)
 
 	xfree(ee.e_hostname);
 	ee.e_hostname = xstrdup(hname);
+	xfree(ee.e_realpath);
+	ee.e_realpath = NULL;
 
 	if (strlen(path) >= sizeof(ee.e_path)) {
 		xlog(L_ERROR, "path name %s too long", path);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v2 5/7] Add helpers to read/write to a file through the chrooted thread
  2019-05-21 12:46       ` [RFC PATCH v2 4/7] Add a helper to return the real path given an export entry Trond Myklebust
@ 2019-05-21 12:46         ` Trond Myklebust
  2019-05-21 12:47           ` [RFC PATCH v2 6/7] Add support for the nfsd rootdir configuration option to rpc.mountd Trond Myklebust
  0 siblings, 1 reply; 21+ messages in thread
From: Trond Myklebust @ 2019-05-21 12:46 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

Add helper functions to do synchronous I/O to a file from inside the
chrooted environment. This ensures that calls to kern_path() in the
kernel resolves to paths that are relative to the nfsd root directory.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 support/include/workqueue.h |  4 ++
 support/misc/workqueue.c    | 78 +++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/support/include/workqueue.h b/support/include/workqueue.h
index 518be82f1b34..21b1ee6bc873 100644
--- a/support/include/workqueue.h
+++ b/support/include/workqueue.h
@@ -15,4 +15,8 @@ void xthread_work_run_sync(struct xthread_workqueue *wq,
 void xthread_workqueue_chroot(struct xthread_workqueue *wq,
 		const char *path);
 
+ssize_t xthread_read(struct xthread_workqueue *wq,
+		int fd, char *buf, size_t len);
+ssize_t xthread_write(struct xthread_workqueue *wq,
+		int fd, const char *buf, size_t len);
 #endif
diff --git a/support/misc/workqueue.c b/support/misc/workqueue.c
index b8d03446f2c7..9d967cef6547 100644
--- a/support/misc/workqueue.c
+++ b/support/misc/workqueue.c
@@ -1,3 +1,4 @@
+#include <errno.h>
 #include <stdlib.h>
 #include <unistd.h>
 
@@ -197,6 +198,72 @@ void xthread_workqueue_chroot(struct xthread_workqueue *wq,
 	xthread_work_run_sync(wq, xthread_workqueue_do_chroot, (void *)path);
 }
 
+struct xthread_read_data {
+	int fd;
+	char *buf;
+	size_t len;
+	ssize_t ret;
+	int err;
+};
+
+static void xthread_readfunc(void *data)
+{
+	struct xthread_read_data *d = data;
+
+	d->ret = read(d->fd, d->buf, d->len);
+	if (d->ret < 0)
+		d->err = errno;
+}
+
+ssize_t xthread_read(struct xthread_workqueue *wq,
+		int fd, char *buf, size_t len)
+{
+	struct xthread_read_data data = {
+		fd,
+		buf,
+		len,
+		0,
+		0
+	};
+	xthread_work_run_sync(wq, xthread_readfunc, &data);
+	if (data.ret < 0)
+		errno = data.err;
+	return data.ret;
+}
+
+struct xthread_write_data {
+	int fd;
+	const char *buf;
+	size_t len;
+	ssize_t ret;
+	int err;
+};
+
+static void xthread_writefunc(void *data)
+{
+	struct xthread_write_data *d = data;
+
+	d->ret = write(d->fd, d->buf, d->len);
+	if (d->ret < 0)
+		d->err = errno;
+}
+
+ssize_t xthread_write(struct xthread_workqueue *wq,
+		int fd, const char *buf, size_t len)
+{
+	struct xthread_write_data data = {
+		fd,
+		buf,
+		len,
+		0,
+		0
+	};
+	xthread_work_run_sync(wq, xthread_writefunc, &data);
+	if (data.ret < 0)
+		errno = data.err;
+	return data.ret;
+}
+
 #else
 
 struct xthread_workqueue {
@@ -225,4 +292,15 @@ void xthread_workqueue_chroot(struct xthread_workqueue *wq,
 	xlog_err("Unable to run as chroot");
 }
 
+ssize_t xthread_read(struct xthread_workqueue *wq,
+		int fd, char *buf, size_t len)
+{
+	return read(fd, buf, len);
+}
+
+ssize_t xthread_write(struct xthread_workqueue *wq,
+		int fd, const char *buf, size_t len)
+{
+	return write(fd, buf, len);
+}
 #endif /* defined(HAVE_SCHED_H) && defined(HAVE_LIBPTHREAD) && defined(HAVE_UNSHARE) */
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v2 6/7] Add support for the nfsd rootdir configuration option to rpc.mountd
  2019-05-21 12:46         ` [RFC PATCH v2 5/7] Add helpers to read/write to a file through the chrooted thread Trond Myklebust
@ 2019-05-21 12:47           ` Trond Myklebust
  2019-05-21 12:47             ` [RFC PATCH v2 7/7] Add support for the nfsd root directory to exportfs Trond Myklebust
  0 siblings, 1 reply; 21+ messages in thread
From: Trond Myklebust @ 2019-05-21 12:47 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

Ensure that I/O to those pseudo files that resolve filehandles and exports
go through the chrooted threads, so that we can use paths that are relative
to the nfsd root directory.
Ensure that any stat() or lstat() calls go through their nfsd_path_*
equivalent so that they too can be resolved correctly.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 nfs.conf                  |  1 +
 support/misc/mountpoint.c |  5 ++-
 systemd/nfs.conf.man      |  3 +-
 utils/mountd/cache.c      | 79 ++++++++++++++++++++++++++++-----------
 utils/mountd/mountd.c     |  8 ++--
 utils/nfsd/nfsd.man       |  6 +++
 6 files changed, 74 insertions(+), 28 deletions(-)

diff --git a/nfs.conf b/nfs.conf
index 27e962c8a2a9..4d3bc512c4be 100644
--- a/nfs.conf
+++ b/nfs.conf
@@ -60,6 +60,7 @@
 # vers4.1=y
 # vers4.2=y
 # rdma=n
+# root_dir=/export
 #
 [statd]
 # debug=0
diff --git a/support/misc/mountpoint.c b/support/misc/mountpoint.c
index 9f9ce44ec1e3..9723dcede79d 100644
--- a/support/misc/mountpoint.c
+++ b/support/misc/mountpoint.c
@@ -7,6 +7,7 @@
 #include "xcommon.h"
 #include <sys/stat.h>
 #include "misc.h"
+#include "nfsd_path.h"
 
 int
 is_mountpoint(char *path)
@@ -26,8 +27,8 @@ is_mountpoint(char *path)
 	dotdot = xmalloc(strlen(path)+4);
 
 	strcat(strcpy(dotdot, path), "/..");
-	if (lstat(path, &stb) != 0 ||
-	    lstat(dotdot, &pstb) != 0)
+	if (nfsd_path_lstat(path, &stb) != 0 ||
+	    nfsd_path_lstat(dotdot, &pstb) != 0)
 		rv = 0;
 	else
 		if (stb.st_dev != pstb.st_dev ||
diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
index e3654a3c2c2b..a88799769365 100644
--- a/systemd/nfs.conf.man
+++ b/systemd/nfs.conf.man
@@ -136,7 +136,8 @@ Recognized values:
 .BR vers4.0 ,
 .BR vers4.1 ,
 .BR vers4.2 ,
-.BR rdma .
+.BR rdma ,
+.BR root_dir .
 
 Version and protocol values are Boolean values as described above,
 and are also used by
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index bdbd1904eb76..6e15ecd986aa 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -27,17 +27,21 @@
 #include <grp.h>
 #include <mntent.h>
 #include "misc.h"
+#include "nfsd_path.h"
 #include "nfslib.h"
 #include "exportfs.h"
 #include "mountd.h"
 #include "fsloc.h"
 #include "pseudoflavors.h"
+#include "workqueue.h"
 #include "xcommon.h"
 
 #ifdef USE_BLKID
 #include "blkid/blkid.h"
 #endif
 
+static struct xthread_workqueue *cache_workqueue;
+
 /*
  * Invoked by RPC service loop
  */
@@ -55,6 +59,34 @@ enum nfsd_fsid {
 	FSID_UUID16_INUM,
 };
 
+static ssize_t cache_read(int fd, char *buf, size_t len)
+{
+	if (cache_workqueue)
+		return xthread_read(cache_workqueue, fd, buf, len);
+	return read(fd, buf, len);
+}
+
+static ssize_t cache_write(int fd, const char *buf, size_t len)
+{
+	if (cache_workqueue)
+		return xthread_write(cache_workqueue, fd, buf, len);
+	return write(fd, buf, len);
+}
+
+static void
+cache_setup_workqueue(void)
+{
+	const char *chroot;
+
+	chroot = nfsd_path_nfsd_rootdir();
+	if (!chroot)
+		return;
+	cache_workqueue = xthread_workqueue_alloc();
+	if (!cache_workqueue)
+		return;
+	xthread_workqueue_chroot(cache_workqueue, chroot);
+}
+
 /*
  * Support routines for text-based upcalls.
  * Fields are separated by spaces.
@@ -221,7 +253,7 @@ static const char *get_uuid_blkdev(char *path)
 	if (cache == NULL)
 		blkid_get_cache(&cache, NULL);
 
-	if (stat(path, &stb) != 0)
+	if (nfsd_path_stat(path, &stb) != 0)
 		return NULL;
 	devname = blkid_devno_to_devname(stb.st_dev);
 	if (!devname)
@@ -373,21 +405,22 @@ static char *next_mnt(void **v, char *p)
 	FILE *f;
 	struct mntent *me;
 	size_t l = strlen(p);
+	char *mnt_dir = NULL;
+
 	if (*v == NULL) {
 		f = setmntent("/etc/mtab", "r");
 		*v = f;
 	} else
 		f = *v;
-	while ((me = getmntent(f)) != NULL && l > 1 &&
-	       (strncmp(me->mnt_dir, p, l) != 0 ||
-		me->mnt_dir[l] != '/'))
-		;
-	if (me == NULL) {
-		endmntent(f);
-		*v = NULL;
-		return NULL;
+	while ((me = getmntent(f)) != NULL && l > 1) {
+		mnt_dir = nfsd_path_strip_root(me->mnt_dir);
+
+		if (strncmp(mnt_dir, p, l) == 0 && mnt_dir[l] != '/')
+			return mnt_dir;
 	}
-	return me->mnt_dir;
+	endmntent(f);
+	*v = NULL;
+	return NULL;
 }
 
 /* same_path() check is two paths refer to the same directory.
@@ -458,9 +491,9 @@ fallback:
 	 * bind-mounted in two places and both are exported, it
 	 * could give a false positive
 	 */
-	if (lstat(p, &sc) != 0)
+	if (nfsd_path_lstat(p, &sc) != 0)
 		return 0;
-	if (lstat(parent, &sp) != 0)
+	if (nfsd_path_lstat(parent, &sp) != 0)
 		return 0;
 	if (sc.st_dev != sp.st_dev)
 		return 0;
@@ -610,7 +643,7 @@ static bool match_fsid(struct parsed_fsid *parsed, nfs_export *exp, char *path)
 	int type;
 	char u[16];
 
-	if (stat(path, &stb) != 0)
+	if (nfsd_path_stat(path, &stb) != 0)
 		return false;
 	if (!S_ISDIR(stb.st_mode) && !S_ISREG(stb.st_mode))
 		return false;
@@ -692,7 +725,7 @@ static void nfsd_fh(int f)
 	char buf[RPC_CHAN_BUF_SIZE], *bp;
 	int blen;
 
-	blen = read(f, buf, sizeof(buf));
+	blen = cache_read(f, buf, sizeof(buf));
 	if (blen <= 0 || buf[blen-1] != '\n') return;
 	buf[blen-1] = 0;
 
@@ -829,7 +862,7 @@ static void nfsd_fh(int f)
 	if (found)
 		qword_add(&bp, &blen, found_path);
 	qword_addeol(&bp, &blen);
-	if (blen <= 0 || write(f, buf, bp - buf) != bp - buf)
+	if (blen <= 0 || cache_write(f, buf, bp - buf) != bp - buf)
 		xlog(L_ERROR, "nfsd_fh: error writing reply");
 out:
 	if (found_path)
@@ -921,7 +954,7 @@ static int dump_to_cache(int f, char *buf, int buflen, char *domain,
 		qword_adduint(&bp, &blen, now + ttl);
 	qword_addeol(&bp, &blen);
 	if (blen <= 0) return -1;
-	if (write(f, buf, bp - buf) != bp - buf) return -1;
+	if (cache_write(f, buf, bp - buf) != bp - buf) return -1;
 	return 0;
 }
 
@@ -1298,7 +1331,7 @@ static void nfsd_export(int f)
 	char buf[RPC_CHAN_BUF_SIZE], *bp;
 	int blen;
 
-	blen = read(f, buf, sizeof(buf));
+	blen = cache_read(f, buf, sizeof(buf));
 	if (blen <= 0 || buf[blen-1] != '\n') return;
 	buf[blen-1] = 0;
 
@@ -1381,6 +1414,8 @@ extern int manage_gids;
 void cache_open(void) 
 {
 	int i;
+
+	cache_setup_workqueue();
 	for (i=0; cachelist[i].cache_name; i++ ) {
 		char path[100];
 		if (!manage_gids && cachelist[i].cache_handle == auth_unix_gid)
@@ -1456,7 +1491,7 @@ static int cache_export_ent(char *buf, int buflen, char *domain, struct exporten
 		if (strlen(path) <= l || path[l] != '/' ||
 		    strncmp(exp->e_path, path, l) != 0)
 			break;
-		if (stat(exp->e_path, &stb) != 0)
+		if (nfsd_path_stat(exp->e_path, &stb) != 0)
 			break;
 		dev = stb.st_dev;
 		while(path[l] == '/') {
@@ -1469,7 +1504,7 @@ static int cache_export_ent(char *buf, int buflen, char *domain, struct exporten
 				l++;
 			c = path[l];
 			path[l] = 0;
-			err2 = lstat(path, &stb);
+			err2 = nfsd_path_lstat(path, &stb);
 			path[l] = c;
 			if (err2 < 0)
 				break;
@@ -1508,7 +1543,7 @@ int cache_export(nfs_export *exp, char *path)
 	qword_adduint(&bp, &blen, time(0) + exp->m_export.e_ttl);
 	qword_add(&bp, &blen, exp->m_client->m_hostname);
 	qword_addeol(&bp, &blen);
-	if (blen <= 0 || write(f, buf, bp - buf) != bp - buf) blen = -1;
+	if (blen <= 0 || cache_write(f, buf, bp - buf) != bp - buf) blen = -1;
 	close(f);
 	if (blen < 0) return -1;
 
@@ -1546,12 +1581,12 @@ cache_get_filehandle(nfs_export *exp, int len, char *p)
 	qword_add(&bp, &blen, p);
 	qword_addint(&bp, &blen, len);
 	qword_addeol(&bp, &blen);
-	if (blen <= 0 || write(f, buf, bp - buf) != bp - buf) {
+	if (blen <= 0 || cache_write(f, buf, bp - buf) != bp - buf) {
 		close(f);
 		return NULL;
 	}
 	bp = buf;
-	blen = read(f, buf, sizeof(buf));
+	blen = cache_read(f, buf, sizeof(buf));
 	close(f);
 
 	if (blen <= 0 || buf[blen-1] != '\n')
diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index 88a207b3a85a..db9891269051 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -29,6 +29,7 @@
 #include "mountd.h"
 #include "rpcmisc.h"
 #include "pseudoflavors.h"
+#include "nfsd_path.h"
 #include "nfslib.h"
 
 extern void my_svc_run(void);
@@ -374,7 +375,7 @@ mount_pathconf_2_svc(struct svc_req *rqstp, dirpath *path, ppathcnf *res)
 	exp = auth_authenticate("pathconf", sap, p);
 	if (exp == NULL)
 		return 1;
-	else if (stat(p, &stb) < 0) {
+	else if (nfsd_path_stat(p, &stb) < 0) {
 		xlog(L_WARNING, "can't stat exported dir %s: %s",
 				p, strerror(errno));
 		return 1;
@@ -483,7 +484,7 @@ get_rootfh(struct svc_req *rqstp, dirpath *path, nfs_export **expret,
 		*error = MNT3ERR_ACCES;
 		return NULL;
 	}
-	if (stat(p, &stb) < 0) {
+	if (nfsd_path_stat(p, &stb) < 0) {
 		xlog(L_WARNING, "can't stat exported dir %s: %s",
 				p, strerror(errno));
 		if (errno == ENOENT)
@@ -497,7 +498,7 @@ get_rootfh(struct svc_req *rqstp, dirpath *path, nfs_export **expret,
 		*error = MNT3ERR_NOTDIR;
 		return NULL;
 	}
-	if (stat(exp->m_export.e_path, &estb) < 0) {
+	if (nfsd_path_stat(exp->m_export.e_path, &estb) < 0) {
 		xlog(L_WARNING, "can't stat export point %s: %s",
 		     p, strerror(errno));
 		*error = MNT3ERR_NOENT;
@@ -886,6 +887,7 @@ main(int argc, char **argv)
 	if (num_threads > 1)
 		fork_workers();
 
+	nfsd_path_init();
 	/* Open files now to avoid sharing descriptors among forked processes */
 	cache_open();
 
diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
index d83ef869d26e..1f3bdf75a2b4 100644
--- a/utils/nfsd/nfsd.man
+++ b/utils/nfsd/nfsd.man
@@ -167,6 +167,12 @@ Setting these to "off" or similar will disable the selected minor
 versions.  Setting to "on" will enable them.  The default values
 are determined by the kernel, and usually minor versions default to
 being enabled once the implementation is sufficiently complete.
+.B root_dir
+Setting this to a valid path causes the nfs server to act as if the
+supplied path is being prefixed to all the exported entries. For
+instance, if "root_dir=/my/root", and there is an entry in /etc/exports
+for '/filesystem', then the client can mount '/filesystem', but the
+actual path on the server will resolve to '/my/root/filesystem'.
 
 .SH NOTES
 If the program is built with TI-RPC support, it will enable any protocol and
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC PATCH v2 7/7] Add support for the nfsd root directory to exportfs
  2019-05-21 12:47           ` [RFC PATCH v2 6/7] Add support for the nfsd rootdir configuration option to rpc.mountd Trond Myklebust
@ 2019-05-21 12:47             ` Trond Myklebust
  0 siblings, 0 replies; 21+ messages in thread
From: Trond Myklebust @ 2019-05-21 12:47 UTC (permalink / raw)
  To: SteveD; +Cc: linux-nfs

Ensure that exportfs also resolves paths relative to the nfsd root
directory

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 utils/exportfs/Makefile.am |  2 +-
 utils/exportfs/exportfs.c  | 32 ++++++++++++++++++++++++++++++--
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/utils/exportfs/Makefile.am b/utils/exportfs/Makefile.am
index 4b291610d19b..96524c729359 100644
--- a/utils/exportfs/Makefile.am
+++ b/utils/exportfs/Makefile.am
@@ -10,6 +10,6 @@ exportfs_SOURCES = exportfs.c
 exportfs_LDADD = ../../support/export/libexport.a \
 	       	 ../../support/nfs/libnfs.la \
 		 ../../support/misc/libmisc.a \
-		 $(LIBWRAP) $(LIBNSL)
+		 $(LIBWRAP) $(LIBNSL) $(LIBPTHREAD)
 
 MAINTAINERCLEANFILES = Makefile.in
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index 333eadcd0228..05481ad3f896 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -33,8 +33,10 @@
 
 #include "sockaddr.h"
 #include "misc.h"
+#include "nfsd_path.h"
 #include "nfslib.h"
 #include "exportfs.h"
+#include "workqueue.h"
 #include "xlog.h"
 #include "conffile.h"
 
@@ -52,6 +54,29 @@ static const char *lockfile = EXP_LOCKFILE;
 static int _lockfd = -1;
 
 struct state_paths etab;
+static struct xthread_workqueue *exportfs_wq;
+
+static ssize_t exportfs_write(int fd, const char *buf, size_t len)
+{
+	if (exportfs_wq)
+		return xthread_write(exportfs_wq, fd, buf, len);
+	return write(fd, buf, len);
+}
+
+static void
+exportfs_setup_workqueue(void)
+{
+	const char *chroot = nfsd_path_nfsd_rootdir();
+
+	if (!chroot || chroot[0] == '\0')
+		return;
+	if (chroot[0] == '/' && chroot[1] == '\0')
+		return;
+	exportfs_wq = xthread_workqueue_alloc();
+	if (!exportfs_wq)
+		return;
+	xthread_workqueue_chroot(exportfs_wq, chroot);
+}
 
 /*
  * If we aren't careful, changes made by exportfs can be lost
@@ -109,6 +134,7 @@ main(int argc, char **argv)
 
 	conf_init_file(NFS_CONFFILE);
 	xlog_from_conffile("exportfs");
+	nfsd_path_init();
 
 	/* NOTE: following uses "mountd" section of nfs.conf !!!! */
 	s = conf_get_str("mountd", "state-directory-path");
@@ -181,6 +207,8 @@ main(int argc, char **argv)
 		}
 	}
 
+	exportfs_setup_workqueue();
+
 	/*
 	 * Serialize things as best we can
 	 */
@@ -505,7 +533,7 @@ static int test_export(nfs_export *exp, int with_fsid)
 	fd = open("/proc/net/rpc/nfsd.export/channel", O_WRONLY);
 	if (fd < 0)
 		return 0;
-	n = write(fd, buf, strlen(buf));
+	n = exportfs_write(fd, buf, strlen(buf));
 	close(fd);
 	if (n < 0)
 		return 0;
@@ -521,7 +549,7 @@ validate_export(nfs_export *exp)
 	 * otherwise trial-export to '-test-client-' and check for failure.
 	 */
 	struct stat stb;
-	char *path = exp->m_export.e_path;
+	char *path = exportent_realpath(&exp->m_export);
 	struct statfs64 stf;
 	int fs_has_fsid = 0;
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-21 12:46 [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf Trond Myklebust
  2019-05-21 12:46 ` [RFC PATCH v2 1/7] mountd: Ensure we don't share cache file descriptors among processes Trond Myklebust
@ 2019-05-21 17:40 ` Chuck Lever
  2019-05-21 18:17   ` Trond Myklebust
  1 sibling, 1 reply; 21+ messages in thread
From: Chuck Lever @ 2019-05-21 17:40 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Steve Dickson, Linux NFS Mailing List

Hi Trond -

> On May 21, 2019, at 8:46 AM, Trond Myklebust <trondmy@gmail.com> wrote:
> 
> The following patchset adds support for the 'root_dir' configuration
> option for nfsd in nfs.conf. If a user sets this option to a valid
> directory path, then nfsd will act as if it is confined to a chroot
> jail based on that directory. All paths in /etc/exporfs and from
> exportfs are then resolved relative to that directory.

What about files under /proc that mountd might access? I assume these
pathnames are not affected.

Aren't there also one or two other files that maintain export state
like /var/lib/nfs/rmtab? Are those affected?

IMHO it could be less confusing to administrators to make root_dir an
[exportfs] option instead of a [mountd] option, if this is not a true
chroot of mountd.


> Trond Myklebust (7):
>  mountd: Ensure we don't share cache file descriptors among processes.
>  Add a simple workqueue mechanism
>  Add utilities for resolving nfsd paths and stat()ing them
>  Add a helper to return the real path given an export entry
>  Add helpers to read/write to a file through the chrooted thread
>  Add support for the nfsd rootdir configuration option to rpc.mountd
>  Add support for the nfsd root directory to exportfs
> 
> aclocal/libpthread.m4       |  13 +-
> configure.ac                |   6 +-
> nfs.conf                    |   1 +
> support/export/export.c     |  24 +++
> support/include/Makefile.am |   2 +
> support/include/exportfs.h  |   1 +
> support/include/nfsd_path.h |  17 ++
> support/include/nfslib.h    |   1 +
> support/include/workqueue.h |  22 +++
> support/misc/Makefile.am    |   3 +-
> support/misc/mountpoint.c   |   5 +-
> support/misc/nfsd_path.c    | 175 +++++++++++++++++++++
> support/misc/workqueue.c    | 306 ++++++++++++++++++++++++++++++++++++
> support/nfs/exports.c       |   4 +
> systemd/nfs.conf.man        |   3 +-
> utils/exportfs/Makefile.am  |   2 +-
> utils/exportfs/exportfs.c   |  32 +++-
> utils/mountd/Makefile.am    |   3 +-
> utils/mountd/cache.c        |  79 +++++++---
> utils/mountd/mountd.c       |  13 +-
> utils/nfsd/nfsd.man         |   6 +
> 21 files changed, 676 insertions(+), 42 deletions(-)
> create mode 100644 support/include/nfsd_path.h
> create mode 100644 support/include/workqueue.h
> create mode 100644 support/misc/nfsd_path.c
> create mode 100644 support/misc/workqueue.c
> 
> -- 
> 2.21.0
> 

--
Chuck Lever
chucklever@gmail.com




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-21 17:40 ` [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf Chuck Lever
@ 2019-05-21 18:17   ` Trond Myklebust
  2019-05-21 18:59     ` Trond Myklebust
  2019-05-21 19:06     ` Chuck Lever
  0 siblings, 2 replies; 21+ messages in thread
From: Trond Myklebust @ 2019-05-21 18:17 UTC (permalink / raw)
  To: chucklever; +Cc: linux-nfs, SteveD

On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
> Hi Trond -
> 
> > On May 21, 2019, at 8:46 AM, Trond Myklebust <trondmy@gmail.com>
> > wrote:
> > 
> > The following patchset adds support for the 'root_dir'
> > configuration
> > option for nfsd in nfs.conf. If a user sets this option to a valid
> > directory path, then nfsd will act as if it is confined to a chroot
> > jail based on that directory. All paths in /etc/exporfs and from
> > exportfs are then resolved relative to that directory.
> 
> What about files under /proc that mountd might access? I assume these
> pathnames are not affected.
> 
That's why we have 2 threads. One thread is root jailed using chroot,
and is used to talk to knfsd. The other thread is not root jailed (or
at least not by root_dir) and so has full access to /etc, /proc, /var,
...

> Aren't there also one or two other files that maintain export state
> like /var/lib/nfs/rmtab? Are those affected?

See above. They are not affected.

> IMHO it could be less confusing to administrators to make root_dir an
> [exportfs] option instead of a [mountd] option, if this is not a true
> chroot of mountd.

It is neither. I made in a [nfsd] option, since it governs the way that
both exportfs and mountd talk to nfsd.

Cheers
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-21 18:17   ` Trond Myklebust
@ 2019-05-21 18:59     ` Trond Myklebust
  2019-05-21 19:06     ` Chuck Lever
  1 sibling, 0 replies; 21+ messages in thread
From: Trond Myklebust @ 2019-05-21 18:59 UTC (permalink / raw)
  To: chucklever; +Cc: linux-nfs, SteveD

On Tue, 2019-05-21 at 18:17 +0000, Trond Myklebust wrote:
> On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
> > Hi Trond -
> > 
> > > On May 21, 2019, at 8:46 AM, Trond Myklebust <trondmy@gmail.com>
> > > wrote:
> > > 
> > > The following patchset adds support for the 'root_dir'
> > > configuration
> > > option for nfsd in nfs.conf. If a user sets this option to a
> > > valid
> > > directory path, then nfsd will act as if it is confined to a
> > > chroot
> > > jail based on that directory. All paths in /etc/exporfs and from
> > > exportfs are then resolved relative to that directory.
> > 
> > What about files under /proc that mountd might access? I assume
> > these
> > pathnames are not affected.
> > 
> That's why we have 2 threads. One thread is root jailed using chroot,
> and is used to talk to knfsd. The other thread is not root jailed (or
> at least not by root_dir) and so has full access to /etc, /proc,
> /var,
> ...

I should perhaps note that the reason why I used a second thread,
rather than using fork()ed processes like the rest of the mountd code
is to allow the sharing of file descriptors, so that the unconfined
thread can open files that can then be easily used by the root jailed
thread.

This means that if you have an old glibc that does not support POSIX
threads, then the 'root_dir' functionality is disabled. Ditto if you
have a kernel that does not support the unshare() system call or if it
does not support openat()+fstatat().

> > Aren't there also one or two other files that maintain export state
> > like /var/lib/nfs/rmtab? Are those affected?
> 
> See above. They are not affected.
> 
> > IMHO it could be less confusing to administrators to make root_dir
> > an
> > [exportfs] option instead of a [mountd] option, if this is not a
> > true
> > chroot of mountd.
> 
> It is neither. I made in a [nfsd] option, since it governs the way
> that
> both exportfs and mountd talk to nfsd.
> 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-21 18:17   ` Trond Myklebust
  2019-05-21 18:59     ` Trond Myklebust
@ 2019-05-21 19:06     ` Chuck Lever
  2019-05-21 19:58       ` Trond Myklebust
  1 sibling, 1 reply; 21+ messages in thread
From: Chuck Lever @ 2019-05-21 19:06 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Linux NFS Mailing List, Steve Dickson



> On May 21, 2019, at 2:17 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
>> Hi Trond -
>> 
>>> On May 21, 2019, at 8:46 AM, Trond Myklebust <trondmy@gmail.com>
>>> wrote:
>>> 
>>> The following patchset adds support for the 'root_dir'
>>> configuration
>>> option for nfsd in nfs.conf. If a user sets this option to a valid
>>> directory path, then nfsd will act as if it is confined to a chroot
>>> jail based on that directory. All paths in /etc/exporfs and from
>>> exportfs are then resolved relative to that directory.
>> 
>> What about files under /proc that mountd might access? I assume these
>> pathnames are not affected.
>> 
> That's why we have 2 threads. One thread is root jailed using chroot,
> and is used to talk to knfsd. The other thread is not root jailed (or
> at least not by root_dir) and so has full access to /etc, /proc, /var,
> ...
> 
>> Aren't there also one or two other files that maintain export state
>> like /var/lib/nfs/rmtab? Are those affected?
> 
> See above. They are not affected.
> 
>> IMHO it could be less confusing to administrators to make root_dir an
>> [exportfs] option instead of a [mountd] option, if this is not a true
>> chroot of mountd.
> 
> It is neither. I made in a [nfsd] option, since it governs the way that
> both exportfs and mountd talk to nfsd.

My point is not about implementation, it's about how this functionality
is presented to administrators.

In nfs.conf, [nfsd] looks like it controls what options are passed via
rpc.nfsd. That still seems like a confusing admin interface.

IMO admins won't care about who is talking to whom. They will care about
how the export pathnames are interpreted. That seems like it belongs
squarely with the exportfs interface.


--
Chuck Lever
chucklever@gmail.com




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-21 19:06     ` Chuck Lever
@ 2019-05-21 19:58       ` Trond Myklebust
  2019-05-28 15:25         ` Steve Dickson
  2019-05-28 15:30         ` Chuck Lever
  0 siblings, 2 replies; 21+ messages in thread
From: Trond Myklebust @ 2019-05-21 19:58 UTC (permalink / raw)
  To: chucklever; +Cc: linux-nfs, SteveD

On Tue, 2019-05-21 at 15:06 -0400, Chuck Lever wrote:
> > On May 21, 2019, at 2:17 PM, Trond Myklebust <
> > trondmy@hammerspace.com> wrote:
> > 
> > On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
> > > Hi Trond -
> > > 
> > > > On May 21, 2019, at 8:46 AM, Trond Myklebust <trondmy@gmail.com
> > > > >
> > > > wrote:
> > > > 
> > > > The following patchset adds support for the 'root_dir'
> > > > configuration
> > > > option for nfsd in nfs.conf. If a user sets this option to a
> > > > valid
> > > > directory path, then nfsd will act as if it is confined to a
> > > > chroot
> > > > jail based on that directory. All paths in /etc/exporfs and
> > > > from
> > > > exportfs are then resolved relative to that directory.
> > > 
> > > What about files under /proc that mountd might access? I assume
> > > these
> > > pathnames are not affected.
> > > 
> > That's why we have 2 threads. One thread is root jailed using
> > chroot,
> > and is used to talk to knfsd. The other thread is not root jailed
> > (or
> > at least not by root_dir) and so has full access to /etc, /proc,
> > /var,
> > ...
> > 
> > > Aren't there also one or two other files that maintain export
> > > state
> > > like /var/lib/nfs/rmtab? Are those affected?
> > 
> > See above. They are not affected.
> > 
> > > IMHO it could be less confusing to administrators to make
> > > root_dir an
> > > [exportfs] option instead of a [mountd] option, if this is not a
> > > true
> > > chroot of mountd.
> > 
> > It is neither. I made in a [nfsd] option, since it governs the way
> > that
> > both exportfs and mountd talk to nfsd.
> 
> My point is not about implementation, it's about how this
> functionality
> is presented to administrators.
> 
> In nfs.conf, [nfsd] looks like it controls what options are passed
> via
> rpc.nfsd. That still seems like a confusing admin interface.
> 
> IMO admins won't care about who is talking to whom. They will care
> about
> how the export pathnames are interpreted. That seems like it belongs
> squarely with the exportfs interface.
> 

With the exportfs interface, yes. However it is not specific to the
exportfs utility, so to me [exportfs] is more confusing than what
exists now.

OK, so what if we put it in [general] instead, and perhaps rename it
"export_rootdir"?

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-21 19:58       ` Trond Myklebust
@ 2019-05-28 15:25         ` Steve Dickson
  2019-05-28 16:44           ` Trond Myklebust
  2019-05-28 15:30         ` Chuck Lever
  1 sibling, 1 reply; 21+ messages in thread
From: Steve Dickson @ 2019-05-28 15:25 UTC (permalink / raw)
  To: Trond Myklebust, chucklever; +Cc: linux-nfs



On 5/21/19 3:58 PM, Trond Myklebust wrote:
> On Tue, 2019-05-21 at 15:06 -0400, Chuck Lever wrote:
>>> On May 21, 2019, at 2:17 PM, Trond Myklebust <
>>> trondmy@hammerspace.com> wrote:
>>>
>>> On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
>>>> Hi Trond -
>>>>
>>>>> On May 21, 2019, at 8:46 AM, Trond Myklebust <trondmy@gmail.com
>>>>>>
>>>>> wrote:
>>>>>
>>>>> The following patchset adds support for the 'root_dir'
>>>>> configuration
>>>>> option for nfsd in nfs.conf. If a user sets this option to a
>>>>> valid
>>>>> directory path, then nfsd will act as if it is confined to a
>>>>> chroot
>>>>> jail based on that directory. All paths in /etc/exporfs and
>>>>> from
>>>>> exportfs are then resolved relative to that directory.
>>>>
>>>> What about files under /proc that mountd might access? I assume
>>>> these
>>>> pathnames are not affected.
>>>>
>>> That's why we have 2 threads. One thread is root jailed using
>>> chroot,
>>> and is used to talk to knfsd. The other thread is not root jailed
>>> (or
>>> at least not by root_dir) and so has full access to /etc, /proc,
>>> /var,
>>> ...
>>>
>>>> Aren't there also one or two other files that maintain export
>>>> state
>>>> like /var/lib/nfs/rmtab? Are those affected?
>>>
>>> See above. They are not affected.
>>>
>>>> IMHO it could be less confusing to administrators to make
>>>> root_dir an
>>>> [exportfs] option instead of a [mountd] option, if this is not a
>>>> true
>>>> chroot of mountd.
>>>
>>> It is neither. I made in a [nfsd] option, since it governs the way
>>> that
>>> both exportfs and mountd talk to nfsd.
>>
>> My point is not about implementation, it's about how this
>> functionality
>> is presented to administrators.
>>
>> In nfs.conf, [nfsd] looks like it controls what options are passed
>> via
>> rpc.nfsd. That still seems like a confusing admin interface.
>>
>> IMO admins won't care about who is talking to whom. They will care
>> about
>> how the export pathnames are interpreted. That seems like it belongs
>> squarely with the exportfs interface.
>>
> 
> With the exportfs interface, yes. However it is not specific to the
> exportfs utility, so to me [exportfs] is more confusing than what
> exists now.
> 
> OK, so what if we put it in [general] instead, and perhaps rename it
> "export_rootdir"?
> 
I'm just catching up... my apologies tartness...

So setting root_dir effects *all* exports in /etc/exports? 
If that is the case, that one variable can change hundreds
of export... is that what we really want?

Wouldn't be better to have a little more granularity? 

As for where root_dir should go, I think it makes senses
to create a new [exportfs] section and have mountd read it
from there. I think that would be more straightforward if
we continue with the big hammer approach where any and all
exports are effected. 

steved.  

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-21 19:58       ` Trond Myklebust
  2019-05-28 15:25         ` Steve Dickson
@ 2019-05-28 15:30         ` Chuck Lever
  1 sibling, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2019-05-28 15:30 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Linux NFS Mailing List, Steve Dickson



> On May 21, 2019, at 3:58 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Tue, 2019-05-21 at 15:06 -0400, Chuck Lever wrote:
>>> On May 21, 2019, at 2:17 PM, Trond Myklebust <
>>> trondmy@hammerspace.com> wrote:
>>> 
>>> On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
>>>> Hi Trond -
>>>> 
>>>>> On May 21, 2019, at 8:46 AM, Trond Myklebust <trondmy@gmail.com
>>>>>> 
>>>>> wrote:
>>>>> 
>>>>> The following patchset adds support for the 'root_dir'
>>>>> configuration
>>>>> option for nfsd in nfs.conf. If a user sets this option to a
>>>>> valid
>>>>> directory path, then nfsd will act as if it is confined to a
>>>>> chroot
>>>>> jail based on that directory. All paths in /etc/exporfs and
>>>>> from
>>>>> exportfs are then resolved relative to that directory.
>>>> 
>>>> What about files under /proc that mountd might access? I assume
>>>> these
>>>> pathnames are not affected.
>>>> 
>>> That's why we have 2 threads. One thread is root jailed using
>>> chroot,
>>> and is used to talk to knfsd. The other thread is not root jailed
>>> (or
>>> at least not by root_dir) and so has full access to /etc, /proc,
>>> /var,
>>> ...
>>> 
>>>> Aren't there also one or two other files that maintain export
>>>> state
>>>> like /var/lib/nfs/rmtab? Are those affected?
>>> 
>>> See above. They are not affected.
>>> 
>>>> IMHO it could be less confusing to administrators to make
>>>> root_dir an
>>>> [exportfs] option instead of a [mountd] option, if this is not a
>>>> true
>>>> chroot of mountd.
>>> 
>>> It is neither. I made in a [nfsd] option, since it governs the way
>>> that
>>> both exportfs and mountd talk to nfsd.
>> 
>> My point is not about implementation, it's about how this
>> functionality
>> is presented to administrators.
>> 
>> In nfs.conf, [nfsd] looks like it controls what options are passed
>> via
>> rpc.nfsd. That still seems like a confusing admin interface.
>> 
>> IMO admins won't care about who is talking to whom. They will care
>> about
>> how the export pathnames are interpreted. That seems like it belongs
>> squarely with the exportfs interface.
>> 
> 
> With the exportfs interface, yes. However it is not specific to the
> exportfs utility, so to me [exportfs] is more confusing than what
> exists now.
> 
> OK, so what if we put it in [general] instead, and perhaps rename it
> "export_rootdir"?

For the record, I prefer export_rootdir to root_dir. I haven't
thought of a better choice.

I also like not putting this option under [nfsd]. [general] is
a better choice IMO, though I'm still not crazy about it.

It might be nice to have a new section called "[exports]" (no "f").


--
Chuck Lever




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-28 15:25         ` Steve Dickson
@ 2019-05-28 16:44           ` Trond Myklebust
  2019-05-28 16:47             ` Chuck Lever
  2019-05-28 17:40             ` Steve Dickson
  0 siblings, 2 replies; 21+ messages in thread
From: Trond Myklebust @ 2019-05-28 16:44 UTC (permalink / raw)
  To: chucklever, SteveD; +Cc: linux-nfs

On Tue, 2019-05-28 at 11:25 -0400, Steve Dickson wrote:
> 
> On 5/21/19 3:58 PM, Trond Myklebust wrote:
> > On Tue, 2019-05-21 at 15:06 -0400, Chuck Lever wrote:
> > > > On May 21, 2019, at 2:17 PM, Trond Myklebust <
> > > > trondmy@hammerspace.com> wrote:
> > > > 
> > > > On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
> > > > > Hi Trond -
> > > > > 
> > > > > > On May 21, 2019, at 8:46 AM, Trond Myklebust <
> > > > > > trondmy@gmail.com
> > > > > > wrote:
> > > > > > 
> > > > > > The following patchset adds support for the 'root_dir'
> > > > > > configuration
> > > > > > option for nfsd in nfs.conf. If a user sets this option to
> > > > > > a
> > > > > > valid
> > > > > > directory path, then nfsd will act as if it is confined to
> > > > > > a
> > > > > > chroot
> > > > > > jail based on that directory. All paths in /etc/exporfs and
> > > > > > from
> > > > > > exportfs are then resolved relative to that directory.
> > > > > 
> > > > > What about files under /proc that mountd might access? I
> > > > > assume
> > > > > these
> > > > > pathnames are not affected.
> > > > > 
> > > > That's why we have 2 threads. One thread is root jailed using
> > > > chroot,
> > > > and is used to talk to knfsd. The other thread is not root
> > > > jailed
> > > > (or
> > > > at least not by root_dir) and so has full access to /etc,
> > > > /proc,
> > > > /var,
> > > > ...
> > > > 
> > > > > Aren't there also one or two other files that maintain export
> > > > > state
> > > > > like /var/lib/nfs/rmtab? Are those affected?
> > > > 
> > > > See above. They are not affected.
> > > > 
> > > > > IMHO it could be less confusing to administrators to make
> > > > > root_dir an
> > > > > [exportfs] option instead of a [mountd] option, if this is
> > > > > not a
> > > > > true
> > > > > chroot of mountd.
> > > > 
> > > > It is neither. I made in a [nfsd] option, since it governs the
> > > > way
> > > > that
> > > > both exportfs and mountd talk to nfsd.
> > > 
> > > My point is not about implementation, it's about how this
> > > functionality
> > > is presented to administrators.
> > > 
> > > In nfs.conf, [nfsd] looks like it controls what options are
> > > passed
> > > via
> > > rpc.nfsd. That still seems like a confusing admin interface.
> > > 
> > > IMO admins won't care about who is talking to whom. They will
> > > care
> > > about
> > > how the export pathnames are interpreted. That seems like it
> > > belongs
> > > squarely with the exportfs interface.
> > > 
> > 
> > With the exportfs interface, yes. However it is not specific to the
> > exportfs utility, so to me [exportfs] is more confusing than what
> > exists now.
> > 
> > OK, so what if we put it in [general] instead, and perhaps rename
> > it
> > "export_rootdir"?
> > 
> I'm just catching up... my apologies tartness...
> 
> So setting root_dir effects *all* exports in /etc/exports? 
> If that is the case, that one variable can change hundreds
> of export... is that what we really want?
> 
> Wouldn't be better to have a little more granularity? 

Can you explain what you mean? The intention here is that if you have
all your exported filesystems set up in a subtree under
/mnt/my/exports, then you can remove that unnecessary prefix.

So, for instance, if I'm trying to export /mnt/my/exports/foo and
/mnt/my/exports/bar, then I can make those two filesystems appear as
/foo, and /bar to the remote clients.

If an admin wants to rearrange all the paths in /etc/exports, and make
a custom namespace, then that is possible using bind mounts: just
create a directory /my_exports, and use mount --bind to attach the
necessary mountpoints into the right spots in /my_exports, then use
export_rootdir to remove the /my_exports prefix.

> As for where root_dir should go, I think it makes senses
> to create a new [exportfs] section and have mountd read it
> from there. I think that would be more straightforward if
> we continue with the big hammer approach where any and all
> exports are effected. 
> 

Fair enough, I can add the [exports] section if you all agree that is
an appropriate place.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-28 16:44           ` Trond Myklebust
@ 2019-05-28 16:47             ` Chuck Lever
  2019-05-28 16:50               ` Trond Myklebust
  2019-05-28 17:40             ` Steve Dickson
  1 sibling, 1 reply; 21+ messages in thread
From: Chuck Lever @ 2019-05-28 16:47 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Steve Dickson, Linux NFS Mailing List



> On May 28, 2019, at 12:44 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Tue, 2019-05-28 at 11:25 -0400, Steve Dickson wrote:
>> 
>> On 5/21/19 3:58 PM, Trond Myklebust wrote:
>>> On Tue, 2019-05-21 at 15:06 -0400, Chuck Lever wrote:
>>>>> On May 21, 2019, at 2:17 PM, Trond Myklebust <
>>>>> trondmy@hammerspace.com> wrote:
>>>>> 
>>>>> On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
>>>>>> Hi Trond -
>>>>>> 
>>>>>>> On May 21, 2019, at 8:46 AM, Trond Myklebust <
>>>>>>> trondmy@gmail.com
>>>>>>> wrote:
>>>>>>> 
>>>>>>> The following patchset adds support for the 'root_dir'
>>>>>>> configuration
>>>>>>> option for nfsd in nfs.conf. If a user sets this option to
>>>>>>> a
>>>>>>> valid
>>>>>>> directory path, then nfsd will act as if it is confined to
>>>>>>> a
>>>>>>> chroot
>>>>>>> jail based on that directory. All paths in /etc/exporfs and
>>>>>>> from
>>>>>>> exportfs are then resolved relative to that directory.
>>>>>> 
>>>>>> What about files under /proc that mountd might access? I
>>>>>> assume
>>>>>> these
>>>>>> pathnames are not affected.
>>>>>> 
>>>>> That's why we have 2 threads. One thread is root jailed using
>>>>> chroot,
>>>>> and is used to talk to knfsd. The other thread is not root
>>>>> jailed
>>>>> (or
>>>>> at least not by root_dir) and so has full access to /etc,
>>>>> /proc,
>>>>> /var,
>>>>> ...
>>>>> 
>>>>>> Aren't there also one or two other files that maintain export
>>>>>> state
>>>>>> like /var/lib/nfs/rmtab? Are those affected?
>>>>> 
>>>>> See above. They are not affected.
>>>>> 
>>>>>> IMHO it could be less confusing to administrators to make
>>>>>> root_dir an
>>>>>> [exportfs] option instead of a [mountd] option, if this is
>>>>>> not a
>>>>>> true
>>>>>> chroot of mountd.
>>>>> 
>>>>> It is neither. I made in a [nfsd] option, since it governs the
>>>>> way
>>>>> that
>>>>> both exportfs and mountd talk to nfsd.
>>>> 
>>>> My point is not about implementation, it's about how this
>>>> functionality
>>>> is presented to administrators.
>>>> 
>>>> In nfs.conf, [nfsd] looks like it controls what options are
>>>> passed
>>>> via
>>>> rpc.nfsd. That still seems like a confusing admin interface.
>>>> 
>>>> IMO admins won't care about who is talking to whom. They will
>>>> care
>>>> about
>>>> how the export pathnames are interpreted. That seems like it
>>>> belongs
>>>> squarely with the exportfs interface.
>>>> 
>>> 
>>> With the exportfs interface, yes. However it is not specific to the
>>> exportfs utility, so to me [exportfs] is more confusing than what
>>> exists now.
>>> 
>>> OK, so what if we put it in [general] instead, and perhaps rename
>>> it
>>> "export_rootdir"?
>>> 
>> I'm just catching up... my apologies tartness...
>> 
>> So setting root_dir effects *all* exports in /etc/exports? 
>> If that is the case, that one variable can change hundreds
>> of export... is that what we really want?
>> 
>> Wouldn't be better to have a little more granularity? 
> 
> Can you explain what you mean? The intention here is that if you have
> all your exported filesystems set up in a subtree under
> /mnt/my/exports, then you can remove that unnecessary prefix.
> 
> So, for instance, if I'm trying to export /mnt/my/exports/foo and
> /mnt/my/exports/bar, then I can make those two filesystems appear as
> /foo, and /bar to the remote clients.
> 
> If an admin wants to rearrange all the paths in /etc/exports, and make
> a custom namespace, then that is possible using bind mounts: just
> create a directory /my_exports, and use mount --bind to attach the
> necessary mountpoints into the right spots in /my_exports, then use
> export_rootdir to remove the /my_exports prefix.

Just to be clear, do you expect that each mount namespace on a
Linux NFS server would have its own /etc/exports and /etc/nfs.conf ?

Maybe you stated that before, and I missed it.


>> As for where root_dir should go, I think it makes senses
>> to create a new [exportfs] section and have mountd read it
>> from there. I think that would be more straightforward if
>> we continue with the big hammer approach where any and all
>> exports are effected. 
>> 
> 
> Fair enough, I can add the [exports] section if you all agree that is
> an appropriate place.
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com

--
Chuck Lever
chucklever@gmail.com




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-28 16:47             ` Chuck Lever
@ 2019-05-28 16:50               ` Trond Myklebust
  0 siblings, 0 replies; 21+ messages in thread
From: Trond Myklebust @ 2019-05-28 16:50 UTC (permalink / raw)
  To: chucklever; +Cc: linux-nfs, SteveD

On Tue, 2019-05-28 at 12:47 -0400, Chuck Lever wrote:
> > On May 28, 2019, at 12:44 PM, Trond Myklebust <
> > trondmy@hammerspace.com> wrote:
> > 
> > On Tue, 2019-05-28 at 11:25 -0400, Steve Dickson wrote:
> > > On 5/21/19 3:58 PM, Trond Myklebust wrote:
> > > > On Tue, 2019-05-21 at 15:06 -0400, Chuck Lever wrote:
> > > > > > On May 21, 2019, at 2:17 PM, Trond Myklebust <
> > > > > > trondmy@hammerspace.com> wrote:
> > > > > > 
> > > > > > On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
> > > > > > > Hi Trond -
> > > > > > > 
> > > > > > > > On May 21, 2019, at 8:46 AM, Trond Myklebust <
> > > > > > > > trondmy@gmail.com
> > > > > > > > wrote:
> > > > > > > > 
> > > > > > > > The following patchset adds support for the 'root_dir'
> > > > > > > > configuration
> > > > > > > > option for nfsd in nfs.conf. If a user sets this option
> > > > > > > > to
> > > > > > > > a
> > > > > > > > valid
> > > > > > > > directory path, then nfsd will act as if it is confined
> > > > > > > > to
> > > > > > > > a
> > > > > > > > chroot
> > > > > > > > jail based on that directory. All paths in /etc/exporfs
> > > > > > > > and
> > > > > > > > from
> > > > > > > > exportfs are then resolved relative to that directory.
> > > > > > > 
> > > > > > > What about files under /proc that mountd might access? I
> > > > > > > assume
> > > > > > > these
> > > > > > > pathnames are not affected.
> > > > > > > 
> > > > > > That's why we have 2 threads. One thread is root jailed
> > > > > > using
> > > > > > chroot,
> > > > > > and is used to talk to knfsd. The other thread is not root
> > > > > > jailed
> > > > > > (or
> > > > > > at least not by root_dir) and so has full access to /etc,
> > > > > > /proc,
> > > > > > /var,
> > > > > > ...
> > > > > > 
> > > > > > > Aren't there also one or two other files that maintain
> > > > > > > export
> > > > > > > state
> > > > > > > like /var/lib/nfs/rmtab? Are those affected?
> > > > > > 
> > > > > > See above. They are not affected.
> > > > > > 
> > > > > > > IMHO it could be less confusing to administrators to make
> > > > > > > root_dir an
> > > > > > > [exportfs] option instead of a [mountd] option, if this
> > > > > > > is
> > > > > > > not a
> > > > > > > true
> > > > > > > chroot of mountd.
> > > > > > 
> > > > > > It is neither. I made in a [nfsd] option, since it governs
> > > > > > the
> > > > > > way
> > > > > > that
> > > > > > both exportfs and mountd talk to nfsd.
> > > > > 
> > > > > My point is not about implementation, it's about how this
> > > > > functionality
> > > > > is presented to administrators.
> > > > > 
> > > > > In nfs.conf, [nfsd] looks like it controls what options are
> > > > > passed
> > > > > via
> > > > > rpc.nfsd. That still seems like a confusing admin interface.
> > > > > 
> > > > > IMO admins won't care about who is talking to whom. They will
> > > > > care
> > > > > about
> > > > > how the export pathnames are interpreted. That seems like it
> > > > > belongs
> > > > > squarely with the exportfs interface.
> > > > > 
> > > > 
> > > > With the exportfs interface, yes. However it is not specific to
> > > > the
> > > > exportfs utility, so to me [exportfs] is more confusing than
> > > > what
> > > > exists now.
> > > > 
> > > > OK, so what if we put it in [general] instead, and perhaps
> > > > rename
> > > > it
> > > > "export_rootdir"?
> > > > 
> > > I'm just catching up... my apologies tartness...
> > > 
> > > So setting root_dir effects *all* exports in /etc/exports? 
> > > If that is the case, that one variable can change hundreds
> > > of export... is that what we really want?
> > > 
> > > Wouldn't be better to have a little more granularity? 
> > 
> > Can you explain what you mean? The intention here is that if you
> > have
> > all your exported filesystems set up in a subtree under
> > /mnt/my/exports, then you can remove that unnecessary prefix.
> > 
> > So, for instance, if I'm trying to export /mnt/my/exports/foo and
> > /mnt/my/exports/bar, then I can make those two filesystems appear
> > as
> > /foo, and /bar to the remote clients.
> > 
> > If an admin wants to rearrange all the paths in /etc/exports, and
> > make
> > a custom namespace, then that is possible using bind mounts: just
> > create a directory /my_exports, and use mount --bind to attach the
> > necessary mountpoints into the right spots in /my_exports, then use
> > export_rootdir to remove the /my_exports prefix.
> 
> Just to be clear, do you expect that each mount namespace on a
> Linux NFS server would have its own /etc/exports and /etc/nfs.conf ?
> 
> Maybe you stated that before, and I missed it.

Yes, if you are running in a containerised environment, then you will
have your own /etc/exports, /etc/nfs.conf, etc.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-28 16:44           ` Trond Myklebust
  2019-05-28 16:47             ` Chuck Lever
@ 2019-05-28 17:40             ` Steve Dickson
  2019-05-28 18:19               ` Trond Myklebust
  1 sibling, 1 reply; 21+ messages in thread
From: Steve Dickson @ 2019-05-28 17:40 UTC (permalink / raw)
  To: Trond Myklebust, chucklever; +Cc: linux-nfs



On 5/28/19 12:44 PM, Trond Myklebust wrote:
> On Tue, 2019-05-28 at 11:25 -0400, Steve Dickson wrote:
>>
>> On 5/21/19 3:58 PM, Trond Myklebust wrote:
>>> On Tue, 2019-05-21 at 15:06 -0400, Chuck Lever wrote:
>>>>> On May 21, 2019, at 2:17 PM, Trond Myklebust <
>>>>> trondmy@hammerspace.com> wrote:
>>>>>
>>>>> On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
>>>>>> Hi Trond -
>>>>>>
>>>>>>> On May 21, 2019, at 8:46 AM, Trond Myklebust <
>>>>>>> trondmy@gmail.com
>>>>>>> wrote:
>>>>>>>
>>>>>>> The following patchset adds support for the 'root_dir'
>>>>>>> configuration
>>>>>>> option for nfsd in nfs.conf. If a user sets this option to
>>>>>>> a
>>>>>>> valid
>>>>>>> directory path, then nfsd will act as if it is confined to
>>>>>>> a
>>>>>>> chroot
>>>>>>> jail based on that directory. All paths in /etc/exporfs and
>>>>>>> from
>>>>>>> exportfs are then resolved relative to that directory.
>>>>>>
>>>>>> What about files under /proc that mountd might access? I
>>>>>> assume
>>>>>> these
>>>>>> pathnames are not affected.
>>>>>>
>>>>> That's why we have 2 threads. One thread is root jailed using
>>>>> chroot,
>>>>> and is used to talk to knfsd. The other thread is not root
>>>>> jailed
>>>>> (or
>>>>> at least not by root_dir) and so has full access to /etc,
>>>>> /proc,
>>>>> /var,
>>>>> ...
>>>>>
>>>>>> Aren't there also one or two other files that maintain export
>>>>>> state
>>>>>> like /var/lib/nfs/rmtab? Are those affected?
>>>>>
>>>>> See above. They are not affected.
>>>>>
>>>>>> IMHO it could be less confusing to administrators to make
>>>>>> root_dir an
>>>>>> [exportfs] option instead of a [mountd] option, if this is
>>>>>> not a
>>>>>> true
>>>>>> chroot of mountd.
>>>>>
>>>>> It is neither. I made in a [nfsd] option, since it governs the
>>>>> way
>>>>> that
>>>>> both exportfs and mountd talk to nfsd.
>>>>
>>>> My point is not about implementation, it's about how this
>>>> functionality
>>>> is presented to administrators.
>>>>
>>>> In nfs.conf, [nfsd] looks like it controls what options are
>>>> passed
>>>> via
>>>> rpc.nfsd. That still seems like a confusing admin interface.
>>>>
>>>> IMO admins won't care about who is talking to whom. They will
>>>> care
>>>> about
>>>> how the export pathnames are interpreted. That seems like it
>>>> belongs
>>>> squarely with the exportfs interface.
>>>>
>>>
>>> With the exportfs interface, yes. However it is not specific to the
>>> exportfs utility, so to me [exportfs] is more confusing than what
>>> exists now.
>>>
>>> OK, so what if we put it in [general] instead, and perhaps rename
>>> it
>>> "export_rootdir"?
>>>
>> I'm just catching up... my apologies tartness...
>>
>> So setting root_dir effects *all* exports in /etc/exports? 
>> If that is the case, that one variable can change hundreds
>> of export... is that what we really want?
>>
>> Wouldn't be better to have a little more granularity? 
> 
> Can you explain what you mean? The intention here is that if you have
> all your exported filesystems set up in a subtree under
> /mnt/my/exports, then you can remove that unnecessary prefix.
> 
> So, for instance, if I'm trying to export /mnt/my/exports/foo and
> /mnt/my/exports/bar, then I can make those two filesystems appear as
> /foo, and /bar to the remote clients.
By granularity I meant have different roots for different exports.
Meaning /mnt/foo/exports/foo and /mnt/bar/exports/bar
would still appear as /foo and /bar

As you explain later in this thread, there is going to be a nfs.conf
and exports for each container so maybe this is not necessary?? 

Maybe I'm misunderstanding how this feature should/will be used.

> 
> If an admin wants to rearrange all the paths in /etc/exports, and make
> a custom namespace, then that is possible using bind mounts: just
> create a directory /my_exports, and use mount --bind to attach the
> necessary mountpoints into the right spots in /my_exports, then use
> export_rootdir to remove the /my_exports prefix.
> 
>> As for where root_dir should go, I think it makes senses
>> to create a new [exportfs] section and have mountd read it
>> from there. I think that would be more straightforward if
>> we continue with the big hammer approach where any and all
>> exports are effected. 
>>
> 
> Fair enough, I can add the [exports] section if you all agree that is
> an appropriate place.
> 
I think a new exports sections with a rootdir variable makes sense.
It is changing the root of the exports... 

But I could also live with a export_rootdir in the general section.

Question:
How is this different than pseudo root? 

Isn't this basically a way to set the pseudo for v3? 

What is going to override whom? Meaning if both 
fsid=/mnt/foo and rootdir=/mnt/bar which one will be used?

steved.

steved.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-28 17:40             ` Steve Dickson
@ 2019-05-28 18:19               ` Trond Myklebust
  2019-05-28 19:33                 ` Steve Dickson
  0 siblings, 1 reply; 21+ messages in thread
From: Trond Myklebust @ 2019-05-28 18:19 UTC (permalink / raw)
  To: chucklever, SteveD; +Cc: linux-nfs

On Tue, 2019-05-28 at 13:40 -0400, Steve Dickson wrote:
> 
> On 5/28/19 12:44 PM, Trond Myklebust wrote:
> > On Tue, 2019-05-28 at 11:25 -0400, Steve Dickson wrote:
> > > On 5/21/19 3:58 PM, Trond Myklebust wrote:
> > > > On Tue, 2019-05-21 at 15:06 -0400, Chuck Lever wrote:
> > > > > > On May 21, 2019, at 2:17 PM, Trond Myklebust <
> > > > > > trondmy@hammerspace.com> wrote:
> > > > > > 
> > > > > > On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
> > > > > > > Hi Trond -
> > > > > > > 
> > > > > > > > On May 21, 2019, at 8:46 AM, Trond Myklebust <
> > > > > > > > trondmy@gmail.com
> > > > > > > > wrote:
> > > > > > > > 
> > > > > > > > The following patchset adds support for the 'root_dir'
> > > > > > > > configuration
> > > > > > > > option for nfsd in nfs.conf. If a user sets this option
> > > > > > > > to
> > > > > > > > a
> > > > > > > > valid
> > > > > > > > directory path, then nfsd will act as if it is confined
> > > > > > > > to
> > > > > > > > a
> > > > > > > > chroot
> > > > > > > > jail based on that directory. All paths in /etc/exporfs
> > > > > > > > and
> > > > > > > > from
> > > > > > > > exportfs are then resolved relative to that directory.
> > > > > > > 
> > > > > > > What about files under /proc that mountd might access? I
> > > > > > > assume
> > > > > > > these
> > > > > > > pathnames are not affected.
> > > > > > > 
> > > > > > That's why we have 2 threads. One thread is root jailed
> > > > > > using
> > > > > > chroot,
> > > > > > and is used to talk to knfsd. The other thread is not root
> > > > > > jailed
> > > > > > (or
> > > > > > at least not by root_dir) and so has full access to /etc,
> > > > > > /proc,
> > > > > > /var,
> > > > > > ...
> > > > > > 
> > > > > > > Aren't there also one or two other files that maintain
> > > > > > > export
> > > > > > > state
> > > > > > > like /var/lib/nfs/rmtab? Are those affected?
> > > > > > 
> > > > > > See above. They are not affected.
> > > > > > 
> > > > > > > IMHO it could be less confusing to administrators to make
> > > > > > > root_dir an
> > > > > > > [exportfs] option instead of a [mountd] option, if this
> > > > > > > is
> > > > > > > not a
> > > > > > > true
> > > > > > > chroot of mountd.
> > > > > > 
> > > > > > It is neither. I made in a [nfsd] option, since it governs
> > > > > > the
> > > > > > way
> > > > > > that
> > > > > > both exportfs and mountd talk to nfsd.
> > > > > 
> > > > > My point is not about implementation, it's about how this
> > > > > functionality
> > > > > is presented to administrators.
> > > > > 
> > > > > In nfs.conf, [nfsd] looks like it controls what options are
> > > > > passed
> > > > > via
> > > > > rpc.nfsd. That still seems like a confusing admin interface.
> > > > > 
> > > > > IMO admins won't care about who is talking to whom. They will
> > > > > care
> > > > > about
> > > > > how the export pathnames are interpreted. That seems like it
> > > > > belongs
> > > > > squarely with the exportfs interface.
> > > > > 
> > > > 
> > > > With the exportfs interface, yes. However it is not specific to
> > > > the
> > > > exportfs utility, so to me [exportfs] is more confusing than
> > > > what
> > > > exists now.
> > > > 
> > > > OK, so what if we put it in [general] instead, and perhaps
> > > > rename
> > > > it
> > > > "export_rootdir"?
> > > > 
> > > I'm just catching up... my apologies tartness...
> > > 
> > > So setting root_dir effects *all* exports in /etc/exports? 
> > > If that is the case, that one variable can change hundreds
> > > of export... is that what we really want?
> > > 
> > > Wouldn't be better to have a little more granularity? 
> > 
> > Can you explain what you mean? The intention here is that if you
> > have
> > all your exported filesystems set up in a subtree under
> > /mnt/my/exports, then you can remove that unnecessary prefix.
> > 
> > So, for instance, if I'm trying to export /mnt/my/exports/foo and
> > /mnt/my/exports/bar, then I can make those two filesystems appear
> > as
> > /foo, and /bar to the remote clients.
> By granularity I meant have different roots for different exports.
> Meaning /mnt/foo/exports/foo and /mnt/bar/exports/bar
> would still appear as /foo and /bar

No. That should be done using bind mounts. Otherwise we end up with
/etc/nfs.conf and /etc/exports depending on being mutually consistent.
That would be awkward.

> As you explain later in this thread, there is going to be a nfs.conf
> and exports for each container so maybe this is not necessary?? 
> 
> Maybe I'm misunderstanding how this feature should/will be used.

As I've already said, it can be used to do what you are proposing, but
only in conjunction with bind mounts.

> 
> > If an admin wants to rearrange all the paths in /etc/exports, and
> > make
> > a custom namespace, then that is possible using bind mounts: just
> > create a directory /my_exports, and use mount --bind to attach the
> > necessary mountpoints into the right spots in /my_exports, then use
> > export_rootdir to remove the /my_exports prefix.
> > 
> > > As for where root_dir should go, I think it makes senses
> > > to create a new [exportfs] section and have mountd read it
> > > from there. I think that would be more straightforward if
> > > we continue with the big hammer approach where any and all
> > > exports are effected. 
> > > 
> > 
> > Fair enough, I can add the [exports] section if you all agree that
> > is
> > an appropriate place.
> > 
> I think a new exports sections with a rootdir variable makes sense.
> It is changing the root of the exports... 
> 
> But I could also live with a export_rootdir in the general section.
> 
> Question:
> How is this different than pseudo root? 
> 
> Isn't this basically a way to set the pseudo for v3? 

Sort of, yes.

> What is going to override whom? Meaning if both 
> fsid=/mnt/foo and rootdir=/mnt/bar which one will be used?
> 
> 
Both. However the entry in /etc/exports will be relative to /mnt/bar.
In other words, the NFSv4 root would be fsid=/mnt/foo, which translates
as /mnt/bar/mnt/foo in the 'init' namespace.

-- 
Trond Myklebust
CTO, Hammerspace Inc
4300 El Camino Real, Suite 105
Los Altos, CA 94022
www.hammer.space



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf
  2019-05-28 18:19               ` Trond Myklebust
@ 2019-05-28 19:33                 ` Steve Dickson
  0 siblings, 0 replies; 21+ messages in thread
From: Steve Dickson @ 2019-05-28 19:33 UTC (permalink / raw)
  To: Trond Myklebust, chucklever; +Cc: linux-nfs



On 5/28/19 2:19 PM, Trond Myklebust wrote:
> On Tue, 2019-05-28 at 13:40 -0400, Steve Dickson wrote:
>>
>> On 5/28/19 12:44 PM, Trond Myklebust wrote:
>>> On Tue, 2019-05-28 at 11:25 -0400, Steve Dickson wrote:
>>>> On 5/21/19 3:58 PM, Trond Myklebust wrote:
>>>>> On Tue, 2019-05-21 at 15:06 -0400, Chuck Lever wrote:
>>>>>>> On May 21, 2019, at 2:17 PM, Trond Myklebust <
>>>>>>> trondmy@hammerspace.com> wrote:
>>>>>>>
>>>>>>> On Tue, 2019-05-21 at 13:40 -0400, Chuck Lever wrote:
>>>>>>>> Hi Trond -
>>>>>>>>
>>>>>>>>> On May 21, 2019, at 8:46 AM, Trond Myklebust <
>>>>>>>>> trondmy@gmail.com
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> The following patchset adds support for the 'root_dir'
>>>>>>>>> configuration
>>>>>>>>> option for nfsd in nfs.conf. If a user sets this option
>>>>>>>>> to
>>>>>>>>> a
>>>>>>>>> valid
>>>>>>>>> directory path, then nfsd will act as if it is confined
>>>>>>>>> to
>>>>>>>>> a
>>>>>>>>> chroot
>>>>>>>>> jail based on that directory. All paths in /etc/exporfs
>>>>>>>>> and
>>>>>>>>> from
>>>>>>>>> exportfs are then resolved relative to that directory.
>>>>>>>>
>>>>>>>> What about files under /proc that mountd might access? I
>>>>>>>> assume
>>>>>>>> these
>>>>>>>> pathnames are not affected.
>>>>>>>>
>>>>>>> That's why we have 2 threads. One thread is root jailed
>>>>>>> using
>>>>>>> chroot,
>>>>>>> and is used to talk to knfsd. The other thread is not root
>>>>>>> jailed
>>>>>>> (or
>>>>>>> at least not by root_dir) and so has full access to /etc,
>>>>>>> /proc,
>>>>>>> /var,
>>>>>>> ...
>>>>>>>
>>>>>>>> Aren't there also one or two other files that maintain
>>>>>>>> export
>>>>>>>> state
>>>>>>>> like /var/lib/nfs/rmtab? Are those affected?
>>>>>>>
>>>>>>> See above. They are not affected.
>>>>>>>
>>>>>>>> IMHO it could be less confusing to administrators to make
>>>>>>>> root_dir an
>>>>>>>> [exportfs] option instead of a [mountd] option, if this
>>>>>>>> is
>>>>>>>> not a
>>>>>>>> true
>>>>>>>> chroot of mountd.
>>>>>>>
>>>>>>> It is neither. I made in a [nfsd] option, since it governs
>>>>>>> the
>>>>>>> way
>>>>>>> that
>>>>>>> both exportfs and mountd talk to nfsd.
>>>>>>
>>>>>> My point is not about implementation, it's about how this
>>>>>> functionality
>>>>>> is presented to administrators.
>>>>>>
>>>>>> In nfs.conf, [nfsd] looks like it controls what options are
>>>>>> passed
>>>>>> via
>>>>>> rpc.nfsd. That still seems like a confusing admin interface.
>>>>>>
>>>>>> IMO admins won't care about who is talking to whom. They will
>>>>>> care
>>>>>> about
>>>>>> how the export pathnames are interpreted. That seems like it
>>>>>> belongs
>>>>>> squarely with the exportfs interface.
>>>>>>
>>>>>
>>>>> With the exportfs interface, yes. However it is not specific to
>>>>> the
>>>>> exportfs utility, so to me [exportfs] is more confusing than
>>>>> what
>>>>> exists now.
>>>>>
>>>>> OK, so what if we put it in [general] instead, and perhaps
>>>>> rename
>>>>> it
>>>>> "export_rootdir"?
>>>>>
>>>> I'm just catching up... my apologies tartness...
>>>>
>>>> So setting root_dir effects *all* exports in /etc/exports? 
>>>> If that is the case, that one variable can change hundreds
>>>> of export... is that what we really want?
>>>>
>>>> Wouldn't be better to have a little more granularity? 
>>>
>>> Can you explain what you mean? The intention here is that if you
>>> have
>>> all your exported filesystems set up in a subtree under
>>> /mnt/my/exports, then you can remove that unnecessary prefix.
>>>
>>> So, for instance, if I'm trying to export /mnt/my/exports/foo and
>>> /mnt/my/exports/bar, then I can make those two filesystems appear
>>> as
>>> /foo, and /bar to the remote clients.
>> By granularity I meant have different roots for different exports.
>> Meaning /mnt/foo/exports/foo and /mnt/bar/exports/bar
>> would still appear as /foo and /bar
> 
> No. That should be done using bind mounts. Otherwise we end up with
> /etc/nfs.conf and /etc/exports depending on being mutually consistent.
> That would be awkward.
Fine... 

> 
>> As you explain later in this thread, there is going to be a nfs.conf
>> and exports for each container so maybe this is not necessary?? 
>>
>> Maybe I'm misunderstanding how this feature should/will be used.
> 
> As I've already said, it can be used to do what you are proposing, but
> only in conjunction with bind mounts.
> 
>>
>>> If an admin wants to rearrange all the paths in /etc/exports, and
>>> make
>>> a custom namespace, then that is possible using bind mounts: just
>>> create a directory /my_exports, and use mount --bind to attach the
>>> necessary mountpoints into the right spots in /my_exports, then use
>>> export_rootdir to remove the /my_exports prefix.
>>>
>>>> As for where root_dir should go, I think it makes senses
>>>> to create a new [exportfs] section and have mountd read it
>>>> from there. I think that would be more straightforward if
>>>> we continue with the big hammer approach where any and all
>>>> exports are effected. 
>>>>
>>>
>>> Fair enough, I can add the [exports] section if you all agree that
>>> is
>>> an appropriate place.
>>>
>> I think a new exports sections with a rootdir variable makes sense.
>> It is changing the root of the exports... 
>>
>> But I could also live with a export_rootdir in the general section.
>>
>> Question:
>> How is this different than pseudo root? 
>>
>> Isn't this basically a way to set the pseudo for v3? 
> 
> Sort of, yes.
> 
>> What is going to override whom? Meaning if both 
>> fsid=/mnt/foo and rootdir=/mnt/bar which one will be used?
>>
>>
> Both. However the entry in /etc/exports will be relative to /mnt/bar.
> In other words, the NFSv4 root would be fsid=/mnt/foo, which translates
> as /mnt/bar/mnt/foo in the 'init' namespace.
> 
Ok... 

So what do you want to do... 

[exports]
 rootdir=/mnt/foo

or 

[general]
   export_rootdir=/mnt/bar

steved.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2019-05-28 19:33 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-21 12:46 [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf Trond Myklebust
2019-05-21 12:46 ` [RFC PATCH v2 1/7] mountd: Ensure we don't share cache file descriptors among processes Trond Myklebust
2019-05-21 12:46   ` [RFC PATCH v2 2/7] Add a simple workqueue mechanism Trond Myklebust
2019-05-21 12:46     ` [RFC PATCH v2 3/7] Add utilities for resolving nfsd paths and stat()ing them Trond Myklebust
2019-05-21 12:46       ` [RFC PATCH v2 4/7] Add a helper to return the real path given an export entry Trond Myklebust
2019-05-21 12:46         ` [RFC PATCH v2 5/7] Add helpers to read/write to a file through the chrooted thread Trond Myklebust
2019-05-21 12:47           ` [RFC PATCH v2 6/7] Add support for the nfsd rootdir configuration option to rpc.mountd Trond Myklebust
2019-05-21 12:47             ` [RFC PATCH v2 7/7] Add support for the nfsd root directory to exportfs Trond Myklebust
2019-05-21 17:40 ` [RFC PATCH v2 0/7] Add a root_dir option to nfs.conf Chuck Lever
2019-05-21 18:17   ` Trond Myklebust
2019-05-21 18:59     ` Trond Myklebust
2019-05-21 19:06     ` Chuck Lever
2019-05-21 19:58       ` Trond Myklebust
2019-05-28 15:25         ` Steve Dickson
2019-05-28 16:44           ` Trond Myklebust
2019-05-28 16:47             ` Chuck Lever
2019-05-28 16:50               ` Trond Myklebust
2019-05-28 17:40             ` Steve Dickson
2019-05-28 18:19               ` Trond Myklebust
2019-05-28 19:33                 ` Steve Dickson
2019-05-28 15:30         ` Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).