Bläddra i källkod

Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6:
  ima: always maintain counters
  AppArmor: Fix locking from removal of profile namespace
  AppArmor: Fix splitting an fqname into separate namespace and profile names
  AppArmor: Fix security_task_setrlimit logic for 2.6.36 changes
  AppArmor: Drop hack to remove appended " (deleted)" string
Linus Torvalds 14 år sedan
förälder
incheckning
e6f901bb85

+ 2 - 2
security/apparmor/include/resource.h

@@ -33,8 +33,8 @@ struct aa_rlimit {
 };
 
 int aa_map_resource(int resource);
-int aa_task_setrlimit(struct aa_profile *profile, unsigned int resource,
-		      struct rlimit *new_rlim);
+int aa_task_setrlimit(struct aa_profile *profile, struct task_struct *,
+		      unsigned int resource, struct rlimit *new_rlim);
 
 void __aa_transition_rlimits(struct aa_profile *old, struct aa_profile *new);
 

+ 1 - 1
security/apparmor/lib.c

@@ -40,6 +40,7 @@ char *aa_split_fqname(char *fqname, char **ns_name)
 	*ns_name = NULL;
 	if (name[0] == ':') {
 		char *split = strchr(&name[1], ':');
+		*ns_name = skip_spaces(&name[1]);
 		if (split) {
 			/* overwrite ':' with \0 */
 			*split = 0;
@@ -47,7 +48,6 @@ char *aa_split_fqname(char *fqname, char **ns_name)
 		} else
 			/* a ns name without a following profile is allowed */
 			name = NULL;
-		*ns_name = &name[1];
 	}
 	if (name && *name == 0)
 		name = NULL;

+ 1 - 1
security/apparmor/lsm.c

@@ -614,7 +614,7 @@ static int apparmor_task_setrlimit(struct task_struct *task,
 	int error = 0;
 
 	if (!unconfined(profile))
-		error = aa_task_setrlimit(profile, resource, new_rlim);
+		error = aa_task_setrlimit(profile, task, resource, new_rlim);
 
 	return error;
 }

+ 11 - 27
security/apparmor/path.c

