[LU-16210] Lustre on RHEL8 with selinux disabled tries to retrieve security.selinux xattr Created: 05/Oct/22 Updated: 28/Mar/23 Resolved: 13/Jan/23 |
|
| Status: | Resolved |
| Project: | Lustre |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | Lustre 2.16.0, Lustre 2.15.3 |
| Type: | Bug | Priority: | Major |
| Reporter: | Etienne Aujames | Assignee: | Etienne Aujames |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | SELinux, performance | ||
| Severity: | 3 |
| Rank (Obsolete): | 9223372036854775807 |
| Description |
|
On Centos7, selinux_is_enabled() was used to prevent this but on RHEL8 this interface disappears (on 5.1 kernel): static int ll_xattr_get_common() ... /* LU-549: Disable security.selinux when selinux is disabled */ if (handler->flags == XATTR_SECURITY_T && !selinux_is_enabled() && !strcmp(name, "selinux")) RETURN(-EOPNOTSUPP); The patch https://review.whamcloud.com/38480 " So on RHEL8 with selinux disabled, we send unneeded RPCs on the MDS to retrieved security.selinux xattr. This has performance issues. Here the behavior with "ls": Centos 7 :
getenforce 0
Disabled
strace -e getxattr,lgetxattr ls -lda “directory”
lgetxattr("directory", "security.selinux", 0x1096100, 255) = -1 EOPNOTSUPP (Operation not supported)
getxattr("directory", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("directory", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
Centos 8 :
getenforce 0
Disabled
lgetxattr("directory", "security.selinux", 0x1096100, 255) = -1 ENODATA (No data available)
getxattr("directory", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("directory", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
Here the "perf report" diff on getfattr: Centos 7 : + 10.59% 0.00% getfattr libc-2.17.so [.] __GI___lxstat64 (inlined) + 10.59% 0.00% getfattr [kernel.vmlinux] [k] system_call + 10.59% 0.00% getfattr [kernel.vmlinux] [k] sys_newlstat + 10.59% 0.00% getfattr [kernel.vmlinux] [k] SYSC_newlstat + 10.59% 0.00% getfattr [kernel.vmlinux] [k] vfs_fstatat + 10.59% 0.00% getfattr [kernel.vmlinux] [k] user_path_at + 10.59% 0.00% getfattr [kernel.vmlinux] [k] user_path_at_empty + 10.59% 0.00% getfattr [kernel.vmlinux] [k] filename_lookup + 10.59% 0.00% getfattr [kernel.vmlinux] [k] path_lookupat + 10.59% 0.00% getfattr [kernel.vmlinux] [k] link_path_walk + 10.59% 0.00% getfattr [kernel.vmlinux] [k] inode_permission + 10.59% 0.00% getfattr [kernel.vmlinux] [k] __inode_permission + 10.59% 0.00% getfattr [lustre] [k] ll_inode_permission + 10.59% 0.00% getfattr [lustre] [k] ll_inode_revalidate + 10.59% 0.00% getfattr [lmv] [k] lmv_intent_lock Centos 8 : + 15.74% 0.00% getfattr libc-2.28.so [.] __GI_getxattr (inlined) + 15.74% 0.00% getfattr [kernel.kallsyms] [k] path_getxattr + 15.74% 0.00% getfattr [kernel.kallsyms] [k] getxattr + 15.74% 0.00% getfattr [kernel.kallsyms] [k] vfs_getxattr + 15.74% 0.00% getfattr [kernel.kallsyms] [k] __vfs_getxattr + 15.74% 0.00% getfattr [kernel.kallsyms] [k] ll_xattr_get_common + 15.74% 0.00% getfattr [kernel.kallsyms] [k] ll_xattr_list + 15.74% 0.00% getfattr [kernel.kallsyms] [k] lmv_getxattr + 15.74% 0.00% getfattr [kernel.kallsyms] [k] mdc_getxattr + 15.74% 0.00% getfattr [kernel.kallsyms] [k] mdc_xattr_common <---------- + 15.74% 0.00% getfattr [kernel.kallsyms] [k] ptlrpc_queue_wait + 15.74% 0.00% getfattr [kernel.kallsyms] [k] ptlrpc_set_wait + 15.74% 0.00% getfattr [kernel.kallsyms] [k] ptlrpc_check_set.part |
| Comments |
| Comment by Etienne Aujames [ 05/Oct/22 ] | |||||||||
|
The CEA proposes to implement something like Ceph/NFS to fix this issue: bool ceph_security_xattr_wanted(struct inode *in) { return in->i_security != NULL; } | |||||||||
| Comment by Sebastien Buisson [ 05/Oct/22 ] | |||||||||
|
Thanks for reporting this. Centos 7 :
getenforce 0
Disabled
strace -e getxattr,lgetxattr ls -lda “directory”
lgetxattr("directory", "security.selinux", 0x1096100, 255) = -1 EOPNOTSUPP (Operation not supported)
getxattr("directory", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("directory", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
Centos 8 :
getenforce 0
Disabled
lgetxattr("directory", "security.selinux", 0x1096100, 255) = -1 ENODATA (No data available)
getxattr("directory", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("directory", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
Except the different return code (EOPNOTSUPP vs ENODATA), I do not see a different behavior between CentOS 7 and CentOS 8. Both ls are doing a getxattr on security.selinux, so what is the problem you are seeing? | |||||||||
| Comment by Aurelien Cedeyn [ 05/Oct/22 ] | |||||||||
|
Hi Sebastien, The problem is, when ENODATA is reported, it means an action on the MDT side, but not when EOPNOTSUPP, the client will not try to fetch any security xattrs. In Etienne behavior description, `ls` check only the directory dentry, but with the full `ls` of the directory, you can see : ## CentOS 8
$ strace -e getxattr,lgetxattr ls -l
lgetxattr("squashfs", "security.selinux", 0x564355468230, 255) = -1 ENODATA (No data available)
getxattr("squashfs", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("squashfs", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
lgetxattr("repository.app", "security.selinux", 0x564355477cc0, 255) = -1 ENODATA (No data available)
getxattr("repository.app", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("repository.app", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
lgetxattr("repository", "security.selinux", 0x564355477dd0, 255) = -1 ENODATA (No data available)
getxattr("repository", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("repository", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
lgetxattr("libpcocc_slurm.so", "security.selinux", 0x564355478cf0, 255) = -1 ENODATA (No data available)
getxattr("libpcocc_slurm.so", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
lgetxattr("ubuntu-mate", "security.selinux", 0x564355478e20, 255) = -1 ENODATA (No data available)
getxattr("ubuntu-mate", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("ubuntu-mate", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
## CentOS 7
$ strace -e getxattr,lgetxattr ls -l
lgetxattr("squashfs", "security.selinux", 0xba0140, 255) = -1 EOPNOTSUPP (Operation not supported)
getxattr("squashfs", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("squashfs", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
getxattr("repository.app", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("repository.app", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
getxattr("repository", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("repository", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
getxattr("libpcocc_slurm.so", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("libpcocc_slurm.so", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
getxattr("ubuntu-mate", "system.posix_acl_access", NULL, 0) = -1 ENODATA (No data available)
getxattr("ubuntu-mate", "system.posix_acl_default", NULL, 0) = -1 ENODATA (No data available)
So there are some optimisation in userspace saying that if a lgetxattr returns EOPNOTSUPP, don't try to gather selinux xattr anymore. | |||||||||
| Comment by Etienne Aujames [ 05/Oct/22 ] | |||||||||
|
There is no issue from users/applications standpoint. | |||||||||
| Comment by Sebastien Buisson [ 05/Oct/22 ] | |||||||||
|
OK, it is clear to me now, thanks. I looked at how the i_security field is set in the kernel. Only SELinux sets it, via the following path: alloc_inode
inode_init_always
security_inode_alloc --> sets i_security to NULL
inode_alloc_security
selinux_inode_alloc_security
inode_alloc_security --> sets i_security to non-NULL
So indeed, we could consider that once inode has been allocated, i_security is not NULL if SELinux is enabled. | |||||||||
| Comment by Sebastien Buisson [ 05/Oct/22 ] | |||||||||
|
eaujames please add me as a reviewer to the patch you are cooking. | |||||||||
| Comment by Gerrit Updater [ 06/Oct/22 ] | |||||||||
|
"Etienne AUJAMES <eaujames@ddn.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/48796 | |||||||||
| Comment by Gerrit Updater [ 14/Oct/22 ] | |||||||||
|
"Etienne AUJAMES <eaujames@ddn.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/48875 | |||||||||
| Comment by Etienne Aujames [ 27/Oct/22 ] | |||||||||
|
*https://review.whamcloud.com/48796 : implements a solution with a simple i_security check. | |||||||||
| Comment by Etienne Aujames [ 27/Oct/22 ] | |||||||||
|
Those patches restore the behavior of the http://review.whamcloud.com/2503 when SElinux is disabled: commit 7cc542fd4c26ccb117ceb13a47ac8ced3107b9b3
Author: Yevheniy Demchenko <zheka@uvt.cz>
Date: Tue Apr 10 22:01:14 2012 +0200
LU-549 llite: Improve statfs performance if selinux is disabled
Even if selinux is disabled, client still tries to get selinux
attributes from MDS. As xattrs are not yet cached, this significantly
slows down xattr heavy operations like ls -l. This patch forces
to return -EOPNOTSUPP on the client side if selinux is disabled.
It speeds up ls -l 25% for cold-cache case and 50% for hot-cache
case.
Tests with "strace -c ls -l" with 100000 files on root in a multi VMs env (on Rocky 9). FS is remounted for each tests (cache is cleaned) and selinux is disabled.
"ls -l" uses lgetxattr to get "security.selinux". | |||||||||
| Comment by Gerrit Updater [ 13/Jan/23 ] | |||||||||
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/48875/ | |||||||||
| Comment by Peter Jones [ 13/Jan/23 ] | |||||||||
|
Landed for 2.16 | |||||||||
| Comment by Gerrit Updater [ 13/Jan/23 ] | |||||||||
|
"Sebastien Buisson <sbuisson@ddn.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/49630 | |||||||||
| Comment by Gerrit Updater [ 24/Mar/23 ] | |||||||||
|
"Etienne AUJAMES <eaujames@ddn.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50404 | |||||||||
| Comment by Gerrit Updater [ 28/Mar/23 ] | |||||||||
|
"Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/49630/ |