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

[Steve Beattie]

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>
---
 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.

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150828/7c658f69/attachment.pgp>