From 9689e3a39feb4e377509924f1796e8c31f714cbf Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Wed, 13 Nov 2024 14:34:55 -0800 Subject: [PATCH 1/5] Clarify duplicate insertion logic in parser_alias.c:process_entries Signed-off-by: Ryan Lee --- parser/parser_alias.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/parser/parser_alias.c b/parser/parser_alias.c index 827128e87..635f8c880 100644 --- a/parser/parser_alias.c +++ b/parser/parser_alias.c @@ -142,8 +142,10 @@ static void process_entries(const void *nodep, VISIT value, int level unused) } if (dup) { dup->alias_ignore = true; - /* adds to the front of the list, list iteratition - * will skip it + /* The original entry->next is in dup->next, so we don't lose + * any of the original elements of the linked list. Also, by + * setting dup->alias_ignore, we trigger the check at the start + * of the loop, skipping the new entry we just inserted. */ entry->next = dup; From d9ee417dc3ce365b97e92e7d7bc1c48e4591dbcb Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Wed, 13 Nov 2024 14:45:55 -0800 Subject: [PATCH 2/5] Add missing null check in mount.cc:extract_flags There is a null check before storing invflags into inv, but not before initializing the value at inv to 0. Assuming the null check is needed, it should be there in both places. Signed-off-by: Ryan Lee --- parser/mount.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/parser/mount.cc b/parser/mount.cc index 3a966a82a..371fea43c 100644 --- a/parser/mount.cc +++ b/parser/mount.cc @@ -349,7 +349,8 @@ int is_valid_mnt_cond(const char *name, int src) static unsigned int extract_flags(struct value_list **list, unsigned int *inv) { unsigned int flags = 0, invflags = 0; - *inv = 0; + if (inv) + *inv = 0; struct value_list *entry, *tmp, *prev = NULL; list_for_each_safe(*list, entry, tmp) { From 5b391a5a4fc5433c776f4e45daf22650d87b3c36 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Wed, 13 Nov 2024 14:51:08 -0800 Subject: [PATCH 3/5] Remove unused list_remove and list_find_prev helper macros in parser.h Signed-off-by: Ryan Lee --- parser/parser.h | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/parser/parser.h b/parser/parser.h index 2d46ce3f3..5d657804c 100644 --- a/parser/parser.h +++ b/parser/parser.h @@ -242,17 +242,6 @@ do { \ len; \ }) -#define list_find_prev(LIST, ENTRY) \ -({ \ - typeof(ENTRY) tmp, prev = NULL; \ - list_for_each((LIST), tmp) { \ - if (tmp == (ENTRY)) \ - break; \ - prev = tmp; \ - } \ - prev; \ -}) - #define list_pop(LIST) \ ({ \ typeof(LIST) _entry = (LIST); \ @@ -270,12 +259,6 @@ do { \ (LIST) = (ENTRY)->next; \ (ENTRY)->next = NULL; \ -#define list_remove(LIST, ENTRY) \ -do { \ - typeof(ENTRY) prev = list_find_prev((LIST), (ENTRY)); \ - list_remove_at((LIST), prev, (ENTRY)); \ -} while (0) - #define DUP_STRING(orig, new, field, fail_target) \ do { \ From 88719dbb7b908443808c76867a153ddce5984dce Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Wed, 13 Nov 2024 15:46:38 -0800 Subject: [PATCH 4/5] Fix infinite loop in chfa.cc:weld_file_to_policy This is simple enough to fix even if weld_file_to_policy isn't used in practice with the compat layer that uses it being a target for deletion Signed-off-by: Ryan Lee --- parser/libapparmor_re/chfa.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/parser/libapparmor_re/chfa.cc b/parser/libapparmor_re/chfa.cc index f6e103681..b340c5130 100644 --- a/parser/libapparmor_re/chfa.cc +++ b/parser/libapparmor_re/chfa.cc @@ -25,6 +25,8 @@ #include #include +#include + #include #include #include @@ -587,10 +589,11 @@ void CHFA::weld_file_to_policy(CHFA &file_chfa, size_t &new_start, // to repeat assert(accept.size() == old_base_size); accept.resize(accept.size() + file_chfa.accept.size()); - size_t size = policy_perms.size(); + assert(policy_perms.size() < std::numeric_limits::max()); + ssize_t size = (ssize_t) policy_perms.size(); policy_perms.resize(size*2 + file_perms.size()); // shift and double the policy perms - for (size_t i = size - 1; size >= 0; i--) { + for (ssize_t i = size - 1; i >= 0; i--) { policy_perms[i*2] = policy_perms[i]; policy_perms[i*2 + 1] = policy_perms[i]; } From ae5a16bfd1018796807c2f65f0e37993562ed1b5 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Thu, 14 Nov 2024 10:27:07 -0800 Subject: [PATCH 5/5] Use list_remove_at macro in mount.cc:extract_flags Signed-off-by: Ryan Lee --- parser/mount.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/parser/mount.cc b/parser/mount.cc index 371fea43c..80f1e3bc4 100644 --- a/parser/mount.cc +++ b/parser/mount.cc @@ -363,11 +363,7 @@ static unsigned int extract_flags(struct value_list **list, unsigned int *inv) " => req: 0x%x inv: 0x%x\n", entry->value, mnt_opts_table[i].set, mnt_opts_table[i].clear, flags, invflags); - if (prev) - prev->next = tmp; - if (entry == *list) - *list = tmp; - entry->next = NULL; + list_remove_at(*list, prev, entry); free_value_list(entry); } else prev = entry;