NFS export symlink vulnerability fix - Replaced dangerous use of realpath within support/nfs/export.c with nfsd_realpath variant that is executed within the chrooted thread rather than main thread. - Implemented nfsd_path.h methods to work securely within

Commit Message

Christopher Bii Jan. 7, 2025, 9:11 p.m. UTC

Signed-off-by: Christopher Bii <[e-mail]>
---
 support/include/nfsd_path.h |   5 +-
 support/misc/nfsd_path.c    | 255 +++++++++++-------------------------
 support/nfs/exports.c       |   3 +-
 3 files changed, 80 insertions(+), 183 deletions(-)

Patch

diff --git a/support/include/nfsd_path.h b/support/include/nfsd_path.h
index 214bde47..8085ddfd 100644
--- a/support/include/nfsd_path.h
+++ b/support/include/nfsd_path.h
@@ -8,6 +8,7 @@ 
 
 struct file_handle;
 struct statfs;
+struct nfsd_task_t;
 
 void 		nfsd_path_init(void);
 
@@ -23,8 +24,8 @@  int		nfsd_path_statfs(const char *pathname,
 
 char *		nfsd_realpath(const char *path, char *resolved_path);
 
-ssize_t		nfsd_path_read(int fd, char *buf, size_t len);
-ssize_t		nfsd_path_write(int fd, const char *buf, size_t len);
+ssize_t		nfsd_path_read(int fd, void* buf, size_t len);
+ssize_t		nfsd_path_write(int fd, void* buf, size_t len);
 
 int		nfsd_name_to_handle_at(int fd, const char *path,
 				       struct file_handle *fh,
diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c
index 0f727d3b..38f0a394 100644
--- a/support/misc/nfsd_path.c
+++ b/support/misc/nfsd_path.c
@@ -19,10 +19,21 @@ 
 #include "nfsd_path.h"
 #include "workqueue.h"
 
-static struct xthread_workqueue *nfsd_wq;
+static struct xthread_workqueue *nfsd_wq = NULL;
 const char      *rootdir;
 size_t          rootdir_pathlen = 0;
 
+struct nfsd_task_t {
+        int             ret;
+        void*           data;
+};
+/* Function used to offload tasks that must be ran within the correct
+ * chroot environment.
+ * */
+static void
+nfsd_run_task(void (*func)(void*), void* data){
+        nfsd_wq ? xthread_work_run_sync(nfsd_wq, func, data) : func(data);
+};
 
 const char*
 nfsd_path_rootdir(void)
@@ -97,224 +108,119 @@  nfsd_path_init(void)
 }
 
 struct nfsd_stat_data {
-	const char *pathname;
-	struct stat *statbuf;
-	int ret;
-	int err;
+	const char      *pathname;
+	struct stat     *statbuf;
+        int             (*stat_handler)(const char*, struct stat*);
 };
 
 static void
-nfsd_statfunc(void *data)
-{
-	struct nfsd_stat_data *d = data;
-
-	d->ret = xstat(d->pathname, d->statbuf);
-	if (d->ret < 0)
-		d->err = errno;
-}
-
-static void
-nfsd_lstatfunc(void *data)
+nfsd_handle_stat(void *data)
 {
-	struct nfsd_stat_data *d = data;
-
-	d->ret = xlstat(d->pathname, d->statbuf);
-	if (d->ret < 0)
-		d->err = errno;
+        struct nfsd_task_t*     t = data;
+	struct nfsd_stat_data*  d = t->data;
+        t->ret = d->stat_handler(d->pathname, d->statbuf);
 }
 
 static int
-nfsd_run_stat(struct xthread_workqueue *wq,
-		void (*func)(void *),
-		const char *pathname,
-		struct stat *statbuf)
+nfsd_run_stat(const char *pathname,
+	        struct stat *statbuf,
+                int (*handler)(const char*, struct stat*))
 {
-	struct nfsd_stat_data data = {
-		pathname,
-		statbuf,
-		0,
-		0
-	};
-	xthread_work_run_sync(wq, func, &data);
-	if (data.ret < 0)
-		errno = data.err;
-	return data.ret;
+        struct nfsd_task_t      t;
+        struct nfsd_stat_data   d = { pathname, statbuf, handler };
+        t.data = &d;
+        nfsd_run_task(nfsd_handle_stat, &t);
+	return t.ret;
 }
 
 int
 nfsd_path_stat(const char *pathname, struct stat *statbuf)
 {
-	if (!nfsd_wq)
-		return xstat(pathname, statbuf);
-	return nfsd_run_stat(nfsd_wq, nfsd_statfunc, pathname, statbuf);
+        return nfsd_run_stat(pathname, statbuf, stat);
 }
 
 int
-nfsd_path_lstat(const char *pathname, struct stat *statbuf)
-{
-	if (!nfsd_wq)
-		return xlstat(pathname, statbuf);
-	return nfsd_run_stat(nfsd_wq, nfsd_lstatfunc, pathname, statbuf);
-}
-
-struct nfsd_statfs_data {
-	const char *pathname;
-	struct statfs *statbuf;
-	int ret;
-	int err;
+nfsd_path_lstat(const char* pathname, struct stat* statbuf){
+        return nfsd_run_stat(pathname, statbuf, lstat);
 };
 
-static void
-nfsd_statfsfunc(void *data)
-{
-	struct nfsd_statfs_data *d = data;
-
-	d->ret = statfs(d->pathname, d->statbuf);
-	if (d->ret < 0)
-		d->err = errno;
-}
-
-static int
-nfsd_run_statfs(struct xthread_workqueue *wq,
-		  const char *pathname,
-		  struct statfs *statbuf)
-{
-	struct nfsd_statfs_data data = {
-		pathname,
-		statbuf,
-		0,
-		0
-	};
-	xthread_work_run_sync(wq, nfsd_statfsfunc, &data);
-	if (data.ret < 0)
-		errno = data.err;
-	return data.ret;
-}
-
 int
-nfsd_path_statfs(const char *pathname, struct statfs *statbuf)
+nfsd_path_statfs(const char* pathname, struct statfs* statbuf)
 {
-	if (!nfsd_wq)
-		return statfs(pathname, statbuf);
-	return nfsd_run_statfs(nfsd_wq, pathname, statbuf);
-}
+        return nfsd_run_stat(pathname, (struct stat*)statbuf, (int (*)(const char*, struct stat*))statfs);
+};
 
-struct nfsd_realpath_data {
-	const char *pathname;
-	char *resolved;
-	int err;
+struct nfsd_realpath_t {
+        const char*     path;
+        char*           resolved_buf;
+        char*           res_ptr;
 };
 
 static void
 nfsd_realpathfunc(void *data)
 {
-	struct nfsd_realpath_data *d = data;
-
-	d->resolved = realpath(d->pathname, d->resolved);
-	if (!d->resolved)
-		d->err = errno;
+        struct nfsd_realpath_t *d = data;
+        d->res_ptr = realpath(d->path, d->resolved_buf);
 }
 
-char *
-nfsd_realpath(const char *path, char *resolved_path)
+char*
+nfsd_realpath(const char *path, char *resolved_buf)
 {
-	struct nfsd_realpath_data data = {
-		path,
-		resolved_path,
-		0
-	};
-
-	if (!nfsd_wq)
-		return realpath(path, resolved_path);
-
-	xthread_work_run_sync(nfsd_wq, nfsd_realpathfunc, &data);
-	if (!data.resolved)
-		errno = data.err;
-	return data.resolved;
+        struct nfsd_realpath_t realpath_buf = {
+                .path = path,
+                .resolved_buf = resolved_buf
+        };
+        nfsd_run_task(nfsd_realpathfunc, &realpath_buf);
+        return realpath_buf.res_ptr;
 }
 
-struct nfsd_read_data {
-	int fd;
-	char *buf;
-	size_t len;
-	ssize_t ret;
-	int err;
+struct nfsd_rw_data {
+	int             fd;
+	void*           buf;
+	size_t          len;
+        ssize_t         bytes_read;
 };
 
 static void
 nfsd_readfunc(void *data)
 {
-	struct nfsd_read_data *d = data;
-
-	d->ret = read(d->fd, d->buf, d->len);
-	if (d->ret < 0)
-		d->err = errno;
+        struct nfsd_rw_data* t = (struct nfsd_rw_data*)data;
+        t->bytes_read = read(t->fd, t->buf, t->len);
 }
 
 static ssize_t
-nfsd_run_read(struct xthread_workqueue *wq, int fd, char *buf, size_t len)
+nfsd_run_read(int fd, void* buf, size_t len)
 {
-	struct nfsd_read_data data = {
-		fd,
-		buf,
-		len,
-		0,
-		0
-	};
-	xthread_work_run_sync(wq, nfsd_readfunc, &data);
-	if (data.ret < 0)
-		errno = data.err;
-	return data.ret;
+        struct nfsd_rw_data d = { .fd = fd, .buf = buf, .len = len };
+        nfsd_run_task(nfsd_readfunc, &d);
+	return d.bytes_read;
 }
 
 ssize_t
-nfsd_path_read(int fd, char *buf, size_t len)
+nfsd_path_read(int fd, void* buf, size_t len)
 {
-	if (!nfsd_wq)
-		return read(fd, buf, len);
-	return nfsd_run_read(nfsd_wq, fd, buf, len);
+	return nfsd_run_read(fd, buf, len);
 }
 
-struct nfsd_write_data {
-	int fd;
-	const char *buf;
-	size_t len;
-	ssize_t ret;
-	int err;
-};
-
 static void
 nfsd_writefunc(void *data)
 {
-	struct nfsd_write_data *d = data;
-
-	d->ret = write(d->fd, d->buf, d->len);
-	if (d->ret < 0)
-		d->err = errno;
+	struct nfsd_rw_data* d = data;
+	d->bytes_read = write(d->fd, d->buf, d->len);
 }
 
 static ssize_t
-nfsd_run_write(struct xthread_workqueue *wq, int fd, const char *buf, size_t len)
+nfsd_run_write(int fd, void* buf, size_t len)
 {
-	struct nfsd_write_data data = {
-		fd,
-		buf,
-		len,
-		0,
-		0
-	};
-	xthread_work_run_sync(wq, nfsd_writefunc, &data);
-	if (data.ret < 0)
-		errno = data.err;
-	return data.ret;
+        struct nfsd_rw_data d = { .fd = fd, .buf = buf, .len = len };
+        nfsd_run_task(nfsd_writefunc, &d);
+	return d.bytes_read;
 }
 
 ssize_t
-nfsd_path_write(int fd, const char *buf, size_t len)
+nfsd_path_write(int fd, void* buf, size_t len)
 {
-	if (!nfsd_wq)
-		return write(fd, buf, len);
-	return nfsd_run_write(nfsd_wq, fd, buf, len);
+	return nfsd_run_write(fd, buf, len);
 }
 
 #if defined(HAVE_NAME_TO_HANDLE_AT)
@@ -325,23 +231,18 @@  struct nfsd_handle_data {
 	int *mount_id;
 	int flags;
 	int ret;
-	int err;
 };
 
 static void
 nfsd_name_to_handle_func(void *data)
 {
 	struct nfsd_handle_data *d = data;
-
-	d->ret = name_to_handle_at(d->fd, d->path,
-			d->fh, d->mount_id, d->flags);
-	if (d->ret < 0)
-		d->err = errno;
+	d->ret = name_to_handle_at(d->fd, d->path, d->fh, d->mount_id, d->flags);
 }
 
 static int
-nfsd_run_name_to_handle_at(struct xthread_workqueue *wq,
-		int fd, const char *path, struct file_handle *fh,
+nfsd_run_name_to_handle_at(int fd, const char *path,
+                struct file_handle *fh,
 		int *mount_id, int flags)
 {
 	struct nfsd_handle_data data = {
@@ -350,25 +251,19 @@  nfsd_run_name_to_handle_at(struct xthread_workqueue *wq,
 		fh,
 		mount_id,
 		flags,
-		0,
 		0
 	};
 
-	xthread_work_run_sync(wq, nfsd_name_to_handle_func, &data);
-	if (data.ret < 0)
-		errno = data.err;
+	nfsd_run_task(nfsd_name_to_handle_func, &data);
 	return data.ret;
 }
 
 int
-nfsd_name_to_handle_at(int fd, const char *path, struct file_handle *fh,
+nfsd_name_to_handle_at(int fd, const char *path,
+                struct file_handle *fh,
 		int *mount_id, int flags)
 {
-	if (!nfsd_wq)
-		return name_to_handle_at(fd, path, fh, mount_id, flags);
-
-	return nfsd_run_name_to_handle_at(nfsd_wq, fd, path, fh,
-			mount_id, flags);
+        return nfsd_run_name_to_handle_at(fd, path, fh, mount_id, flags);
 }
 #else
 int
diff --git a/support/nfs/exports.c b/support/nfs/exports.c
index a6816e60..21ec6486 100644
--- a/support/nfs/exports.c
+++ b/support/nfs/exports.c
@@ -32,6 +32,7 @@ 
 #include "xio.h"
 #include "pseudoflavors.h"
 #include "reexport.h"
+#include "nfsd_path.h"
 
 #define EXPORT_DEFAULT_FLAGS	\
   (NFSEXP_READONLY|NFSEXP_ROOTSQUASH|NFSEXP_GATHERED_WRITES|NFSEXP_NOSUBTREECHECK)
@@ -200,7 +201,7 @@  getexportent(int fromkernel)
 		return NULL;
 	}
 	/* resolve symlinks */
-	if (realpath(ee.e_path, rpath) != NULL) {
+	if (nfsd_realpath(ee.e_path, rpath) != NULL) {
 		rpath[sizeof (rpath) - 1] = '\0';
 		strncpy(ee.e_path, rpath, sizeof (ee.e_path) - 1);
 		ee.e_path[sizeof (ee.e_path) - 1] = '\0';