linux-kernel - Re: [AppArmor 39/41] AppArmor: Profile loading and manipulation, pathname matching

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <200704151621.52906.agruen@suse.de>
Date:	Sun, 15 Apr 2007 16:21:52 +0200
From:	Andreas Gruenbacher <agruen@...e.de>
To:	Andi Kleen <andi@...stfloor.org>
Cc:	jjohansen@...e.de, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, chrisw@...s-sol.org
Subject: Re: [AppArmor 39/41] AppArmor: Profile loading and manipulation, pathname matching

On Thursday 12 April 2007 15:46, Andi Kleen wrote:
> > +struct aa_dfa {
> > +	struct table_header *tables[YYTD_ID_NXT];
> > +};
> 
> If that is passed in from user space you would need special compat
> code for 64bit kernels who support 32bit userland.
> Better to avoid pointers.

This struct is not passed between the kernel and user space; no problem here. 
In fact the profiles format used is fully machine independent.

> > +
> > +	/* get optional subprofiles */
> > +	if (aa_is_nameX(e, AA_LIST, "hats")) {
> > +		while (!aa_is_nameX(e, AA_LISTEND, NULL)) {
> > +			struct aa_profile *subprofile;
> > +			subprofile = aa_unpack_profile(e);
> 
> Is there any check that would guard the recursion from stack
> overflow on malicious input?  

It's nice to check for consistency though, so we're adding that. Profile 
loading is a trusted operation, at least so far, and so security wise we 
don't actually have to care --- if loading an invalid profile can bring down 
the system, then that's no worse than an arbitrary module that crashes the 
machine. Not sure if there will ever be user loadable profiles; at least at 
that point we had to care.

> > +	/*
> > +	 * Replacement needs to allocate a new aa_task_context for each
> > +	 * task confined by old_profile.  To do this the profile locks
> > +	 * are only held when the actual switch is done per task.  While
> > +	 * looping to allocate a new aa_task_context the old_task list
> > +	 * may get shorter if tasks exit/change their profile but will
> > +	 * not get longer as new task will not use old_profile detecting
> > +	 * that is stale.
> > +	 */
> > +	do {
> > +		new_cxt = aa_alloc_task_context(GFP_KERNEL | __GFP_NOFAIL);
> 
> NOFAIL is usually a bad sign. It should be only used if there is no
> alternative.

At this point there is no secure alternative to allocating a task context --- 
except killing the task, maybe.

Thanks,
Andreas
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/