[apparmor] [PATCH 02/24] apparmor: convert profile lists to RCU based locking

John Johansen john.johansen at canonical.com
Sat Mar 2 02:29:22 UTC 2013


On 03/01/2013 06:17 PM, Seth Arnold wrote:
> In the code below, the "if (error) return error;" near the top of the
> loop feels a bit out of place -- if one policy loads fine and a second
> policy fails the header check, a profile is on the list_head and never
> cleaned up. (I mentioned the caller of this function in a different
> email -- this is the case I was referring to.)
> 
> 
yep its fixed in the next patch (sorry)

> int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns)
> {
> 	struct aa_profile *tmp, *profile = NULL;
> 	int error;
> 	struct aa_ext e = {
> 		.start = udata,
> 		.end = udata + size,
> 		.pos = udata,
> 	};
> 
> 	*ns = NULL;
> 	while (e.pos < e.end) {
> 		error = verify_header(&e, e.pos == e.start, ns);
> 		if (error)
> 			return error; /* XXX if the header fails on a
> 					 second or subsequent profile,
> 					 we'll leave the list_head
> 					 intact XXX */
> 
> 		/* alignment adjust needed by dfa unpack */
> 		e.adj = (e.pos - e.start) & 7;
> 		profile = unpack_profile(&e);
> 		if (IS_ERR(profile)) {
> 			error = PTR_ERR(profile);
> 			goto fail;
> 		}
> 
> 		error = verify_profile(profile);
> 		if (error) {
> 			aa_put_profile(profile);
> 			goto fail;
> 		}
> 
> 		/* transfer refcount to list */
> 		list_add_tail(&profile->base.list, lh);
> 	}
> 
> 	return 0;
> 
> fail:
> 	/*
> 	 * outside references to the list of profiles are not possible
> 	 * as they have not been added to the global list yet, so rcu
> 	 * is not necessary
> 	 */
> 	list_for_each_entry_safe(profile, tmp, lh, base.list) {
> 			list_del_init(&profile->base.list);
> 			aa_put_profile(profile);
> 	}
> 
> 	return error;
> }
> 
> 
> 




More information about the AppArmor mailing list