From 803f17da7c19a7e3475cdc14605e45e6b932f0e5 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. When setting user namespace xattrs, do not prefix the namespace to the name. If the xattr is already present with the namespace prefix, remove it so only the non-prefixed version persists. This ensures other platforms will be able to read the xattr with the correct name. 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. Make xattr namespace compatibility dependent on a new feature. New pools will use the compatible namespace encoding by default, and existing pools will continue using the old Linux-specific encoding until the feature is enabled. Allow choosing between cross-platform compatability and legacy Linux compatibility with a per-dataset property. This facilitates replication between hosts with different compatibility needs. Add xattr_fallback property to skip fallbacks. This property currently defaults to "on" so we do not miss xattrs in datasets from porrls for which the encoding cannot be known or may even be a mix of both. It can be turned "off" for a performance boost when it is known only the configured xattr_compat format is relevant. Future work will investigate how we can change the default to "off" and automatically force it to "on" when we cannot know whether skipping the fallbacks is safe. TODO: * New tests should be added. * Performance optimizations should be investigated. Signed-off-by: Ryan Moeller --- include/os/freebsd/zfs/sys/zfs_vfsops_os.h | 5 +- include/os/linux/zfs/sys/zfs_vfsops_os.h | 4 +- include/sys/fs/zfs.h | 38 ++- include/zfeature_common.h | 1 + man/man7/zfsprops.7 | 43 +++ module/os/freebsd/zfs/zfs_vfsops.c | 51 ++++ module/os/freebsd/zfs/zfs_vnops_os.c | 272 ++++++++++++------ module/os/linux/zfs/zfs_vfsops.c | 49 ++++ module/os/linux/zfs/zpl_xattr.c | 144 +++++++--- module/zcommon/zfeature_common.c | 13 + module/zcommon/zfs_prop.c | 13 + tests/zfs-tests/include/properties.shlib | 6 +- .../cli_root/zpool_get/zpool_get.cfg | 1 + 13 files changed, 516 insertions(+), 124 deletions(-) diff --git a/include/os/freebsd/zfs/sys/zfs_vfsops_os.h b/include/os/freebsd/zfs/sys/zfs_vfsops_os.h index ccbbf4f73224..12d9d06e8566 100644 --- a/include/os/freebsd/zfs/sys/zfs_vfsops_os.h +++ b/include/os/freebsd/zfs/sys/zfs_vfsops_os.h @@ -239,7 +239,10 @@ struct zfsvfs { RW_WRITE_HELD(&(zfsvfs)->z_teardown_inactive_lock) #endif -#define ZSB_XATTR 0x0001 /* Enable user xattrs */ +#define ZSB_XATTR 0x0001 /* Enable user xattrs */ +#define ZSB_XATTR_COMPAT 0x0002 /* Enable cross-platform user xattrs */ +#define ZSB_XATTR_FALLBACK 0x0004 /* Enable user xattr compat fallback */ + /* * Normal filesystems (those not under .zfs/snapshot) have a total * file ID size limited to 12 bytes (including the length field) due to diff --git a/include/os/linux/zfs/sys/zfs_vfsops_os.h b/include/os/linux/zfs/sys/zfs_vfsops_os.h index 8e03ae99a7fd..936abb57b2b8 100644 --- a/include/os/linux/zfs/sys/zfs_vfsops_os.h +++ b/include/os/linux/zfs/sys/zfs_vfsops_os.h @@ -170,7 +170,9 @@ struct zfsvfs { #define ZFS_TEARDOWN_HELD(zfsvfs) \ RRM_LOCK_HELD(&(zfsvfs)->z_teardown_lock) -#define ZSB_XATTR 0x0001 /* Enable user xattrs */ +#define ZSB_XATTR 0x0001 /* Enable user xattrs */ +#define ZSB_XATTR_COMPAT 0x0002 /* Enable cross-platform user xattrs */ +#define ZSB_XATTR_FALLBACK 0x0004 /* Enable user xattr compat fallback */ /* * Allow a maximum number of links. While ZFS does not internally limit diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index 5d43750594cd..696ce9d1cddd 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -186,6 +186,8 @@ typedef enum { ZFS_PROP_IVSET_GUID, /* not exposed to the user */ ZFS_PROP_REDACTED, ZFS_PROP_REDACT_SNAPS, + ZFS_PROP_XATTR_COMPAT, + ZFS_PROP_XATTR_FALLBACK, ZFS_NUM_PROPS } zfs_prop_t; @@ -460,6 +462,11 @@ typedef enum zfs_key_location { #define DEFAULT_PBKDF2_ITERATIONS 350000 #define MIN_PBKDF2_ITERATIONS 100000 +typedef enum zfs_xattr_compat { + ZFS_XATTR_COMPAT_LINUX = 0, + ZFS_XATTR_COMPAT_ALL, +} zfs_xattr_compat_t; + /* * On-disk version number. */ @@ -1616,7 +1623,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 @@ -1638,7 +1644,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, @@ -1656,6 +1661,35 @@ 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.") + +/* BEGIN CSTYLED */ +#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)) +/* END CSTYLED */ #ifdef __cplusplus } diff --git a/include/zfeature_common.h b/include/zfeature_common.h index 874cbd9ff714..26f4fc52053e 100644 --- a/include/zfeature_common.h +++ b/include/zfeature_common.h @@ -75,6 +75,7 @@ typedef enum spa_feature { SPA_FEATURE_DEVICE_REBUILD, SPA_FEATURE_ZSTD_COMPRESS, SPA_FEATURE_DRAID, + SPA_FEATURE_XATTR_COMPAT, SPA_FEATURES } spa_feature_t; diff --git a/man/man7/zfsprops.7 b/man/man7/zfsprops.7 index 3f3ddcebf320..33d9007728e3 100644 --- a/man/man7/zfsprops.7 +++ b/man/man7/zfsprops.7 @@ -1863,6 +1863,49 @@ are equivalent to the and .Sy noxattr mount options. +.It Sy xattr_compat Ns = Ns Sy all Ns | Ns Sy linux +Controls the preferred encoding of xattrs in the user namespace. +When set to +.Sy all +(the default) with +.Sy feature Ns @ Ns Ar xattr_compat +enabled on the pool, xattrs written in the user namespace are stored in a +format compatible across all supported platforms, and xattrs in the user +namespace from all platforms are accessible. +There is no notion of xattr namespaces on illumos, so all xattrs from +illumos are presented in the user namespace on other platforms. +The xattrs not in the user namespace are considered platform-specific and are +not exposed on other platforms. +Existing xattrs in the +.Sy xattr_compat Ns = Ns Sy linux +format are accessible and are replaced with the cross-platform compatible +format when written. +When +.Sy feature Ns @ Ns Ar xattr_compat +is disabled, xattrs behave as with +.Sy xattr_compat Ns = Ns Sy linux +on Linux and as with +.Sy xattr_compat Ns = Ns Sy all +elsewhere. +When set to +.Sy linux , +xattrs written in the user namespace are stored in a format that is compatible +with ZFS on Linux prior to +.Sy feature Ns @ Ns Ar xattr_compat +but not compatible with ZFS on other platforms prior to this feature. +See +.Sy feature Ns @ Ns Ar xattr_compat +in +.Xr zpool-features 5 +for more information. +.It Sy xattr_fallback Ns = Ns Sy on Ns | Ns Sy off +Controls whether to fall back to the alternative encoding of xattrs in the +user namespace for lookups. +If accessing an xattr in the user namespace fails for the format configured by +.Sy xattr_compat , +when set to +.Sy on +(the default), a second attempt will be made using the other xattr name format. .It Sy jailed Ns = Ns Sy off Ns | Ns Sy on Controls whether the dataset is managed from a jail. See diff --git a/module/os/freebsd/zfs/zfs_vfsops.c b/module/os/freebsd/zfs/zfs_vfsops.c index 42e11eeb183d..da28b2d43a57 100644 --- a/module/os/freebsd/zfs/zfs_vfsops.c +++ b/module/os/freebsd/zfs/zfs_vfsops.c @@ -56,6 +56,7 @@ #include #include #include +#include #include #include #include @@ -493,6 +494,42 @@ xattr_changed_cb(void *arg, uint64_t newval) } } +static void +xattr_compat_changed_cb(void *arg, uint64_t newval) +{ + zfsvfs_t *zfsvfs = arg; + + /* + * Force the old cross-platform compatible behavior if + * feature@xattr_compat is disabled. This contrasts with + * Linux where the behavior prior to feature@xattr_compat + * was to use the incompatible Linux-only xattr format. + */ + if (!spa_feature_is_enabled(dmu_objset_spa(zfsvfs->z_os), + SPA_FEATURE_XATTR_COMPAT)) + newval = ZFS_XATTR_COMPAT_ALL; + + switch (newval) { + case ZFS_XATTR_COMPAT_ALL: + zfsvfs->z_flags |= ZSB_XATTR_COMPAT; + break; + case ZFS_XATTR_COMPAT_LINUX: + zfsvfs->z_flags &= ~ZSB_XATTR_COMPAT; + break; + } +} + +static void +xattr_fallback_changed_cb(void *arg, uint64_t newval) +{ + zfsvfs_t *zfsvfs = arg; + + if (newval) + zfsvfs->z_flags |= ZSB_XATTR_FALLBACK; + else + zfsvfs->z_flags &= ~ZSB_XATTR_FALLBACK; +} + static void blksz_changed_cb(void *arg, uint64_t newval) { @@ -728,6 +765,12 @@ zfs_register_callbacks(vfs_t *vfsp) zfs_prop_to_name(ZFS_PROP_ATIME), atime_changed_cb, zfsvfs); error = error ? error : dsl_prop_register(ds, zfs_prop_to_name(ZFS_PROP_XATTR), xattr_changed_cb, zfsvfs); + error = error ? error : dsl_prop_register(ds, + zfs_prop_to_name(ZFS_PROP_XATTR_COMPAT), xattr_compat_changed_cb, + zfsvfs); + error = error ? error : dsl_prop_register(ds, + zfs_prop_to_name(ZFS_PROP_XATTR_FALLBACK), + xattr_fallback_changed_cb, zfsvfs); error = error ? error : dsl_prop_register(ds, zfs_prop_to_name(ZFS_PROP_RECORDSIZE), blksz_changed_cb, zfsvfs); error = error ? error : dsl_prop_register(ds, @@ -1242,6 +1285,14 @@ zfs_domount(vfs_t *vfsp, char *osname) "xattr", &pval, NULL))) goto out; xattr_changed_cb(zfsvfs, pval); + if ((error = dsl_prop_get_integer(osname, + "xattr_compat", &pval, NULL))) + goto out; + xattr_compat_changed_cb(zfsvfs, pval); + if ((error = dsl_prop_get_integer(osname, + "xattr_fallback", &pval, NULL))) + goto out; + xattr_fallback_changed_cb(zfsvfs, pval); if ((error = dsl_prop_get_integer(osname, "acltype", &pval, NULL))) goto out; diff --git a/module/os/freebsd/zfs/zfs_vnops_os.c b/module/os/freebsd/zfs/zfs_vnops_os.c index 846b4b60531f..3f43608ce4d5 100644 --- a/module/os/freebsd/zfs/zfs_vnops_os.c +++ b/module/os/freebsd/zfs/zfs_vnops_os.c @@ -5233,43 +5233,54 @@ zfs_freebsd_pathconf(struct vop_pathconf_args *ap) } } +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 all (none, can be used to access ZFS + * fsattr(5) attributes created on Solaris) + * user linux user. */ static int zfs_create_attrname(int attrnamespace, const char *name, char *attrname, - size_t size) + size_t size, boolean_t xattr_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 (xattr_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 feature@xattr_compat, but nothing + * else. + */ + prefix = ""; + namespace = "user"; + suffix = "."; + } break; case EXTATTR_NAMESPACE_SYSTEM: prefix = "freebsd:"; @@ -5337,8 +5348,7 @@ zfs_getextattr_dir(struct vop_getextattr_args *ap, const char *attrname) return (error); flags = FREAD; - NDINIT_ATVP(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, attrname, - xvp, td); + NDINIT_ATVP(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, attrname, xvp, td); error = vn_open_cred(&nd, &flags, 0, VN_OPEN_INVFS, ap->a_cred, NULL); vp = nd.ni_vp; NDFREE(&nd, NDF_ONLY_PNBUF); @@ -5387,6 +5397,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. */ @@ -5395,7 +5426,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; /* @@ -5409,8 +5439,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); @@ -5418,10 +5447,21 @@ 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); + + boolean_t compat = (zfsvfs->z_flags & ZSB_XATTR_COMPAT) != 0; + boolean_t fallback = (zfsvfs->z_flags & ZSB_XATTR_FALLBACK) != 0; + error = zfs_getextattr_impl(ap, compat); + if (fallback && error == ENOENT && + 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, !compat); + } if (error == ENOENT) - error = zfs_getextattr_dir(ap, attrname); + error = SET_ERROR(ENOATTR); + rw_exit(&zp->z_xattr_lock); ZFS_EXIT(zfsvfs); if (error == ENOENT) @@ -5500,6 +5540,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. */ @@ -5508,7 +5569,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; /* @@ -5522,32 +5582,28 @@ 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); + + boolean_t compat = (zfsvfs->z_flags & ZSB_XATTR_COMPAT) != 0; + boolean_t fallback = (zfsvfs->z_flags & ZSB_XATTR_FALLBACK) != 0; + error = zfs_deleteextattr_impl(ap, compat); + if (fallback && error == ENOENT && + 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, !compat); } + if (error == ENOENT) + error = SET_ERROR(ENOATTR); + rw_exit(&zp->z_xattr_lock); ZFS_EXIT(zfsvfs); if (error == ENOENT) @@ -5643,30 +5699,16 @@ 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); @@ -5676,27 +5718,69 @@ zfs_setextattr(struct vop_setextattr_args *ap) .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) { error = zfs_setextattr_dir(ap, attrname); - if (error == 0) + if (error == 0) { /* * Successfully put into dir, we need to clear the one * in SA if present. */ zfs_deleteextattr_sa(&vda, attrname); + } + } + boolean_t fallback = (zfsvfs->z_flags & ZSB_XATTR_FALLBACK) != 0; + if (fallback && 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); + + boolean_t compat = (zfsvfs->z_flags & ZSB_XATTR_COMPAT) != 0; + error = zfs_setextattr_impl(ap, compat); + rw_exit(&zp->z_xattr_lock); ZFS_EXIT(zfsvfs); return (error); @@ -5771,7 +5855,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; @@ -5819,7 +5903,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; @@ -5846,6 +5930,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. */ @@ -5854,7 +5958,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) @@ -5871,18 +5974,19 @@ 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); + + boolean_t compat = (zfsvfs->z_flags & ZSB_XATTR_COMPAT) != 0; + boolean_t fallback = (zfsvfs->z_flags & ZSB_XATTR_FALLBACK) != 0; + error = zfs_listextattr_impl(ap, compat); + if (fallback && error == 0 && + ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) { + /* Also list user xattrs with the alternate format. */ + error = zfs_listextattr_impl(ap, !compat); + } + rw_exit(&zp->z_xattr_lock); ZFS_EXIT(zfsvfs); return (error); diff --git a/module/os/linux/zfs/zfs_vfsops.c b/module/os/linux/zfs/zfs_vfsops.c index ff0b0d9df8f0..d50c18dc2108 100644 --- a/module/os/linux/zfs/zfs_vfsops.c +++ b/module/os/linux/zfs/zfs_vfsops.c @@ -58,6 +58,7 @@ #include #include #include +#include #include #include #include "zfs_comutil.h" @@ -346,6 +347,40 @@ xattr_changed_cb(void *arg, uint64_t newval) } } +static void +xattr_compat_changed_cb(void *arg, uint64_t newval) +{ + zfsvfs_t *zfsvfs = arg; + + /* + * Force the old incompatible Linux behavior + * if feature@xattr_compat is disabled. + */ + if (!spa_feature_is_enabled(dmu_objset_spa(zfsvfs->z_os), + SPA_FEATURE_XATTR_COMPAT)) + newval = ZFS_XATTR_COMPAT_LINUX; + + switch (newval) { + case ZFS_XATTR_COMPAT_ALL: + zfsvfs->z_flags |= ZSB_XATTR_COMPAT; + break; + case ZFS_XATTR_COMPAT_LINUX: + zfsvfs->z_flags &= ~ZSB_XATTR_COMPAT; + break; + } +} + +static void +xattr_fallback_changed_cb(void *arg, uint64_t newval) +{ + zfsvfs_t *zfsvfs = arg; + + if (newval) + zfsvfs->z_flags |= ZSB_XATTR_FALLBACK; + else + zfsvfs->z_flags &= ~ZSB_XATTR_FALLBACK; +} + static void acltype_changed_cb(void *arg, uint64_t newval) { @@ -486,6 +521,12 @@ zfs_register_callbacks(vfs_t *vfsp) zfs_prop_to_name(ZFS_PROP_RELATIME), relatime_changed_cb, zfsvfs); error = error ? error : dsl_prop_register(ds, zfs_prop_to_name(ZFS_PROP_XATTR), xattr_changed_cb, zfsvfs); + error = error ? error : dsl_prop_register(ds, + zfs_prop_to_name(ZFS_PROP_XATTR_COMPAT), xattr_compat_changed_cb, + zfsvfs); + error = error ? error : dsl_prop_register(ds, + zfs_prop_to_name(ZFS_PROP_XATTR_FALLBACK), + xattr_fallback_changed_cb, zfsvfs); error = error ? error : dsl_prop_register(ds, zfs_prop_to_name(ZFS_PROP_RECORDSIZE), blksz_changed_cb, zfsvfs); error = error ? error : dsl_prop_register(ds, @@ -1502,6 +1543,14 @@ zfs_domount(struct super_block *sb, zfs_mnt_t *zm, int silent) "xattr", &pval, NULL))) goto out; xattr_changed_cb(zfsvfs, pval); + if ((error = dsl_prop_get_integer(osname, + "xattr_compat", &pval, NULL))) + goto out; + xattr_compat_changed_cb(zfsvfs, pval); + if ((error = dsl_prop_get_integer(osname, + "xattr_fallback", &pval, NULL))) + goto out; + xattr_fallback_changed_cb(zfsvfs, pval); if ((error = dsl_prop_get_integer(osname, "acltype", &pval, NULL))) goto out; diff --git a/module/os/linux/zfs/zpl_xattr.c b/module/os/linux/zfs/zpl_xattr.c index 66f197e4c77a..c8e65c04e5cc 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,8 @@ 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; - - 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 enum xattr_permission zpl_xattr_permission(xattr_filldir_t *, + const char *, int); /* * Determine is a given xattr name should be visible and if so copy it @@ -126,10 +107,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 +704,33 @@ static int __zpl_xattr_user_get(struct inode *ip, const char *name, void *value, size_t size) { - char *xattr_name; + boolean_t compat = !!(ITOZSB(ip)->z_flags & ZSB_XATTR_COMPAT); + boolean_t fallback = !!(ITOZSB(ip)->z_flags & ZSB_XATTR_FALLBACK); 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 if fallback is allowed. + */ + error = -ENODATA; + if (compat || fallback) + error = zpl_xattr_get(ip, name, value, size); + if (!compat || (fallback && 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); } @@ -729,19 +741,44 @@ __zpl_xattr_user_set(struct inode *ip, const char *name, const void *value, size_t size, int flags) { char *xattr_name; - int error; + boolean_t compat = !!(ITOZSB(ip)->z_flags & ZSB_XATTR_COMPAT); + boolean_t fallback = !!(ITOZSB(ip)->z_flags & ZSB_XATTR_FALLBACK); + 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); + /* + * Remove any namespaced version of the xattr so we only set the + * version compatible with other platforms. + * + * The following flags must be handled correctly: + * + * XATTR_CREATE: fail if xattr already exists + * XATTR_REPLACE: fail if xattr does not exist + */ xattr_name = kmem_asprintf("%s%s", XATTR_USER_PREFIX, name); - error = zpl_xattr_set(ip, xattr_name, value, size, flags); + if (compat && fallback) + error = zpl_xattr_set(ip, xattr_name, NULL, 0, flags); + if (!compat) + error = zpl_xattr_set(ip, xattr_name, value, size, flags); kmem_strfree(xattr_name); + if (!compat || error == -EEXIST) + return (error); + if (fallback && error == 0 && (flags & XATTR_REPLACE)) + flags &= ~XATTR_REPLACE; + error = zpl_xattr_set(ip, name, value, size, flags); + + dsl_dataset_t *ds = dmu_objset_ds(ITOZSB(ip)->z_os); + ds->ds_feature_activation[SPA_FEATURE_XATTR_COMPAT] = (void *)B_TRUE; + return (error); } ZPL_XATTR_SET_WRAPPER(zpl_xattr_user_set); @@ -1396,6 +1433,45 @@ 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 = xf->dentry; + boolean_t compat = !!(ITOZSB(d->d_inode)->z_flags & ZSB_XATTR_COMPAT); + enum xattr_permission perm = XAPERM_ALLOW; + + handler = zpl_xattr_handler(name); + if (handler == NULL) { + if (!compat) + return (XAPERM_DENY); + /* 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; diff --git a/module/zcommon/zfeature_common.c b/module/zcommon/zfeature_common.c index fc0e09605eef..48c90d384ff2 100644 --- a/module/zcommon/zfeature_common.c +++ b/module/zcommon/zfeature_common.c @@ -598,6 +598,19 @@ zpool_feature_init(void) zfeature_register(SPA_FEATURE_DRAID, "org.openzfs:draid", "draid", "Support for distributed spare RAID", ZFEATURE_FLAG_MOS, ZFEATURE_TYPE_BOOLEAN, NULL); + + + { + static const spa_feature_t xattr_compat_deps[] = { + SPA_FEATURE_EXTENSIBLE_DATASET, + SPA_FEATURE_NONE + }; + zfeature_register(SPA_FEATURE_XATTR_COMPAT, + "com.ixsystems:xattr_compat", "xattr_compat", + "Support for cross-platform compatible xattr namespace encoding", + ZFEATURE_FLAG_PER_DATASET, ZFEATURE_TYPE_BOOLEAN, + xattr_compat_deps); + } } #if defined(_KERNEL) diff --git a/module/zcommon/zfs_prop.c b/module/zcommon/zfs_prop.c index d17321990809..03bdf82cafd8 100644 --- a/module/zcommon/zfs_prop.c +++ b/module/zcommon/zfs_prop.c @@ -357,6 +357,12 @@ zfs_prop_init(void) { NULL } }; + static zprop_index_t xattr_compat_table[] = { + { "linux", ZFS_XATTR_COMPAT_LINUX }, + { "all", ZFS_XATTR_COMPAT_ALL }, + { NULL } + }; + static zprop_index_t dnsize_table[] = { { "legacy", ZFS_DNSIZE_LEGACY }, { "auto", ZFS_DNSIZE_AUTO }, @@ -459,6 +465,10 @@ zfs_prop_init(void) zprop_register_index(ZFS_PROP_XATTR, "xattr", ZFS_XATTR_DIR, PROP_INHERIT, ZFS_TYPE_FILESYSTEM | ZFS_TYPE_SNAPSHOT, "on | off | dir | sa", "XATTR", xattr_table); + zprop_register_index(ZFS_PROP_XATTR_COMPAT, "xattr_compat", + ZFS_XATTR_COMPAT_ALL, PROP_INHERIT, + ZFS_TYPE_FILESYSTEM | ZFS_TYPE_SNAPSHOT, + "all | linux", "XATTR_COMPAT", xattr_compat_table); zprop_register_index(ZFS_PROP_DNODESIZE, "dnodesize", ZFS_DNSIZE_LEGACY, PROP_INHERIT, ZFS_TYPE_FILESYSTEM, "legacy | auto | 1k | 2k | 4k | 8k | 16k", "DNSIZE", dnsize_table); @@ -498,6 +508,9 @@ zfs_prop_init(void) boolean_table); zprop_register_index(ZFS_PROP_OVERLAY, "overlay", 1, PROP_INHERIT, ZFS_TYPE_FILESYSTEM, "on | off", "OVERLAY", boolean_table); + zprop_register_index(ZFS_PROP_XATTR_FALLBACK, "xattr_fallback", 1, + PROP_INHERIT, ZFS_TYPE_FILESYSTEM | ZFS_TYPE_SNAPSHOT, "on | off", + "XATTR_FALLBACK", boolean_table); /* default index properties */ zprop_register_index(ZFS_PROP_VERSION, "version", 0, PROP_DEFAULT, diff --git a/tests/zfs-tests/include/properties.shlib b/tests/zfs-tests/include/properties.shlib index 6d467b60051d..6eecc2a9fce8 100644 --- a/tests/zfs-tests/include/properties.shlib +++ b/tests/zfs-tests/include/properties.shlib @@ -31,10 +31,11 @@ typeset -a redundant_metadata_prop_vals=('all' 'most') typeset -a secondarycache_prop_vals=('all' 'none' 'metadata') typeset -a snapdir_prop_vals=('hidden' 'visible') typeset -a sync_prop_vals=('standard' 'always' 'disabled') +typeset -a xattr_compat_prop_vals=('all' 'linux') typeset -a fs_props=('compress' 'checksum' 'recsize' 'canmount' 'copies' 'logbias' 'primarycache' 'redundant_metadata' - 'secondarycache' 'snapdir' 'sync') + 'secondarycache' 'snapdir' 'sync' 'xattr_compat') typeset -a vol_props=('compress' 'checksum' 'copies' 'logbias' 'primarycache' 'secondarycache' 'redundant_metadata' 'sync') @@ -92,7 +93,8 @@ function get_rand_large_recsize # # Functions to toggle on/off properties # -typeset -a binary_props=('atime' 'devices' 'exec' 'readonly' 'setuid' 'xattr') +typeset -a binary_props=('atime' 'devices' 'exec' 'readonly' 'setuid' 'xattr' + 'xattr_fallback') if is_freebsd; then binary_props+=('jailed') diff --git a/tests/zfs-tests/tests/functional/cli_root/zpool_get/zpool_get.cfg b/tests/zfs-tests/tests/functional/cli_root/zpool_get/zpool_get.cfg index 6075e1f1abbd..20d29c9169e6 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zpool_get/zpool_get.cfg +++ b/tests/zfs-tests/tests/functional/cli_root/zpool_get/zpool_get.cfg @@ -82,6 +82,7 @@ typeset -a properties=( "feature@log_spacemap" "feature@device_rebuild" "feature@draid" + "feature@xattr_compat" ) if is_linux || is_freebsd; then