Whamcloud - gitweb
LU-16210 llite: replace selinux_is_enabled() 75/48875/10
authorEtienne AUJAMES <etienne.aujames@cea.fr>
Thu, 6 Oct 2022 13:30:54 +0000 (15:30 +0200)
committerOleg Drokin <green@whamcloud.com>
Fri, 13 Jan 2023 07:17:25 +0000 (07:17 +0000)
selinux_is_enabled() was removed from kernel 5.1.
The commit 39e5bfa add the kernel support by assuming SELinux to be
enabled if the function selinux_is_enabled() does not exist.

This has performances impacts: on older kernel (e.g: Centos7) getxattr
RPCs was not send for "security.selinux" if selinux was disabled.
Utilities like "ls -l" always try to get "security.selinux".
See the LU-549 for more information.

This patch uses security_inode_listsecurity() when mounting the
client to know if a LSM module (selinux) required a xattr to store
file contexts. If a xattr is returned we store it and use it for in
request security context.

For getxattr/setxattr we use the stored LSM's xattr to filter xattr
security contexts like security.selinux. If xattr does not match the
stored xattr name we returned -EOPNOTSUPP to userspace.

It adds also the s_security check for security_inode_notifysecctx() to
avoid calling this function if selinux is disabled (as in
nfs_setsecurity()).

For "Enforcing SELinux Policy Check" functionnality, the selinux check
have been moved in l_getsepol: -ENODEV is returned if selinux is
disabled.

Add a regresion test "sanity test_434" for this use case.

*Note:*
This patch detects that selinux is disabled without explicitly
disabled it in kernel cmdline. This is recommended for RHEL >= 8.5.

*Performances:*
Tests with "strace -c ls -l" with 100000 files on root in a multi VMs
env (on Rocky 9). FS is remount for each tests (cache is cleaned) and
selinux is disabled.
 __________________ ___________ _________
| Total time %     | lgetxattr | statx   |
|__________________|___________|_________|
|Without the patch:|    29%    |   51%   |
|__________________|___________|_________|
|With the patch:   |    0%     |   87%   |
|__________________|___________|_________|
"ls -l" uses lgetxattr to get "security.selinux".

Linux-commit: 3d252529480c68bfd6a6774652df7c8968b28e41

Fixes: 39e5bfa ("LU-12355 llite: include file linux/selinux.h removed")
Fixes: 9bcac0b ("LU-549 llite: Improve statfs performance if selinux is disabled")
Test-Parameters: clientselinux=false clientdistro=el7.9 testlist=sanity env=ONLY=434,ONLY_REPEAT=20
Test-Parameters: clientselinux=false clientdistro=el8.5 testlist=sanity env=ONLY=434,ONLY_REPEAT=20
Test-Parameters: clientselinux=false clientdistro=el8.6 testlist=sanity env=ONLY=434,ONLY_REPEAT=20
Test-Parameters: clientselinux clientdistro=el8.6 testlist=sanity-selinux
Test-Parameters: clientselinux clientdistro=el8.6 testlist=sanity-selinux
Test-Parameters: clientselinux clientdistro=el7.9 testlist=sanity-selinux
Test-Parameters: clientselinux clientdistro=el7.9 testlist=sanity-selinux
Signed-off-by: Etienne AUJAMES <etienne.aujames@cea.fr>
Change-Id: I4dac87ac0341b45a1c2fef836cdce0361017b3f5
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/48875
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Sebastien Buisson <sbuisson@ddn.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
12 files changed:
lustre/autoconf/lustre-core.m4
lustre/include/lustre_compat.h
lustre/llite/dir.c
lustre/llite/llite_internal.h
lustre/llite/llite_lib.c
lustre/llite/namei.c
lustre/llite/xattr.c
lustre/llite/xattr_cache.c
lustre/llite/xattr_security.c
lustre/ptlrpc/sec.c
lustre/tests/sanity.sh
lustre/utils/l_getsepol.c

index 99a30be..8ccc779 100644 (file)
@@ -2459,28 +2459,6 @@ EXTRA_KCFLAGS="$tmp_flags"
 ]) # LC_HAVE_SUNRPC_CACHE_HASH_LOCK_IS_A_SPINLOCK
 
 #
