[PATCH 08/11] AppArmor: Make sure to unmap aliases for vmalloced dfas before they are live

[Andy Whitcroft]

On Tue, Apr 13, 2010 at 12:09:37AM -0700, john.johansen at canonical.com wrote:
> From: John Johansen <john.johansen at canonical.com>
> 
> OriginalAuthor: John Johansen <john.johansen at canonical.com>
> OriginalLocation: git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparm$
> commit: 6857acf643ba19eddfc29125fc011a3ce48fe87b
> BugLink: http://bugs.launchpad.net/bugs/529288
> 
> vmalloc doesn't guarentee that the tlbs of all cpus will be flushed
> when it completes.  Instead the tlbs gets flushed lazily, however for
> AppArmor this is a problem as the dfa becomes live to all cpus as
> soon as the profile replacedby value is written (this is even before
> locking of the lists are removed).
> 
> It is possible for another cpu to be in a state where it has an old
> tlb mapping for the vmalloc address (this will be caused by putting
> a reference on an old profile while replacing to the current),
> so that it references to the wrong memory location when doing dfa
> lookups.
> 
> Replacement is not a common operation so make sure all memory
> aliases are removed before the dfa goes live.
> 
> Signed-off-by: John Johansen <john.johansen at canonical.com>
> ---
>  security/apparmor/match.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> index 5a55959..afc2dd2 100644
> --- a/security/apparmor/match.c
> +++ b/security/apparmor/match.c
> @@ -50,6 +50,7 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
>  {
>  	struct table_header *table = NULL;
>  	struct table_header th;
> +	int unmap_alias = 0;
>  	size_t tsize;
>  
>  	if (bsize < sizeof(struct table_header))
> @@ -73,8 +74,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
>  
>  	/* freed by free_table */
>  	table = kmalloc(tsize, GFP_KERNEL | __GFP_NOWARN);
> -	if (!table)
> +	if (!table) {
> +		unmap_alias = 1;
>  		table = vmalloc(tsize);

It is possible this could be below more optimally:

		table = vmalloc(tsize);
		if (table)
			unmap_alias = 1

> +	}
>  	if (table) {
>  		*table = th;
>  		if (th.td_flags == YYTD_DATA8)
> @@ -91,6 +94,8 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
>  	}
>  
>  out:
> +	if (unmap_alias)
> +		vm_unmap_aliases();
>  	return table;
>  fail:
>  	free_table(table);

Even without the above it seems sane and necessary:

Acked-by: Andy Whitcroft <apw at canonical.com>

-apw