* [PATCH 0/7] nfs-utils fixes @ 2020-04-16 22:12 trondmy 2020-04-16 22:12 ` [PATCH 1/7] mountd: Add a helper nfsd_path_statfs64() for uuid_by_path() trondmy 2020-05-08 14:13 ` [PATCH 0/7] nfs-utils fixes Steve Dickson 0 siblings, 2 replies; 11+ messages in thread From: trondmy @ 2020-04-16 22:12 UTC (permalink / raw) To: Steve Dickson; +Cc: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> This patchset fixes a couple of missed API changes in mountd to ensure that the [exports]rootdir root jail works correctly. It fixes up the 'same_path' function, as well as 'uuid_by_path'. It also improves the error handling, and tries to distinguish between bona fide path resolution problems, and other transient issues in order to avoid having knfsd return spurious ESTALE errors. Trond Myklebust (7): mountd: Add a helper nfsd_path_statfs64() for uuid_by_path() nfsd: Support running nfsd_name_to_handle_at() in the root jail mountd: Fix up path checking helper same_path() Fix autoconf probe for 'struct nfs_filehandle' mountd: Ensure dump_to_cache() sets errno appropriately mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() mountd: Check the stat() return values in match_fsid() configure.ac | 7 +- support/include/nfsd_path.h | 9 ++ support/misc/nfsd_path.c | 109 ++++++++++++++++++++++ utils/mountd/cache.c | 174 ++++++++++++++++++++++++------------ 4 files changed, 242 insertions(+), 57 deletions(-) -- 2.25.2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/7] mountd: Add a helper nfsd_path_statfs64() for uuid_by_path() 2020-04-16 22:12 [PATCH 0/7] nfs-utils fixes trondmy @ 2020-04-16 22:12 ` trondmy 2020-04-16 22:12 ` [PATCH 2/7] nfsd: Support running nfsd_name_to_handle_at() in the root jail trondmy 2020-05-08 14:13 ` [PATCH 0/7] nfs-utils fixes Steve Dickson 1 sibling, 1 reply; 11+ messages in thread From: trondmy @ 2020-04-16 22:12 UTC (permalink / raw) To: Steve Dickson; +Cc: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> Ensure uuid_by_path() works correctly when 'rootdir' is set in the [exports] section of nfs.conf. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- support/include/nfsd_path.h | 5 +++++ support/misc/nfsd_path.c | 43 +++++++++++++++++++++++++++++++++++++ utils/mountd/cache.c | 2 +- 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/support/include/nfsd_path.h b/support/include/nfsd_path.h index b42416bbff58..8331ff96a277 100644 --- a/support/include/nfsd_path.h +++ b/support/include/nfsd_path.h @@ -6,6 +6,8 @@ #include <sys/stat.h> +struct statfs64; + void nfsd_path_init(void); const char * nfsd_path_nfsd_rootdir(void); @@ -15,6 +17,9 @@ 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); +int nfsd_path_statfs64(const char *pathname, + struct statfs64 *statbuf); + char * nfsd_realpath(const char *path, char *resolved_path); ssize_t nfsd_path_read(int fd, char *buf, size_t len); diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c index f078a668fb8f..ab6c98dbe395 100644 --- a/support/misc/nfsd_path.c +++ b/support/misc/nfsd_path.c @@ -5,6 +5,7 @@ #include <errno.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/vfs.h> #include <limits.h> #include <stdlib.h> #include <unistd.h> @@ -180,6 +181,48 @@ nfsd_path_lstat(const char *pathname, struct stat *statbuf) return nfsd_run_stat(nfsd_wq, nfsd_lstatfunc, pathname, statbuf); } +struct nfsd_statfs64_data { + const char *pathname; + struct statfs64 *statbuf; + int ret; + int err; +}; + +static void +nfsd_statfs64func(void *data) +{ + struct nfsd_statfs64_data *d = data; + + d->ret = statfs64(d->pathname, d->statbuf); + if (d->ret < 0) + d->err = errno; +} + +static int +nfsd_run_statfs64(struct xthread_workqueue *wq, + const char *pathname, + struct statfs64 *statbuf) +{ + struct nfsd_statfs64_data data = { + pathname, + statbuf, + 0, + 0 + }; + xthread_work_run_sync(wq, nfsd_statfs64func, &data); + if (data.ret < 0) + errno = data.err; + return data.ret; +} + +int +nfsd_path_statfs64(const char *pathname, struct statfs64 *statbuf) +{ + if (!nfsd_wq) + return statfs64(pathname, statbuf); + return nfsd_run_statfs64(nfsd_wq, pathname, statbuf); +} + struct nfsd_realpath_data { const char *pathname; char *resolved; diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c index 8f54e37b7936..7d8657c91323 100644 --- a/utils/mountd/cache.c +++ b/utils/mountd/cache.c @@ -352,7 +352,7 @@ static int uuid_by_path(char *path, int type, size_t uuidlen, char *uuid) const char *val; int rc; - rc = statfs64(path, &st); + rc = nfsd_path_statfs64(path, &st); if (type == 0 && rc == 0) { const unsigned long *bad; -- 2.25.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/7] nfsd: Support running nfsd_name_to_handle_at() in the root jail 2020-04-16 22:12 ` [PATCH 1/7] mountd: Add a helper nfsd_path_statfs64() for uuid_by_path() trondmy @ 2020-04-16 22:12 ` trondmy 2020-04-16 22:12 ` [PATCH 3/7] mountd: Fix up path checking helper same_path() trondmy 0 siblings, 1 reply; 11+ messages in thread From: trondmy @ 2020-04-16 22:12 UTC (permalink / raw) To: Steve Dickson; +Cc: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> When running nfsd_name_to_handle_at(), we usually want to see the same namespace as knfsd, so add helpers. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- support/include/nfsd_path.h | 4 +++ support/misc/nfsd_path.c | 66 +++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/support/include/nfsd_path.h b/support/include/nfsd_path.h index 8331ff96a277..3b73aadd8af7 100644 --- a/support/include/nfsd_path.h +++ b/support/include/nfsd_path.h @@ -6,6 +6,7 @@ #include <sys/stat.h> +struct file_handle; struct statfs64; void nfsd_path_init(void); @@ -25,4 +26,7 @@ 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); +int nfsd_name_to_handle_at(int fd, const char *path, + struct file_handle *fh, + int *mount_id, int flags); #endif diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c index ab6c98dbe395..1f6dfd4b642b 100644 --- a/support/misc/nfsd_path.c +++ b/support/misc/nfsd_path.c @@ -6,6 +6,7 @@ #include <sys/types.h> #include <sys/stat.h> #include <sys/vfs.h> +#include <fcntl.h> #include <limits.h> #include <stdlib.h> #include <unistd.h> @@ -14,6 +15,7 @@ #include "xmalloc.h" #include "xlog.h" #include "xstat.h" +#include "nfslib.h" #include "nfsd_path.h" #include "workqueue.h" @@ -340,3 +342,67 @@ nfsd_path_write(int fd, const char *buf, size_t len) return write(fd, buf, len); return nfsd_run_write(nfsd_wq, fd, buf, len); } + +#if defined(HAVE_NAME_TO_HANDLE_AT) +struct nfsd_handle_data { + int fd; + const char *path; + struct file_handle *fh; + 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; +} + +static int +nfsd_run_name_to_handle_at(struct xthread_workqueue *wq, + int fd, const char *path, struct file_handle *fh, + int *mount_id, int flags) +{ + struct nfsd_handle_data data = { + fd, + path, + fh, + mount_id, + flags, + 0, + 0 + }; + + xthread_work_run_sync(wq, nfsd_name_to_handle_func, &data); + if (data.ret < 0) + errno = data.err; + return data.ret; +} + +int +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); +} +#else +int +nfsd_name_to_handle_at(int UNUSED(fd), const char *UNUSED(path), + struct file_handle *UNUSED(fh), + int *UNUSED(mount_id), int UNUSED(flags)) +{ + errno = ENOSYS; + return -1; +} +#endif -- 2.25.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/7] mountd: Fix up path checking helper same_path() 2020-04-16 22:12 ` [PATCH 2/7] nfsd: Support running nfsd_name_to_handle_at() in the root jail trondmy @ 2020-04-16 22:12 ` trondmy 2020-04-16 22:12 ` [PATCH 4/7] Fix autoconf probe for 'struct nfs_filehandle' trondmy 0 siblings, 1 reply; 11+ messages in thread From: trondmy @ 2020-04-16 22:12 UTC (permalink / raw) To: Steve Dickson; +Cc: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> Convert 'same_path()' so that it works when 'rootdir' is set in the [exports] section of nfs.conf. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- utils/mountd/cache.c | 83 +++++++++++++++++++++++++++++++------------- 1 file changed, 59 insertions(+), 24 deletions(-) diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c index 7d8657c91323..94e9e44b46b9 100644 --- a/utils/mountd/cache.c +++ b/utils/mountd/cache.c @@ -72,6 +72,18 @@ static ssize_t cache_write(int fd, const char *buf, size_t len) return nfsd_path_write(fd, buf, len); } +static bool path_lookup_error(int err) +{ + switch (err) { + case ELOOP: + case ENAMETOOLONG: + case ENOENT: + case ENOTDIR: + return 1; + } + return 0; +} + /* * Support routines for text-based upcalls. * Fields are separated by spaces. @@ -430,23 +442,9 @@ static inline int count_slashes(char *p) return cnt; } -static int same_path(char *child, char *parent, int len) +#if defined(HAVE_STRUCT_FILE_HANDLE) +static int check_same_path_by_handle(const char *child, const char *parent) { - static char p[PATH_MAX]; - struct stat sc, sp; - - if (len <= 0) - len = strlen(child); - strncpy(p, child, len); - p[len] = 0; - if (strcmp(p, parent) == 0) - return 1; - - /* If number of '/' are different, they must be different */ - if (count_slashes(p) != count_slashes(parent)) - return 0; - -#if defined(HAVE_NAME_TO_HANDLE_AT) && defined(HAVE_STRUCT_FILE_HANDLE) struct { struct file_handle fh; unsigned char handle[128]; @@ -455,13 +453,17 @@ static int same_path(char *child, char *parent, int len) fchild.fh.handle_bytes = 128; fparent.fh.handle_bytes = 128; - if (name_to_handle_at(AT_FDCWD, p, &fchild.fh, &mnt_child, 0) != 0) { - if (errno == ENOSYS) - goto fallback; - return 0; + + /* This process should have the CAP_DAC_READ_SEARCH capability */ + if (nfsd_name_to_handle_at(AT_FDCWD, child, &fchild.fh, &mnt_child, 0) < 0) + return -1; + if (nfsd_name_to_handle_at(AT_FDCWD, parent, &fparent.fh, &mnt_parent, 0) < 0) { + /* If the child resolved, but the parent did not, they differ */ + if (path_lookup_error(errno)) + return 0; + /* Otherwise, we just don't know */ + return -1; } - if (name_to_handle_at(AT_FDCWD, parent, &fparent.fh, &mnt_parent, 0) != 0) - return 0; if (mnt_child != mnt_parent || fchild.fh.handle_bytes != fparent.fh.handle_bytes || @@ -471,14 +473,24 @@ static int same_path(char *child, char *parent, int len) return 0; return 1; -fallback: +} +#else +static int check_same_path_by_handle(const char *child, const char *parent) +{ + errno = ENOSYS; + return -1; +} #endif +static int check_same_path_by_inode(const char *child, const char *parent) +{ + struct stat sc, sp; + /* This is nearly good enough. However if a directory is * bind-mounted in two places and both are exported, it * could give a false positive */ - if (nfsd_path_lstat(p, &sc) != 0) + if (nfsd_path_lstat(child, &sc) != 0) return 0; if (nfsd_path_lstat(parent, &sp) != 0) return 0; @@ -490,6 +502,29 @@ fallback: return 1; } +static int same_path(char *child, char *parent, int len) +{ + static char p[PATH_MAX]; + int err; + + if (len <= 0) + len = strlen(child); + strncpy(p, child, len); + p[len] = 0; + if (strcmp(p, parent) == 0) + return 1; + + /* If number of '/' are different, they must be different */ + if (count_slashes(p) != count_slashes(parent)) + return 0; + + /* Try to use filehandle approach before falling back to stat() */ + err = check_same_path_by_handle(p, parent); + if (err != -1) + return err; + return check_same_path_by_inode(p, parent); +} + static int is_subdirectory(char *child, char *parent) { /* Check is child is strictly a subdirectory of -- 2.25.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/7] Fix autoconf probe for 'struct nfs_filehandle' 2020-04-16 22:12 ` [PATCH 3/7] mountd: Fix up path checking helper same_path() trondmy @ 2020-04-16 22:12 ` trondmy 2020-04-16 22:12 ` [PATCH 5/7] mountd: Ensure dump_to_cache() sets errno appropriately trondmy 0 siblings, 1 reply; 11+ messages in thread From: trondmy @ 2020-04-16 22:12 UTC (permalink / raw) To: Steve Dickson; +Cc: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> It was failing because fcntl.h is not one of the standard includes. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- configure.ac | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 00b32800c526..df88e58fd0d4 100644 --- a/configure.ac +++ b/configure.ac @@ -531,7 +531,12 @@ AC_TYPE_PID_T AC_TYPE_SIZE_T AC_HEADER_TIME AC_STRUCT_TM -AC_CHECK_TYPES([struct file_handle]) +AC_CHECK_TYPES([struct file_handle], [], [], [[ + #define _GNU_SOURCE + #include <sys/types.h> + #include <sys/stat.h> + #include <fcntl.h> + ]]) dnl ************************************************************* dnl Check for functions -- 2.25.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/7] mountd: Ensure dump_to_cache() sets errno appropriately 2020-04-16 22:12 ` [PATCH 4/7] Fix autoconf probe for 'struct nfs_filehandle' trondmy @ 2020-04-16 22:12 ` trondmy 2020-04-16 22:12 ` [PATCH 6/7] mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() trondmy 0 siblings, 1 reply; 11+ messages in thread From: trondmy @ 2020-04-16 22:12 UTC (permalink / raw) To: Steve Dickson; +Cc: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> cache_write() will set errno if it returns -1, so that callers can handle errors appropriately, however dump_to_cache() needs to do so too. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- utils/mountd/cache.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c index 94e9e44b46b9..0f323226b12a 100644 --- a/utils/mountd/cache.c +++ b/utils/mountd/cache.c @@ -936,12 +936,13 @@ static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_m } -static int dump_to_cache(int f, char *buf, int buflen, char *domain, +static int dump_to_cache(int f, char *buf, int blen, char *domain, char *path, struct exportent *exp, int ttl) { char *bp = buf; - int blen = buflen; time_t now = time(0); + size_t buflen; + ssize_t err; if (ttl <= 1) ttl = DEFAULT_TTL; @@ -974,8 +975,18 @@ static int dump_to_cache(int f, char *buf, int buflen, char *domain, } else qword_adduint(&bp, &blen, now + ttl); qword_addeol(&bp, &blen); - if (blen <= 0) return -1; - if (cache_write(f, buf, bp - buf) != bp - buf) return -1; + if (blen <= 0) { + errno = ENOBUFS; + return -1; + } + buflen = bp - buf; + err = cache_write(f, buf, buflen); + if (err < 0) + return err; + if ((size_t)err != buflen) { + errno = ENOSPC; + return -1; + } return 0; } -- 2.25.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 6/7] mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() 2020-04-16 22:12 ` [PATCH 5/7] mountd: Ensure dump_to_cache() sets errno appropriately trondmy @ 2020-04-16 22:12 ` trondmy 2020-04-16 22:12 ` [PATCH 7/7] mountd: Check the stat() return values in match_fsid() trondmy 2020-04-27 18:38 ` [PATCH 6/7] mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() J. Bruce Fields 0 siblings, 2 replies; 11+ messages in thread From: trondmy @ 2020-04-16 22:12 UTC (permalink / raw) To: Steve Dickson; +Cc: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> In nfsd_fh(), if the error returned by the downcall is transient, then we should ignore it. Only reject the export if the filesystem path is truly not exportable. This fixes a case where we can see spurious NFSERR_STALE errors being returned by knfsd. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- utils/mountd/cache.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c index 0f323226b12a..79d3ee085a90 100644 --- a/utils/mountd/cache.c +++ b/utils/mountd/cache.c @@ -843,8 +843,14 @@ static void nfsd_fh(int f) } } } - if (found && - found->e_mountpoint && + + if (!found) { + /* The missing dev could be what we want, so just be + * quiet rather than returning stale yet + */ + if (dev_missing) + goto out; + } else if (found->e_mountpoint && !is_mountpoint(found->e_mountpoint[0]? found->e_mountpoint: found->e_path)) { @@ -855,17 +861,12 @@ static void nfsd_fh(int f) */ /* FIXME we need to make sure we re-visit this later */ goto out; + } else if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0) { + if (!path_lookup_error(errno)) + goto out; + /* The kernel is saying the path is unexportable */ + found = NULL; } - if (!found && dev_missing) { - /* The missing dev could be what we want, so just be - * quite rather than returning stale yet - */ - goto out; - } - - if (found) - if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0) - found = 0; bp = buf; blen = sizeof(buf); qword_add(&bp, &blen, dom); -- 2.25.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 7/7] mountd: Check the stat() return values in match_fsid() 2020-04-16 22:12 ` [PATCH 6/7] mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() trondmy @ 2020-04-16 22:12 ` trondmy 2020-04-27 18:38 ` [PATCH 6/7] mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() J. Bruce Fields 1 sibling, 0 replies; 11+ messages in thread From: trondmy @ 2020-04-16 22:12 UTC (permalink / raw) To: Steve Dickson; +Cc: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> Propagate errors from the stat() calls in match_fsid() so that the caller can be more careful about how they are handled. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- utils/mountd/cache.c | 45 +++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c index 79d3ee085a90..6cba2883026f 100644 --- a/utils/mountd/cache.c +++ b/utils/mountd/cache.c @@ -659,55 +659,66 @@ static int parse_fsid(int fsidtype, int fsidlen, char *fsid, return 0; } -static bool match_fsid(struct parsed_fsid *parsed, nfs_export *exp, char *path) +static int match_fsid(struct parsed_fsid *parsed, nfs_export *exp, char *path) { struct stat stb; int type; char u[16]; if (nfsd_path_stat(path, &stb) != 0) - return false; + goto path_error; if (!S_ISDIR(stb.st_mode) && !S_ISREG(stb.st_mode)) - return false; + goto nomatch; switch (parsed->fsidtype) { case FSID_DEV: case FSID_MAJOR_MINOR: case FSID_ENCODE_DEV: if (stb.st_ino != parsed->inode) - return false; + goto nomatch; if (parsed->major != major(stb.st_dev) || parsed->minor != minor(stb.st_dev)) - return false; - return true; + goto nomatch; + goto match; case FSID_NUM: if (((exp->m_export.e_flags & NFSEXP_FSID) == 0 || exp->m_export.e_fsid != parsed->fsidnum)) - return false; - return true; + goto nomatch; + goto match; case FSID_UUID4_INUM: case FSID_UUID16_INUM: if (stb.st_ino != parsed->inode) - return false; + goto nomatch; goto check_uuid; case FSID_UUID8: case FSID_UUID16: - if (!is_mountpoint(path)) - return false; + errno = 0; + if (!is_mountpoint(path)) { + if (!errno) + goto nomatch; + goto path_error; + } check_uuid: if (exp->m_export.e_uuid) { get_uuid(exp->m_export.e_uuid, parsed->uuidlen, u); if (memcmp(u, parsed->fhuuid, parsed->uuidlen) == 0) - return true; + goto match; } else for (type = 0; uuid_by_path(path, type, parsed->uuidlen, u); type++) if (memcmp(u, parsed->fhuuid, parsed->uuidlen) == 0) - return true; + goto match; } - return false; +nomatch: + return 0; +match: + return 1; +path_error: + if (path_lookup_error(errno)) + goto nomatch; + return -1; } static struct addrinfo *lookup_client_addr(char *dom) @@ -815,8 +826,12 @@ static void nfsd_fh(int f) exp->m_export.e_path)) dev_missing ++; - if (!match_fsid(&parsed, exp, path)) + switch(match_fsid(&parsed, exp, path)) { + case 0: continue; + case -1: + goto out; + } if (is_ipaddr_client(dom) && !ipaddr_client_matches(exp, ai)) continue; -- 2.25.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 6/7] mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() 2020-04-16 22:12 ` [PATCH 6/7] mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() trondmy 2020-04-16 22:12 ` [PATCH 7/7] mountd: Check the stat() return values in match_fsid() trondmy @ 2020-04-27 18:38 ` J. Bruce Fields 2020-04-27 18:39 ` J. Bruce Fields 1 sibling, 1 reply; 11+ messages in thread From: J. Bruce Fields @ 2020-04-27 18:38 UTC (permalink / raw) To: trondmy; +Cc: Steve Dickson, linux-nfs On Thu, Apr 16, 2020 at 06:12:51PM -0400, trondmy@kernel.org wrote: > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > In nfsd_fh(), if the error returned by the downcall is transient, > then we should ignore it. Only reject the export if the filesystem > path is truly not exportable. > This fixes a case where we can see spurious NFSERR_STALE errors > being returned by knfsd. > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > --- > utils/mountd/cache.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) > > diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c > index 0f323226b12a..79d3ee085a90 100644 > --- a/utils/mountd/cache.c > +++ b/utils/mountd/cache.c > @@ -843,8 +843,14 @@ static void nfsd_fh(int f) > } > } > } > - if (found && > - found->e_mountpoint && > + > + if (!found) { > + /* The missing dev could be what we want, so just be > + * quiet rather than returning stale yet > + */ > + if (dev_missing) > + goto out; > + } else if (found->e_mountpoint && > !is_mountpoint(found->e_mountpoint[0]? > found->e_mountpoint: > found->e_path)) { > @@ -855,17 +861,12 @@ static void nfsd_fh(int f) > */ > /* FIXME we need to make sure we re-visit this later */ > goto out; > + } else if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0) { > + if (!path_lookup_error(errno)) > + goto out; > + /* The kernel is saying the path is unexportable */ > + found = NULL; > } > - if (!found && dev_missing) { > - /* The missing dev could be what we want, so just be > - * quite rather than returning stale yet > - */ > - goto out; > - } > - > - if (found) > - if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0) > - found = 0; > > bp = buf; blen = sizeof(buf); > qword_add(&bp, &blen, dom); Is everybody else better at reading this kind of patch? Between the code reshuffling and the conditionals, it's almost totally opaque to me. I'd have split it up something like the below.--b. commit b0b623281017 Author: Trond Myklebust <trond.myklebust@hammerspace.com> Date: Thu Apr 16 18:12:51 2020 -0400 mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() In nfsd_fh(), if the error returned by the downcall is transient, then we should ignore it. Only reject the export if the filesystem path is truly not exportable. This fixes a case where we can see spurious NFSERR_STALE errors being returned by knfsd. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c index bcd4b3ce5bb2..0426b171f040 100644 --- a/utils/mountd/cache.c +++ b/utils/mountd/cache.c @@ -826,8 +826,12 @@ static void nfsd_fh(int f) */ /* FIXME we need to make sure we re-visit this later */ goto out; - } else if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0) - found = 0; + } else if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0) { + if (!path_lookup_error(errno)) + goto out; + /* The kernel is saying the path is unexportable */ + found = NULL; + } bp = buf; blen = sizeof(buf); qword_add(&bp, &blen, dom); commit 931a2e77f954 Author: J. Bruce Fields <bfields@redhat.com> Date: Mon Apr 27 13:22:30 2020 -0400 rearrange logic; no change in behavior diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c index 8f54e37b7936..bcd4b3ce5bb2 100644 --- a/utils/mountd/cache.c +++ b/utils/mountd/cache.c @@ -808,8 +808,14 @@ static void nfsd_fh(int f) } } } - if (found && - found->e_mountpoint && + + if (!found) { + /* The missing dev could be what we want, so just be + * quiet rather than returning stale yet + */ + if (dev_missing) + goto out; + } else if (found->e_mountpoint && !is_mountpoint(found->e_mountpoint[0]? found->e_mountpoint: found->e_path)) { @@ -820,16 +826,7 @@ static void nfsd_fh(int f) */ /* FIXME we need to make sure we re-visit this later */ goto out; - } - if (!found && dev_missing) { - /* The missing dev could be what we want, so just be - * quite rather than returning stale yet - */ - goto out; - } - - if (found) - if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0) + } else if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0) found = 0; bp = buf; blen = sizeof(buf); ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 6/7] mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() 2020-04-27 18:38 ` [PATCH 6/7] mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() J. Bruce Fields @ 2020-04-27 18:39 ` J. Bruce Fields 0 siblings, 0 replies; 11+ messages in thread From: J. Bruce Fields @ 2020-04-27 18:39 UTC (permalink / raw) To: trondmy; +Cc: Steve Dickson, linux-nfs Ack to the series, otherwise, looks good.--b. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/7] nfs-utils fixes 2020-04-16 22:12 [PATCH 0/7] nfs-utils fixes trondmy 2020-04-16 22:12 ` [PATCH 1/7] mountd: Add a helper nfsd_path_statfs64() for uuid_by_path() trondmy @ 2020-05-08 14:13 ` Steve Dickson 1 sibling, 0 replies; 11+ messages in thread From: Steve Dickson @ 2020-05-08 14:13 UTC (permalink / raw) To: trondmy; +Cc: linux-nfs On 4/16/20 6:12 PM, trondmy@kernel.org wrote: > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > This patchset fixes a couple of missed API changes in mountd to > ensure that the [exports]rootdir root jail works correctly. It > fixes up the 'same_path' function, as well as 'uuid_by_path'. > It also improves the error handling, and tries to distinguish > between bona fide path resolution problems, and other transient > issues in order to avoid having knfsd return spurious ESTALE > errors. > > Trond Myklebust (7): > mountd: Add a helper nfsd_path_statfs64() for uuid_by_path() > nfsd: Support running nfsd_name_to_handle_at() in the root jail > mountd: Fix up path checking helper same_path() > Fix autoconf probe for 'struct nfs_filehandle' > mountd: Ensure dump_to_cache() sets errno appropriately > mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() > mountd: Check the stat() return values in match_fsid() > > configure.ac | 7 +- > support/include/nfsd_path.h | 9 ++ > support/misc/nfsd_path.c | 109 ++++++++++++++++++++++ > utils/mountd/cache.c | 174 ++++++++++++++++++++++++------------ > 4 files changed, 242 insertions(+), 57 deletions(-) > Committed the series (tag: nfs-utils-2-4-4-rc4) steved. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-05-08 14:13 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-16 22:12 [PATCH 0/7] nfs-utils fixes trondmy 2020-04-16 22:12 ` [PATCH 1/7] mountd: Add a helper nfsd_path_statfs64() for uuid_by_path() trondmy 2020-04-16 22:12 ` [PATCH 2/7] nfsd: Support running nfsd_name_to_handle_at() in the root jail trondmy 2020-04-16 22:12 ` [PATCH 3/7] mountd: Fix up path checking helper same_path() trondmy 2020-04-16 22:12 ` [PATCH 4/7] Fix autoconf probe for 'struct nfs_filehandle' trondmy 2020-04-16 22:12 ` [PATCH 5/7] mountd: Ensure dump_to_cache() sets errno appropriately trondmy 2020-04-16 22:12 ` [PATCH 6/7] mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() trondmy 2020-04-16 22:12 ` [PATCH 7/7] mountd: Check the stat() return values in match_fsid() trondmy 2020-04-27 18:38 ` [PATCH 6/7] mountd: Ignore transient and non-fatal filesystem errors in nfsd_fh() J. Bruce Fields 2020-04-27 18:39 ` J. Bruce Fields 2020-05-08 14:13 ` [PATCH 0/7] nfs-utils fixes Steve Dickson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.