apparmor/kernel-patches/for-mainline/apparmor-aa_permission.diff

Refactor some duplicated code patterns

lsm.c duplicates the pattern (get active profile; do access check; put
active profile) all over the place. This can easily be abstracted away
as in the attached patch.

Index: b/security/apparmor/lsm.c
===================================================================
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -277,26 +277,27 @@ out:
 	return error;
 }
 
-static int apparmor_inode_create(struct inode *dir, struct dentry *dentry,
-				 struct vfsmount *mnt, int mask)
+static int aa_permission(struct inode *inode, struct dentry *dentry,
+			 struct vfsmount *mnt, int mask)
 {
-	struct aa_profile *active;
 	int error = 0;
 
-	if (!mnt || !mediated_filesystem(dir))
-		goto out;
-
-	active = get_active_aa_profile();
-
-	/* At a minimum, need write perm to create */
-	if (active)
-		error = aa_perm(active, dentry, mnt, MAY_WRITE);
+	if (mnt && mediated_filesystem(inode)) {
+		struct aa_profile *active = get_active_aa_profile();
 
-	put_aa_profile(active);
-out:
+		if (active)
+			error = aa_perm(active, dentry, mnt, mask);
+		put_aa_profile(active);
+	}
 	return error;
 }
 
+static int apparmor_inode_create(struct inode *dir, struct dentry *dentry,
+				 struct vfsmount *mnt, int mask)
+{
+	return aa_permission(dir, dentry, mnt, MAY_WRITE);
+}
+
 static int apparmor_inode_link(struct dentry *old_dentry,
 			       struct vfsmount *old_mnt, struct inode *dir,
 			       struct dentry *new_dentry,
@@ -324,61 +325,19 @@ static int apparmor_inode_unlink(struct 
 				 struct dentry *dentry,
 				 struct vfsmount *mnt)
 {
-	struct aa_profile *active;
-	int error = 0;
-
-	if (!mnt || !mediated_filesystem(dir))
-		goto out;
-
-	active = get_active_aa_profile();
-
-	if (active)
-		error = aa_perm(active, dentry, mnt, MAY_WRITE);
-
-	put_aa_profile(active);
-
-out:
-	return error;
+	return aa_permission(dir, dentry, mnt, MAY_WRITE);
 }
 
 static int apparmor_inode_symlink(struct inode *dir, struct dentry *dentry,
 				  struct vfsmount *mnt, const char *old_name)
 {
-	struct aa_profile *active;
-	int error = 0;
-
-	if (!mnt || !mediated_filesystem(dir))
-		goto out;
-
-	active = get_active_aa_profile();
-
-	if (active)
-		error = aa_perm(active, dentry, mnt, MAY_WRITE);
-
-	put_aa_profile(active);
-
-out:
-	return error;
+	return aa_permission(dir, dentry, mnt, MAY_WRITE);
 }
 
 static int apparmor_inode_mknod(struct inode *dir, struct dentry *dentry,
 				struct vfsmount *mnt, int mode, dev_t dev)
 {
-	struct aa_profile *active;
-	int error = 0;
-
-	if (!mnt || !mediated_filesystem(dir))
-		goto out;
-
-	active = get_active_aa_profile();
-
-	if (active)
-		error = aa_perm(active, dentry, mnt, MAY_WRITE);
-
-	put_aa_profile(active);
-
-out:
-	return error;
+	return aa_permission(dir, dentry, mnt, MAY_WRITE);
 }
 
 static int apparmor_inode_rename(struct inode *old_dir,
@@ -415,21 +374,10 @@ out:
 static int apparmor_inode_permission(struct inode *inode, int mask,
 				     struct nameidata *nd)
 {
-	int error = 0;
-
-	/* Do not perform check on pipes or sockets
-	 * Same as apparmor_file_permission
-	 */
-	if (nd && mediated_filesystem(inode)) {
-		struct aa_profile *active;
-
-		active = get_active_aa_profile();
-		if (active)
-			error = aa_perm(active, nd->dentry, nd->mnt, mask);
-		put_aa_profile(active);
-	}
-
-	return error;
+	if (!nd)
+		return 0;
+	return aa_permission(inode, nd->dentry, nd->mnt,
+			     mask & (MAY_READ | MAY_WRITE | MAY_EXEC));
 }
 
 static int apparmor_inode_setattr(struct dentry *dentry, struct vfsmount *mnt,
@@ -458,91 +406,49 @@ out:
 	return error;
 }
 
-static int apparmor_inode_setxattr(struct dentry *dentry, struct vfsmount *mnt,
-				   char *name, void *value, size_t size,
-				   int flags)
+static int aa_xattr_permission(struct dentry *dentry, struct vfsmount *mnt,
+			       const char *name, const char *operation,
+			       int mask)
 {
 	int error = 0;
 
-	if (!mnt)
-		goto out;
-
-	if (mediated_filesystem(dentry->d_inode)) {
-		struct aa_profile *active;
-
-		active = get_active_aa_profile();
+	if (mnt && mediated_filesystem(dentry->d_inode)) {
+		struct aa_profile *active = get_active_aa_profile();
+
 		if (active)
 			error = aa_perm_xattr(active, dentry, mnt, name,
-					      "xattr set", AA_MAY_WRITE);
+					      operation, mask);
 		put_aa_profile(active);
 	}
 
-out:
 	return error;
 }
 
+static int apparmor_inode_setxattr(struct dentry *dentry, struct vfsmount *mnt,
+				   char *name, void *value, size_t size,
+				   int flags)
+{
+	return aa_xattr_permission(dentry, mnt, name, "xattr set",
+				   AA_MAY_WRITE);
+}
+
 static int apparmor_inode_getxattr(struct dentry *dentry, struct vfsmount *mnt,
 				   char *name)
 {
-	int error = 0;
-
-	if (!mnt)
-		goto out;
-
-	if (mediated_filesystem(dentry->d_inode)) {
-		struct aa_profile *active;
-
-		active = get_active_aa_profile();
-		if (active)
-			error = aa_perm_xattr(active, dentry, mnt, name,
-					      "xattr get", AA_MAY_READ);
-		put_aa_profile(active);
-	}
-
-out:
-	return error;
+	return aa_xattr_permission(dentry, mnt, name, "xattr get", AA_MAY_READ);
 }
+
 static int apparmor_inode_listxattr(struct dentry *dentry, struct vfsmount *mnt)
 {
-	int error = 0;
-
-	if (!mnt)
-		goto out;
-
-	if (mediated_filesystem(dentry->d_inode)) {
-		struct aa_profile *active;
-
-		active = get_active_aa_profile();
-		if (active)
-			error = aa_perm_xattr(active, dentry, mnt, NULL,
-					      "xattr list", AA_MAY_READ);;
-		put_aa_profile(active);
-	}
-
-out:
-	return error;
+	return aa_xattr_permission(dentry, mnt, NULL, "xattr list",
+				   AA_MAY_READ);
 }
 
 static int apparmor_inode_removexattr(struct dentry *dentry,
 				      struct vfsmount *mnt, char *name)
 {
-	int error = 0;
-
-	if (!mnt)
-		goto out;
-
-	if (mediated_filesystem(dentry->d_inode)) {
-		struct aa_profile *active;
-
-		active = get_active_aa_profile();
-		if (active)
-			error = aa_perm_xattr(active, dentry, mnt, name,
-					      "xattr remove", AA_MAY_WRITE);
-		put_aa_profile(active);
-	}
-
-out:
-	return error;
+	return aa_xattr_permission(dentry, mnt, name, "xattr remove",
+				   AA_MAY_WRITE);
 }
 
 static int apparmor_file_permission(struct file *file, int mask)
@@ -551,15 +457,16 @@ static int apparmor_file_permission(stru
 	struct aa_profile *file_profile = (struct aa_profile*)file->f_security;
 	int error = 0;
 
-	/* bail out early if this isn't a mediated file */
-	if (!file_profile || !mediated_filesystem(file->f_dentry->d_inode))
+	/* FIXME: get rid of revalidation. */
+	if (!file_profile)
 		goto out;
 
 	active = get_active_aa_profile();
 	if (active && file_profile != active) {
-		/* FIXME: get rid of revalidation. */
-		error = aa_perm(active, file->f_dentry, file->f_vfsmnt,
-				mask & (MAY_EXEC | MAY_WRITE | MAY_READ));
+		struct dentry *dentry = file->f_dentry;
+
+		error = aa_permission(dentry->d_inode, dentry, file->f_vfsmnt,
+				      mask & (MAY_READ | MAY_WRITE | MAY_EXEC));
 	}
 	put_aa_profile(active);
 
@@ -591,32 +498,22 @@ static void apparmor_file_free_security(
 static inline int aa_mmap(struct file *file, unsigned long prot,
 			  unsigned long flags)
 {
-	int error = 0, mask = 0;
-	struct aa_profile *active;
+	int mask = 0;
 
-	active = get_active_aa_profile();
-	if (!active || !file || !mediated_filesystem(file->f_dentry->d_inode))
-		goto out;
+	if (!file)
+		return 0;
 
 	if (prot & PROT_READ)
 		mask |= MAY_READ;
-
 	/* Private mappings don't require write perms since they don't
 	 * write back to the files */
-	if (prot & PROT_WRITE && !(flags & MAP_PRIVATE))
+	if ((prot & PROT_WRITE) && !(flags & MAP_PRIVATE))
 		mask |= MAY_WRITE;
 	if (prot & PROT_EXEC)
 		mask |= AA_EXEC_MMAP;
 
-	AA_DEBUG("%s: 0x%x\n", __FUNCTION__, mask);
-
-	if (mask)
-		error = aa_perm(active, file->f_dentry, file->f_vfsmnt, mask);
-
-	put_aa_profile(active);
-
-out:
-	return error;
+	return aa_permission(file->f_dentry->d_inode, file->f_dentry,
+			     file->f_vfsmnt, mask);
 }
 
 static int apparmor_file_mmap(struct file *file, unsigned long reqprot,