From f93aefef197849ae75b36a32f72b0ef9c318d93a Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Tue, 13 Apr 2021 17:30:19 -0400 Subject: [PATCH] Cross-platform xattr user namespace compatibility ZFS on Linux originally implemented xattr namespaces in a way that is incompatible with other operating systems. On illumos, xattrs do not have namespaces. Every xattr name is visible. FreeBSD has two universally defined namespaces: EXTATTR_NAMESPACE_USER and EXTATTR_NAMESPACE_SYSTEM. The system namespace is used for protected FreeBSD-specific attributes such as MAC labels and pnfs state. These attributes have the namespace string "freebsd:system:" prefixed to the name in the encoding scheme used by ZFS. The user namespace is used for general purpose user attributes and obeys normal access control mechanisms. These attributes have no namespace string prefixed, so xattrs written on illumos are accessible in the user namespace on FreeBSD, and xattrs written to the user namespace on FreeBSD are accessible by the same name on illumos. Linux has several xattr namespaces. On Linux, ZFS encodes the namespace in the xattr name for every namespace, including the user namespace. As a consequence, an xattr in the user namespace with the name "foo" is stored by ZFS with the name "user.foo" and therefore appears on FreeBSD and illumos to have the name "user.foo" rather than "foo". Conversely, none of the xattrs written on FreeBSD or illumos are accessible on Linux unless the name happens to be prefixed with one of the Linux xattr namespaces, in which case the namespace is stripped from the name. This makes xattrs entirely incompatible between Linux and other platforms. We want to make the encoding of user namespace xattrs compatible across platforms. A critical requirement of this compatibility is for xattrs from existing pools from FreeBSD and illumos to be accessible by the same names in the user namespace on Linux. It is also necessary that existing pools with xattrs written by Linux retain access to those xattrs by the same names on Linux. Making user namespace xattrs from Linux accessible by the correct names on other platforms is important. The handling of other namespaces is not required to be consistent. Add a fallback mechanism for listing and getting xattrs to treat xattrs as being in the user namespace if they do not match a known prefix. Do not allow setting or getting xattrs with a name that is prefixed with one of the namespace names used by ZFS on supported platforms. Allow choosing between legacy illumos and FreeBSD compatability and legacy Linux compatibility with a new tunable. This facilitates replication and migration of pools between hosts with different compatibility needs. The tunable controls whether or not to prefix the namespace to the name. If the xattr is already present with the alternate prefix, remove it so only the new version persists. By default the platform's existing convention is used. Signed-off-by: Ryan Moeller --- include/sys/fs/zfs.h | 30 +- man/man4/zfs.4 | 24 ++ module/os/freebsd/zfs/zfs_vnops_os.c | 266 ++++++++++++------ module/os/linux/zfs/zpl_xattr.c | 162 ++++++++--- tests/runfiles/common.run | 2 +- tests/runfiles/sanity.run | 2 +- tests/zfs-tests/include/tunables.cfg | 1 + .../tests/functional/xattr/Makefile.am | 3 +- .../tests/functional/xattr/xattr_compat.ksh | 92 ++++++ 9 files changed, 456 insertions(+), 126 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/xattr/xattr_compat.ksh diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index e7fc2f459e18..6c0463e139da 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -1709,7 +1709,6 @@ typedef enum { #define ZFS_EV_HIST_DSID "history_dsid" #define ZFS_EV_RESILVER_TYPE "resilver_type" - /* * We currently support block sizes from 512 bytes to 16MB. * The benefits of larger blocks, and thus larger IO, need to be weighed @@ -1731,7 +1730,6 @@ typedef enum { #define SPA_OLD_MAXBLOCKSIZE (1ULL << SPA_OLD_MAXBLOCKSHIFT) #define SPA_MAXBLOCKSIZE (1ULL << SPA_MAXBLOCKSHIFT) - /* supported encryption algorithms */ enum zio_encrypt { ZIO_CRYPT_INHERIT = 0, @@ -1749,6 +1747,34 @@ enum zio_encrypt { #define ZIO_CRYPT_ON_VALUE ZIO_CRYPT_AES_256_GCM #define ZIO_CRYPT_DEFAULT ZIO_CRYPT_OFF +/* + * xattr namespace prefixes. These are forbidden in xattr names. + * + * For cross-platform compatibility, xattrs in the user namespace should not be + * prefixed with the namespace name, but for backwards compatibility with older + * ZFS on Linux versions we do prefix the namespace. + */ +#define ZFS_XA_NS_FREEBSD_PREFIX "freebsd:" +#define ZFS_XA_NS_FREEBSD_PREFIX_LEN strlen("freebsd:") +#define ZFS_XA_NS_LINUX_SECURITY_PREFIX "security." +#define ZFS_XA_NS_LINUX_SECURITY_PREFIX_LEN strlen("security.") +#define ZFS_XA_NS_LINUX_SYSTEM_PREFIX "system." +#define ZFS_XA_NS_LINUX_SYSTEM_PREFIX_LEN strlen("system.") +#define ZFS_XA_NS_LINUX_TRUSTED_PREFIX "trusted." +#define ZFS_XA_NS_LINUX_TRUSTED_PREFIX_LEN strlen("trusted.") +#define ZFS_XA_NS_LINUX_USER_PREFIX "user." +#define ZFS_XA_NS_LINUX_USER_PREFIX_LEN strlen("user.") + +#define ZFS_XA_NS_PREFIX_MATCH(ns, name) \ + (strncmp(name, ZFS_XA_NS_##ns##_PREFIX, \ + ZFS_XA_NS_##ns##_PREFIX_LEN) == 0) + +#define ZFS_XA_NS_PREFIX_FORBIDDEN(name) \ + (ZFS_XA_NS_PREFIX_MATCH(FREEBSD, name) || \ + ZFS_XA_NS_PREFIX_MATCH(LINUX_SECURITY, name) || \ + ZFS_XA_NS_PREFIX_MATCH(LINUX_SYSTEM, name) || \ + ZFS_XA_NS_PREFIX_MATCH(LINUX_TRUSTED, name) || \ + ZFS_XA_NS_PREFIX_MATCH(LINUX_USER, name)) #ifdef __cplusplus } diff --git a/man/man4/zfs.4 b/man/man4/zfs.4 index b2e850f840a1..bb3cd2243ad3 100644 --- a/man/man4/zfs.4 +++ b/man/man4/zfs.4 @@ -2129,6 +2129,30 @@ When enabled, the maximum number of pending allocations per top-level vdev is limited by .Sy zfs_vdev_queue_depth_pct . . +.It Sy zfs_xattr_compat Ns = Ns 0 Ns | Ns 1 Pq int +Control the naming scheme used when setting new xattrs in the user namespace. +If +.Sy 0 +.Pq the default on Linux , +user namespace xattr names are prefixed with the namespace, to be backwards +compatible with previous versions of ZFS on Linux. +If +.Sy 1 +.Pq the default on Fx , +user namespace xattr names are not prefixed, to be backwards compatible with +previous versions of ZFS on illumos and +.Fx . +.Pp +Either naming scheme can be read on this and future versions of ZFS, regardless +of this tunable, but legacy ZFS on illumos or +.Fx +are unable to read user namespace xattrs written in the Linux format, and +legacy versions of ZFS on Linux are unable to read user namespace xattrs written +in the legacy ZFS format. +.Pp +An existing xattr with the alternate naming scheme is removed when overwriting +the xattr so as to not accumulate duplicates. +. .It Sy zio_requeue_io_start_cut_in_line Ns = Ns Sy 0 Ns | Ns 1 Pq int Prioritize requeued I/O. . diff --git a/module/os/freebsd/zfs/zfs_vnops_os.c b/module/os/freebsd/zfs/zfs_vnops_os.c index e2f52029d9cd..dbf02bc6446f 100644 --- a/module/os/freebsd/zfs/zfs_vnops_os.c +++ b/module/os/freebsd/zfs/zfs_vnops_os.c @@ -5253,43 +5253,56 @@ zfs_freebsd_pathconf(struct vop_pathconf_args *ap) } } +static int zfs_xattr_compat = 1; + +static int +zfs_check_attrname(const char *name) +{ + /* We don't allow '/' character in attribute name. */ + if (strchr(name, '/') != NULL) + return (SET_ERROR(EINVAL)); + /* We don't allow attribute names that start with a namespace prefix. */ + if (ZFS_XA_NS_PREFIX_FORBIDDEN(name)) + return (SET_ERROR(EINVAL)); + return (0); +} + /* * FreeBSD's extended attributes namespace defines file name prefix for ZFS' * extended attribute name: * - * NAMESPACE PREFIX - * system freebsd:system: - * user (none, can be used to access ZFS fsattr(5) attributes - * created on Solaris) + * NAMESPACE XATTR_COMPAT PREFIX + * system * freebsd:system: + * user 1 (none, can be used to access ZFS + * fsattr(5) attributes created on Solaris) + * user 0 user. */ static int zfs_create_attrname(int attrnamespace, const char *name, char *attrname, - size_t size) + size_t size, boolean_t compat) { const char *namespace, *prefix, *suffix; - /* We don't allow '/' character in attribute name. */ - if (strchr(name, '/') != NULL) - return (SET_ERROR(EINVAL)); - /* We don't allow attribute names that start with "freebsd:" string. */ - if (strncmp(name, "freebsd:", 8) == 0) - return (SET_ERROR(EINVAL)); - bzero(attrname, size); switch (attrnamespace) { case EXTATTR_NAMESPACE_USER: -#if 0 - prefix = "freebsd:"; - namespace = EXTATTR_NAMESPACE_USER_STRING; - suffix = ":"; -#else - /* - * This is the default namespace by which we can access all - * attributes created on Solaris. - */ - prefix = namespace = suffix = ""; -#endif + if (compat) { + /* + * This is the default namespace by which we can access + * all attributes created on Solaris. + */ + prefix = namespace = suffix = ""; + } else { + /* + * This is compatible with the user namespace encoding + * on Linux prior to xattr_compat, but nothing + * else. + */ + prefix = ""; + namespace = "user"; + suffix = "."; + } break; case EXTATTR_NAMESPACE_SYSTEM: prefix = "freebsd:"; @@ -5411,6 +5424,27 @@ zfs_getextattr_sa(struct vop_getextattr_args *ap, const char *attrname) return (0); } +static int +zfs_getextattr_impl(struct vop_getextattr_args *ap, boolean_t compat) +{ + znode_t *zp = VTOZ(ap->a_vp); + zfsvfs_t *zfsvfs = ZTOZSB(zp); + char attrname[EXTATTR_MAXNAMELEN+1]; + int error; + + error = zfs_create_attrname(ap->a_attrnamespace, ap->a_name, attrname, + sizeof (attrname), compat); + if (error != 0) + return (error); + + error = ENOENT; + if (zfsvfs->z_use_sa && zp->z_is_sa) + error = zfs_getextattr_sa(ap, attrname); + if (error == ENOENT) + error = zfs_getextattr_dir(ap, attrname); + return (error); +} + /* * Vnode operation to retrieve a named extended attribute. */ @@ -5419,7 +5453,6 @@ zfs_getextattr(struct vop_getextattr_args *ap) { znode_t *zp = VTOZ(ap->a_vp); zfsvfs_t *zfsvfs = ZTOZSB(zp); - char attrname[EXTATTR_MAXNAMELEN+1]; int error; /* @@ -5433,8 +5466,7 @@ zfs_getextattr(struct vop_getextattr_args *ap) if (error != 0) return (SET_ERROR(error)); - error = zfs_create_attrname(ap->a_attrnamespace, ap->a_name, attrname, - sizeof (attrname)); + error = zfs_check_attrname(ap->a_name); if (error != 0) return (error); @@ -5442,10 +5474,17 @@ zfs_getextattr(struct vop_getextattr_args *ap) ZFS_ENTER(zfsvfs); ZFS_VERIFY_ZP(zp) rw_enter(&zp->z_xattr_lock, RW_READER); - if (zfsvfs->z_use_sa && zp->z_is_sa) - error = zfs_getextattr_sa(ap, attrname); - if (error == ENOENT) - error = zfs_getextattr_dir(ap, attrname); + + error = zfs_getextattr_impl(ap, zfs_xattr_compat); + if ((error == ENOENT || error == ENOATTR) && + ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) { + /* + * Fall back to the alternate namespace format if we failed to + * find a user xattr. + */ + error = zfs_getextattr_impl(ap, !zfs_xattr_compat); + } + rw_exit(&zp->z_xattr_lock); ZFS_EXIT(zfsvfs); if (error == ENOENT) @@ -5528,6 +5567,27 @@ zfs_deleteextattr_sa(struct vop_deleteextattr_args *ap, const char *attrname) return (error); } +static int +zfs_deleteextattr_impl(struct vop_deleteextattr_args *ap, boolean_t compat) +{ + znode_t *zp = VTOZ(ap->a_vp); + zfsvfs_t *zfsvfs = ZTOZSB(zp); + char attrname[EXTATTR_MAXNAMELEN+1]; + int error; + + error = zfs_create_attrname(ap->a_attrnamespace, ap->a_name, attrname, + sizeof (attrname), compat); + if (error != 0) + return (error); + + error = ENOENT; + if (zfsvfs->z_use_sa && zp->z_is_sa) + error = zfs_deleteextattr_sa(ap, attrname); + if (error == ENOENT) + error = zfs_deleteextattr_dir(ap, attrname); + return (error); +} + /* * Vnode operation to remove a named attribute. */ @@ -5536,7 +5596,6 @@ zfs_deleteextattr(struct vop_deleteextattr_args *ap) { znode_t *zp = VTOZ(ap->a_vp); zfsvfs_t *zfsvfs = ZTOZSB(zp); - char attrname[EXTATTR_MAXNAMELEN+1]; int error; /* @@ -5550,32 +5609,24 @@ zfs_deleteextattr(struct vop_deleteextattr_args *ap) if (error != 0) return (SET_ERROR(error)); - error = zfs_create_attrname(ap->a_attrnamespace, ap->a_name, attrname, - sizeof (attrname)); + error = zfs_check_attrname(ap->a_name); if (error != 0) return (error); - size_t size = 0; - struct vop_getextattr_args vga = { - .a_vp = ap->a_vp, - .a_size = &size, - .a_cred = ap->a_cred, - .a_td = ap->a_td, - }; - error = ENOENT; ZFS_ENTER(zfsvfs); ZFS_VERIFY_ZP(zp); rw_enter(&zp->z_xattr_lock, RW_WRITER); - if (zfsvfs->z_use_sa && zp->z_is_sa) { - error = zfs_getextattr_sa(&vga, attrname); - if (error == 0) - error = zfs_deleteextattr_sa(ap, attrname); - } - if (error == ENOENT) { - error = zfs_getextattr_dir(&vga, attrname); - if (error == 0) - error = zfs_deleteextattr_dir(ap, attrname); + + error = zfs_deleteextattr_impl(ap, zfs_xattr_compat); + if ((error == ENOENT || error == ENOATTR) && + ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) { + /* + * Fall back to the alternate namespace format if we failed to + * find a user xattr. + */ + error = zfs_deleteextattr_impl(ap, !zfs_xattr_compat); } + rw_exit(&zp->z_xattr_lock); ZFS_EXIT(zfsvfs); if (error == ENOENT) @@ -5675,60 +5726,87 @@ zfs_setextattr_sa(struct vop_setextattr_args *ap, const char *attrname) return (error); } -/* - * Vnode operation to set a named attribute. - */ static int -zfs_setextattr(struct vop_setextattr_args *ap) +zfs_setextattr_impl(struct vop_setextattr_args *ap, boolean_t compat) { znode_t *zp = VTOZ(ap->a_vp); zfsvfs_t *zfsvfs = ZTOZSB(zp); char attrname[EXTATTR_MAXNAMELEN+1]; int error; - /* - * If the xattr property is off, refuse the request. - */ - if (!(zfsvfs->z_flags & ZSB_XATTR)) - return (SET_ERROR(EOPNOTSUPP)); - - error = extattr_check_cred(ap->a_vp, ap->a_attrnamespace, - ap->a_cred, ap->a_td, VWRITE); - if (error != 0) - return (SET_ERROR(error)); - error = zfs_create_attrname(ap->a_attrnamespace, ap->a_name, attrname, - sizeof (attrname)); + sizeof (attrname), compat); if (error != 0) return (error); struct vop_deleteextattr_args vda = { .a_vp = ap->a_vp, + .a_attrnamespace = ap->a_attrnamespace, + .a_name = ap->a_name, .a_cred = ap->a_cred, .a_td = ap->a_td, }; error = ENOENT; - ZFS_ENTER(zfsvfs); - ZFS_VERIFY_ZP(zp); - rw_enter(&zp->z_xattr_lock, RW_WRITER); if (zfsvfs->z_use_sa && zp->z_is_sa && zfsvfs->z_xattr_sa) { error = zfs_setextattr_sa(ap, attrname); - if (error == 0) + if (error == 0) { /* * Successfully put into SA, we need to clear the one * in dir if present. */ zfs_deleteextattr_dir(&vda, attrname); + } } - if (error) { + if (error != 0) { error = zfs_setextattr_dir(ap, attrname); - if (error == 0 && zp->z_is_sa) + if (error == 0 && zp->z_is_sa) { /* * Successfully put into dir, we need to clear the one * in SA if present. */ zfs_deleteextattr_sa(&vda, attrname); + } } + if (error == 0 && ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) { + /* + * Also clear all versions of the alternate compat name. + */ + zfs_deleteextattr_impl(&vda, !compat); + } + return (error); +} + +/* + * Vnode operation to set a named attribute. + */ +static int +zfs_setextattr(struct vop_setextattr_args *ap) +{ + znode_t *zp = VTOZ(ap->a_vp); + zfsvfs_t *zfsvfs = ZTOZSB(zp); + int error; + + /* + * If the xattr property is off, refuse the request. + */ + if (!(zfsvfs->z_flags & ZSB_XATTR)) + return (SET_ERROR(EOPNOTSUPP)); + + error = extattr_check_cred(ap->a_vp, ap->a_attrnamespace, + ap->a_cred, ap->a_td, VWRITE); + if (error != 0) + return (SET_ERROR(error)); + + error = zfs_check_attrname(ap->a_name); + if (error != 0) + return (error); + + ZFS_ENTER(zfsvfs); + ZFS_VERIFY_ZP(zp); + rw_enter(&zp->z_xattr_lock, RW_WRITER); + + error = zfs_setextattr_impl(ap, zfs_xattr_compat); + rw_exit(&zp->z_xattr_lock); ZFS_EXIT(zfsvfs); return (error); @@ -5808,7 +5886,7 @@ zfs_listextattr_dir(struct vop_listextattr_args *ap, const char *attrprefix) if (dp->d_type != DT_REG && dp->d_type != DT_UNKNOWN) continue; else if (plen == 0 && - strncmp(dp->d_name, "freebsd:", 8) == 0) + ZFS_XA_NS_PREFIX_FORBIDDEN(dp->d_name)) continue; else if (strncmp(dp->d_name, attrprefix, plen) != 0) continue; @@ -5856,7 +5934,7 @@ zfs_listextattr_sa(struct vop_listextattr_args *ap, const char *attrprefix) ASSERT3U(nvpair_type(nvp), ==, DATA_TYPE_BYTE_ARRAY); const char *name = nvpair_name(nvp); - if (plen == 0 && strncmp(name, "freebsd:", 8) == 0) + if (plen == 0 && ZFS_XA_NS_PREFIX_FORBIDDEN(name)) continue; else if (strncmp(name, attrprefix, plen) != 0) continue; @@ -5883,6 +5961,26 @@ zfs_listextattr_sa(struct vop_listextattr_args *ap, const char *attrprefix) return (error); } +static int +zfs_listextattr_impl(struct vop_listextattr_args *ap, boolean_t compat) +{ + znode_t *zp = VTOZ(ap->a_vp); + zfsvfs_t *zfsvfs = ZTOZSB(zp); + char attrprefix[16]; + int error; + + error = zfs_create_attrname(ap->a_attrnamespace, "", attrprefix, + sizeof (attrprefix), compat); + if (error != 0) + return (error); + + if (zfsvfs->z_use_sa && zp->z_is_sa) + error = zfs_listextattr_sa(ap, attrprefix); + if (error == 0) + error = zfs_listextattr_dir(ap, attrprefix); + return (error); +} + /* * Vnode operation to retrieve extended attributes on a vnode. */ @@ -5891,7 +5989,6 @@ zfs_listextattr(struct vop_listextattr_args *ap) { znode_t *zp = VTOZ(ap->a_vp); zfsvfs_t *zfsvfs = ZTOZSB(zp); - char attrprefix[16]; int error; if (ap->a_size != NULL) @@ -5908,18 +6005,16 @@ zfs_listextattr(struct vop_listextattr_args *ap) if (error != 0) return (SET_ERROR(error)); - error = zfs_create_attrname(ap->a_attrnamespace, "", attrprefix, - sizeof (attrprefix)); - if (error != 0) - return (error); - ZFS_ENTER(zfsvfs); ZFS_VERIFY_ZP(zp); rw_enter(&zp->z_xattr_lock, RW_READER); - if (zfsvfs->z_use_sa && zp->z_is_sa) - error = zfs_listextattr_sa(ap, attrprefix); - if (error == 0) - error = zfs_listextattr_dir(ap, attrprefix); + + error = zfs_listextattr_impl(ap, zfs_xattr_compat); + if (error == 0 && ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) { + /* Also list user xattrs with the alternate format. */ + error = zfs_listextattr_impl(ap, !zfs_xattr_compat); + } + rw_exit(&zp->z_xattr_lock); ZFS_EXIT(zfsvfs); return (error); @@ -6257,3 +6352,6 @@ struct vop_vector zfs_shareops = { #endif }; VFS_VOP_VECTOR_REGISTER(zfs_shareops); + +ZFS_MODULE_PARAM(zfs, zfs_, xattr_compat, INT, ZMOD_RW, + "Use legacy ZFS xattr naming for writing new user namespace xattrs"); diff --git a/module/os/linux/zfs/zpl_xattr.c b/module/os/linux/zfs/zpl_xattr.c index a1921ed08863..f3e42144b20a 100644 --- a/module/os/linux/zfs/zpl_xattr.c +++ b/module/os/linux/zfs/zpl_xattr.c @@ -84,6 +84,12 @@ #include #include +enum xattr_permission { + XAPERM_DENY, + XAPERM_ALLOW, + XAPERM_COMPAT, +}; + typedef struct xattr_filldir { size_t size; size_t offset; @@ -91,33 +97,10 @@ typedef struct xattr_filldir { struct dentry *dentry; } xattr_filldir_t; -static const struct xattr_handler *zpl_xattr_handler(const char *); - -static int -zpl_xattr_permission(xattr_filldir_t *xf, const char *name, int name_len) -{ - static const struct xattr_handler *handler; - struct dentry *d = xf->dentry; +static enum xattr_permission zpl_xattr_permission(xattr_filldir_t *, + const char *, int); - handler = zpl_xattr_handler(name); - if (!handler) - return (0); - - if (handler->list) { -#if defined(HAVE_XATTR_LIST_SIMPLE) - if (!handler->list(d)) - return (0); -#elif defined(HAVE_XATTR_LIST_DENTRY) - if (!handler->list(d, NULL, 0, name, name_len, 0)) - return (0); -#elif defined(HAVE_XATTR_LIST_HANDLER) - if (!handler->list(handler, d, NULL, 0, name, name_len)) - return (0); -#endif - } - - return (1); -} +static int zfs_xattr_compat = 0; /* * Determine is a given xattr name should be visible and if so copy it @@ -126,10 +109,27 @@ zpl_xattr_permission(xattr_filldir_t *xf, const char *name, int name_len) static int zpl_xattr_filldir(xattr_filldir_t *xf, const char *name, int name_len) { + enum xattr_permission perm; + /* Check permissions using the per-namespace list xattr handler. */ - if (!zpl_xattr_permission(xf, name, name_len)) + perm = zpl_xattr_permission(xf, name, name_len); + if (perm == XAPERM_DENY) return (0); + /* Prefix the name with "user." if it does not have a namespace. */ + if (perm == XAPERM_COMPAT) { + if (xf->buf) { + if (xf->offset + XATTR_USER_PREFIX_LEN + 1 > xf->size) + return (-ERANGE); + + memcpy(xf->buf + xf->offset, XATTR_USER_PREFIX, + XATTR_USER_PREFIX_LEN); + xf->buf[xf->offset + XATTR_USER_PREFIX_LEN] = '\0'; + } + + xf->offset += XATTR_USER_PREFIX_LEN; + } + /* When xf->buf is NULL only calculate the required size. */ if (xf->buf) { if (xf->offset + name_len + 1 > xf->size) @@ -706,19 +706,29 @@ static int __zpl_xattr_user_get(struct inode *ip, const char *name, void *value, size_t size) { - char *xattr_name; int error; /* xattr_resolve_name will do this for us if this is defined */ #ifndef HAVE_XATTR_HANDLER_NAME if (strcmp(name, "") == 0) return (-EINVAL); #endif + if (ZFS_XA_NS_PREFIX_FORBIDDEN(name)) + return (-EINVAL); if (!(ITOZSB(ip)->z_flags & ZSB_XATTR)) return (-EOPNOTSUPP); - xattr_name = kmem_asprintf("%s%s", XATTR_USER_PREFIX, name); - error = zpl_xattr_get(ip, xattr_name, value, size); - kmem_strfree(xattr_name); + /* + * Try to look up the name without the namespace prefix first for + * compatibility with xattrs from other platforms. If that fails, + * try again with the namespace prefix. + */ + error = zpl_xattr_get(ip, name, value, size); + if (error == -ENODATA) { + char *xattr_name; + xattr_name = kmem_asprintf("%s%s", XATTR_USER_PREFIX, name); + error = zpl_xattr_get(ip, xattr_name, value, size); + kmem_strfree(xattr_name); + } return (error); } @@ -728,20 +738,59 @@ static int __zpl_xattr_user_set(struct inode *ip, const char *name, const void *value, size_t size, int flags) { - char *xattr_name; - int error; + int error = 0; /* xattr_resolve_name will do this for us if this is defined */ #ifndef HAVE_XATTR_HANDLER_NAME if (strcmp(name, "") == 0) return (-EINVAL); #endif + if (ZFS_XA_NS_PREFIX_FORBIDDEN(name)) + return (-EINVAL); if (!(ITOZSB(ip)->z_flags & ZSB_XATTR)) return (-EOPNOTSUPP); - xattr_name = kmem_asprintf("%s%s", XATTR_USER_PREFIX, name); - error = zpl_xattr_set(ip, xattr_name, value, size, flags); - kmem_strfree(xattr_name); - + /* + * Remove alternate compat version of the xattr so we only set the + * version specified by the zfs_xattr_compat tunable. + * + * The following flags must be handled correctly: + * + * XATTR_CREATE: fail if xattr already exists + * XATTR_REPLACE: fail if xattr does not exist + */ + char *prefixed_name = kmem_asprintf("%s%s", XATTR_USER_PREFIX, name); + const char *clear_name, *set_name; + if (zfs_xattr_compat) { + clear_name = prefixed_name; + set_name = name; + } else { + clear_name = name; + set_name = prefixed_name; + } + /* + * Clear the old value with the alternative name format, if it exists. + */ + error = zpl_xattr_set(ip, clear_name, NULL, 0, flags); + /* + * XATTR_CREATE was specified and we failed to clear the xattr + * because it already exists. Stop here. + */ + if (error == -EEXIST) + goto out; + /* + * If XATTR_REPLACE was specified and we succeeded to clear + * an xattr, we don't need to replace anything when setting + * the new value. If we failed with -ENODATA that's fine, + * there was nothing to be cleared and we can ignore the error. + */ + if (error == 0) + flags &= ~XATTR_REPLACE; + /* + * Set the new value with the configured name format. + */ + error = zpl_xattr_set(ip, set_name, value, size, flags); +out: + kmem_strfree(prefixed_name); return (error); } ZPL_XATTR_SET_WRAPPER(zpl_xattr_user_set); @@ -1411,6 +1460,42 @@ zpl_xattr_handler(const char *name) return (NULL); } +static enum xattr_permission +zpl_xattr_permission(xattr_filldir_t *xf, const char *name, int name_len) +{ + const struct xattr_handler *handler; + struct dentry *d __maybe_unused = xf->dentry; + enum xattr_permission perm = XAPERM_ALLOW; + + handler = zpl_xattr_handler(name); + if (handler == NULL) { + /* Do not expose FreeBSD system namespace xattrs. */ + if (ZFS_XA_NS_PREFIX_MATCH(FREEBSD, name)) + return (XAPERM_DENY); + /* + * Anything that doesn't match a known namespace gets put in the + * user namespace for compatibility with other platforms. + */ + perm = XAPERM_COMPAT; + handler = &zpl_xattr_user_handler; + } + + if (handler->list) { +#if defined(HAVE_XATTR_LIST_SIMPLE) + if (!handler->list(d)) + return (XAPERM_DENY); +#elif defined(HAVE_XATTR_LIST_DENTRY) + if (!handler->list(d, NULL, 0, name, name_len, 0)) + return (XAPERM_DENY); +#elif defined(HAVE_XATTR_LIST_HANDLER) + if (!handler->list(handler, d, NULL, 0, name, name_len)) + return (XAPERM_DENY); +#endif + } + + return (perm); +} + #if !defined(HAVE_POSIX_ACL_RELEASE) || defined(HAVE_POSIX_ACL_RELEASE_GPL_ONLY) struct acl_rel_struct { struct acl_rel_struct *next; @@ -1510,3 +1595,6 @@ zpl_posix_acl_release_impl(struct posix_acl *acl) NULL, TQ_SLEEP, ddi_get_lbolt() + ACL_REL_SCHED); } #endif + +ZFS_MODULE_PARAM(zfs, zfs_, xattr_compat, INT, ZMOD_RW, + "Use legacy ZFS xattr naming for writing new user namespace xattrs"); diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index a7c34d0eac33..0dfac459d8e3 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -925,7 +925,7 @@ tags = ['functional', 'write_dirs'] [tests/functional/xattr] tests = ['xattr_001_pos', 'xattr_002_neg', 'xattr_003_neg', 'xattr_004_pos', 'xattr_005_pos', 'xattr_006_pos', 'xattr_007_neg', - 'xattr_011_pos', 'xattr_012_pos', 'xattr_013_pos'] + 'xattr_011_pos', 'xattr_012_pos', 'xattr_013_pos', 'xattr_compat'] tags = ['functional', 'xattr'] [tests/functional/zvol/zvol_ENOSPC] diff --git a/tests/runfiles/sanity.run b/tests/runfiles/sanity.run index 8057d8148c93..383308bb59ff 100644 --- a/tests/runfiles/sanity.run +++ b/tests/runfiles/sanity.run @@ -602,7 +602,7 @@ tags = ['functional', 'vdev_zaps'] [tests/functional/xattr] tests = ['xattr_001_pos', 'xattr_002_neg', 'xattr_003_neg', 'xattr_004_pos', 'xattr_005_pos', 'xattr_006_pos', 'xattr_007_neg', - 'xattr_011_pos', 'xattr_013_pos'] + 'xattr_011_pos', 'xattr_013_pos', 'xattr_compat'] tags = ['functional', 'xattr'] [tests/functional/zvol/zvol_ENOSPC] diff --git a/tests/zfs-tests/include/tunables.cfg b/tests/zfs-tests/include/tunables.cfg index fff43e469165..eea2af2edcf0 100644 --- a/tests/zfs-tests/include/tunables.cfg +++ b/tests/zfs-tests/include/tunables.cfg @@ -87,6 +87,7 @@ VDEV_VALIDATE_SKIP vdev.validate_skip vdev_validate_skip VOL_INHIBIT_DEV UNSUPPORTED zvol_inhibit_dev VOL_MODE vol.mode zvol_volmode VOL_RECURSIVE vol.recursive UNSUPPORTED +XATTR_COMPAT xattr_compat zfs_xattr_compat ZEVENT_LEN_MAX zevent.len_max zfs_zevent_len_max ZEVENT_RETAIN_MAX zevent.retain_max zfs_zevent_retain_max ZIO_SLOW_IO_MS zio.slow_io_ms zio_slow_io_ms diff --git a/tests/zfs-tests/tests/functional/xattr/Makefile.am b/tests/zfs-tests/tests/functional/xattr/Makefile.am index 17001885f1ca..0cbd799aac85 100644 --- a/tests/zfs-tests/tests/functional/xattr/Makefile.am +++ b/tests/zfs-tests/tests/functional/xattr/Makefile.am @@ -14,7 +14,8 @@ dist_pkgdata_SCRIPTS = \ xattr_010_neg.ksh \ xattr_011_pos.ksh \ xattr_012_pos.ksh \ - xattr_013_pos.ksh + xattr_013_pos.ksh \ + xattr_compat.ksh dist_pkgdata_DATA = \ xattr_common.kshlib \ diff --git a/tests/zfs-tests/tests/functional/xattr/xattr_compat.ksh b/tests/zfs-tests/tests/functional/xattr/xattr_compat.ksh new file mode 100755 index 000000000000..7e29d3de16a9 --- /dev/null +++ b/tests/zfs-tests/tests/functional/xattr/xattr_compat.ksh @@ -0,0 +1,92 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or http://www.opensolaris.org/os/licensing. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# +# Copyright 2007 Sun Microsystems, Inc. All rights reserved. +# Use is subject to license terms. +# + +# +# Copyright 2022 iXsystems, Inc. +# + +. $STF_SUITE/include/libtest.shlib + +# +# DESCRIPTION: +# The zfs_xattr_compat tunable and fallback works as expected. +# +# STRATEGY: +# For both of xattr=sa and xattr=dir: +# 1. Create a filesystem with zfs_xattr_compat=0 +# 2. Create a file on the filesystem and add some xattrs to it +# 3. Change the zfs_xattr_compat tunable to 1 +# 4. Verify that the xattrs can still be accessed and modified +# 5. Change zfs_xattr_compat back to 0 again +# 6. Verify that the xattrs can still be accessed and modified +# + +function cleanup { + rm -f $TESTFILE $TMPFILE + zfs set xattr=sa $TESTPOOL/$TESTFS + set_tunable32 XATTR_COMPAT $OLD_XATTR_COMPAT +} + +log_assert "The zfs_xattr_compat tunable and fallback works as expected" +log_onexit cleanup + +TESTFILE=$TESTDIR/testfile.$$ +TMPFILE=$TEST_BASE_DIR/tmpfile.$$ +OLD_XATTR_COMPAT=$(get_tunable XATTR_COMPAT) + +for x in sa dir; do + log_must zfs set xattr=$x $TESTPOOL/$TESTFS + log_must set_tunable32 XATTR_COMPAT 0 + log_must touch $TESTFILE + log_must set_xattr testattr1 value1 $TESTFILE + log_must set_xattr testattr2 value2 $TESTFILE + log_must set_xattr testattr3 value3 $TESTFILE + log_must ls_xattr $TESTFILE + + log_must set_tunable32 XATTR_COMPAT 1 + log_must ls_xattr $TESTFILE + log_must eval "get_xattr testattr1 $TESTFILE > $TMPFILE" + log_must test $(<$TMPFILE) = value1 + log_must set_xattr testattr2 newvalue2 $TESTFILE + log_must rm_xattr testattr3 $TESTFILE + log_must set_xattr testattr4 value4 $TESTFILE + log_must ls_xattr $TESTFILE + + log_must set_tunable32 XATTR_COMPAT 0 + log_must ls_xattr $TESTFILE + log_must eval "get_xattr testattr1 $TESTFILE > $TMPFILE" + log_must test $(<$TMPFILE) = value1 + log_must eval "get_xattr testattr2 $TESTFILE > $TMPFILE" + log_must test $(<$TMPFILE) = newvalue2 + log_mustnot get_xattr testattr3 $TESTFILE + log_must set_xattr testattr3 value3 $TESTFILE + log_must eval "get_xattr testattr4 $TESTFILE > $TMPFILE" + log_must test $(<$TMPFILE) = value4 + log_must ls_xattr $TESTFILE + + log_must rm $TESTFILE +done + +log_pass "The zfs_xattr_compat tunable and fallback works as expected"