[PATCH 11/11] AppArmor: use the kernel shared workqueue to free vmalloc'ed dfas

[John Johansen]

On 04/13/2010 01:22 AM, Stefan Bader wrote:
> Dunno, this looks a bit scary. Assuming the size of table is big enough to
> contain a work struct,
well yes the size is big enough, but we could bound the table size on alloc
so it is documented in the code.

> but is the struct really never touched again after
> calling the worker function?
> 
If it is its a bug.  The only way this code gets reached is if all the
refcounts on the profile and the dfa have been put.

> 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-apparmor.git AA2.5-2.6.33
>> commit: 2b0c4ab1a947adfef6a09619454b7f8eac46df96
>> BugLink: http://bugs.launchpad.net/bugs/562044
>>
>> AppArmor falls back to allocating dfas with vmalloc when memory becomes
>> fragmented.  However dfa life cycle is often dependent on credentials
>> which can be freed during interrupt context.  This can in cause the dfa
>> to be freed during interrupt context, as well but vfree can not be
>> used in interrupt context.
>>
>> So for dfas that are allocated with vmalloc delay freeing them to
>> a later time by placing them on the kernel shared workqueue.
>>
>> Signed-off-by: John Johansen <john.johansen at canonical.com>
>> ---
>>  security/apparmor/match.c |   25 ++++++++++++++++++++++---
>>  1 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>> index afc2dd2..d2cd554 100644
>> --- a/security/apparmor/match.c
>> +++ b/security/apparmor/match.c
>> @@ -17,12 +17,26 @@
>>  #include <linux/mm.h>
>>  #include <linux/slab.h>
>>  #include <linux/vmalloc.h>
>> +#include <linux/workqueue.h>
>>  #include <linux/err.h>
>>  #include <linux/kref.h>
>>  
>>  #include "include/match.h"
>>  
>>  /**
>> + * do_vfree - workqueue routine for freeing vmalloced memory
>> + * @work: data to be freed
>> + *
>> + * The work_struct is overlayed to the data being freed, as at the point
>> + * the work is scheduled the data is no longer valid, be its freeing
>> + * needs to be delayed until safe.
>> + */
>> +static void do_vfree(struct work_struct *work)
>> +{
>> +	vfree(work);
>> +}
>> +
>> +/**
>>   * free_table - free a table allocated by unpack table
>>   * @table: table to unpack  (MAYBE NULL)
>>   */
>> @@ -31,9 +45,14 @@ static void free_table(struct table_header *table)
>>  	if (!table)
>>  		return;
>>  
>> -	if (is_vmalloc_addr(table))
>> -		vfree(table);
>> -	else
>> +	if (is_vmalloc_addr(table)) {
>> +		/* Data is no longer valid so just use the allocated space
>> +		 * as the work_struct
>> +		 */
>> +		struct work_struct *work = (struct work_struct *) table;
>> +		INIT_WORK(work, do_vfree);
>> +		schedule_work(work);	  
>> +	} else
>>  		kzfree(table);
>>  }
>>  
>