-# LC_HAS_LINUX_SELINUX_ENABLED
-#
-# kernel 5.1 commit 3d252529480c68bfd6a6774652df7c8968b28e41
-# SELinux: Remove unused selinux_is_enabled
-#
-AC_DEFUN([LC_HAS_LINUX_SELINUX_ENABLED], [
-tmp_flags="$EXTRA_KCFLAGS"
-EXTRA_KCFLAGS="-Werror"
-LB_CHECK_COMPILE([if linux/selinux.h exists],
-selinux_is_enabled, [
-       #include <linux/selinux.h>
-],[
-       bool has_selinux = selinux_is_enabled();
-       (void)has_selinux;
-],[
-       AC_DEFINE(HAVE_LINUX_SELINUX_IS_ENABLED, 1,
-               [if linux/selinux.h exists])
-])
-EXTRA_KCFLAGS="$tmp_flags"
-]) # LC_HAS_LINUX_SELINUX_ENABLED
-
-#
 # LB_HAVE_BVEC_ITER_ALL
 #
 # kernel 5.1 commit 6dc4f100c175dd0511ae8674786e7c9006cdfbfa
@@ -2999,7 +2977,6 @@ AC_DEFUN([LC_PROG_LINUX], [
        LC_HAVE_SUNRPC_CACHE_HASH_LOCK_IS_A_SPINLOCK
 
        # 5.1
-       LC_HAS_LINUX_SELINUX_ENABLED
        LB_HAVE_BVEC_ITER_ALL
 
        # 5.2
index e8c6128..aca622a 100644 (file)
@@ -446,9 +446,14 @@ static inline struct timespec current_time(struct inode *inode)
        kmem_cache_create(name, size, align, flags, ctor)
 #endif
 
-#ifndef HAVE_LINUX_SELINUX_IS_ENABLED
-#define selinux_is_enabled() 1
+static inline bool ll_security_xattr_wanted(struct inode *in)
+{
+#ifdef CONFIG_SECURITY
+       return in->i_security && in->i_sb->s_security;
+#else
+       return false;
 #endif
+}
 
 static inline int ll_vfs_getxattr(struct dentry *dentry, struct inode *inode,
                                  const char *name,
index b692661..869fc9f 100644 (file)
@@ -458,6 +458,7 @@ static int ll_dir_setdirstripe(struct dentry *dparent, struct lmv_user_md *lump,
                        .hash = ll_full_name_hash(dparent, dirname,
                                                  strlen(dirname)),
                },
+               .d_sb = dparent->d_sb,
        };
        bool encrypt = false;
        int hash_flags;
@@ -539,9 +540,9 @@ static int ll_dir_setdirstripe(struct dentry *dparent, struct lmv_user_md *lump,
                /* selinux_dentry_init_security() uses dentry->d_parent and name
                 * to determine the security context for the file. So our fake
                 * dentry should be real enough for this purpose. */
-               err = ll_dentry_init_security(parent,
-                                             &dentry, mode, &dentry.d_name,
+               err = ll_dentry_init_security(&dentry, mode, &dentry.d_name,
                                              &op_data->op_file_secctx_name,
+                                             &op_data->op_file_secctx_name_size,
                                              &op_data->op_file_secctx,
                                              &op_data->op_file_secctx_size);
                if (err < 0)
@@ -573,17 +574,12 @@ static int ll_dir_setdirstripe(struct dentry *dparent, struct lmv_user_md *lump,
 
        dentry.d_inode = inode;
 
-       if (test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags)) {
-               /* no need to protect selinux_inode_setsecurity() by
-                * inode_lock. Taking it would lead to a client deadlock
-                * LU-13617
-                */
-               err = security_inode_notifysecctx(inode,
-                                                 op_data->op_file_secctx,
-                                                 op_data->op_file_secctx_size);
-       } else {
+       if (test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags))
+               err = ll_inode_notifysecctx(inode, op_data->op_file_secctx,
+                                           op_data->op_file_secctx_size);
+       else
                err = ll_inode_init_security(&dentry, inode, parent);
-       }
+
        if (err)
                GOTO(out_inode, err);
 
index 6a1161f..45ab7b1 100644 (file)
@@ -471,15 +471,36 @@ static inline void obd_connect_set_secctx(struct obd_connect_data *data)
 #endif
 }
 
-int ll_dentry_init_security(struct inode *parent, struct dentry *dentry,
-                           int mode, struct qstr *name,
-                           const char **secctx_name, void **secctx,
-                           __u32 *secctx_size);
+/* Only smack and selinux is known to use security contexts */
+static inline bool ll_xattr_is_seclabel(const char *name)
+{
+       return !strcmp(name, XATTR_NAME_SELINUX) ||
+               !strcmp(name, XATTR_NAME_SMACK);
+}
+
+static inline bool ll_xattr_suffix_is_seclabel(const char *suffix)
+{
+       return !strcmp(suffix, XATTR_SELINUX_SUFFIX) ||
+               !strcmp(suffix, XATTR_SMACK_SUFFIX);
+}
+
+int ll_dentry_init_security(struct dentry *dentry, int mode, struct qstr *name,
+                           const char **secctx_name, __u32 *secctx_name_size,
+                           void **secctx, __u32 *secctx_size);
 int ll_inode_init_security(struct dentry *dentry, struct inode *inode,
                           struct inode *dir);
 
-int ll_listsecurity(struct inode *inode, char *secctx_name,
-                   size_t secctx_name_size);
+int ll_inode_notifysecctx(struct inode *inode,
+                         void *secctx, __u32 secctxlen);
+
+void ll_secctx_name_free(struct ll_sb_info *sbi);
+
+int ll_secctx_name_store(struct inode *in);
+
+__u32 ll_secctx_name_get(struct ll_sb_info *sbi, const char **secctx_name);
+
+int ll_security_secctx_name_filter(struct ll_sb_info *sbi, int xattr_type,
+                                  const char *suffix);
 
 static inline bool obd_connect_has_enc(struct obd_connect_data *data)
 {
@@ -814,6 +835,10 @@ struct ll_sb_info {
        struct ll_foreign_symlink_upcall_item *ll_foreign_symlink_upcall_items;
        /* foreign symlink path upcall nb infos */
        unsigned int              ll_foreign_symlink_upcall_nb_items;
+
+       /* cached file security context xattr name. e.g: security.selinux */
+       char *ll_secctx_name;
+       __u32 ll_secctx_name_size;
 };
 
 #define SBI_DEFAULT_HEAT_DECAY_WEIGHT  ((80 * 256 + 50) / 100)
index e097c33..e67ab23 100644 (file)
@@ -147,6 +147,9 @@ static struct ll_sb_info *ll_init_sbi(void)
         * not enabled by default
         */
 
+       sbi->ll_secctx_name = NULL;
+       sbi->ll_secctx_name_size = 0;
+
        sbi->ll_ra_info.ra_max_pages =
                min(pages / 32, SBI_DEFAULT_READ_AHEAD_MAX);
        sbi->ll_ra_info.ra_max_pages_per_file =
@@ -257,6 +260,9 @@ static void ll_free_sbi(struct super_block *sb)
                                sizeof(struct ll_foreign_symlink_upcall_item));
                        sbi->ll_foreign_symlink_upcall_items = NULL;
                }
+               if (sbi->ll_secctx_name)
+                       ll_secctx_name_free(sbi);
+
                ll_free_rw_stats_info(sbi);
                pcc_super_fini(&sbi->ll_pcc_super);
                OBD_FREE(sbi, sizeof(*sbi));
@@ -726,6 +732,12 @@ retry_connect:
                GOTO(out_root, err);
        }
 
