2019-05-14 20:45:58

by Trond Myklebust

[permalink] [raw]
Subject: [RFC PATCH 0/5] Add a chroot option to nfs.conf

The following patchset aims to allow the configuration of a 'chroot
jail' to rpc.nfsd, and allowing us to export a filesystem /foo (and
possibly subtrees) as '/'.

Trond Myklebust (5):
mountd: Ensure we don't share cache file descriptors among processes.
Add a simple workqueue mechanism
Add a helper to write to a file through the chrooted thread
Add support for chrooted exports
Add support for chroot in exportfs

aclocal/libpthread.m4 | 13 +-
configure.ac | 6 +-
nfs.conf | 1 +
support/include/misc.h | 11 ++
support/misc/Makefile.am | 2 +-
support/misc/workqueue.c | 267 +++++++++++++++++++++++++++++++++++++
systemd/nfs.conf.man | 3 +-
utils/exportfs/Makefile.am | 2 +-
utils/exportfs/exportfs.c | 31 ++++-
utils/mountd/Makefile.am | 3 +-
utils/mountd/cache.c | 39 +++++-
utils/mountd/mountd.c | 5 +-
utils/nfsd/nfsd.man | 4 +
13 files changed, 369 insertions(+), 18 deletions(-)
create mode 100644 support/misc/workqueue.c

--
2.21.0


2019-05-14 20:46:24

by Trond Myklebust

[permalink] [raw]
Subject: [RFC PATCH 1/5] mountd: Ensure we don't share cache file descriptors among processes.

Sharing cache descriptors without using locking can be very bad.

Signed-off-by: Trond Myklebust <[email protected]>
---
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

2019-05-14 20:46:25

by Trond Myklebust

[permalink] [raw]
Subject: [RFC PATCH 2/5] Add a simple workqueue mechanism

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 <[email protected]>
---
aclocal/libpthread.m4 | 13 +--
configure.ac | 6 +-
support/include/misc.h | 9 ++
support/misc/Makefile.am | 2 +-
support/misc/workqueue.c | 228 +++++++++++++++++++++++++++++++++++++++
utils/mountd/Makefile.am | 3 +-
6 files changed, 252 insertions(+), 9 deletions(-)
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/misc.h b/support/include/misc.h
index 06e2a0c7b061..40fb9a37621a 100644
--- a/support/include/misc.h
+++ b/support/include/misc.h
@@ -20,6 +20,15 @@ _Bool generic_setup_basedir(const char *, const char *, char *, const size_t);

extern int is_mountpoint(char *path);

+/* Naive stack implementation */
+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);
+
/* size of the file pointer buffers for rpc procfs files */
#define RPC_CHAN_BUF_SIZE 32768

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..16e95e1f6c86
--- /dev/null
+++ b/support/misc/workqueue.c
@@ -0,0 +1,228 @@
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "config.h"
+#include "misc.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(L_FATAL, "unshare() failed: %m");
+ return;
+ }
+ if (chroot(path) != 0)
+ xlog(L_FATAL, "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(L_FATAL, "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

2019-05-14 20:46:26

by Trond Myklebust

[permalink] [raw]
Subject: [RFC PATCH 3/5] Add a helper to write to a file through the chrooted thread

Signed-off-by: Trond Myklebust <[email protected]>
---
support/include/misc.h | 2 ++
support/misc/workqueue.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+)

diff --git a/support/include/misc.h b/support/include/misc.h
index 40fb9a37621a..0632df101bbb 100644
--- a/support/include/misc.h
+++ b/support/include/misc.h
@@ -28,6 +28,8 @@ 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);
+ssize_t xthread_write(struct xthread_workqueue *wq,
+ int fd, const char *buf, size_t len);

/* size of the file pointer buffers for rpc procfs files */
#define RPC_CHAN_BUF_SIZE 32768
diff --git a/support/misc/workqueue.c b/support/misc/workqueue.c
index 16e95e1f6c86..7af76a84e8dd 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,39 @@ void xthread_workqueue_chroot(struct xthread_workqueue *wq,
xthread_work_run_sync(wq, xthread_workqueue_do_chroot, (void *)path);
}

+struct xthread_io_data {
+ int fd;
+ const char *buf;
+ size_t len;
+ ssize_t ret;
+ int err;
+};
+
+static void xthread_writefunc(void *data)
+{
+ struct xthread_io_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_io_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 +259,9 @@ void xthread_workqueue_chroot(struct xthread_workqueue *wq,
xlog(L_FATAL, "Unable to run as chroot");
}

+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

2019-05-14 20:46:27

by Trond Myklebust

[permalink] [raw]
Subject: [RFC PATCH 4/5] Add support for chrooted exports

Signed-off-by: Trond Myklebust <[email protected]>
---
nfs.conf | 1 +
systemd/nfs.conf.man | 3 ++-
utils/mountd/cache.c | 39 +++++++++++++++++++++++++++++++++++----
utils/nfsd/nfsd.man | 4 ++++
4 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/nfs.conf b/nfs.conf
index 27e962c8a2a9..aad73035a466 100644
--- a/nfs.conf
+++ b/nfs.conf
@@ -60,6 +60,7 @@
# vers4.1=y
# vers4.2=y
# rdma=n
+# chroot=/export
#
[statd]
# debug=0
diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
index e3654a3c2c2b..bd83e57dd6da 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 chroot .

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..25b0fb84f753 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -33,11 +33,14 @@
#include "fsloc.h"
#include "pseudoflavors.h"
#include "xcommon.h"
+#include "conffile.h"

#ifdef USE_BLKID
#include "blkid/blkid.h"
#endif

+static struct xthread_workqueue *cache_workqueue;
+
/*
* Invoked by RPC service loop
*/
@@ -55,6 +58,32 @@ enum nfsd_fsid {
FSID_UUID16_INUM,
};

+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 = conf_get_str("nfsd", "chroot");
+ if (!chroot || *chroot == '\0')
+ return;
+ /* Strip leading '/' */
+ while (chroot[0] == '/' && chroot[1] == '/')
+ chroot++;
+ if (chroot[0] == '/' && chroot[1] == '\0')
+ 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.
@@ -829,7 +858,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 +950,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;
}

