| Message ID | 20200623223927.31795-1-fllinden@amazon.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | server side user xattr support (RFC 8276) | expand |
On Tue, Jun 23, 2020 at 10:39:17PM +0000, Frank van der Linden wrote: > v3: > * Rebase to v5.8-rc2 > * Use length probe + allocate + query for the listxattr and setxattr > operations to avoid allocating unneeded space. > * Because of the above, drop the 'use kvmalloc for svcxdr_tmpalloc' patch, > as it's no longer needed. > > v2: > * As per the discussion, user extended attributes are enabled if > the client and server support them (e.g. they support 4.2 and > advertise the user extended attribute FATTR). There are no longer > options to switch them off. > * The code is no longer conditioned on a config option. > * The number of patches has been reduced somewhat by merging > smaller, related ones. > * Renamed some functions and added parameter comments as requested. > > v1: > > * Split in to client and server (changed from the original RFC patch). > > Original RFC combined set is here: > > https://www.spinics.net/lists/linux-nfs/msg74843.html > > In general, these patches were, both server and client, tested as > follows: > * stress-ng-xattr with 1000 workers > * Test all corner cases (XATTR_SIZE_*) > * Test all failure cases (no xattr, setxattr with different or > invalid flags, etc). > * Verify the content of xattrs across several operations. Do you have some code to share for these tests? --b. > * Interop run against FreeBSD server/client implementation. > * Ran xfstests-dev, with no unexpected/new failures as compared > to an unpatched kernel. To fully use xfstests-dev, it needed > some modifications, as it expects to either use all xattr > namespaces, or none. Whereas NFS only suppors the "user." > namespace (+ optional ACLs). I will send the changes in > seperately. > > > Frank van der Linden (10): > xattr: break delegations in {set,remove}xattr > xattr: add a function to check if a namespace is supported > nfs,nfsd: NFSv4.2 extended attribute protocol definitions > nfsd: split off the write decode code in to a separate function > nfsd: add defines for NFSv4.2 extended attribute support > nfsd: define xattr functions to call in to their vfs counterparts > nfsd: take xattr bits in to account for permission checks > nfsd: add structure definitions for xattr requests / responses > nfsd: implement the xattr functions and en/decode logic > nfsd: add fattr support for user extended attributes > > fs/nfsd/nfs4proc.c | 128 ++++++++- > fs/nfsd/nfs4xdr.c | 531 +++++++++++++++++++++++++++++++++++--- > fs/nfsd/nfsd.h | 5 +- > fs/nfsd/vfs.c | 239 +++++++++++++++++ > fs/nfsd/vfs.h | 10 + > fs/nfsd/xdr4.h | 31 +++ > fs/xattr.c | 111 +++++++- > include/linux/nfs4.h | 22 +- > include/linux/xattr.h | 4 + > include/uapi/linux/nfs4.h | 3 + > 10 files changed, 1044 insertions(+), 40 deletions(-) > > -- > 2.17.2
By the way, I can't remember if I asked this before: is there a particular use case that motivates this xattr work? --b. On Tue, Jun 23, 2020 at 10:39:17PM +0000, Frank van der Linden wrote: > v3: > * Rebase to v5.8-rc2 > * Use length probe + allocate + query for the listxattr and setxattr > operations to avoid allocating unneeded space. > * Because of the above, drop the 'use kvmalloc for svcxdr_tmpalloc' patch, > as it's no longer needed. > > v2: > * As per the discussion, user extended attributes are enabled if > the client and server support them (e.g. they support 4.2 and > advertise the user extended attribute FATTR). There are no longer > options to switch them off. > * The code is no longer conditioned on a config option. > * The number of patches has been reduced somewhat by merging > smaller, related ones. > * Renamed some functions and added parameter comments as requested. > > v1: > > * Split in to client and server (changed from the original RFC patch). > > Original RFC combined set is here: > > https://www.spinics.net/lists/linux-nfs/msg74843.html > > In general, these patches were, both server and client, tested as > follows: > * stress-ng-xattr with 1000 workers > * Test all corner cases (XATTR_SIZE_*) > * Test all failure cases (no xattr, setxattr with different or > invalid flags, etc). > * Verify the content of xattrs across several operations. > * Use KASAN and KMEMLEAK for a longer mix of testruns to verify > that there were no leaks (after unmounting the filesystem). > * Interop run against FreeBSD server/client implementation. > * Ran xfstests-dev, with no unexpected/new failures as compared > to an unpatched kernel. To fully use xfstests-dev, it needed > some modifications, as it expects to either use all xattr > namespaces, or none. Whereas NFS only suppors the "user." > namespace (+ optional ACLs). I will send the changes in > seperately. > > > Frank van der Linden (10): > xattr: break delegations in {set,remove}xattr > xattr: add a function to check if a namespace is supported > nfs,nfsd: NFSv4.2 extended attribute protocol definitions > nfsd: split off the write decode code in to a separate function > nfsd: add defines for NFSv4.2 extended attribute support > nfsd: define xattr functions to call in to their vfs counterparts > nfsd: take xattr bits in to account for permission checks > nfsd: add structure definitions for xattr requests / responses > nfsd: implement the xattr functions and en/decode logic > nfsd: add fattr support for user extended attributes > > fs/nfsd/nfs4proc.c | 128 ++++++++- > fs/nfsd/nfs4xdr.c | 531 +++++++++++++++++++++++++++++++++++--- > fs/nfsd/nfsd.h | 5 +- > fs/nfsd/vfs.c | 239 +++++++++++++++++ > fs/nfsd/vfs.h | 10 + > fs/nfsd/xdr4.h | 31 +++ > fs/xattr.c | 111 +++++++- > include/linux/nfs4.h | 22 +- > include/linux/xattr.h | 4 + > include/uapi/linux/nfs4.h | 3 + > 10 files changed, 1044 insertions(+), 40 deletions(-) > > -- > 2.17.2
On Thu, Jun 25, 2020 at 12:50:38PM -0400, J. Bruce Fields wrote: > > In general, these patches were, both server and client, tested as > > follows: > > * stress-ng-xattr with 1000 workers > > * Test all corner cases (XATTR_SIZE_*) > > * Test all failure cases (no xattr, setxattr with different or > > invalid flags, etc). > > * Verify the content of xattrs across several operations. > > Do you have some code to share for these tests? > > --b. Sure, my main piece of test code is attached. - Frank #define _GNU_SOURCE #include <sys/types.h> #include <sys/stat.h> #include <sys/xattr.h> #include <stdio.h> #include <stdlib.h> #include <malloc.h> #include <unistd.h> #include <fcntl.h> #include <errno.h> #include <string.h> #include <time.h> #include <stdarg.h> #include <limits.h> #ifndef XATTR_SIZE_MAX #define XATTR_SIZE_MAX 65536 #endif #ifndef XATTR_LIST_MAX #define XATR_LIST_MAX 65536 #endif #define XATTR_TEST_ATTRLEN (5 + 4 + 1) /* "user.NNNN\0" */ #define XATTR_TEST_NATTR_MAX (XATTR_LIST_MAX / XATTR_TEST_ATTRLEN) static const char *xt_dir; typedef struct xattr_test_handle { const char *xt_test; char xt_filename[256]; char xt_attrkey[2 * 256]; /* Long to test limit */ char *xt_attrbuf; ssize_t xt_attrlen; char xt_fillc; ssize_t xt_failoff; int xt_fd; char *xt_listbuf; ssize_t xt_listlen; } xt_hdl; #define xt_value_len(xth) ((xth)->xt_attrlen) #define xt_value_failoff(xth) ((xth)->xt_failoff) #define xt_fillc(xth) ((xth)->xt_fillc) #define xt_filename(xth) ((xth)->xt_filename) #define PAGE_SIZE sysconf(_SC_PAGE_SIZE) static void usage(void) { fprintf(stderr, "usage: xattr <dir>\n"); exit(1); } static void fail(xt_hdl *xth, const char *fmt, ...) { va_list ap; printf("%-24s FAIL: ", xth->xt_test); va_start(ap, fmt); vprintf(fmt, ap); va_end(ap); printf("\n"); } static void pass(xt_hdl *xth, const char *fmt, ...) { va_list ap; printf("%-24s PASS: ", xth->xt_test); va_start(ap, fmt); vprintf(fmt, ap); va_end(ap); printf("\n"); } static int xt_set_fs(const char *dir) { int ret; struct stat st; ret = stat(dir, &st); if (ret < 0) return ret; if (!S_ISDIR(st.st_mode)) return -ENOTDIR; xt_dir = dir; return 0; } static xt_hdl * xt_init(const char *test) { xt_hdl *xth; if (xt_dir == NULL) { errno = EINVAL; return NULL; } xth = calloc(1, sizeof (xt_hdl)); if (xth == NULL) return NULL; xth->xt_test = test; snprintf(xth->xt_filename, sizeof xth->xt_filename, "%s/%sXXXXXX", xt_dir, test); /* * Default value, can be changed. */ snprintf(xth->xt_attrkey, sizeof xth->xt_attrkey, "user.%s", test); return xth; } static int xt_file_create(xt_hdl *xth) { int fd; int ret; fd = mkostemp(xth->xt_filename, O_TRUNC); if (fd < 0) return -errno; close(fd); return 0; } static void xt_file_remove(xt_hdl *xth) { unlink(xth->xt_filename); } static int xt_file_open(xt_hdl *xth, int flags) { int fd; fd = open(xth->xt_filename, flags); if (fd < 0) return errno; xth->xt_fd = fd; return fd; } static int xt_file_close(xt_hdl *xth) { if (xth->xt_fd != -1) { close(xth->xt_fd); xth->xt_fd = -1; } } static void xt_value_fill(xt_hdl *xth) { char c; char *p; ssize_t i; if (xth->xt_attrlen == 0) return; p = xth->xt_attrbuf; c = random() & 0xff; if (c == 0) c++; for (i = 0; i < xth->xt_attrlen; i++) p[i] = c; xth->xt_fillc = c; } static void xt_value_free(xt_hdl *xth) { if (xth->xt_attrbuf != NULL) { free(xth->xt_attrbuf); xth->xt_attrbuf = NULL; xth->xt_attrlen = 0; } } static int xt_value_alloc(xt_hdl *xth, ssize_t len) { char *p; xt_value_free(xth); if (len == 0) return 0; p = malloc(len); if (p == NULL) return ENOMEM; xth->xt_attrbuf = p; xth->xt_attrlen = len; xth->xt_failoff = 0; xth->xt_fd = -1; xt_value_fill(xth); return 0; } static int __xt_value_check(xt_hdl *xth, ssize_t len, char c) { ssize_t i; for (i = 0; i < len; i++) { if (xth->xt_attrbuf[i] != c) { xth->xt_failoff = i; return -EINVAL; } } return 0; } static int xt_value_check(xt_hdl *xth, ssize_t len) { return __xt_value_check(xth, len, xth->xt_fillc); } static int xt_list_alloc(xt_hdl *xth, ssize_t size) { char *p; p = calloc(1, size); if (p == NULL) return -ENOMEM; xth->xt_listbuf = p; xth->xt_listlen = size; return 0; } static void xt_value_clear(xt_hdl *xth) { memset(xth->xt_attrbuf, 0, xth->xt_attrlen); } static void xt_list_free(xt_hdl *xth) { if (xth->xt_attrbuf != NULL) { free(xth->xt_listbuf); xth->xt_listbuf = NULL; xth->xt_listlen = 0; } } static void xt_free(xt_hdl *xth) { xt_value_free(xth); xt_list_free(xth); free(xth); } static int xt_set_attr_name(xt_hdl *xth, const char *name) { if (strlen(name) > (sizeof xth->xt_attrkey) - 1) return -EINVAL; strcpy(xth->xt_attrkey, name); return 0; } static int xt_getxattr(xt_hdl *xth) { ssize_t ret; ret = getxattr(xth->xt_filename, xth->xt_attrkey, xth->xt_attrbuf, xth->xt_attrlen); return ret < 0 ? -errno : ret; } static int xt_fgetxattr(xt_hdl *xth) { ssize_t ret; ret = fgetxattr(xth->xt_fd, xth->xt_attrkey, xth->xt_attrbuf, xth->xt_attrlen); return ret < 0 ? -errno : ret; } static int xt_setxattr(xt_hdl *xth, int flags) { int ret; ret = setxattr(xth->xt_filename, xth->xt_attrkey, xth->xt_attrbuf, xth->xt_attrlen, flags); return ret < 0 ? -errno : 0; } static int xt_fsetxattr(xt_hdl *xth, int flags) { int ret; ret = fsetxattr(xth->xt_fd, xth->xt_attrkey, xth->xt_attrbuf, xth->xt_attrlen, flags); return ret < 0 ? -errno : 0; } static int xt_removexattr(xt_hdl *xth) { int ret; ret = removexattr(xth->xt_filename, xth->xt_attrkey); return ret < 0 ? -errno : 0; } static int xt_fremovexattr(xt_hdl *xth) { int ret; ret = fremovexattr(xth->xt_fd, xth->xt_attrkey); return ret < 0 ? -errno : 0; } static int cmpstr(const void *p1, const void *p2) { char *s1, *s2; s1 = *(char * const *) p1; s2 = *(char * const *) p2; if (s1 == s2) return 0; if (s1 == NULL) return 1; if (s2 == NULL) return -1; return strcmp(s1, s2); } static void xattrs_sort(char **list, ssize_t count) { qsort(list, count, sizeof (char *), cmpstr); } static int xattrs_alloc(char ***listp, ssize_t count, ssize_t *lenp) { ssize_t i, n, len, j; size_t slen; char **list; n = count; len = 0; #ifdef LIST_MAX_CHECK if (n > (XATTR_LIST_MAX / 6)) return -EINVAL; #endif list = calloc(n, sizeof (char *)); if (list == NULL) return -ENOMEM; for (i = 0; i < n; i++) { #ifdef LIST_MAX_CHECK if ((len + 10) > XATTR_LIST_MAX) break; #endif len += 10; if (asprintf(&list[i], "user.%04d", (int)i) < 0) break; } if (i < n) { for (j = 0; j < i; j++) { free(list[j]); } free(list); return -ENOMEM; } if (lenp) *lenp = len; *listp = list; return 0; } static int xattrs_free(char **list, ssize_t count) { ssize_t i; for (i = 0; i < count; i++) { if (list[i] != NULL) free(list[i]); } free(list); } static int xattrs_buf2list(char *buf, ssize_t buflen, char ***listp, ssize_t *countp) { ssize_t slen, count, len, i; char *p; char **list; p = buf; len = buflen; count = 0; while (len > 0) { slen = strlen(p) + 1; if (!strncmp(p, "user.", 5)) count++; p += slen; len -= slen; } if (count == 0) { list = NULL; goto out; } p = buf; len = buflen; list = calloc(count, sizeof (char *)); if (list == NULL) return -ENOMEM; i = 0; while (len > 0) { slen = strlen(p) + 1; if (!strncmp(p, "user.", 5)) list[i++] = strdup(p); p += slen; len -= slen; } xattrs_sort(list, count); out: *listp = list; *countp = count; return 0; } static int xattrs_file2list(const char *filename, char ***listp, ssize_t *countp) { ssize_t len, ret; int error; char *buf; len = listxattr(filename, NULL, 0); if (len < 0) return -errno; buf = malloc(len); if (buf == NULL) return -ENOMEM; ret = listxattr(filename, buf, len); if (ret < 0) { error = errno; *listp = NULL; free(buf); return -error; } error = xattrs_buf2list(buf, ret, listp, countp); free(buf); return error; } /* * Compare two string arrays that may have NULL entries. Counts are * the number of entries that include possible NULL entries. */ static int xattrs_cmplists(char **list1, ssize_t count1, char **list2, ssize_t count2) { ssize_t n1, n2; for (n1 = n2 = 0; n1 < count1 && n2 < count2;) { while (n1 < count1 && list1[n1] == NULL) n1++; while (n2 < count2 && list2[n2] == NULL) n2++; if (n1 >= count1) { if (n2 >= count2) return 0; return -1; } if (n2 >= count2) { if (n1 >= count1) return 0; return -1; } if (strcmp(list1[n1++], list2[n2++])) return -1; } return 0; } static int xattrs_listset(const char *filename, char **list, ssize_t len) { ssize_t i; int ret; for (i = 0; i < len; i++) { if (list[i] == NULL) continue; ret = setxattr(filename, list[i], NULL, 0, 0); if (ret < 0) { ret = -errno; fprintf(stderr, "%s: failed at %d (%s)\n", __func__, i, strerror(ret)); return ret; } } return 0; } static int xattrs_listrm(const char *filename, char **list, ssize_t len) { ssize_t i; int ret; for (i = 0; i < len; i++) { if (list[i] == NULL) continue; ret = removexattr(filename, list[i]); if (ret < 0) return -errno; } return 0; } static void __test_xattr_len(xt_hdl *xth, ssize_t len) { int ret; ssize_t xlen; ret = xt_file_create(xth); if (ret < 0) { fail(xth, "file create errno %d (%s)", errno, strerror(errno)); return; } ret = xt_value_alloc(xth, len); if (ret < 0) { fail(xth, "value alloc errno %d (%s)", errno, strerror(errno)); goto cleanup; } ret = xt_setxattr(xth, 0); if (ret < 0) { fail(xth, "setxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } xt_value_clear(xth); xlen = xt_getxattr(xth); if (xlen < 0) { fail(xth, "getxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } if (xlen != xt_value_len(xth)) { fail(xth, "getxattr length mismatch (expected %lld got %lld)", (long long)xt_value_len(xth), (long long)xlen); goto cleanup; } ret = xt_value_check(xth, len); if (ret < 0) { fail(xth, "value check failed at offset %lld", xt_value_failoff(xth)); goto cleanup; } pass(xth, "success setting / getting %lld length xattr", (long long)len); cleanup: xt_file_remove(xth); } void test_xattr_probe() { int ret; xt_hdl *xth; ssize_t xlen; char c; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } ret = xt_file_create(xth); if (ret < 0) { fail(xth, "file create errno %d (%s)", errno, strerror(errno)); return; } ret = xt_value_alloc(xth, 37); if (ret < 0) { fail(xth, "value alloc failure"); goto cleanup; } c = xt_fillc(xth); ret = xt_setxattr(xth, 0); if (ret < 0) { fail(xth, "setxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } ret = xt_value_alloc(xth, 0); if (ret < 0) { fail(xth, "value reset failure"); goto cleanup; } xlen = xt_getxattr(xth); if (xlen < 0) { fail(xth, "getxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } if (xlen != 37) { fail(xth, "getxattr length mismatch (expected 37, got %lld)", (long long)xlen); goto cleanup; } ret = xt_value_alloc(xth, 37); if (ret < 0) { fail(xth, "value realloc failure"); goto cleanup; } xlen = xt_getxattr(xth); if (xlen < 0) { fail(xth, "getxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } if (__xt_value_check(xth, 37, c) < 0) { fail(xth, "value check failed at offset %lld", xt_value_failoff(xth)); goto cleanup; } pass(xth, "getxattr length probe works correctly"); cleanup: xt_file_remove(xth); xt_free(xth); } void test_xattr_0() { int ret; xt_hdl *xth; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } __test_xattr_len(xth, 0); xt_free(xth); } void test_xattr_1() { xt_hdl *xth; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } __test_xattr_len(xth, 1); xt_free(xth); } void test_xattr_256() { xt_hdl *xth; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } __test_xattr_len(xth, 256); xt_free(xth); } void test_xattr_1page() { xt_hdl *xth; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } __test_xattr_len(xth, PAGE_SIZE); xt_free(xth); } void test_xattr_2pages() { xt_hdl *xth; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } __test_xattr_len(xth, 2 * PAGE_SIZE); xt_free(xth); } void test_xattr_max() { xt_hdl *xth; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } __test_xattr_len(xth, 65536); xt_free(xth); } void test_xattr_toolarge() { xt_hdl *xth; ssize_t ret; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } ret = xt_file_create(xth); if (ret < 0) { fail(xth, "file create errno %d (%s)", errno, strerror(errno)); return; } ret = xt_value_alloc(xth, XATTR_SIZE_MAX + 1); if (ret < 0) { fail(xth, "value alloc errno %d (%s)", errno, strerror(errno)); goto cleanup; } ret = xt_setxattr(xth, 0); if (ret != -E2BIG) { fail(xth, "expected %d got %d (%s)", E2BIG, -ret, ret == 0 ? "no error" : strerror(-ret)); goto cleanup; } pass(xth, "setxattr beyond max length fails as expected"); cleanup: xt_file_remove(xth); xt_free(xth); } void test_setxattr_create() { xt_hdl *xth; int ret; ssize_t xlen; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } ret = xt_file_create(xth); if (ret < 0) { fail(xth, "file create errno %d (%s)", errno, strerror(errno)); return; } ret = xt_value_alloc(xth, 16); if (ret < 0) { fail(xth, "value alloc errno %d (%s)", errno, strerror(errno)); goto cleanup; } /* * XATTR_CREATE for a non-existing xattr ==> OK */ ret = xt_setxattr(xth, XATTR_CREATE); if (ret < 0) { fail(xth, "setxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } /* * XATTR_CREATE for an existing xattr ==> EEXIST */ ret = xt_setxattr(xth, XATTR_CREATE); if (ret != -EEXIST) { fail(xth, "expected %d got %d (%s)", EEXIST, -ret, ret == 0 ? "no error" : strerror(-ret)); goto cleanup; } /* * Verify value (xattr should still exist). */ xt_value_clear(xth); xlen = xt_getxattr(xth); if (xlen < 0) { fail(xth, "getxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } if (xlen != xt_value_len(xth)) { fail(xth, "getxattr length mismatch (expected %lld got %lld)", (long long)xt_value_len(xth), (long long)xlen); goto cleanup; } ret = xt_value_check(xth, 16); if (ret < 0) { fail(xth, "value check failed at offset %lld", xt_value_failoff(xth)); goto cleanup; } pass(xth, "setxattr with XATTR_CREATE behaves correctly"); cleanup: xt_file_remove(xth); xt_free(xth); } void test_setxattr_replace() { xt_hdl *xth; int ret; ssize_t xlen; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } ret = xt_file_create(xth); if (ret < 0) { fail(xth, "file create errno %d (%s)", errno, strerror(errno)); return; } ret = xt_value_alloc(xth, 16); if (ret < 0) { fail(xth, "value alloc errno %d (%s)", errno, strerror(errno)); goto cleanup; } /* * XATTR_REPLACE for a non-existing xattr ==> ENODATA */ ret = xt_setxattr(xth, XATTR_REPLACE); if (ret != -ENODATA) { fail(xth, "expected %d got %d (%s)", ENODATA, -ret, ret == 0 ? "no error" : strerror(-ret)); goto cleanup; } /* * Create it, try to replace it afterwards. */ ret = xt_setxattr(xth, 0); if (ret < 0) { fail(xth, "setxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } xt_value_fill(xth); /* * XATTR_REPLACE should work. */ ret = xt_setxattr(xth, XATTR_REPLACE); if (ret < 0) { fail(xth, "setxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } /* * Verify value (xattr should still exist). */ xt_value_clear(xth); xlen = xt_getxattr(xth); if (xlen < 0) { fail(xth, "getxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } if (xlen != xt_value_len(xth)) { fail(xth, "getxattr length mismatch (expected %lld got %lld)", (long long)xt_value_len(xth), (long long)xlen); goto cleanup; } ret = xt_value_check(xth, 16); if (ret < 0) { fail(xth, "value check failed at offset %lld", xt_value_failoff(xth)); goto cleanup; } pass(xth, "setxattr with XATTR_REPLACE behaves correctly"); cleanup: xt_file_remove(xth); xt_free(xth); } void test_setxattr_badflag() { xt_hdl *xth; int ret; ssize_t xlen; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } ret = xt_file_create(xth); if (ret < 0) { fail(xth, "file create errno %d (%s)", errno, strerror(errno)); return; } ret = xt_value_alloc(xth, 16); if (ret < 0) { fail(xth, "value alloc errno %d (%s)", errno, strerror(errno)); goto cleanup; } /* * XATTR_REPLACE for a non-existing xattr ==> ENODATA */ ret = xt_setxattr(xth, XATTR_REPLACE); if (ret != -ENODATA) { fail(xth, "expected %d got %d (%s)", ENODATA, -ret, ret == 0 ? "no error" : strerror(-ret)); goto cleanup; } ret = xt_setxattr(xth, -1); if (ret != -EINVAL) { fail(xth, "setxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } pass(xth, "setxattr with a bad flag behaves correctly"); cleanup: xt_file_remove(xth); xt_free(xth); } void test_removexattr() { xt_hdl *xth; int ret; ssize_t xlen; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } ret = xt_file_create(xth); if (ret < 0) { fail(xth, "file create errno %d (%s)", errno, strerror(errno)); return; } ret = xt_value_alloc(xth, 16); if (ret < 0) { fail(xth, "value alloc errno %d (%s)", errno, strerror(errno)); goto cleanup; } ret = xt_removexattr(xth); if (ret != -ENODATA) { fail(xth, "expected %d got %d (%s)", ENODATA, -ret, ret == 0 ? "no error" : strerror(-ret)); goto cleanup; } /* * Create it, try to remove it afterwards. */ ret = xt_setxattr(xth, 0); if (ret < 0) { fail(xth, "setxattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } ret = xt_removexattr(xth); if (ret < 0) { fail(xth, "removexattr errno %d (%s)", errno, strerror(errno)); goto cleanup; } /* * See if it's actually gone. */ xlen = xt_getxattr(xth); if (xlen != -ENODATA) { fail(xth, "expected %d got %d (%s)", ENODATA, -xlen, xlen == 0 ? "no error" : strerror(xlen)); goto cleanup; } pass(xth, "removexattr behaves correctly"); cleanup: xt_file_remove(xth); xt_free(xth); } void __test_listxattr(xt_hdl *xth, ssize_t nattr) { ssize_t len, alen; char **list1, **list2; int ret; list1 = list2 = NULL; ret = xattrs_alloc(&list1, nattr, &alen); if (ret < 0) { fail(xth, "failed to allocate xattr names"); return; } ret = xattrs_listset(xt_filename(xth), list1, nattr); if (ret < 0) { fail(xth, "failed to set xattrs"); goto out; } ret = xattrs_file2list(xt_filename(xth), &list2, &len); if (ret < 0) { if (alen > XATTR_SIZE_MAX && ret == -E2BIG) { /* * Expected failure. */ goto rm; } fail(xth, "failed to get xattrs"); goto out; } if (xattrs_cmplists(list1, nattr, list2, len)) { ret = -EINVAL; /* XXX */ fail(xth, "list comparison failed"); goto out; } rm: ret = xattrs_listrm(xt_filename(xth), list1, nattr); if (ret < 0) { fail(xth, "failed to remove xattrs"); goto out; } pass(xth, "expected results listing and removing %lld xattrs", (long long)nattr); out: if (list2 != NULL) xattrs_free(list2, len); if (list1 != NULL) xattrs_free(list1, nattr); } void test_listxattr_1() { xt_hdl *xth; int ret; char **list1, **list2; ssize_t len; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } ret = xt_file_create(xth); if (ret < 0) { fail(xth, "file create errno %d (%s)", errno, strerror(errno)); return; } __test_listxattr(xth, 1); xt_file_remove(xth); xt_free(xth); } void test_listxattr_256() { xt_hdl *xth; int ret; char **list1, **list2; ssize_t len; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } ret = xt_file_create(xth); if (ret < 0) { fail(xth, "file create errno %d (%s)", errno, strerror(errno)); return; } __test_listxattr(xth, 256); xt_file_remove(xth); xt_free(xth); } void test_listxattr_large() { xt_hdl *xth; int ret; char **list1, **list2; ssize_t len; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } ret = xt_file_create(xth); if (ret < 0) { fail(xth, "file create errno %d (%s)", errno, strerror(errno)); return; } __test_listxattr(xth, XATTR_TEST_NATTR_MAX - 2); xt_file_remove(xth); xt_free(xth); } void test_listxattr_2big() { xt_hdl *xth; int ret; char **list1, **list2; ssize_t len; xth = xt_init(__func__); if (xth == NULL) { printf("%-24s FAIL: can't initialize handle", __func__); return; } ret = xt_file_create(xth); if (ret < 0) { fail(xth, "file create errno %d (%s)", errno, strerror(errno)); return; } __test_listxattr(xth, XATTR_TEST_NATTR_MAX + 1); xt_file_remove(xth); xt_free(xth); } int main(int argc, char **argv) { struct stat st; int fd, ret; if (argc < 2) usage(); ret = xt_set_fs(argv[1]); if (ret != 0) { fprintf(stderr, "xattr: set fs: %s\n", strerror(ret)); exit(1); } srandom(time(NULL)); test_xattr_probe(); test_xattr_0(); test_xattr_1(); test_xattr_256(); test_xattr_1page(); test_xattr_2pages(); test_xattr_max(); test_xattr_toolarge(); test_setxattr_create(); test_setxattr_replace(); test_setxattr_badflag(); test_removexattr(); test_listxattr_1(); test_listxattr_256(); test_listxattr_large(); test_listxattr_2big(); return 0; }
On Thu, Jun 25, 2020 at 12:53:47PM -0400, J. Bruce Fields wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. > > > > By the way, I can't remember if I asked this before: is there a > particular use case that motivates this xattr work? > > --b. There's one use case that I can't really talk about publicly at this point (and it's not my code either, so I wouldn't have all the details). Nothing super secret or anything - it's just something that is not mine, so I won't try to speak for anyone. We wanted to get this upstreamed first, as that's the right thing to do. Since I posted my first RFC, I did get contacted off-list by several readers of linux-nfs who wanted to use the feature in practice, too, so there's definitely interest out there. - Frank
On Thu, Jun 25, 2020 at 05:39:16PM +0000, Frank van der Linden wrote: > On Thu, Jun 25, 2020 at 12:53:47PM -0400, J. Bruce Fields wrote: > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. > > > > By the way, I can't remember if I asked this before: is there a > > particular use case that motivates this xattr work? > > There's one use case that I can't really talk about publicly at this point > (and it's not my code either, so I wouldn't have all the details). Nothing > super secret or anything - it's just something that is not mine, > so I won't try to speak for anyone. We wanted to get this upstreamed first, > as that's the right thing to do. > > Since I posted my first RFC, I did get contacted off-list by several > readers of linux-nfs who wanted to use the feature in practice, too, so > there's definitely interest out there. Yeah, I always hear a lot of interest but then have trouble sorting through it for the cases that are actually *user* xattr cases, where the server has to just act as a dumb store of the values. There are some. But unfortunately xattrs are best known for enabling selinux and posix acls, and to a lesser extent accessing random other filesystem features, so that tends to be what comes to people's minds first, though it's not what we're doing. --b.
> On Jun 23, 2020, at 6:39 PM, Frank van der Linden <fllinden@amazon.com> wrote: > > v3: > * Rebase to v5.8-rc2 > * Use length probe + allocate + query for the listxattr and setxattr > operations to avoid allocating unneeded space. > * Because of the above, drop the 'use kvmalloc for svcxdr_tmpalloc' patch, > as it's no longer needed. v3 of this series has been applied to nfsd-5.9. Thanks! See: git://git.linux-nfs.org/projects/cel/cel-2.6.git nfsd-5.9 Still waiting for Acks on 01/13 and 02/13. > v2: > * As per the discussion, user extended attributes are enabled if > the client and server support them (e.g. they support 4.2 and > advertise the user extended attribute FATTR). There are no longer > options to switch them off. > * The code is no longer conditioned on a config option. > * The number of patches has been reduced somewhat by merging > smaller, related ones. > * Renamed some functions and added parameter comments as requested. > > v1: > > * Split in to client and server (changed from the original RFC patch). > > Original RFC combined set is here: > > https://www.spinics.net/lists/linux-nfs/msg74843.html > > In general, these patches were, both server and client, tested as > follows: > * stress-ng-xattr with 1000 workers > * Test all corner cases (XATTR_SIZE_*) > * Test all failure cases (no xattr, setxattr with different or > invalid flags, etc). > * Verify the content of xattrs across several operations. > * Use KASAN and KMEMLEAK for a longer mix of testruns to verify > that there were no leaks (after unmounting the filesystem). > * Interop run against FreeBSD server/client implementation. > * Ran xfstests-dev, with no unexpected/new failures as compared > to an unpatched kernel. To fully use xfstests-dev, it needed > some modifications, as it expects to either use all xattr > namespaces, or none. Whereas NFS only suppors the "user." > namespace (+ optional ACLs). I will send the changes in > seperately. > > > Frank van der Linden (10): > xattr: break delegations in {set,remove}xattr > xattr: add a function to check if a namespace is supported > nfs,nfsd: NFSv4.2 extended attribute protocol definitions > nfsd: split off the write decode code in to a separate function > nfsd: add defines for NFSv4.2 extended attribute support > nfsd: define xattr functions to call in to their vfs counterparts > nfsd: take xattr bits in to account for permission checks > nfsd: add structure definitions for xattr requests / responses > nfsd: implement the xattr functions and en/decode logic > nfsd: add fattr support for user extended attributes > > fs/nfsd/nfs4proc.c | 128 ++++++++- > fs/nfsd/nfs4xdr.c | 531 +++++++++++++++++++++++++++++++++++--- > fs/nfsd/nfsd.h | 5 +- > fs/nfsd/vfs.c | 239 +++++++++++++++++ > fs/nfsd/vfs.h | 10 + > fs/nfsd/xdr4.h | 31 +++ > fs/xattr.c | 111 +++++++- > include/linux/nfs4.h | 22 +- > include/linux/xattr.h | 4 + > include/uapi/linux/nfs4.h | 3 + > 10 files changed, 1044 insertions(+), 40 deletions(-) > > -- > 2.17.2 > -- Chuck Lever
v3: * Rebase to v5.8-rc2 * Use length probe + allocate + query for the listxattr and setxattr operations to avoid allocating unneeded space. * Because of the above, drop the 'use kvmalloc for svcxdr_tmpalloc' patch, as it's no longer needed. v2: * As per the discussion, user extended attributes are enabled if the client and server support them (e.g. they support 4.2 and advertise the user extended attribute FATTR). There are no longer options to switch them off. * The code is no longer conditioned on a config option. * The number of patches has been reduced somewhat by merging smaller, related ones. * Renamed some functions and added parameter comments as requested. v1: * Split in to client and server (changed from the original RFC patch). Original RFC combined set is here: https://www.spinics.net/lists/linux-nfs/msg74843.html In general, these patches were, both server and client, tested as follows: * stress-ng-xattr with 1000 workers * Test all corner cases (XATTR_SIZE_*) * Test all failure cases (no xattr, setxattr with different or invalid flags, etc). * Verify the content of xattrs across several operations. * Use KASAN and KMEMLEAK for a longer mix of testruns to verify that there were no leaks (after unmounting the filesystem). * Interop run against FreeBSD server/client implementation. * Ran xfstests-dev, with no unexpected/new failures as compared to an unpatched kernel. To fully use xfstests-dev, it needed some modifications, as it expects to either use all xattr namespaces, or none. Whereas NFS only suppors the "user." namespace (+ optional ACLs). I will send the changes in seperately. Frank van der Linden (10): xattr: break delegations in {set,remove}xattr xattr: add a function to check if a namespace is supported nfs,nfsd: NFSv4.2 extended attribute protocol definitions nfsd: split off the write decode code in to a separate function nfsd: add defines for NFSv4.2 extended attribute support nfsd: define xattr functions to call in to their vfs counterparts nfsd: take xattr bits in to account for permission checks nfsd: add structure definitions for xattr requests / responses nfsd: implement the xattr functions and en/decode logic nfsd: add fattr support for user extended attributes fs/nfsd/nfs4proc.c | 128 ++++++++- fs/nfsd/nfs4xdr.c | 531 +++++++++++++++++++++++++++++++++++--- fs/nfsd/nfsd.h | 5 +- fs/nfsd/vfs.c | 239 +++++++++++++++++ fs/nfsd/vfs.h | 10 + fs/nfsd/xdr4.h | 31 +++ fs/xattr.c | 111 +++++++- include/linux/nfs4.h | 22 +- include/linux/xattr.h | 4 + include/uapi/linux/nfs4.h | 3 + 10 files changed, 1044 insertions(+), 40 deletions(-)