[apparmor] Memory leaks in parser/parser_regex.c (at least)

[John Johansen]

On 08/28/2015 05:26 PM, Steve Beattie wrote:
> On Fri, Aug 28, 2015 at 04:36:35PM +0200, intrigeri wrote:
>> Steve Beattie wrote (27 Aug 2015 20:44:58 GMT) :
>>> It looks like the parser never drops it's reference to the policy cache
>>> object. This patch looks to fix the issue:
>>
>> It helps! With your 5 patches applied, including the two ones you've
>> sent yesterday, the build now stops at:
>>
>> FAIL: test case failed for testcase_dmesg_rename_dest: child process exited abnormally
>> FAIL: test case failed for testcase_dmesg_link_01: child process exited abnormally
>> FAIL: test case failed for testcase_dmesg_rename_src: child process exited abnormally
>> FAIL: test case failed for testcase_dmesg_mkdir: child process exited abnormally
>> FAIL: test case failed for avc_audit_invalid_audit_id: child process exited abnormally
>> FAIL: test case failed for syslog_other_02: child process exited abnormally
>> FAIL: test case failed for testcase_dmesg_truncate: child process exited abnormally
>> FAIL: test case failed for testcase_dmesg_changehat_negative_error: child process exited abnormally
>> FAIL: test case failed for syslog_other_03: child process exited abnormally
>> FAIL: test case failed for testcase_dmesg_changeprofile_01: child process exited abnormally
>> FAIL: test case failed for syslog_other_04: child process exited abnormally
>> FAIL: test case failed for testcase_dmesg_capability: child process exited abnormally
>> FAIL: test case failed for syslog_other_01: child process exited abnormally
>> FAIL: test case failed for testcase_dmesg_status_offset: child process exited abnormally
> 
> The following patch should address these:
> 
> Signed-off-by: Steve Beattie <steve at nxnw.org>

Acked-by: John Johansen <john.johansen at canonical.com>

> ---
>  libraries/libapparmor/src/grammar.y |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> Index: b/libraries/libapparmor/src/grammar.y
> ===================================================================
> --- a/libraries/libapparmor/src/grammar.y
> +++ b/libraries/libapparmor/src/grammar.y
> @@ -166,6 +166,11 @@ aa_record_event_type lookup_aa_event(uns
>  %token TOK_SYSLOG_KERNEL
>  %token TOK_SYSLOG_USER
>  
> +%destructor { free($$); } TOK_QUOTED_STRING TOK_ID TOK_MODE TOK_DMESG_STAMP
> +%destructor { free($$); } TOK_AUDIT_DIGITS TOK_DATE_MONTH TOK_DATE TOK_TIME
> +%destructor { free($$); } TOK_HEXSTRING TOK_TYPE_OTHER TOK_MSG_REST
> +%destructor { free($$); } TOK_IP_ADDR
> +
>  %%
>  
>  log_message: audit_type
> @@ -201,7 +206,7 @@ other_audit: TOK_TYPE_OTHER audit_msg TO
>  	;
>  
>  dmesg_type: TOK_DMESG_STAMP TOK_AUDIT TOK_COLON key_type audit_id key_list
> -	{ ret_record->version = AA_RECORD_SYNTAX_V2; }
> +	{ ret_record->version = AA_RECORD_SYNTAX_V2; free($1); }
>  	;
>  
>  syslog_type:
> 
> The latter fix is a legit oversight. The former deconstructors are
> for when bison can't find a solution and needs to pop state back off
> its stack (in our case, when we've failed to parse a log message).
> 
> However, this results in log parsing failures that have very little
> content; for some of our failures we populate the unparsed portion
> of log entry in the info field (e.g. parsing old old old non longer
> used anywhere style log messages), but for many we do not. It would
> be good if we could better and consistent here.
> 
> 
>