+       err = ll_secctx_name_store(root);
+       if (err < 0 && ll_security_xattr_wanted(root))
+               CWARN("%s: file security contextes not supported: rc = %d\n",
+                     sbi->ll_fsname, err);
+
+       err = 0;
        if (encctxlen) {
                CDEBUG(D_SEC,
                       "server returned encryption ctx for root inode "DFID"\n",
index 9d2e37e..549b887 100644 (file)
@@ -734,19 +734,8 @@ static int ll_lookup_it_finish(struct ptlrpc_request *request,
                                       PFID(ll_inode2fid(inode)));
                }
 
-               if (secctx != NULL && secctxlen != 0) {
-                       /* no need to protect selinux_inode_setsecurity() by
-                        * inode_lock. Taking it would lead to a client deadlock
-                        * LU-13617
-                        */
-                       rc = security_inode_notifysecctx(inode, secctx,
-                                                        secctxlen);
-                       if (rc)
-                               CWARN("%s: cannot set security context for "DFID": rc = %d\n",
-                                     ll_i2sbi(inode)->ll_fsname,
-                                     PFID(ll_inode2fid(inode)),
-                                     rc);
-               }
+               /* resume normally on error */
+               ll_inode_notifysecctx(inode, secctx, secctxlen);
        }
 
        /* Only hash *de if it is unhashed (new dentry).
@@ -832,14 +821,14 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
        struct ptlrpc_request *req = NULL;
        struct md_op_data *op_data = NULL;
        struct lov_user_md *lum = NULL;
+       struct ll_sb_info *sbi = ll_i2sbi(parent);
        __u32 opc;
        int rc;
-       char secctx_name[XATTR_NAME_MAX + 1];
        struct llcrypt_name fname;
        struct lu_fid fid;
        ENTRY;
 
-       if (dentry->d_name.len > ll_i2sbi(parent)->ll_namelen)
+       if (dentry->d_name.len > sbi->ll_namelen)
                RETURN(ERR_PTR(-ENAMETOOLONG));
 
        CDEBUG(D_VFSTRACE, "VFS Op:name=%pd, dir="DFID"(%p), intent=%s\n",
@@ -902,11 +891,11 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
                it->it_create_mode &= ~current_umask();
 
        if (it->it_op & IT_CREAT &&
-           test_bit(LL_SBI_FILE_SECCTX, ll_i2sbi(parent)->ll_flags)) {
-               rc = ll_dentry_init_security(parent,
-                                            dentry, it->it_create_mode,
+           test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags)) {
+               rc = ll_dentry_init_security(dentry, it->it_create_mode,
                                             &dentry->d_name,
                                             &op_data->op_file_secctx_name,
+                                            &op_data->op_file_secctx_name_size,
                                             &op_data->op_file_secctx,
                                             &op_data->op_file_secctx_size);
                if (rc < 0)
@@ -1002,22 +991,12 @@ inherit:
                        *encctxlen = 0;
        }
 
-       /* ask for security context upon intent */
-       if (it->it_op & (IT_LOOKUP | IT_GETATTR | IT_OPEN)) {
-               /* get name of security xattr to request to server */
-               rc = ll_listsecurity(parent, secctx_name,
-                                    sizeof(secctx_name));
-               if (rc < 0) {
-                       CDEBUG(D_SEC, "cannot get security xattr name for "
-                              DFID": rc = %d\n",
-                              PFID(ll_inode2fid(parent)), rc);
-               } else if (rc > 0) {
-                       op_data->op_file_secctx_name = secctx_name;
-                       op_data->op_file_secctx_name_size = rc;
-                       CDEBUG(D_SEC, "'%.*s' is security xattr for "DFID"\n",
-                              rc, secctx_name, PFID(ll_inode2fid(parent)));
-               }
-       }
+       /* ask for security context upon intent:
+        * get name of security xattr to request to server
+        */
+       if (it->it_op & (IT_LOOKUP | IT_GETATTR | IT_OPEN))
+               op_data->op_file_secctx_name_size =
+                       ll_secctx_name_get(sbi, &op_data->op_file_secctx_name);
 
        if (pca && pca->pca_dataset) {
                OBD_ALLOC_PTR(lum);
@@ -1435,19 +1414,13 @@ static int ll_create_it(struct inode *dir, struct dentry *dentry,
        if (IS_ERR(inode))
                RETURN(PTR_ERR(inode));
 
-       if (test_bit(LL_SBI_FILE_SECCTX, ll_i2sbi(inode)->ll_flags) &&
-           secctx) {
-               /* must be done before d_instantiate, because it calls
-                * security_d_instantiate, which means a getxattr if security
-                * context is not set yet */
-               /* no need to protect selinux_inode_setsecurity() by
-                * inode_lock. Taking it would lead to a client deadlock
-                * LU-13617
-                */
-               rc = security_inode_notifysecctx(inode, secctx, secctxlen);
-               if (rc)
-                       RETURN(rc);
-       }
+       /* must be done before d_instantiate, because it calls
+        * security_d_instantiate, which means a getxattr if security
+        * context is not set yet
+        */
+       rc = ll_inode_notifysecctx(inode, secctx, secctxlen);
+       if (rc)
+               RETURN(rc);
 
        d_instantiate(dentry, inode);
 
@@ -1585,9 +1558,9 @@ again:
                ll_qos_mkdir_prep(op_data, dir);
 
        if (test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags)) {
-               err = ll_dentry_init_security(dir,
-                                             dchild, mode, &dchild->d_name,
+               err = ll_dentry_init_security(dchild, mode, &dchild->d_name,
                                              &op_data->op_file_secctx_name,
+                                             &op_data->op_file_secctx_name_size,
                                              &op_data->op_file_secctx,
                                              &op_data->op_file_secctx_size);
                if (err < 0)
@@ -1727,20 +1700,15 @@ again:
        if (err)
                GOTO(err_exit, err);
 
-       if (test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags)) {
-               /* must be done before d_instantiate, because it calls
-                * security_d_instantiate, which means a getxattr if security
-                * context is not set yet */
-               /* no need to protect selinux_inode_setsecurity() by
-                * inode_lock. Taking it would lead to a client deadlock
-                * LU-13617
-                */
-               err = security_inode_notifysecctx(inode,
-                                                 op_data->op_file_secctx,
-                                                 op_data->op_file_secctx_size);
-               if (err)
-                       GOTO(err_exit, err);
-       }
+       /* must be done before d_instantiate, because it calls
+        * security_d_instantiate, which means a getxattr if security
+        * context is not set yet
+        */
+       err = ll_inode_notifysecctx(inode,
+                                   op_data->op_file_secctx,
+                                   op_data->op_file_secctx_size);
+       if (err)
+               GOTO(err_exit, err);
 
        d_instantiate(dchild, inode);
 
index 27157ba..9231650 100644 (file)
@@ -138,10 +138,9 @@ static int ll_xattr_set_common(const struct xattr_handler *handler,
             (handler->flags == XATTR_LUSTRE_T && !strcmp(name, "lov"))))
                RETURN(0);
 
-       /* LU-549:  Disable security.selinux when selinux is disabled */
-       if (handler->flags == XATTR_SECURITY_T && !selinux_is_enabled() &&
-           strcmp(name, "selinux") == 0)
-               RETURN(-EOPNOTSUPP);
+       rc = ll_security_secctx_name_filter(sbi, handler->flags, name);
+       if (rc)
+               RETURN(rc);
 
        /*
         * In user.* namespace, only regular files and directories can have
@@ -398,7 +397,7 @@ int ll_xattr_list(struct inode *inode, const char *name, int type, void *buffer,
                GOTO(out_xattr, rc = -EPERM);
 
        if (sbi->ll_xattr_cache_enabled && type != XATTR_ACL_ACCESS_T &&
-           (type != XATTR_SECURITY_T || strcmp(name, "security.selinux")) &&
+           (type != XATTR_SECURITY_T || !ll_xattr_is_seclabel(name)) &&
            (type != XATTR_TRUSTED_T || strcmp(name, XATTR_NAME_SOM))) {
                rc = ll_xattr_cache_get(inode, name, buffer, size, valid);
                if (rc == -EAGAIN)
@@ -471,10 +470,9 @@ static int ll_xattr_get_common(const struct xattr_handler *handler,
        if (rc)
                RETURN(rc);
 
-       /* LU-549:  Disable security.selinux when selinux is disabled */
-       if (handler->flags == XATTR_SECURITY_T && !selinux_is_enabled() &&
-           !strcmp(name, "selinux"))
-               RETURN(-EOPNOTSUPP);
+       rc = ll_security_secctx_name_filter(sbi, handler->flags, name);
+       if (rc)
+               RETURN(rc);
 
 #ifdef CONFIG_LUSTRE_FS_POSIX_ACL
        /* posix acl is under protection of LOOKUP lock. when calling to this,
index 2064ba7..0a75174 100644 (file)
@@ -505,9 +505,9 @@ static int ll_xattr_cache_refill(struct inode *inode)
                        CDEBUG(D_CACHE, "not caching %s\n",
                               XATTR_NAME_ACL_ACCESS);
                        rc = 0;
-               } else if (!strcmp(xdata, "security.selinux")) {
-                       /* Filter out security.selinux, it is cached in slab */
-                       CDEBUG(D_CACHE, "not caching security.selinux\n");
+               } else if (ll_xattr_is_seclabel(xdata)) {
+                       /* Filter out security label, it is cached in slab */
+                       CDEBUG(D_CACHE, "not caching %s\n", xdata);
                        rc = 0;
                } else if (!strcmp(xdata, XATTR_NAME_SOM)) {
                        /* Filter out trusted.som, it is not cached on client */
index 9bcb800..57e1d7a 100644 (file)
 /*
  * Check for LL_SBI_FILE_SECCTX before calling.
  */
-int ll_dentry_init_security(struct inode *parent, struct dentry *dentry,
-                           int mode, struct qstr *name,
-                           const char **secctx_name, void **secctx,
-                           __u32 *secctx_size)
+int ll_dentry_init_security(struct dentry *dentry, int mode, struct qstr *name,
+                           const char **secctx_name, __u32 *secctx_name_size,
+                           void **secctx, __u32 *secctx_size)
 {
+       struct ll_sb_info *sbi = ll_s2sbi(dentry->d_sb);
+#ifdef HAVE_SECURITY_DENTRY_INIT_WITH_XATTR_NAME_ARG
+       const char *secctx_name_lsm = NULL;
+#endif
        int rc;
 
        /*
-        * security_dentry_init_security() is strange. Like
-        * security_inode_init_security() it may return a context (provided a
-        * Linux security module is enabled) but unlike
-        * security_inode_init_security() it does not return to us the name of
-        * the extended attribute to store the context under (for example
+        * Before kernel 5.15-rc1-20-g15bf32398ad4,
+        * security_inode_init_security() does not return to us the name of the
+        * extended attribute to store the context under (for example
         * "security.selinux"). So we only call it when we think we know what
         * the name of the extended attribute will be. This is OK-ish since
         * SELinux is the only module that implements
@@ -71,36 +72,28 @@ int ll_dentry_init_security(struct inode *parent, struct dentry *dentry,
         * from SELinux.
         */
 
-       if (!selinux_is_enabled())
+       *secctx_name_size = ll_secctx_name_get(sbi, secctx_name);
+       /* xattr name length == 0 means no LSM module manage file contexts */
+       if (*secctx_name_size == 0)
                return 0;
 
-       /* fetch length of security xattr name */
-       rc = security_inode_listsecurity(parent, NULL, 0);
-       /* xattr name length == 0 means SELinux is disabled */
-       if (rc == 0)
-               return 0;
-       /* we support SELinux only */
-       if (rc != strlen(XATTR_NAME_SELINUX) + 1)
-               return -EOPNOTSUPP;
-
        rc = security_dentry_init_security(dentry, mode, name,
 #ifdef HAVE_SECURITY_DENTRY_INIT_WITH_XATTR_NAME_ARG
-                                          secctx_name,
+                                          &secctx_name_lsm,
 #endif
                                           secctx, secctx_size);
-       /* Usually, security_dentry_init_security() returns -EOPNOTSUPP when
-        * SELinux is disabled.
-        * But on some kernels (e.g. rhel 8.5) it returns 0 when SELinux is
-        * disabled, and in this case the security context is empty.
-        */
-       if (rc == -EOPNOTSUPP || (rc == 0 && *secctx_size == 0))
-               /* do nothing */
+       /* ignore error if the hook is not supported by the LSM module */
+       if (rc == -EOPNOTSUPP)
                return 0;
        if (rc < 0)
                return rc;
 
-#ifndef HAVE_SECURITY_DENTRY_INIT_WITH_XATTR_NAME_ARG
-       *secctx_name = XATTR_NAME_SELINUX;
+#ifdef HAVE_SECURITY_DENTRY_INIT_WITH_XATTR_NAME_ARG
+       if (strncmp(*secctx_name, secctx_name_lsm, *secctx_name_size) != 0) {
+               CERROR("%s: LSM secctx_name '%s' does not match the one stored by Lustre '%s'\n",
+                     sbi->ll_fsname, secctx_name_lsm, *secctx_name);
+               return -EOPNOTSUPP;
+       }
 #endif
 
        return 0;
@@ -161,7 +154,7 @@ ll_inode_init_security(struct dentry *dentry, struct inode *inode,
 {
        int rc;
 
-       if (!selinux_is_enabled())
+       if (!ll_security_xattr_wanted(dir))
                return 0;
 
        rc = security_inode_init_security(inode, dir, NULL,
@@ -173,23 +166,146 @@ ll_inode_init_security(struct dentry *dentry, struct inode *inode,
 }
 
 /**
- * Get security context xattr name used by policy.
+ * Notify security context to the security layer
+ *
+ * Notify security context @secctx of inode @inode to the security layer.
  *
- * \retval >= 0     length of xattr name
- * \retval < 0      failure to get security context xattr name
+ * \retval 0        success, or SELinux is disabled or not supported by the fs
+ * \retval < 0      failure to set the security context
  */
-int
-ll_listsecurity(struct inode *inode, char *secctx_name, size_t secctx_name_size)
+int ll_inode_notifysecctx(struct inode *inode,
+                         void *secctx, __u32 secctxlen)
 {
+       struct ll_sb_info *sbi = ll_i2sbi(inode);
        int rc;
 
-       if (!selinux_is_enabled())
+       if (!test_bit(LL_SBI_FILE_SECCTX, sbi->ll_flags) ||
+           !ll_security_xattr_wanted(inode) ||
+           !secctx || !secctxlen)
                return 0;
 
-       rc = security_inode_listsecurity(inode, secctx_name, secctx_name_size);
-       if (rc >= secctx_name_size)
+       /* no need to protect selinux_inode_setsecurity() by
+        * inode_lock. Taking it would lead to a client deadlock
+        * LU-13617
+        */
+       rc = security_inode_notifysecctx(inode, secctx, secctxlen);
+       if (rc)
+               CWARN("%s: cannot set security context for "DFID": rc = %d\n",
+                     sbi->ll_fsname, PFID(ll_inode2fid(inode)), rc);
+
+       return rc;
+}
+
+/**
+ * Free the security context xattr name used by policy
+ */
+void ll_secctx_name_free(struct ll_sb_info *sbi)
+{
+       OBD_FREE(sbi->ll_secctx_name, sbi->ll_secctx_name_size + 1);
+       sbi->ll_secctx_name = NULL;
+       sbi->ll_secctx_name_size = 0;
+}
+
+/**
+ * Get security context xattr name used by policy and save it.
+ *
+ * \retval > 0      length of xattr name
+ * \retval == 0     no LSM module registered supporting security contexts
+ * \retval <= 0     failure to get xattr name or xattr is not supported
+ */
+int ll_secctx_name_store(struct inode *in)
+{
+       struct ll_sb_info *sbi = ll_i2sbi(in);
+       int rc = 0;
+
+       if (!ll_security_xattr_wanted(in))
+               return 0;
+
+       /* get size of xattr name */
+       rc = security_inode_listsecurity(in, NULL, 0);
+       if (rc <= 0)
+               return rc;
+
+       if (sbi->ll_secctx_name)
+               ll_secctx_name_free(sbi);
+
+       OBD_ALLOC(sbi->ll_secctx_name, rc + 1);
+       if (!sbi->ll_secctx_name)
+               return -ENOMEM;
+
+       /* save the xattr name */
+       sbi->ll_secctx_name_size = rc;
+       rc = security_inode_listsecurity(in, sbi->ll_secctx_name,
+                                        sbi->ll_secctx_name_size);
+       if (rc <= 0)
+               goto err_free;
+
+       if (rc > sbi->ll_secctx_name_size) {
                rc = -ERANGE;
-       else if (rc >= 0)
-               secctx_name[rc] = '\0';
+               goto err_free;
+       }
+
+       /* sanity check */
+       sbi->ll_secctx_name[rc] = '\0';
+       if (rc < sizeof(XATTR_SECURITY_PREFIX)) {
+               rc = -EINVAL;
+               goto err_free;
+       }
+       if (strncmp(sbi->ll_secctx_name, XATTR_SECURITY_PREFIX,
+                   sizeof(XATTR_SECURITY_PREFIX) - 1) != 0) {
+               rc = -EOPNOTSUPP;
+               goto err_free;
+       }
+
+       return rc;
+
+err_free:
+       ll_secctx_name_free(sbi);
        return rc;
 }
+
+/**
+ * Retrieved file security context xattr name stored.
+ *
+ * \retval      security context xattr name size stored.
+ * \retval 0   no xattr name stored.
+ */
+__u32 ll_secctx_name_get(struct ll_sb_info *sbi, const char **secctx_name)
+{
+       if (!sbi->ll_secctx_name || !sbi->ll_secctx_name_size)
+               return 0;
+
+       *secctx_name = sbi->ll_secctx_name;
+
+       return sbi->ll_secctx_name_size;
+}
+
+/**
+ * Filter out xattr file security context if not managed by LSM
+ *
+ * This is done to improve performance for application that blindly try to get
+ * file context (like "ls -l" for security.linux).
+ * See LU-549 for more information.
+ *
+ * \retval 0           xattr not filtered
+ * \retval -EOPNOTSUPP no enabled LSM security module supports the xattr
+ */
+int ll_security_secctx_name_filter(struct ll_sb_info *sbi, int xattr_type,
+                                  const char *suffix)
+{
+       const char *cached_suffix = NULL;
+
+       if (xattr_type != XATTR_SECURITY_T ||
+           !ll_xattr_suffix_is_seclabel(suffix))
+               return 0;
+
+       /* is the xattr label used by lsm ? */
+       if (!ll_secctx_name_get(sbi, &cached_suffix))
+               return -EOPNOTSUPP;
+
+       cached_suffix += sizeof(XATTR_SECURITY_PREFIX) - 1;
+       if (strcmp(suffix, cached_suffix) != 0)
+               return -EOPNOTSUPP;
+
+       return 0;
+}
index c308b69..f6b9ace 100644 (file)
@@ -1829,7 +1829,7 @@ static inline int sptlrpc_sepol_needs_check(struct ptlrpc_sec *imp_sec)
 {
        ktime_t checknext;
 
-       if (send_sepol == 0 || !selinux_is_enabled())
+       if (send_sepol == 0)
                return 0;
 
        if (send_sepol == -1)
@@ -1875,7 +1875,7 @@ int sptlrpc_get_sepol(struct ptlrpc_request *req)
        RETURN(0);
 #endif
 
-       if (send_sepol == 0 || !selinux_is_enabled())
+       if (send_sepol == 0)
                RETURN(0);
 
        if (imp_sec == NULL)
@@ -1889,6 +1889,10 @@ int sptlrpc_get_sepol(struct ptlrpc_request *req)
                memcpy(req->rq_sepol, imp_sec->ps_sepol,
                       sizeof(req->rq_sepol));
                spin_unlock(&imp_sec->ps_lock);
+       } else if (rc == -ENODEV) {
+               CDEBUG(D_SEC,
+                      "Client cannot report SELinux status, SELinux is disabled.\n");
+               rc = 0;
        }
 
        RETURN(rc);
index 900de19..cbe3909 100755 (executable)
@@ -27622,6 +27622,41 @@ test_433() {
 }
 run_test 433 "ldlm lock cancel releases dentries and inodes"
 
+test_434() {
+       local file
+       local getxattr_count
+       local mdc_stat_param="mdc.$FSNAME-MDT0000*.md_stats"
+       local p="$TMP/$TESTSUITE-$TESTNAME.parameters"
+
+       [[ $(getenforce) == "Disabled" ]] ||
+               skip "lsm selinux module have to be disabled for this test"
+
+       test_mkdir -i 0 -c1 $DIR/$tdir/ ||
+               error "fail to create $DIR/$tdir/ on MDT0000"
+
+       touch $DIR/$tdir/$tfile-{001..100}
+
+       # disable the xattr cache
+       save_lustre_params client "llite.*.xattr_cache" > $p
+       lctl set_param llite.*.xattr_cache=0
+       stack_trap "restore_lustre_params < $p; rm -f $p" EXIT
+
+       # clear clients mdc stats
+       clear_stats $mdc_stat_param ||
+               error "fail to clear stats on mdc MDT0000"
+
+       for file in $DIR/$tdir/$tfile-{001..100}; do
+               getfattr -n security.selinux $file |&
+                       grep -q "Operation not supported" ||
+                       error "getxattr on security.selinux should return EOPNOTSUPP"
+       done
+
+       getxattr_count=$(calc_stats $mdc_stat_param "getxattr")
+       (( getxattr_count < 100 )) ||
+               error "client sent $getxattr_count getxattr RPCs to the MDS"
+}
+run_test 434 "Client should not send RPCs for security.selinux with SElinux disabled"
+
 prep_801() {
        [[ $MDS1_VERSION -lt $(version_code 2.9.55) ]] ||
        [[ $OST1_VERSION -lt $(version_code 2.9.55) ]] &&
index a3e07c4..990d016 100644 (file)
@@ -69,6 +69,8 @@ static void errlog(const char *fmt, ...)
 
        va_start(args, fmt);
        vsyslog(LOG_NOTICE, fmt, args);
+       if (isatty(STDIN_FILENO))
+               vfprintf(stderr, fmt, args);
        va_end(args);
 
        closelog();
@@ -205,7 +207,7 @@ int get_opts(int argc, char *const argv[])
                ref_pol_mtime = (time_t)strtoul(sel_mtime, &res, 0);
                if (*res != '\0') {
                        /* not a valid number */
-                       errlog("invalid sel_mtime");
+                       errlog("invalid sel_mtime\n");
                        return -EINVAL;
                }
        }
@@ -214,7 +216,7 @@ int get_opts(int argc, char *const argv[])
                ref_selinux_mode = sel_mode[0] - '0';
                if (ref_selinux_mode != 0 && ref_selinux_mode != 1) {
                        /* not a valid enforcing mode */
-                       errlog("invalid sel_mode");
+                       errlog("invalid sel_mode\n");
                        return -EINVAL;
                }
        }
@@ -331,6 +333,7 @@ int main(int argc, char **argv)
        struct stat st;
        time_t policymtime = 0;
        int enforce;
+       int is_selinux;
        char *policy_type = NULL;
        unsigned char *mdval = NULL;
        unsigned int mdsize = 0;
@@ -342,6 +345,19 @@ int main(int argc, char **argv)
        if (rc < 0)
                goto out;
 
+       is_selinux = is_selinux_enabled();
+       if (is_selinux < 0) {
+               errlog("is_selinux_enabled() failed\n");
+               rc = -errno;
+               goto out;
+       }
+
+       if (!is_selinux) {
+               errlog("SELinux is disabled, ptlrpc 'send_sepol' value should be set to 0\n");
+               rc = -ENODEV;
+               goto out;
+       }
+
        /* Max version of loaded policy */
        policyver = security_policyvers();
        if (policyver < 0) {