From 43fa5f88a774a967fbece1687ed32d28674648a4 Mon Sep 17 00:00:00 2001 From: Georgia Garcia Date: Wed, 6 Aug 2025 18:34:46 -0300 Subject: [PATCH] parser: fix cases leaks when new state creation fails For every item in "cases", a new state is created, but if the creation of one of them fails, the rest of the items in that list would not be deleted and would leak. Fix it by continuing to iterate over the items in the list and delete them, and then re-throw the exception. $ /usr/bin/valgrind --leak-check=full --error-exitcode=151 ../apparmor_parser -Q -I simple_tests/ -M ./features_files/features.all simple_tests/xtrans/x-conflict.sd ==564911== 208 (48 direct, 160 indirect) bytes in 1 blocks are definitely lost in loss record 15 of 19 ==564911== at 0x4846FA3: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==564911== by 0x179E74: CharNode::follow(Cases&) (expr-tree.h:447) ==564911== by 0x189F8B: DFA::update_state_transitions(optflags const&, State*) (hfa.cc:376) ==564911== by 0x18A25B: DFA::process_work_queue(char const*, optflags const&) (hfa.cc:442) ==564911== by 0x18CB65: DFA::DFA(Node*, optflags const&, bool) (hfa.cc:486) ==564911== by 0x178263: aare_rules::create_chfa(int*, std::vector >&, optflags const&, bool, bool) (aare_rules.cc:258) ==564911== by 0x178A4F: aare_rules::create_dfablob(unsigned long*, int*, std::vector >&, optflags const&, bool, bool) (aare_rules.cc:359) ==564911== by 0x14E4E1: process_profile_regex(Profile*) (parser_regex.c:791) ==564911== by 0x154CDF: process_profile_rules(Profile*) (parser_policy.c:194) ==564911== by 0x154E0F: post_process_profile(Profile*, int) (parser_policy.c:240) ==564911== by 0x154F7A: post_process_policy_list (parser_policy.c:257) ==564911== by 0x154F7A: post_process_policy(int) (parser_policy.c:267) ==564911== by 0x141B17: process_profile(int, aa_kernel_interface*, char const*, aa_policy_cache*) (parser_main.c:1227) Fixes: https://gitlab.com/apparmor/apparmor/-/issues/534 Signed-off-by: Georgia Garcia --- parser/libapparmor_re/hfa.cc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/parser/libapparmor_re/hfa.cc b/parser/libapparmor_re/hfa.cc index a1fea95b0..83aa19783 100644 --- a/parser/libapparmor_re/hfa.cc +++ b/parser/libapparmor_re/hfa.cc @@ -401,7 +401,17 @@ void DFA::update_state_transitions(optflags const &opts, State *state) */ for (Cases::iterator j = cases.begin(); j != cases.end(); j++) { State *target; - target = add_new_state(opts, j->second, nonmatching); + try { + target = add_new_state(opts, j->second, nonmatching); + } catch (int error) { + /* when add_new_state fails, there could still + * be NodeSets in the rest of cases, so clean + * them up before re-throwing the exception */ + for (Cases::iterator k = ++j; k != cases.end(); k++) { + delete k->second; + } + throw error; + } /* Don't insert transition that the otherwise transition * already covers