Whamcloud - gitweb
LU-13617 llite: don't hold inode_lock for security notify 92/38792/3
authorAlexander Boyko <c17825@cray.com>
Mon, 1 Jun 2020 12:32:11 +0000 (08:32 -0400)
committerOleg Drokin <green@whamcloud.com>
Tue, 23 Jun 2020 08:11:45 +0000 (08:11 +0000)
With selinux enabled client has a dead lock which leads to
client eviction from MDS.
1 thread                    2 thread
do file open                do stat
inode_lock(parend dir)
                            got LDLM_PR(parent dir)
enqueue LDLM_CW(parent dir) waits on inode_lock to notify security
waits
timeout on enqueue
and client eviction because client didn't cancel a LDLM_PR lock

security_inode_notifysecctx()->selinux_inode_notifysecctx()->
selinux_inode_setsecurity()
The call of selinux_inode_setsecurity doesn't need to hold
inode_lock.

Fixes: 1d44980bcb ("LU-8956 llite: set sec ctx on client's inode at create time")
Signed-off-by: Alexander Boyko <alexander.boyko@hpe.com>
Cray-bug-id: LUS-8924
Change-Id: I4727da45590734bde57bee9d378b61c30b5d515a
Reviewed-on: https://review.whamcloud.com/38792
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: Sebastien Buisson <sbuisson@ddn.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andrew Perepechko <andrew.perepechko@hpe.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/llite/dir.c
lustre/llite/namei.c

index a69a1de..a452779 100644 (file)
@@ -517,11 +517,13 @@ static int ll_dir_setdirstripe(struct dentry *dparent, struct lmv_user_md *lump,
        dentry.d_inode = inode;
 
        if (sbi->ll_flags & LL_SBI_FILE_SECCTX) {
-               inode_lock(inode);
+               /* 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);
-               inode_unlock(inode);
        } else {
                err = ll_inode_init_security(&dentry, inode, parent);
        }
index e358e7a..f569da1 100644 (file)
@@ -664,10 +664,12 @@ static int ll_lookup_it_finish(struct ptlrpc_request *request,
                }
 
                if (secctx != NULL && secctxlen != 0) {
-                       inode_lock(inode);
+                       /* 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);
-                       inode_unlock(inode);
                        if (rc)
                                CWARN("cannot set security context for "
                                      DFID": rc = %d\n",
@@ -1205,12 +1207,14 @@ static int ll_create_it(struct inode *dir, struct dentry *dentry,
 
        if ((ll_i2sbi(inode)->ll_flags & LL_SBI_FILE_SECCTX) &&
            secctx != NULL) {
-               inode_lock(inode);
                /* 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);
-               inode_unlock(inode);
                if (rc)
                        RETURN(rc);
        }
@@ -1376,14 +1380,16 @@ again:
                GOTO(err_exit, err);
 
        if (sbi->ll_flags & LL_SBI_FILE_SECCTX) {
-               inode_lock(inode);
                /* 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);
-               inode_unlock(inode);
                if (err)
                        GOTO(err_exit, err);
        }