LU-18095 sec: fix ACL handling on recent kernels

On recent distributions like Ubuntu 24.04, the kernel imposes that
ACLs are fetched via the dedicated .get_acl operation (or
.get_inode_acl) instead of doing this via the xattr handlers.
So in ll_get_acl() explicitly fetch the xattr containing ACLs,
XATTR_NAME_ACL_ACCESS or XATTR_NAME_ACL_DEFAULT. This is going to
populate to xattr cache, hence avoiding multiple requests to the MDS.

Also fix sanity-sec test_23b to make sure variable comparisons are
correct. And fix test cleanup to avoid leftovers.

Test-Parameters: testlist=sanity-sec env=ONLY=23b,ONLY_REPEAT=50 mdscount=2 mdtcount=4 osscount=1 ostcount=8 clientcount=2
Test-Parameters: testlist=sanity-sec env=ONLY=23b,ONLY_REPEAT=50 mdscount=2 mdtcount=4 osscount=1 ostcount=8 clientcount=2 clientdistro=ubuntu2404 serverdistro=el9.4
Test-Parameters: testlist=sanity env=ONLY=103a,ONLY_REPEAT=50 mdscount=2 mdtcount=4 osscount=1 ostcount=8 clientcount=2 clientdistro=ubuntu2404 serverdistro=el9.4
Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
Change-Id: I467d5a558eaa524e823527a8798478934f65abf9
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/56254
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Jian Yu <yujian@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/llite/acl.c b/lustre/llite/acl.c
index 7ae8f13..2afd352 100644 (file)
--- a/lustre/llite/acl.c
+++ b/lustre/llite/acl.c
@@ -52,6 +52,10 @@ struct posix_acl *ll_get_acl(
 #endif
        struct ll_inode_info *lli = ll_i2info(inode);
        struct posix_acl *acl = NULL;
+       char *value = NULL;
+       int len, xtype;
+       char buf[200];
+       char *xname;
        ENTRY;
 
 #ifdef HAVE_GET_ACL_RCU_ARG
@@ -59,11 +63,58 @@ struct posix_acl *ll_get_acl(
                return ERR_PTR(-ECHILD);
 #endif
 
+       if (type == ACL_TYPE_ACCESS && lli->lli_posix_acl)
+               goto lli_acl;
+
+       switch (type) {
+       case ACL_TYPE_ACCESS:
+               xname = XATTR_NAME_ACL_ACCESS;
+               xtype = XATTR_ACL_ACCESS_T;
+               break;
+       case ACL_TYPE_DEFAULT:
+               xname = XATTR_NAME_ACL_DEFAULT;
+               xtype = XATTR_ACL_DEFAULT_T;
+               break;
+       default:
+               return ERR_PTR(-EINVAL);
+       }
+
+       len = ll_xattr_list(inode, xname, xtype, NULL, 0, OBD_MD_FLXATTR);
+       if (len > 0) {
+               if (len > sizeof(buf))
+                       value = kmalloc(len, GFP_NOFS);
+               else
+                       value = buf;
+               if (!value)
+                       return ERR_PTR(-ENOMEM);
+               len = ll_xattr_list(inode, xname, xtype, value, len,
+                                   OBD_MD_FLXATTR);
+       }
+       if (len > 0)
+               acl = posix_acl_from_xattr(&init_user_ns, value, len);
+       else if (len == -ENODATA || len == -ENOSYS)
+               acl = NULL;
+       else
+               acl = ERR_PTR(len);
+       if (value && value != buf)
+               kfree(value);
+
+       if (IS_ERR_OR_NULL(acl))
+               goto out;
+       if (type == ACL_TYPE_DEFAULT) {
+               acl = posix_acl_dup(acl);
+               goto out;
+       }
+       if (type == ACL_TYPE_ACCESS)
+               lli_replace_acl(lli, acl);
+
+lli_acl:
        read_lock(&lli->lli_lock);
        /* VFS' acl_permission_check->check_acl will release the refcount */
        acl = posix_acl_dup(lli->lli_posix_acl);
        read_unlock(&lli->lli_lock);
 
+out:
        RETURN(acl);
 }
 

diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h
index 557370e..c354030 100644 (file)
--- a/lustre/llite/llite_internal.h
+++ b/lustre/llite/llite_internal.h
@@ -487,12 +487,12 @@ static inline void lli_clear_acl(struct ll_inode_info *lli)
 }
 
 static inline void lli_replace_acl(struct ll_inode_info *lli,
-                                  struct lustre_md *md)
+                                  struct posix_acl *acl)
 {
        write_lock(&lli->lli_lock);
        if (lli->lli_posix_acl)
                posix_acl_release(lli->lli_posix_acl);
-       lli->lli_posix_acl = md->posix_acl;
+       lli->lli_posix_acl = acl;
        write_unlock(&lli->lli_lock);
 }
 #else
