From 5b99b881c412993f239bf8e708ae9526ba1bc9e3 Mon Sep 17 00:00:00 2001 From: Patrick Farrell Date: Mon, 1 Apr 2024 12:12:00 -0400 Subject: [PATCH] LU-17628 lfs: add lfs_setstripe admin restrict In some settings, it's not desirable for users to be able to set their own striping. This is purely a 'convenience' restriction, where the admin prefers users not set their own striping to avoid user error, and not a security restriction. This is for sites which have a sensible default striping and prefer users not modify it. The goal here is to avoid user error. However, some applications use the Lustre API to set their own striping, and it's not desirable for such applications to fail. So setstripe fails with an error for the 'lfs' binary, and is silently ignored in other cases. In all cases, the file is created with the default layout. Note we return EACESS for this case rather than EPERM because EPERM is already returned (via a special case) for setting layout on the root of the file system. This is a distinct case because we do not want the special group created by this patch to be able to set the root filesystem layout, so the root of the FS continues to be handled separately. Signed-off-by: Patrick Farrell Change-Id: Id2e8dd175f5e3870f3aa64b69556308706d5317c Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/54341 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Oleg Drokin Reviewed-by: Andreas Dilger Reviewed-by: wangdi --- lustre/doc/lfs-migrate.1 | 6 ++++ lustre/doc/lfs-setstripe.1 | 12 +++++++ lustre/llite/file.c | 21 ++++++++++- lustre/llite/llite_internal.h | 1 + lustre/llite/llite_lib.c | 3 ++ lustre/llite/lproc_llite.c | 29 +++++++++++++++ lustre/tests/sanity.sh | 48 +++++++++++++++++++++++++ lustre/utils/liblustreapi.c | 84 ++++++++++++++++++++++++++++++++++--------- 8 files changed, 187 insertions(+), 17 deletions(-) diff --git a/lustre/doc/lfs-migrate.1 b/lustre/doc/lfs-migrate.1 index 43fc9e4..ae41e01 100644 --- a/lustre/doc/lfs-migrate.1 +++ b/lustre/doc/lfs-migrate.1 @@ -41,6 +41,12 @@ to the new OST(s). In contrast, .B lfs setstripe is used for creating new (empty) files with the specified layout. For OST object migration, there additional options available: +.PP +If setstripe is restricted to specified users, this also applies to +.B lfs migrate. +See +.BR lfs-setstripe (1) +for details. .TP .BR -b , --block Block access to the file by other applications during data migration diff --git a/lustre/doc/lfs-setstripe.1 b/lustre/doc/lfs-setstripe.1 index e9efb35..08c1bc5 100644 --- a/lustre/doc/lfs-setstripe.1 +++ b/lustre/doc/lfs-setstripe.1 @@ -50,6 +50,17 @@ copy the default layout at creation time, but will implicitly inherit the default layout at runtime. The default layout set on a non-root directory will be copied to any new subdirectories created within that directory at the time they are created. +Setstripe can be restricted to privileged users +.RB (with CAP_SYS_RESOURCE , +see +.BR setcap (8)) +by setting the parameter +.BR llite.*.enable_setstripe_gid=0 , +to an administrator group (e.g. +.BR wheel ) +by using its numeric group ID, or +.B -1 +to permit all users this functionality (the default). .TP .B lfs setstripe \fR[\fISTRIPE_OPTIONS\fR ...] \fIDIRECTORY\fR|\fIFILENAME\fR Create a new @@ -566,3 +577,4 @@ layout/LOV EA and flags .BR lfs-mirror-create (1), .BR lfs-mirror-split (1), .BR lustre (7) +.BR setcap (8) diff --git a/lustre/llite/file.c b/lustre/llite/file.c index acda0df..617955e 100644 --- a/lustre/llite/file.c +++ b/lustre/llite/file.c @@ -4457,13 +4457,14 @@ ll_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { struct inode *inode = file_inode(file); struct ll_file_data *fd = file->private_data; + struct ll_sb_info *sbi = ll_i2sbi(inode); void __user *uarg = (void __user *)arg; int flags, rc; ENTRY; CDEBUG(D_VFSTRACE|D_IOCTL, "VFS Op:inode="DFID"(%pK) cmd=%x arg=%lx\n", PFID(ll_inode2fid(inode)), inode, cmd, arg); - ll_stats_ops_tally(ll_i2sbi(inode), LPROC_LL_IOCTL, 1); + ll_stats_ops_tally(sbi, LPROC_LL_IOCTL, 1); /* asm-ppc{,64} declares TCGETS, et. al. as type 't' not 'T' */ if (_IOC_TYPE(cmd) == 'T' || _IOC_TYPE(cmd) == 't') /* tty ioctls */ @@ -4506,6 +4507,24 @@ ll_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg) RETURN(0); case LL_IOC_LOV_SETSTRIPE: case LL_IOC_LOV_SETSTRIPE_NEW: + if (sbi->ll_enable_setstripe_gid != -1 && + !capable(CAP_SYS_RESOURCE) && + /* in_group_p always returns true for gid == 0, so we check + * for this case directly + */ + (sbi->ll_enable_setstripe_gid == 0 || + !in_group_p(KGIDT_INIT(sbi->ll_enable_setstripe_gid)))) { + /* for lfs we return EACCES, so we can print an error + * from the tool + */ + if (!strcmp(current->comm, "lfs")) + RETURN(-EACCES); + /* otherwise, setstripe is refused silently so + * applications do not fail + */ + RETURN(0); + } + RETURN(ll_lov_setstripe(inode, file, uarg)); case LL_IOC_LOV_SETEA: RETURN(ll_lov_setea(inode, file, uarg)); diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h index fafab4d..768d23f 100644 --- a/lustre/llite/llite_internal.h +++ b/lustre/llite/llite_internal.h @@ -899,6 +899,7 @@ struct ll_sb_info { struct mnt_namespace *ll_mnt_ns; DECLARE_BITMAP(ll_flags, LL_SBI_NUM_FLAGS); /* enum ll_sbi_flags */ + gid_t ll_enable_setstripe_gid; /* */ unsigned int ll_xattr_cache_enabled:1, ll_xattr_cache_set:1, /* already set to 0/1 */ ll_client_common_fill_super_succeeded:1, diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c index e79d4a5..183186f 100644 --- a/lustre/llite/llite_lib.c +++ b/lustre/llite/llite_lib.c @@ -230,6 +230,9 @@ static struct ll_sb_info *ll_init_sbi(struct lustre_sb_info *lsi) sbi->ll_hybrid_io_read_threshold_bytes = SBI_DEFAULT_HYBRID_IO_READ_THRESHOLD; + /* setstripe is allowed for all groups by default */ + sbi->ll_enable_setstripe_gid = -1; + INIT_LIST_HEAD(&sbi->ll_all_quota_list); RETURN(sbi); out_destroy_ra: diff --git a/lustre/llite/lproc_llite.c b/lustre/llite/lproc_llite.c index 52dd9b6..373ad0c 100644 --- a/lustre/llite/lproc_llite.c +++ b/lustre/llite/lproc_llite.c @@ -1468,6 +1468,34 @@ static ssize_t hybrid_io_store(struct kobject *kobj, struct attribute *attr, } LUSTRE_RW_ATTR(hybrid_io); +static ssize_t enable_setstripe_gid_show(struct kobject *kobj, + struct attribute *attr, char *buf) +{ + struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info, + ll_kset.kobj); + + return scnprintf(buf, PAGE_SIZE, "%d\n", sbi->ll_enable_setstripe_gid); +} + +static ssize_t enable_setstripe_gid_store(struct kobject *kobj, + struct attribute *attr, + const char *buffer, size_t count) +{ + struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info, + ll_kset.kobj); + unsigned int val; + int rc; + + rc = kstrtoint(buffer, 10, &val); + if (rc) + return rc; + + sbi->ll_enable_setstripe_gid = val; + + return count; +} +LUSTRE_RW_ATTR(enable_setstripe_gid); + static ssize_t max_read_ahead_async_active_show(struct kobject *kobj, struct attribute *attr, char *buf) @@ -2261,6 +2289,7 @@ static struct attribute *llite_attrs[] = { &lustre_attr_parallel_dio.attr, &lustre_attr_unaligned_dio.attr, &lustre_attr_hybrid_io.attr, + &lustre_attr_enable_setstripe_gid.attr, &lustre_attr_file_heat.attr, &lustre_attr_heat_decay_percentage.attr, &lustre_attr_heat_period_second.attr, diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 9a5fdca..b30bd89 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -3634,6 +3634,54 @@ test_27V() { } run_test 27V "creating widely striped file races with deactivating OST" +test_27W() { + [ $PARALLEL == "yes" ] && skip "skip parallel run" + local stripe_count + local defcount=4 + + # Set back to unrestricted + stack_trap "$LCTL set_param llite.$FSNAME-*.enable_setstripe_gid=-1" + + mkdir $DIR/$tdir + # Set a default layout + $LFS setstripe -C $defcount $DIR/$tdir || error "(0) root failed to set default layout" + + chmod a+rw $DIR + chmod a+rw $DIR/$tdir + # Verify this works normally + $RUNAS $LFS setstripe -c 1 $DIR/$tdir/$tfile || error "(1) failed to setstripe" + rm -f $DIR/$tdir/$tfile + + # Limit to CAP_SYS_RESOURCE + $LCTL set_param llite.$FSNAME-*.enable_setstripe_gid=0 + # Prints a messsage, but will use the default layout for the directory + # This is so scripts which use setstripe and don't check error will not fail + $RUNAS $LFS setstripe -c 1 $DIR/$tdir/$tfile || error "(2) setstripe failed" + # Verify default layout was used on file: + stripe_count=$($LFS getstripe -c $DIR/$tdir/$tfile) + ((stripe_count == defcount)) || + error "(3) got stripe_count '$stripe_count', expected $defcount" + rm -f $DIR/$tdir/$tfile + + # Allow for RUNAS group + $LCTL set_param llite.$FSNAME-*.enable_setstripe_gid=$RUNAS_GID + $RUNAS $LFS setstripe -c 1 $DIR/$tdir/$tfile || error "(4) failed to setstripe" + # Confirm we did not use the default layout + stripe_count=$($LFS getstripe -c $DIR/$tdir/$tfile) + ((stripe_count == 1)) || error "(5) got stripe_count '$stripe_count', expected 1" + rm -f $DIR/$tdir/$tfile + + # Set to some other GID + $LCTL set_param llite.$FSNAME-*.enable_setstripe_gid=1 + # Should give warning and succeed + $RUNAS $LFS setstripe -c 1 $DIR/$tdir/$tfile || error "(6) setstripe failed" + # Confirm we used default layout + stripe_count=$($LFS getstripe -c $DIR/$tdir/$tfile) + ((stripe_count == defcount)) || + error "(7) got stripe_count '$stripe_count', expected $defcount" +} +run_test 27W "test enable_setstripe_gid" + # createtest also checks that device nodes are created and # then visible correctly (#2091) test_28() { # bug 2091 diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c index d46da02..addaa14 100644 --- a/lustre/utils/liblustreapi.c +++ b/lustre/utils/liblustreapi.c @@ -562,6 +562,7 @@ int llapi_file_open_param(const char *name, int flags, mode_t mode, char fsname[MAX_OBD_NAME + 1] = { 0 }; struct lov_user_md *lum = NULL; const char *pool_name = param->lsp_pool; + bool use_default_striping = false; size_t lum_size; int fd, rc = 0; @@ -593,7 +594,10 @@ int llapi_file_open_param(const char *name, int flags, mode_t mode, return -ENOMEM; retry_open: - fd = open(name, flags | O_LOV_DELAY_CREATE, mode); + if (!use_default_striping) + fd = open(name, flags | O_LOV_DELAY_CREATE, mode); + else + fd = open(name, flags, mode); if (fd < 0) { if (errno == EISDIR && !(flags & O_DIRECTORY)) { flags = O_DIRECTORY | O_RDONLY; @@ -640,24 +644,45 @@ retry_open: lumv3->lmm_objects[i].l_ost_idx = param->lsp_osts[i]; } - if (ioctl(fd, LL_IOC_LOV_SETSTRIPE, lum) != 0) { - char errmsg[512] = "stripe already set"; + if (!use_default_striping && ioctl(fd, LL_IOC_LOV_SETSTRIPE, lum) != 0) { + char errbuf[512] = "stripe already set"; + char *errmsg = errbuf; rc = -errno; - if (errno != EEXIST && errno != EALREADY) - strncpy(errmsg, strerror(errno), sizeof(errmsg) - 1); + if (rc != -EEXIST && rc != -EALREADY) + strncpy(errbuf, strerror(errno), sizeof(errbuf) - 1); if (rc == -EREMOTEIO) - snprintf(errmsg, sizeof(errmsg), + snprintf(errbuf, sizeof(errbuf), "inactive OST among your specified %d OST(s)", param->lsp_stripe_count); - - llapi_err_noerrno(LLAPI_MSG_ERROR, - "setstripe error for '%s': %s", name, errmsg); - close(fd); + /* the only reason we get EACESS on the ioctl is if setstripe + * has been explicitly restricted, normal permission errors + * happen earlier on open() and we never call ioctl() + */ + if (rc == -EACCES) { + errmsg = "Setstripe is restricted by your administrator, default striping applied"; + llapi_err_noerrno(LLAPI_MSG_WARN, + "setstripe warning for '%s': %s", + name, errmsg); + rc = remove(name); + if (rc) { + llapi_err_noerrno(LLAPI_MSG_ERROR, + "setstripe error for '%s': %s", + name, strerror(errno)); + goto out; + } + use_default_striping = true; + goto retry_open; + } else { + llapi_err_noerrno(LLAPI_MSG_ERROR, + "setstripe error for '%s': %s", name, + errmsg); + } fd = rc; } +out: free(lum); return fd; @@ -703,6 +728,7 @@ int llapi_file_create_foreign(const char *name, mode_t mode, __u32 type, { size_t len; struct lov_foreign_md *lfm; + bool use_default_striping = false; int fd, rc; if (foreign_lov == NULL) { @@ -731,7 +757,11 @@ int llapi_file_create_foreign(const char *name, mode_t mode, __u32 type, goto out_err; } - fd = open(name, O_WRONLY|O_CREAT|O_LOV_DELAY_CREATE, mode); +retry_open: + if (!use_default_striping) + fd = open(name, O_WRONLY|O_CREAT|O_LOV_DELAY_CREATE, mode); + else + fd = open(name, O_WRONLY|O_CREAT, mode); if (fd == -1) { fd = -errno; llapi_error(LLAPI_MSG_ERROR, fd, "open '%s' failed", name); @@ -744,21 +774,43 @@ int llapi_file_create_foreign(const char *name, mode_t mode, __u32 type, lfm->lfm_flags = flags; memcpy(lfm->lfm_value, foreign_lov, len); - if (ioctl(fd, LL_IOC_LOV_SETSTRIPE, lfm) != 0) { - char *errmsg = "stripe already set"; + if (!use_default_striping && ioctl(fd, LL_IOC_LOV_SETSTRIPE, lfm) != 0) { + char *errmsg; rc = -errno; if (errno == ENOTTY) errmsg = "not on a Lustre filesystem"; else if (errno == EEXIST || errno == EALREADY) errmsg = "stripe already set"; + else if (errno == EACCES) + errmsg = "Setstripe is restricted by your administrator, default striping applied"; else errmsg = strerror(errno); - llapi_err_noerrno(LLAPI_MSG_ERROR, - "setstripe error for '%s': %s", name, errmsg); - close(fd); + /* the only reason we get ENOPERM on the ioctl is if setstripe + * has been explicitly restricted, normal permission errors + * happen earlier on open() and we never call ioctl() + */ + if (rc == -EACCES) { + llapi_err_noerrno(LLAPI_MSG_WARN, + "setstripe warning for '%s': %s", + name, errmsg); + rc = remove(name); + if (rc) { + llapi_err_noerrno(LLAPI_MSG_ERROR, + "setstripe error for '%s': %s", + name, strerror(errno)); + goto out_free; + } + use_default_striping = true; + goto retry_open; + } else { + llapi_err_noerrno(LLAPI_MSG_ERROR, + "setstripe error for '%s': %s", name, + errmsg); + } + fd = rc; } -- 1.8.3.1