smack: opt_dentry is never null in in smack_d_instantiate()

PATCH In review Dan Carpenter 3 participants 3 messages Jun 1, 2010 – Jun 2, 2010

Subsystems (3)

Kernel janitors Security (LSM) Smack security module

lore

Share & tools

lore

Dan Carpenter

author

2010-06-01 07:14 UTC #0 lore raw

PATCH 1/1 smack: opt_dentry is never null in in smack_d_instantiate()

This patch removes some unneeded code for if opt_dentry is null because that can never happen.

The function dereferences "opt_dentry" earlier when it checks "if (opt_dentry->d_parent = opt_dentry) {". That code was added in 2008.

This function called from security_d_instantiate(). I checked all the places which call security_d_instantiate() and dentry is always non-null. I also checked the selinux version of this hook and there is a comment which says that dentry should be non-null if called from d_instantiate().

Signed-off-by Dan Carpenter <[email protected]>

security/smack/smack_lsm.c +2 ~~-12~~

index 0f2fc48..07abc9c 100644
@@ -2191,7 +2191,7 @@ static void smack_ipc_getsecid(struct kern_ipc_perm *ipp, u32 *secid)
 
 /**
  * smack_d_instantiate - Make sure the blob is correct on an inode
- * @opt_dentry: unused
+ * @opt_dentry: dentry where inode will be attached
  * @inode: the object
  *
  * Set the inode's security blob if it hasn't been done already.

@@ -2310,20 +2310,10 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
 		/*
 		 * Get the dentry for xattr.
 		 */
-		if (opt_dentry = NULL) {
-			dp = d_find_alias(inode);
-			if (dp = NULL)
-				break;
-		} else {
-			dp = dget(opt_dentry);
-			if (dp = NULL)
-				break;
-		}
-
+		dp = dget(opt_dentry);
 		fetched = smk_fetch(inode, dp);
 		if (fetched != NULL)
 			final = fetched;
-
 		dput(dp);
 		break;
 	}

Casey Schaufler

maintainer Acked-by on v1 1/1

2010-06-02 00:26 UTC #1 lore raw

Quoted 7 lines · Dan Carpenter

This patch removes some unneeded code for if opt_dentry is null because that can never happen.
The function dereferences "opt_dentry" earlier when it checks "if (opt_dentry->d_parent = opt_dentry) {". That code was added in 2008.

Quoted 8 lines

This function called from security_d_instantiate(). I checked all the places which call security_d_instantiate() and dentry is always non-null. I also checked the selinux version of this hook and there is a comment which says that dentry should be non-null if called from d_instantiate().
Signed-off-by Dan Carpenter <[email protected]>

Acked-by Casey Schaufler <[email protected]>

I have tested the change and not had any issues. I recall struggling with this particular bit of code, but that was long enough ago that the circumstances evade my memory. Thank you.

Quoted 38 lines

security/smack/smack_lsm.c +2 ~~-12~~

index 0f2fc48..07abc9c 100644
@@ -2191,7 +2191,7 @@ static void smack_ipc_getsecid(struct kern_ipc_perm *ipp, u32 *secid)
 
 /**
  * smack_d_instantiate - Make sure the blob is correct on an inode
- * @opt_dentry: unused
+ * @opt_dentry: dentry where inode will be attached
  * @inode: the object
  *
  * Set the inode's security blob if it hasn't been done already.

@@ -2310,20 +2310,10 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
 		/*
 		 * Get the dentry for xattr.
 		 */
-		if (opt_dentry = NULL) {
-			dp = d_find_alias(inode);
-			if (dp = NULL)
-				break;
-		} else {
-			dp = dget(opt_dentry);
-			if (dp = NULL)
-				break;
-		}
-
+		dp = dget(opt_dentry);
 		fetched = smk_fetch(inode, dp);
 		if (fetched != NULL)
 			final = fetched;
-
 		dput(dp);
 		break;
 	}

James Morris

maintainer

2010-06-02 01:56 UTC #2 lore raw

In reply to Dan Carpenter

Quoted 7 lines · Dan Carpenter

This patch removes some unneeded code for if opt_dentry is null because that can never happen.
The function dereferences "opt_dentry" earlier when it checks "if (opt_dentry->d_parent = opt_dentry) {". That code was added in 2008.

Quoted 7 lines

This function called from security_d_instantiate(). I checked all the places which call security_d_instantiate() and dentry is always non-null. I also checked the selinux version of this hook and there is a comment which says that dentry should be non-null if called from d_instantiate().
Signed-off-by Dan Carpenter <[email protected]>

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next

Signature · 2 lines

James Morris
<[email protected]>