From 1eace590604d3efc5458a72096bc6a1e2adca05a Mon Sep 17 00:00:00 2001 From: Ameer Hamza Date: Mon, 9 Mar 2026 23:06:22 +0500 Subject: [PATCH] libzfs: use mount_setattr for selective remount including legacy mounts When a namespace property is changed via zfs set, libzfs remounts the filesystem to propagate the new VFS mount flags. The current approach uses mount(2) with MS_REMOUNT, which reads all namespace properties from ZFS and applies them together. This has two problems: 1. Linux VFS resets unspecified per-mount flags on remount. If an administrator sets a temporary flag (e.g. mount -o remount,noatime), a subsequent zfs set on any namespace property clobbers it. 2. Two concurrent zfs set operations on different namespace properties can overwrite each other's mount flags. Additionally, legacy datasets (mountpoint=legacy) were never remounted on namespace property changes since zfs_is_mountable() returns false for them. Add zfs_mount_setattr() which uses mount_setattr(2) to selectively update only the mount flags that correspond to the changed property. For legacy datasets, /proc/mounts is iterated to update all mountpoints. On kernels without mount_setattr (ENOSYS), non-legacy datasets fall back to a full remount; legacy mounts are skipped to avoid clobbering temporary flags. Reviewed-by: Brian Behlendorf Reviewed-by: Alexander Motin Signed-off-by: Ameer Hamza Closes #18257 --- config/user-mount-setattr.m4 | 27 +++++ config/user.m4 | 1 + lib/libzfs/libzfs_dataset.c | 31 ++---- lib/libzfs/libzfs_impl.h | 17 +++ lib/libzfs/libzfs_mount.c | 43 +++++++- lib/libzfs/os/freebsd/libzfs_zmount.c | 11 ++ lib/libzfs/os/linux/libzfs_mount_os.c | 143 ++++++++++++++++++++++++++ 7 files changed, 247 insertions(+), 26 deletions(-) create mode 100644 config/user-mount-setattr.m4 diff --git a/config/user-mount-setattr.m4 b/config/user-mount-setattr.m4 new file mode 100644 index 000000000..efcfd66b2 --- /dev/null +++ b/config/user-mount-setattr.m4 @@ -0,0 +1,27 @@ +dnl # SPDX-License-Identifier: CDDL-1.0 +dnl # +dnl # Check for mount_setattr() and struct mount_attr availability +dnl # +AC_DEFUN([ZFS_AC_CONFIG_USER_MOUNT_SETATTR], [ + AC_CHECK_FUNC([mount_setattr], [ + AC_MSG_CHECKING([for struct mount_attr]) + AC_COMPILE_IFELSE([ + AC_LANG_PROGRAM([[ + #include + ]], [[ + struct mount_attr attr = { + .attr_set = MOUNT_ATTR_RDONLY, + .attr_clr = MOUNT_ATTR_NOEXEC, + }; + (void) attr; + ]]) + ], [ + AC_MSG_RESULT([yes]) + AC_DEFINE([HAVE_MOUNT_SETATTR], [1], + [mount_setattr() and struct mount_attr + are available]) + ], [ + AC_MSG_RESULT([no]) + ]) + ]) +]) diff --git a/config/user.m4 b/config/user.m4 index ffd22c75a..c7a489d5a 100644 --- a/config/user.m4 +++ b/config/user.m4 @@ -20,6 +20,7 @@ AC_DEFUN([ZFS_AC_CONFIG_USER], [ ZFS_AC_CONFIG_USER_LIBUUID ZFS_AC_CONFIG_USER_LIBBLKID ZFS_AC_CONFIG_USER_STATX + ZFS_AC_CONFIG_USER_MOUNT_SETATTR ]) ZFS_AC_CONFIG_USER_LIBTIRPC ZFS_AC_CONFIG_USER_LIBCRYPTO diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index b9b76e0bd..61ac859b3 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -1701,26 +1701,6 @@ zfs_fix_auto_resv(zfs_handle_t *zhp, nvlist_t *nvl) return (1); } -static boolean_t -zfs_is_namespace_prop(zfs_prop_t prop) -{ - switch (prop) { - - case ZFS_PROP_ATIME: - case ZFS_PROP_RELATIME: - case ZFS_PROP_DEVICES: - case ZFS_PROP_EXEC: - case ZFS_PROP_SETUID: - case ZFS_PROP_READONLY: - case ZFS_PROP_XATTR: - case ZFS_PROP_NBMAND: - return (B_TRUE); - - default: - return (B_FALSE); - } -} - /* * Given a property name and value, set the property for the given dataset. */ @@ -1778,7 +1758,7 @@ zfs_prop_set_list_flags(zfs_handle_t *zhp, nvlist_t *props, int flags) int nvl_len = 0; int added_resv = 0; zfs_prop_t prop; - boolean_t nsprop = B_FALSE; + uint32_t nspflags = 0; nvpair_t *elem; (void) snprintf(errbuf, sizeof (errbuf), @@ -1825,7 +1805,7 @@ zfs_prop_set_list_flags(zfs_handle_t *zhp, nvlist_t *props, int flags) elem = nvlist_next_nvpair(nvl, elem)) { prop = zfs_name_to_prop(nvpair_name(elem)); - nsprop |= zfs_is_namespace_prop(prop); + nspflags |= zfs_namespace_prop_flag(prop); assert(cl_idx < nvl_len); /* @@ -1926,8 +1906,8 @@ zfs_prop_set_list_flags(zfs_handle_t *zhp, nvlist_t *props, int flags) * if one of the options handled by the generic * Linux namespace layer has been modified. */ - if (nsprop && zfs_is_mounted(zhp, NULL)) - ret = zfs_mount(zhp, MNTOPT_REMOUNT, 0); + if (nspflags && zfs_is_mounted(zhp, NULL)) + ret = zfs_mount_setattr(zhp, nspflags); } } @@ -2049,7 +2029,8 @@ zfs_prop_inherit(zfs_handle_t *zhp, const char *propname, boolean_t received) */ if (zfs_is_namespace_prop(prop) && zfs_is_mounted(zhp, NULL)) - ret = zfs_mount(zhp, MNTOPT_REMOUNT, 0); + ret = zfs_mount_setattr(zhp, + zfs_namespace_prop_flag(prop)); } error: diff --git a/lib/libzfs/libzfs_impl.h b/lib/libzfs/libzfs_impl.h index ef0524099..b4dce167f 100644 --- a/lib/libzfs/libzfs_impl.h +++ b/lib/libzfs/libzfs_impl.h @@ -89,6 +89,19 @@ struct zfs_handle { uint8_t *zfs_props_table; }; +/* + * Internal namespace property flags for selective remount via + * mount_setattr(2). Passed to zfs_mount_setattr(). + */ +#define ZFS_MNT_PROP_ATIME (1U << 0) +#define ZFS_MNT_PROP_RELATIME (1U << 1) +#define ZFS_MNT_PROP_DEVICES (1U << 2) +#define ZFS_MNT_PROP_EXEC (1U << 3) +#define ZFS_MNT_PROP_SETUID (1U << 4) +#define ZFS_MNT_PROP_READONLY (1U << 5) +#define ZFS_MNT_PROP_XATTR (1U << 6) +#define ZFS_MNT_PROP_NBMAND (1U << 7) + /* * This is different from checking zfs_type, because it will also catch * snapshots of volumes. @@ -181,6 +194,10 @@ extern prop_changelist_t *changelist_gather(zfs_handle_t *, zfs_prop_t, int, extern int changelist_unshare(prop_changelist_t *, const enum sa_protocol *); extern int changelist_haszonedchild(prop_changelist_t *); +extern boolean_t zfs_is_namespace_prop(zfs_prop_t); +extern uint32_t zfs_namespace_prop_flag(zfs_prop_t); +extern boolean_t zfs_is_mountable_internal(zfs_handle_t *); +extern int zfs_mount_setattr(zfs_handle_t *, uint32_t); extern void remove_mountpoint(zfs_handle_t *); extern int create_parents(libzfs_handle_t *, char *, int); diff --git a/lib/libzfs/libzfs_mount.c b/lib/libzfs/libzfs_mount.c index a6287fbe2..130f0a2db 100644 --- a/lib/libzfs/libzfs_mount.c +++ b/lib/libzfs/libzfs_mount.c @@ -103,6 +103,47 @@ zfs_share_protocol_name(enum sa_protocol protocol) return (sa_protocol_names[protocol]); } +/* + * Returns B_TRUE if the property is a namespace property that requires + * a remount to take effect. + */ +boolean_t +zfs_is_namespace_prop(zfs_prop_t prop) +{ + switch (prop) { + case ZFS_PROP_ATIME: + case ZFS_PROP_RELATIME: + case ZFS_PROP_DEVICES: + case ZFS_PROP_EXEC: + case ZFS_PROP_SETUID: + case ZFS_PROP_READONLY: + case ZFS_PROP_XATTR: + case ZFS_PROP_NBMAND: + return (B_TRUE); + default: + return (B_FALSE); + } +} + +/* + * Returns the ZFS_MNT_PROP_* flag for a namespace property. + */ +uint32_t +zfs_namespace_prop_flag(zfs_prop_t prop) +{ + switch (prop) { + case ZFS_PROP_ATIME: return (ZFS_MNT_PROP_ATIME); + case ZFS_PROP_RELATIME: return (ZFS_MNT_PROP_RELATIME); + case ZFS_PROP_DEVICES: return (ZFS_MNT_PROP_DEVICES); + case ZFS_PROP_EXEC: return (ZFS_MNT_PROP_EXEC); + case ZFS_PROP_SETUID: return (ZFS_MNT_PROP_SETUID); + case ZFS_PROP_READONLY: return (ZFS_MNT_PROP_READONLY); + case ZFS_PROP_XATTR: return (ZFS_MNT_PROP_XATTR); + case ZFS_PROP_NBMAND: return (ZFS_MNT_PROP_NBMAND); + default: return (0); + } +} + static boolean_t dir_is_empty_stat(const char *dirname) { @@ -225,7 +266,7 @@ zfs_is_mounted(zfs_handle_t *zhp, char **where) * that the caller has verified the sanity of mounting the dataset at * its mountpoint to the extent the caller wants. */ -static boolean_t +boolean_t zfs_is_mountable_internal(zfs_handle_t *zhp) { if (zfs_prop_get_int(zhp, ZFS_PROP_ZONED) && diff --git a/lib/libzfs/os/freebsd/libzfs_zmount.c b/lib/libzfs/os/freebsd/libzfs_zmount.c index bc7d68b17..3723769d4 100644 --- a/lib/libzfs/os/freebsd/libzfs_zmount.c +++ b/lib/libzfs/os/freebsd/libzfs_zmount.c @@ -116,6 +116,17 @@ do_unmount(zfs_handle_t *zhp, const char *mntpt, int flags) return (0); } +/* + * FreeBSD does not support mount_setattr(2). Fall back to a full + * remount so that the updated namespace property takes effect. + */ +int +zfs_mount_setattr(zfs_handle_t *zhp, uint32_t nspflags) +{ + (void) nspflags; + return (zfs_mount(zhp, MNTOPT_REMOUNT, 0)); +} + int zfs_mount_delegation_check(void) { diff --git a/lib/libzfs/os/linux/libzfs_mount_os.c b/lib/libzfs/os/linux/libzfs_mount_os.c index cdfb432b1..7d8768d12 100644 --- a/lib/libzfs/os/linux/libzfs_mount_os.c +++ b/lib/libzfs/os/linux/libzfs_mount_os.c @@ -409,6 +409,149 @@ do_unmount(zfs_handle_t *zhp, const char *mntpt, int flags) return (rc ? EINVAL : 0); } +#ifdef HAVE_MOUNT_SETATTR +/* + * Build a struct mount_attr for the changed namespace properties. + * Parallel to zfs_add_options() but produces mount_setattr(2) input + * instead of a mount options string. + */ +static void +zfs_add_options_setattr(zfs_handle_t *zhp, struct mount_attr *attr, + uint32_t nspflags) +{ + const char *source; + + if (nspflags & ZFS_MNT_PROP_READONLY) { + if (getprop_uint64(zhp, ZFS_PROP_READONLY, &source)) + attr->attr_set |= MOUNT_ATTR_RDONLY; + else + attr->attr_clr |= MOUNT_ATTR_RDONLY; + } + if (nspflags & ZFS_MNT_PROP_EXEC) { + if (getprop_uint64(zhp, ZFS_PROP_EXEC, &source)) + attr->attr_clr |= MOUNT_ATTR_NOEXEC; + else + attr->attr_set |= MOUNT_ATTR_NOEXEC; + } + if (nspflags & ZFS_MNT_PROP_SETUID) { + if (getprop_uint64(zhp, ZFS_PROP_SETUID, &source)) + attr->attr_clr |= MOUNT_ATTR_NOSUID; + else + attr->attr_set |= MOUNT_ATTR_NOSUID; + } + if (nspflags & ZFS_MNT_PROP_DEVICES) { + if (getprop_uint64(zhp, ZFS_PROP_DEVICES, &source)) + attr->attr_clr |= MOUNT_ATTR_NODEV; + else + attr->attr_set |= MOUNT_ATTR_NODEV; + } + if (nspflags & (ZFS_MNT_PROP_ATIME | ZFS_MNT_PROP_RELATIME)) { + uint64_t atime = getprop_uint64(zhp, ZFS_PROP_ATIME, &source); + uint64_t relatime = getprop_uint64(zhp, + ZFS_PROP_RELATIME, &source); + + attr->attr_clr |= MOUNT_ATTR__ATIME; + if (!atime) + attr->attr_set |= MOUNT_ATTR_NOATIME; + else if (relatime) + attr->attr_set |= MOUNT_ATTR_RELATIME; + else + attr->attr_set |= MOUNT_ATTR_STRICTATIME; + } +} +#endif /* HAVE_MOUNT_SETATTR */ + +/* + * Selectively update per-mount VFS flags for the changed namespace + * properties using mount_setattr(2). Unlike a full remount via mount(2), + * this only modifies the specified flags without resetting others -- + * avoiding clobbering temporary mount flags set by the administrator. + * + * For non-legacy datasets, the single known mountpoint is used. + * For legacy datasets, /proc/mounts is iterated since legacy datasets + * can be mounted at multiple locations. + * + * Falls back to a full remount via zfs_mount() when mount_setattr(2) + * is not available (ENOSYS), except for legacy mounts where a full + * remount would clobber temporary flags. + */ +int +zfs_mount_setattr(zfs_handle_t *zhp, uint32_t nspflags) +{ +#ifdef HAVE_MOUNT_SETATTR + struct mount_attr attr = { 0 }; + char mntpt_prop[ZFS_MAXPROPLEN]; + boolean_t legacy = B_FALSE; + libzfs_handle_t *hdl = zhp->zfs_hdl; + FILE *mnttab; + struct mnttab entry; + int ret = 0; + + if (!zfs_is_mountable_internal(zhp)) + return (0); + + zfs_add_options_setattr(zhp, &attr, nspflags); + if (attr.attr_set == 0 && attr.attr_clr == 0) + return (0); + + if (zfs_prop_valid_for_type(ZFS_PROP_MOUNTPOINT, zhp->zfs_type, + B_FALSE)) { + verify(zfs_prop_get(zhp, ZFS_PROP_MOUNTPOINT, mntpt_prop, + sizeof (mntpt_prop), NULL, NULL, 0, B_FALSE) == 0); + legacy = (strcmp(mntpt_prop, ZFS_MOUNTPOINT_LEGACY) == 0); + } + + if (!legacy) { + char *mntpt = NULL; + + if (!zfs_is_mounted(zhp, &mntpt)) + return (0); + + ret = mount_setattr(AT_FDCWD, mntpt, 0, + &attr, sizeof (attr)); + free(mntpt); + if (ret != 0) { + if (errno == ENOSYS) + return (zfs_mount(zhp, MNTOPT_REMOUNT, 0)); + return (zfs_error_fmt(hdl, EZFS_MOUNTFAILED, + dgettext(TEXT_DOMAIN, "cannot set mount " + "attributes for '%s'"), zhp->zfs_name)); + } + return (0); + } + + /* Legacy: iterate /proc/mounts for all mountpoints. */ + if ((mnttab = fopen(MNTTAB, "re")) == NULL) + return (0); + + while (getmntent(mnttab, &entry) == 0) { + if (strcmp(entry.mnt_fstype, MNTTYPE_ZFS) != 0) + continue; + if (strcmp(entry.mnt_special, zhp->zfs_name) != 0) + continue; + + ret = mount_setattr(AT_FDCWD, entry.mnt_mountp, 0, + &attr, sizeof (attr)); + if (ret != 0) { + if (errno == ENOSYS) { + ret = 0; + break; + } + ret = zfs_error_fmt(hdl, EZFS_MOUNTFAILED, + dgettext(TEXT_DOMAIN, "cannot set mount " + "attributes for '%s'"), zhp->zfs_name); + break; + } + } + + (void) fclose(mnttab); + return (ret); +#else + (void) nspflags; + return (zfs_mount(zhp, MNTOPT_REMOUNT, 0)); +#endif +} + int zfs_mount_delegation_check(void) {