linux-kernel - Re: [PATCH 1/1] Smack:- Remove mutex lock "smk_lock" from inode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a4c51fbb-9f76-ea6e-0288-a46d5a2558bc@schaufler-ca.com>
Date:   Fri, 24 Apr 2020 15:18:58 -0700
From:   Casey Schaufler <casey@...aufler-ca.com>
To:     Vishal Goel <vishal.goel@...sung.com>,
        linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH 1/1] Smack:- Remove mutex lock "smk_lock" from inode_smack
 structure.

On 4/24/2020 5:55 AM, Vishal Goel wrote:
> "smk_lock" mutex is used during inode instantiation in
> smack_d_instantiate()function. It has been used to avoid
> simultaneous access on same inode security structure.
> Since smack related initialization is done only once i.e during
> inode creation. If the inode has already been instantiated then
> smack_d_instantiate() function just returns without doing
> anything.
>
> So it means mutex lock is required only during inode creation.
> But since 2 processes can't create same inodes or files
> simultaneously. Also linking or some other file operation can't
> be done simultaneously when the file is getting created since
> file lookup will fail before dentry inode linkup which is done
> after smack initialization.
> So no mutex lock is required in inode_smack structure.
>
> It will save memory as well as improve some performance.
> If 40000 inodes are created in system, it will save 1.5 MB on
> 32-bit systems & 2.8 MB on 64-bit systems.
>
> Signed-off-by: Vishal Goel <vishal.goel@...sung.com>
> Signed-off-by: Amit Sahrawat <a.sahrawat@...sung.com>

I am taking this for 5.8. Thank you.

> ---
>  security/smack/smack.h     | 1 -
>  security/smack/smack_lsm.c | 8 ++------
>  2 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 62529f3..fd09dc8 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -109,7 +109,6 @@ struct inode_smack {
>  	struct smack_known	*smk_inode;	/* label of the fso */
>  	struct smack_known	*smk_task;	/* label of the task */
>  	struct smack_known	*smk_mmap;	/* label of the mmap domain */
> -	struct mutex		smk_lock;	/* initialization lock */
>  	int			smk_flags;	/* smack inode flags */
>  	struct rcu_head         smk_rcu;	/* for freeing inode_smack */
>  };
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 2862fc3..10c2bdd 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -317,7 +317,6 @@ static void init_inode_smack(struct inode *inode, struct smack_known *skp)
>  
>  	isp->smk_inode = skp;
>  	isp->smk_flags = 0;
> -	mutex_init(&isp->smk_lock);
>  }
>  
>  /**
> @@ -3274,13 +3273,12 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
>  
>  	isp = smack_inode(inode);
>  
> -	mutex_lock(&isp->smk_lock);
>  	/*
>  	 * If the inode is already instantiated
>  	 * take the quick way out
>  	 */
>  	if (isp->smk_flags & SMK_INODE_INSTANT)
> -		goto unlockandout;
> +		return;
>  
>  	sbp = inode->i_sb;
>  	sbsp = sbp->s_security;
> @@ -3331,7 +3329,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
>  			break;
>  		}
>  		isp->smk_flags |= SMK_INODE_INSTANT;
> -		goto unlockandout;
> +		return;
>  	}
>  
>  	/*
> @@ -3466,8 +3464,6 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
>  
>  	isp->smk_flags |= (SMK_INODE_INSTANT | transflag);
>  
> -unlockandout:
> -	mutex_unlock(&isp->smk_lock);
>  	return;
>  }
>