From aa636f8ae6883cef018b859bba70140df9a826c4 Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Wed, 4 Sep 2024 10:36:42 +0200 Subject: [PATCH] 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 Change-Id: I467d5a558eaa524e823527a8798478934f65abf9 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/56254 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Jian Yu Reviewed-by: Oleg Drokin --- lustre/llite/acl.c | 51 +++++++++++++++++++++++++++++++++++++++++++ lustre/llite/llite_internal.h | 6 ++--- lustre/llite/llite_lib.c | 2 +- lustre/tests/sanity-sec.sh | 39 ++++++++++++++++++++++----------- 4 files changed, 81 insertions(+), 17 deletions(-) diff --git a/lustre/llite/acl.c b/lustre/llite/acl.c index 7ae8f13..2afd352 100644 --- 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 --- 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 --- 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 --- 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" -- 1.8.3.1