@@ -1381,6 +1410,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)
@@ -1508,7 +1539,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,7 +1577,7 @@ 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;
}
diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
index d83ef869d26e..8fb23721daf6 100644
--- a/utils/nfsd/nfsd.man
+++ b/utils/nfsd/nfsd.man
@@ -167,6 +167,10 @@ 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 chroot
+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.
+

.SH NOTES
If the program is built with TI-RPC support, it will enable any protocol and
--
2.21.0

2019-05-15 14:04:11

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [RFC PATCH 0/5] Add a chroot option to nfs.conf

On Tue, May 14, 2019 at 04:41:48PM -0400, Trond Myklebust wrote:
> The following patchset aims to allow the configuration of a 'chroot
> jail' to rpc.nfsd, and allowing us to export a filesystem /foo (and
> possibly subtrees) as '/'.

This is great, thanks! Years ago I did an incomplete version that
worked by just string manipulations on paths. Running part of mountd in
a chrooted thread is a neat way to do it.

If I understand right, the only part that's being run in a chroot is the
writes to /proc/net/sunrpc/*/channel files. So that means that the path
included in writes to /proc/net/sunrpc/nfsd_fh/channel will be resolved
with respect to the chroot by the kernel code handling the write.

That's not the only place in mountd that uses export paths, though.
What about:

- next_mnt(), which compares paths from the export file to paths
in /etc/mtab.
- is_mountpoint, which stats export paths.
- match_fsid, which stats export paths.

Etc. Doesn't that stuff also need to be done with respect to the
chroot? Am I missing something?

--b.

>
> Trond Myklebust (5):
> mountd: Ensure we don't share cache file descriptors among processes.
> Add a simple workqueue mechanism
> Add a helper to write to a file through the chrooted thread
> Add support for chrooted exports
> Add support for chroot in exportfs
>
> aclocal/libpthread.m4 | 13 +-
> configure.ac | 6 +-
> nfs.conf | 1 +
> support/include/misc.h | 11 ++
> support/misc/Makefile.am | 2 +-
> support/misc/workqueue.c | 267 +++++++++++++++++++++++++++++++++++++
> systemd/nfs.conf.man | 3 +-
> utils/exportfs/Makefile.am | 2 +-
> utils/exportfs/exportfs.c | 31 ++++-
> utils/mountd/Makefile.am | 3 +-
> utils/mountd/cache.c | 39 +++++-
> utils/mountd/mountd.c | 5 +-
> utils/nfsd/nfsd.man | 4 +
> 13 files changed, 369 insertions(+), 18 deletions(-)
> create mode 100644 support/misc/workqueue.c
>
> --
> 2.21.0

2019-05-15 14:17:14

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [RFC PATCH 4/5] Add support for chrooted exports

On Tue, May 14, 2019 at 04:41:52PM -0400, Trond Myklebust wrote:
> diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
> index d83ef869d26e..8fb23721daf6 100644
> --- a/utils/nfsd/nfsd.man
> +++ b/utils/nfsd/nfsd.man
> @@ -167,6 +167,10 @@ 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 chroot
> +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.

I don't feel like this is completely clear. Maybe add an example like:
"If the export file contains a line like "/path *(rw)", clients will
mount "/path" but the filesystem they see will be the one at
"$chroot/path"". ?

--b.

2019-05-15 14:39:49

by Trond Myklebust

[permalink] [raw]
Subject: Re: [RFC PATCH 0/5] Add a chroot option to nfs.conf

On Wed, 2019-05-15 at 10:01 -0400, J. Bruce Fields wrote:
> On Tue, May 14, 2019 at 04:41:48PM -0400, Trond Myklebust wrote:
> > The following patchset aims to allow the configuration of a 'chroot
> > jail' to rpc.nfsd, and allowing us to export a filesystem /foo (and
> > possibly subtrees) as '/'.
>
> This is great, thanks! Years ago I did an incomplete version that
> worked by just string manipulations on paths. Running part of mountd
> in
> a chrooted thread is a neat way to do it.
>
> If I understand right, the only part that's being run in a chroot is
> the
> writes to /proc/net/sunrpc/*/channel files. So that means that the
> path
> included in writes to /proc/net/sunrpc/nfsd_fh/channel will be
> resolved
> with respect to the chroot by the kernel code handling the write.
>
> That's not the only place in mountd that uses export paths, though.
> What about:
>
> - next_mnt(), which compares paths from the export file to
> paths
> in /etc/mtab.
> - is_mountpoint, which stats export paths.
> - match_fsid, which stats export paths.
>
> Etc. Doesn't that stuff also need to be done with respect to the
> chroot? Am I missing something?
>

Good feedback. Thanks!

Yes, I do need to fix up those comparisons too. I suspect that we want
to do the stat() in the chrooted namespace so that we resolve symlinks
etc correctly. That should be easy to add: the workqueue implementation
is generic, so adding a new operation is pretty trivial.

For the path comparisons in next_mnt(), things are a little murkier.
I'm not overly happy with a solution where user space tries to resolve
paths because the presence of symlinks, bind mounts, etc can and do
mean that user space will get that wrong. Perhaps the right solution is
to do an open() and check that it ends up in the right place (i.e.
check the fsid/inode number with a fstat())?

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]