@@ -59,8 +59,7 @@ static int d_namespace_path(struct path *path, char *buf, int buflen,
 {
 	struct path root, tmp;
 	char *res;
-	int deleted, connected;
-	int error = 0;
+	int connected, error = 0;
 
 	/* Get the root we want to resolve too, released below */
 	if (flags & PATH_CHROOT_REL) {
@@ -74,19 +73,8 @@ static int d_namespace_path(struct path *path, char *buf, int buflen,
 	}
 
 	spin_lock(&dcache_lock);
-	/* There is a race window between path lookup here and the
-	 * need to strip the " (deleted) string that __d_path applies
-	 * Detect the race and relookup the path
-	 *
-	 * The stripping of (deleted) is a hack that could be removed
-	 * with an updated __d_path
-	 */
-	do {
-		tmp = root;
-		deleted = d_unlinked(path->dentry);
-		res = __d_path(path, &tmp, buf, buflen);
-
-	} while (deleted != d_unlinked(path->dentry));
+	tmp = root;
+	res = __d_path(path, &tmp, buf, buflen);
 	spin_unlock(&dcache_lock);
 
 	*name = res;
@@ -98,21 +86,17 @@ static int d_namespace_path(struct path *path, char *buf, int buflen,
 		*name = buf;
 		goto out;
 	}
-	if (deleted) {
-		/* On some filesystems, newly allocated dentries appear to the
-		 * security_path hooks as a deleted dentry except without an
-		 * inode allocated.
-		 *
-		 * Remove the appended deleted text and return as string for
-		 * normal mediation, or auditing.  The (deleted) string is
-		 * guaranteed to be added in this case, so just strip it.
-		 */
-		buf[buflen - 11] = 0;	/* - (len(" (deleted)") +\0) */
 
-		if (path->dentry->d_inode && !(flags & PATH_MEDIATE_DELETED)) {
+	/* Handle two cases:
+	 * 1. A deleted dentry && profile is not allowing mediation of deleted
+	 * 2. On some filesystems, newly allocated dentries appear to the
+	 *    security_path hooks as a deleted dentry except without an inode
+	 *    allocated.
+	 */
+	if (d_unlinked(path->dentry) && path->dentry->d_inode &&
+	    !(flags & PATH_MEDIATE_DELETED)) {
 			error = -ENOENT;
 			goto out;
-		}
 	}
 
 	/* Determine if the path is connected to the expected root */

+ 4 - 2
security/apparmor/policy.c

@@ -1151,12 +1151,14 @@ ssize_t aa_remove_profiles(char *fqname, size_t size)
 		/* released below */
 		ns = aa_get_namespace(root);
 
-	write_lock(&ns->lock);
 	if (!name) {
 		/* remove namespace - can only happen if fqname[0] == ':' */
+		write_lock(&ns->parent->lock);
 		__remove_namespace(ns);
+		write_unlock(&ns->parent->lock);
 	} else {
 		/* remove profile */
+		write_lock(&ns->lock);
 		profile = aa_get_profile(__lookup_profile(&ns->base, name));
 		if (!profile) {
 			error = -ENOENT;
@@ -1165,8 +1167,8 @@ ssize_t aa_remove_profiles(char *fqname, size_t size)
 		}
 		name = profile->base.hname;
 		__remove_profile(profile);
+		write_unlock(&ns->lock);
 	}
-	write_unlock(&ns->lock);
 
 	/* don't fail removal if audit fails */
 	(void) audit_policy(OP_PROF_RM, GFP_KERNEL, name, info, error);

+ 12 - 8
security/apparmor/resource.c

@@ -72,6 +72,7 @@ int aa_map_resource(int resource)
 /**
  * aa_task_setrlimit - test permission to set an rlimit
  * @profile - profile confining the task  (NOT NULL)
+ * @task - task the resource is being set on
  * @resource - the resource being set
  * @new_rlim - the new resource limit  (NOT NULL)
  *
@@ -79,18 +80,21 @@ int aa_map_resource(int resource)
  *
  * Returns: 0 or error code if setting resource failed
  */
-int aa_task_setrlimit(struct aa_profile *profile, unsigned int resource,
-		      struct rlimit *new_rlim)
+int aa_task_setrlimit(struct aa_profile *profile, struct task_struct *task,
+		      unsigned int resource, struct rlimit *new_rlim)
 {
 	int error = 0;
 
-	if (profile->rlimits.mask & (1 << resource) &&
-	    new_rlim->rlim_max > profile->rlimits.limits[resource].rlim_max)
-
-		error = audit_resource(profile, resource, new_rlim->rlim_max,
-			-EACCES);
+	/* TODO: extend resource control to handle other (non current)
+	 * processes.  AppArmor rules currently have the implicit assumption
+	 * that the task is setting the resource of the current process
+	 */
+	if ((task != current->group_leader) ||
+	    (profile->rlimits.mask & (1 << resource) &&
+	     new_rlim->rlim_max > profile->rlimits.limits[resource].rlim_max))
+		error = -EACCES;
 
-	return error;
+	return audit_resource(profile, resource, new_rlim->rlim_max, error);
 }
 
 /**

+ 1 - 0
security/integrity/ima/ima.h

@@ -35,6 +35,7 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
 #define IMA_MEASURE_HTABLE_SIZE (1 << IMA_HASH_BITS)
 
 /* set during initialization */
+extern int iint_initialized;
 extern int ima_initialized;
 extern int ima_used_chip;
 extern char *ima_hash;

+ 3 - 1
security/integrity/ima/ima_iint.c

@@ -22,9 +22,10 @@
 
 RADIX_TREE(ima_iint_store, GFP_ATOMIC);
 DEFINE_SPINLOCK(ima_iint_lock);
-
 static struct kmem_cache *iint_cache __read_mostly;
 
+int iint_initialized = 0;
+
 /* ima_iint_find_get - return the iint associated with an inode
  *
  * ima_iint_find_get gets a reference to the iint. Caller must
@@ -141,6 +142,7 @@ static int __init ima_iintcache_init(void)
 	iint_cache =
 	    kmem_cache_create("iint_cache", sizeof(struct ima_iint_cache), 0,
 			      SLAB_PANIC, init_once);
+	iint_initialized = 1;
 	return 0;
 }
 security_initcall(ima_iintcache_init);

+ 5 - 3
security/integrity/ima/ima_main.c

@@ -148,12 +148,14 @@ void ima_counts_get(struct file *file)
 	struct ima_iint_cache *iint;
 	int rc;
 
-	if (!ima_initialized || !S_ISREG(inode->i_mode))
+	if (!iint_initialized || !S_ISREG(inode->i_mode))
 		return;
 	iint = ima_iint_find_get(inode);
 	if (!iint)
 		return;
 	mutex_lock(&iint->mutex);
+	if (!ima_initialized)
+		goto out;
 	rc = ima_must_measure(iint, inode, MAY_READ, FILE_CHECK);
 	if (rc < 0)
 		goto out;
@@ -213,7 +215,7 @@ void ima_file_free(struct file *file)
 	struct inode *inode = file->f_dentry->d_inode;
 	struct ima_iint_cache *iint;
 
-	if (!ima_initialized || !S_ISREG(inode->i_mode))
+	if (!iint_initialized || !S_ISREG(inode->i_mode))
 		return;
 	iint = ima_iint_find_get(inode);
 	if (!iint)
@@ -230,7 +232,7 @@ static int process_measurement(struct file *file, const unsigned char *filename,
 {
 	struct inode *inode = file->f_dentry->d_inode;
 	struct ima_iint_cache *iint;
-	int rc;
+	int rc = 0;
 
 	if (!ima_initialized || !S_ISREG(inode->i_mode))
 		return 0;