@@ -501,7 +501,7 @@ static inline void lli_clear_acl(struct ll_inode_info *lli)
 }
 
 static inline void lli_replace_acl(struct ll_inode_info *lli,
-                                  struct lustre_md *md)
+                                  struct posix_acl *acl)
 {
 }
 #endif

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index a7828ef..00ad633 100644 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -2817,7 +2817,7 @@ int ll_update_inode(struct inode *inode, struct lustre_md *md)
        }
 
        if (body->mbo_valid & OBD_MD_FLACL)
-               lli_replace_acl(lli, md);
+               lli_replace_acl(lli, md->posix_acl);
 
        api32 = test_bit(LL_SBI_32BIT_API, sbi->ll_flags);
        inode->i_ino = cl_fid_build_ino(&body->mbo_fid1, api32);

diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh
index b1a4638..d5bde69 100755 (executable)
--- a/lustre/tests/sanity-sec.sh
+++ b/lustre/tests/sanity-sec.sh
@@ -1950,13 +1950,14 @@ test_23a() {
 run_test 23a "test mapped regular ACLs"
 
 test_23b() { #LU-9929
-       [ $num_clients -lt 2 ] && skip "Need 2 clients at least"
-       [ "$MGS_VERSION" -lt $(version_code 2.10.53) ] &&
+       (( num_clients >= 2 )) || skip "Need 2 clients at least"
+       (( $MGS_VERSION >= $(version_code 2.10.53) )) ||
                skip "Need MGS >= 2.10.53"
 
+       stack_trap "export SK_UNIQUE_NM=$SK_UNIQUE_NM"
        export SK_UNIQUE_NM=true
        nodemap_test_setup
-       trap nodemap_test_cleanup EXIT
+       stack_trap nodemap_test_cleanup EXIT
 
        local testdir=$DIR/$tdir
        local fs_id=$((IDBASE+10))
@@ -1980,26 +1981,38 @@ test_23b() { #LU-9929
        # set/getfacl default acl on client 1 (unmapped gid=500)
        do_node ${clients_arr[0]} rm -rf $testdir
        do_node ${clients_arr[0]} mkdir -p $testdir
+       echo "$testdir ACLs after mkdir:"
+       do_node ${clients_arr[0]} getfacl $testdir
        # Here, USER0=$(getent passwd | grep :$ID0:$ID0: | cut -d: -f1)
        do_node ${clients_arr[0]} setfacl -R -d -m group:$USER0:rwx $testdir ||
                error "setfacl $testdir on ${clients_arr[0]} failed"
+       do_node ${clients_arr[0]} "sync && stat $testdir > /dev/null"
+       do_node ${clients_arr[0]} \
+               $LCTL set_param -t4 -n "ldlm.namespaces.*.lru_size=clear"
+       echo "$testdir ACLs after setfacl, on ${clients_arr[0]}:"
+       do_node ${clients_arr[0]} getfacl $testdir
        unmapped_id=$(do_node ${clients_arr[0]} getfacl $testdir |
-                       grep -E "default:group:.*:rwx" | awk -F: '{print $3}')
-       [ "$unmapped_id" = "$USER0" ] ||
+                       grep -E "default:group:.+:rwx" | awk -F: '{print $3}')
+       echo unmapped_id=$unmapped_id
+       (( unmapped_id == USER0 )) ||
                error "gid=$ID0 was not unmapped correctly on ${clients_arr[0]}"
 
        # getfacl default acl on client 2 (mapped gid=60010)
+       do_node ${clients_arr[1]} \
+               $LCTL set_param -t4 -n "ldlm.namespaces.*.lru_size=clear"
+       do_node ${clients_arr[1]} "sync && stat $testdir > /dev/null"
+       echo "$testdir ACLs after setfacl, on ${clients_arr[1]}:"
+       do_node ${clients_arr[1]} getfacl $testdir
        mapped_id=$(do_node ${clients_arr[1]} getfacl $testdir |
-                       grep -E "default:group:.*:rwx" | awk -F: '{print $3}')
+                       grep -E "default:group:.+:rwx" | awk -F: '{print $3}')
+       echo mapped_id=$mapped_id
+       [[ -n "$mapped_id" ]] || error "mapped_id empty"
        fs_user=$(do_node ${clients_arr[1]} getent passwd |
                        grep :$fs_id:$fs_id: | cut -d: -f1)
-       [ -z "$fs_user" ] && fs_user=$fs_id
-       [ $mapped_id -eq $fs_id -o "$mapped_id" = "$fs_user" ] ||
-               error "Should return gid=$fs_id or $fs_user on client2"
-
-       rm -rf $testdir
-       nodemap_test_cleanup
-       export SK_UNIQUE_NM=false
+       [[ -n "$fs_user" ]] || fs_user=$fs_id
+       echo fs_user=$fs_user
+       (( mapped_id == fs_id || mapped_id == fs_user )) ||
+               error "Should return user $fs_user id $fs_id on client2"
 }
 run_test 23b "test mapped default ACLs"