LU-11101 quota: fix setattr project check

Similar patch motivated by upstream patch:
ext4: fix setattr project check in fssetxattr ioctl

Currently, project quota could be changed by fssetxattr
ioctl, and existed permission check inode_owner_or_capable()
is obviously not enough, just think that common users could
change project id of file, that could make users to
break project quota easily.

This patch try to follow same regular of xfs project
quota:

"Project Quota ID state is only allowed to change from
within the init namespace. Enforce that restriction only
if we are trying to change the quota ID state.
Everything else is allowed in user namespaces."

Test-Parameters: trivial testlist=sanity-quota,sanity-quota
Change-Id: If03bb120476eca9707b1b4db64e9594bb99df59e
Signed-off-by: Wang Shilong <wshilong@ddn.com>i
Reviewed-on: https://review.whamcloud.com/32730
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Hongchao Zhang <hongchao@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index 8bb2901..73d032b 100644 (file)
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -3029,6 +3029,30 @@ int ll_ioctl_fsgetxattr(struct inode *inode, unsigned int cmd,
        RETURN(0);
 }
 
+int ll_ioctl_check_project(struct inode *inode, struct fsxattr *fa)
+{
+       /*
+        * Project Quota ID state is only allowed to change from within the init
+        * namespace. Enforce that restriction only if we are trying to change
+        * the quota ID state. Everything else is allowed in user namespaces.
+        */
+       if (current_user_ns() == &init_user_ns)
+               return 0;
+
+       if (ll_i2info(inode)->lli_projid != fa->fsx_projid)
+               return -EINVAL;
+
+       if (ll_file_test_flag(ll_i2info(inode), LLIF_PROJECT_INHERIT)) {
+               if (!(fa->fsx_xflags & FS_XFLAG_PROJINHERIT))
+                       return -EINVAL;
+       } else {
+               if (fa->fsx_xflags & FS_XFLAG_PROJINHERIT)
+                       return -EINVAL;
+       }
+
+       return 0;
+}
+
 int ll_ioctl_fssetxattr(struct inode *inode, unsigned int cmd,
                        unsigned long arg)
 {
@@ -3041,20 +3065,20 @@ int ll_ioctl_fssetxattr(struct inode *inode, unsigned int cmd,
        struct iattr *attr;
        int flags;
 
-       /* only root could change project ID */
-       if (!cfs_capable(CFS_CAP_SYS_ADMIN))
-               RETURN(-EPERM);
+       if (copy_from_user(&fsxattr,
+                          (const struct fsxattr __user *)arg,
+                          sizeof(fsxattr)))
+               RETURN(-EFAULT);
+
+       rc = ll_ioctl_check_project(inode, &fsxattr);
+       if (rc)
+               RETURN(rc);
 
        op_data = ll_prep_md_op_data(NULL, inode, NULL, NULL, 0, 0,
                                     LUSTRE_OPC_ANY, NULL);
        if (IS_ERR(op_data))
                RETURN(PTR_ERR(op_data));
 
-       if (copy_from_user(&fsxattr,
-                          (const struct fsxattr __user *)arg,
-                          sizeof(fsxattr)))
-               GOTO(out_fsxattr, rc = -EFAULT);
-
        flags = ll_xflags_to_inode_flags(fsxattr.fsx_xflags);
        op_data->op_attr_flags = ll_inode_to_ext_flags(flags);
        if (fsxattr.fsx_xflags & FS_XFLAG_PROJINHERIT)

diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h
index 282af57..a9a7d96 100644 (file)
--- a/lustre/llite/llite_internal.h
+++ b/lustre/llite/llite_internal.h
@@ -890,6 +890,7 @@ int ll_inode_permission(struct inode *inode, int mask, struct nameidata *nd);
 int ll_inode_permission(struct inode *inode, int mask);
 # endif
 #endif
+int ll_ioctl_check_project(struct inode *inode, struct fsxattr *fa);
 int ll_ioctl_fsgetxattr(struct inode *inode, unsigned int cmd,
                        unsigned long arg);
 int ll_ioctl_fssetxattr(struct inode *inode, unsigned int cmd,

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index 9d15bd7..28acbb6 100644 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -2176,10 +2176,19 @@ int ll_iocontrol(struct inode *inode, struct file *file,
                struct iattr *attr;
                struct md_op_data *op_data;
                struct cl_object *obj;
+               struct fsxattr fa = { 0 };
 
                if (get_user(flags, (int __user *)arg))
                        RETURN(-EFAULT);
 
+               fa.fsx_projid = ll_i2info(inode)->lli_projid;
+               if (flags & LUSTRE_PROJINHERIT_FL)
+                       fa.fsx_xflags = FS_XFLAG_PROJINHERIT;
+
+               rc = ll_ioctl_check_project(inode, &fa);
+               if (rc)
+                       RETURN(rc);
+
                op_data = ll_prep_md_op_data(NULL, inode, NULL, NULL, 0, 0,
                                             LUSTRE_OPC_ANY, NULL);
                if (IS_ERR(op_data))

diff --git a/lustre/tests/sanity-quota.sh b/lustre/tests/sanity-quota.sh
index d16949d..4ecaa5e 100755 (executable)
--- a/lustre/tests/sanity-quota.sh
+++ b/lustre/tests/sanity-quota.sh
@@ -488,7 +488,7 @@ test_0() {
        local free_space=$(lfs_df | grep "summary" | awk '{print $4}')
        [ $free_space -le $((MB * 1024)) ] &&
                skip "not enough space ${free_space} KB, " \
-                       "required $((MB * 1024)) KB" && return
+                       "required $((MB * 1024)) KB"
        setup_quota_test || error "setup quota failed with $?"
        trap cleanup_quota_test EXIT
 
@@ -634,8 +634,7 @@ test_2() {
        local FREE_INODES=$(mdt_free_inodes 0)
        echo "$FREE_INODES free inodes on master MDT"
        [ $FREE_INODES -lt $LIMIT ] &&
-               skip "not enough free inodes $FREE_INODES required $LIMIT" &&
-               return
+               skip "not enough free inodes $FREE_INODES required $LIMIT"
 
        setup_quota_test || error "setup quota failed with $?"
        trap cleanup_quota_test EXIT
@@ -1437,7 +1436,7 @@ run_test 7d "Quota reintegration (Transfer index in multiple bulks)"
 
 # quota reintegration (inode limits)
 test_7e() {
-       [ "$MDSCOUNT" -lt "2" ] && skip "needs >= 2 MDTs" && return
+       [ "$MDSCOUNT" -lt "2" ] && skip "needs >= 2 MDTs"
 
        # LU-2435: skip this quota test if underlying zfs version has not
        # supported native dnode accounting
@@ -1447,8 +1446,7 @@ test_7e() {
                local feature=$(do_facet mds1 $ZPOOL get -H $F $pool)
 
                [[ "$feature" != *" active "* ]] &&
-                       skip "requires zpool with active userobj_accounting" &&
-                       return
+                       skip "requires zpool with active userobj_accounting"
        }
 
        local ilimit=$((1024 * 2)) # 2k inodes
@@ -1682,7 +1680,7 @@ test_11() {
 run_test 11 "Chown/chgrp ignores quota"
 
 test_12a() {
-       [ "$OSTCOUNT" -lt "2" ] && skip "needs >= 2 OSTs" && return
+       [ "$OSTCOUNT" -lt "2" ] && skip "needs >= 2 OSTs"
 
        local blimit=22 # 22M
        local blk_cnt=$((blimit - 5))
@@ -1726,7 +1724,7 @@ test_12a() {
 run_test 12a "Block quota rebalancing"
 
 test_12b() {
-       [ "$MDSCOUNT" -lt "2" ] && skip "needs >= 2 MDTs" && return
+       [ "$MDSCOUNT" -lt "2" ] && skip "needs >= 2 MDTs"
 
        local ilimit=$((1024 * 2)) # 2k inodes
        local TESTFILE0=$DIR/$tdir/$tfile
@@ -2245,7 +2243,7 @@ test_23_sub() {
 test_23() {
        [ $(facet_fstype ost1) == "zfs" ] &&
                skip "Overwrite in place is not guaranteed to be " \
-               "space neutral on ZFS" && return
+               "space neutral on ZFS"
 
        local OST0_MIN=$((6 * 1024)) # 6MB, extra space for meta blocks.
        check_whether_skip && return 0
@@ -2687,7 +2685,7 @@ run_test 35 "Usage is still accessible across reboot"
 # LU-5006
 test_37() {
        [ $(lustre_version_code $SINGLEMDS) -lt $(version_code 2.6.93) ] &&
-               skip "Old server doesn't have LU-5006 fix." && return
+               skip "Old server doesn't have LU-5006 fix."
 
        setup_quota_test || error "setup quota failed with $?"
        trap cleanup_quota_test EXIT
@@ -2719,7 +2717,7 @@ run_test 37 "Quota accounted properly for file created by 'lfs setstripe'"
 # LU-8801
 test_38() {
        [ $(lustre_version_code $SINGLEMDS) -lt $(version_code 2.8.60) ] &&
-               skip "Old server doesn't have LU-8801 fix." && return
+               skip "Old server doesn't have LU-8801 fix."
 
        [ "$UID" != 0 ] && skip_env "must run as root" && return
 
@@ -2767,7 +2765,7 @@ run_test 38 "Quota accounting iterator doesn't skip id entries"
 test_39() {
        local TESTFILE="$DIR/$tdir/project"
        ! is_project_quota_supported &&
-               skip "Project quota is not supported" && return 0
+               skip "Project quota is not supported"
 
        setup_quota_test || error "setup quota failed with $?"
 
@@ -2793,7 +2791,7 @@ run_test 39 "Project ID interface works correctly"
 
 test_40a() {
        ! is_project_quota_supported &&
-               skip "Project quota is not supported" && return 0
+               skip "Project quota is not supported"
        local dir1="$DIR/$tdir/dir1"
        local dir2="$DIR/$tdir/dir2"
 
@@ -2813,7 +2811,7 @@ run_test 40a "Hard link across different project ID"
 
 test_40b() {
        ! is_project_quota_supported &&
-               skip "Project quota is not supported" && return 0
+               skip "Project quota is not supported"
        local dir1="$DIR/$tdir/dir1"
        local dir2="$DIR/$tdir/dir2"
 
@@ -2833,9 +2831,9 @@ test_40b() {
 run_test 40b "Mv across different project ID"
 
 test_40c() {
-       [ "$MDSCOUNT" -lt "2" ] && skip "needs >= 2 MDTs" && return
+       [ "$MDSCOUNT" -lt "2" ] && skip "needs >= 2 MDTs"
                ! is_project_quota_supported &&
-                       skip "Project quota is not supported" && return 0
+                       skip "Project quota is not supported"
 
        setup_quota_test || error "setup quota failed with $?"
        local dir="$DIR/$tdir/dir"
@@ -2863,7 +2861,7 @@ run_test 40c "Remote child Dir inherit project quota properly"
 
 test_50() {
        ! is_project_quota_supported &&
-               skip "Project quota is not supported" && return 0
+               skip "Project quota is not supported"
 
        setup_quota_test || error "setup quota failed with $?"
        local dir="$DIR/$tdir/dir"
@@ -2879,7 +2877,7 @@ run_test 50 "Test if lfs find --projid works"
 
 test_51() {
        ! is_project_quota_supported &&
-               skip "Project quota is not supported" && return 0
+               skip "Project quota is not supported"
        setup_quota_test || error "setup quota failed with $?"
        local dir="$DIR/$tdir/dir"
 
@@ -2911,7 +2909,7 @@ run_test 51 "Test project accounting with mv/cp"
 
 test_52() {
        ! is_project_quota_supported &&
-               skip "Project quota is not supported" && return 0
+               skip "Project quota is not supported"
        setup_quota_test || error "setup quota failed with $?"
        local dir="$DIR/$tdir/dir"
        mkdir $dir && change_project -sp 1 $dir
@@ -2929,7 +2927,7 @@ run_test 52 "Rename across different project ID"
 
 test_53() {
        ! is_project_quota_supported &&
-               skip "Project quota is not supported" && return 0
+               skip "Project quota is not supported"
        setup_quota_test || error "setup quota failed with $?"
        local dir="$DIR/$tdir/dir"
        mkdir $dir && change_project -s $dir
@@ -2946,7 +2944,7 @@ run_test 53 "Project inherit attribute could be cleared"
 
 test_54() {
        ! is_project_quota_supported &&
-               skip "Project quota is not supported" && return 0
+               skip "Project quota is not supported"
        setup_quota_test || error "setup quota failed with $?"
        trap cleanup_quota_test EXIT
        local testfile="$DIR/$tdir/$tfile-0"
@@ -3000,7 +2998,7 @@ run_test 54 "basic lfs project interface test"
 
 test_55() {
        [ $(lustre_version_code $SINGLEMDS) -lt $(version_code 2.10.58) ] &&
-               skip "Not supported before 2.10.58." && return
+               skip "Not supported before 2.10.58."
        setup_quota_test || error "setup quota failed with $?"
 
        set_ost_qtype $QTYPE || error "enable ost quota failed"
@@ -3083,10 +3081,8 @@ test_57() {
 run_test 57 "lfs project could tolerate errors"
 
 test_59() {
-       if [ $(facet_fstype $SINGLEMDS) != ldiskfs ]; then
+       [ "$(facet_fstype $SINGLEMDS)" != "ldiskfs" ] &&
                skip "ldiskfs only test"
-               return
-       fi
        disable_project_quota
        setup_quota_test || error "setup quota failed with $?"
        quota_init
@@ -3131,7 +3127,7 @@ run_test 60 "Test quota for root with setgid"
 # test default quota
 test_default_quota() {
        [ $(lustre_version_code $SINGLEMDS) -lt $(version_code 2.11.51) ] &&
-               skip "Not supported before 2.11.51." && return
+               skip "Not supported before 2.11.51."
 
        local qtype=$1
        local qpool=$2
@@ -3300,6 +3296,30 @@ test_61() {
 }
 run_test 61 "default quota tests"
 
+test_62() {
+       ! is_project_quota_supported &&
+               skip "Project quota is not supported"
+        [[ "$(chattr -h 2>&1)" =~ "project" ]] ||
+               skip "chattr did not support project quota"
+       setup_quota_test || error "setup quota failed with $?"
+       local testdir=$DIR/$tdir/
+
+       $RUNAS mkdir -p $testdir || error "failed to mkdir"
+       change_project -s $testdir
+       [[ $($LFS project -d $testdir) =~ "P" ]] ||
+               error "inherit attribute should be set"
+       # chattr used FS_IOC_SETFLAGS ioctl
+       $RUNAS chattr -P $testdir &&
+               error "regular user clear inherit should fail"
+       [[ $($LFS project -d $testdir) =~ "P" ]] ||
+               error "inherit attribute should still be set"
+       chattr -P $testdir || error "root failed to clear inherit"
+       [[ $($LFS project -d $testdir) =~ "P" ]] &&
+               error "inherit attribute should be cleared"
+       cleanup_quota_test
+}
+run_test 62 "Project inherit should be only changed by root"
+
 quota_fini()
 {
        do_nodes $(comma_list $(nodes_list)) "lctl set_param debug=-quota"