From d9866f0a2492a60df7613fb545561ccf510e31ed Mon Sep 17 00:00:00 2001 From: Georgia Garcia Date: Tue, 5 Aug 2025 15:48:28 -0300 Subject: [PATCH] parser: fix leaking disconnected paths when merging Valgrind showed that the disconnected paths variables were leaking during the merge. That happened because flagvals did not implement a destructor freeing the variables, so they leaked. flagvals cannot implement a destructor, because that would make it a non-trivial union member and parser_yacc.y would not compile. This patch implements a "clear" function that is supposed to act as the destructor. $ /usr/bin/valgrind --leak-check=full --error-exitcode=151 ../apparmor_parser -Q -I simple_tests/ -M ./features_files/features.all flags_ok_disconnected_ipc15.sd ... ==3708747== 5 bytes in 1 blocks are definitely lost in loss record 1 of 11 ==3708747== at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==3708747== by 0x492E35E: strdup (strdup.c:42) ==3708747== by 0x14C74E: set_disconnected_path (profile.h:188) ==3708747== by 0x14C74E: flagvals::init(char const*) (profile.h:223) ==3708747== by 0x14859B: yyparse() (parser_yacc.y:592) ==3708747== by 0x141A99: process_profile(int, aa_kernel_interface*, char const*, aa_policy_cache*) (parser_main.c:1187) ==3708747== by 0x135421: main (parser_main.c:1771) ... Signed-off-by: Georgia Garcia --- parser/parser_yacc.y | 1 + parser/profile.cc | 5 +---- parser/profile.h | 10 ++++++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y index 0b5d4111f..0e4e4a643 100644 --- a/parser/parser_yacc.y +++ b/parser/parser_yacc.y @@ -577,6 +577,7 @@ flags: opt_flags TOK_OPENPAREN flagvals TOK_CLOSEPAREN flagvals: flagvals flagval { $1.merge($2); + $2.clear(); $$ = $1; }; diff --git a/parser/profile.cc b/parser/profile.cc index 52a195b9d..2e46ddf22 100644 --- a/parser/profile.cc +++ b/parser/profile.cc @@ -78,6 +78,7 @@ void ProfileList::dump_profile_names(bool children) Profile::~Profile() { hat_table.clear(); + flags.clear(); free_cod_entries(entries); free_cond_entry_list(xattrs); @@ -97,10 +98,6 @@ Profile::~Profile() free(name); if (attachment) free(attachment); - if (flags.disconnected_path) - free(flags.disconnected_path); - if (flags.disconnected_ipc) - free(flags.disconnected_ipc); if (ns) free(ns); for (int i = (AA_EXEC_LOCAL >> 10) + 1; i < AA_EXEC_COUNT; i++) diff --git a/parser/profile.h b/parser/profile.h index e7df0f90e..313940167 100644 --- a/parser/profile.h +++ b/parser/profile.h @@ -175,6 +175,12 @@ public: signal = 0; error = 0; } + + void clear(void) { + free(disconnected_path); + free(disconnected_ipc); + } + void init(const char *str) { init(); @@ -301,7 +307,7 @@ public: } // same ignore rhs.disconnect_path } else { - disconnected_path = rhs.disconnected_path; + disconnected_path = strdup(rhs.disconnected_path); } } if (rhs.disconnected_ipc) { @@ -311,7 +317,7 @@ public: } // same so do nothing } else { - disconnected_ipc = rhs.disconnected_ipc; + disconnected_ipc = strdup(rhs.disconnected_ipc); } } if (rhs.signal) {