From 42523bae91eb14f8ffb604db76da60892abbdc1c Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 18 Jul 2024 06:15:05 -0700 Subject: [PATCH 1/2] Revert "parser: fix potential padding bug." This reverts commit 78ae95608753b42956f2445a4965b0577fbb76de. Commit 78ae95608753b42956f2445a4965b0577fbb76de causes policy to not to conform to protocol as determined by the kernel. Technically the reverted patch is correct and the kernel is wrong but we can not change 15 years of history. The reason it breaks the policy in the kernel is because the kernel does not use the name field, and does not expect it. It just expects the size with a single trailing 0. This doesn't break because this section is all padded to 64 bytes so writing the extra 0 doesn't hurt as it is effectively just manually adding to the padding. Signed-off-by: John Johansen --- parser/libapparmor_re/chfa.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parser/libapparmor_re/chfa.cc b/parser/libapparmor_re/chfa.cc index 25edf43e5..658218958 100644 --- a/parser/libapparmor_re/chfa.cc +++ b/parser/libapparmor_re/chfa.cc @@ -419,7 +419,7 @@ void CHFA::flex_table(ostream &os, const char *name) /* Write the actual flex parser table. */ /* TODO: add max_oob */ - size_t hsize = pad64(sizeof(th) + sizeof(th_version) + 1 + strlen(name) + 1); + size_t hsize = pad64(sizeof(th) + sizeof(th_version) + strlen(name) + 1); th.th_magic = htonl(YYTH_REGEX_MAGIC); th.th_flags = htons(chfaflags); th.th_hsize = htonl(hsize); @@ -433,7 +433,7 @@ void CHFA::flex_table(ostream &os, const char *name) flex_table_size(check_vec.begin(), check_vec.end())); os.write((char *)&th, sizeof(th)); os << th_version << (char)0 << name << (char)0; - os << fill64(sizeof(th) + 1 + sizeof(th_version) + strlen(name) + 1); + os << fill64(sizeof(th) + sizeof(th_version) + strlen(name) + 1); write_flex_table(os, YYTD_ID_ACCEPT, accept.begin(), accept.end()); write_flex_table(os, YYTD_ID_ACCEPT2, accept2.begin(), accept2.end()); From 792f23c878fb0000e521ba43aba32d2cafdddd2b Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 18 Jul 2024 06:29:44 -0700 Subject: [PATCH 2/2] chfa: get not-flextable size and padding correct The kernel does not expect a name and it is not used even within the parser so drop it. Correct the padding calculation. sizeof(th_version) includes the trailing \0 in the count so we should not be adding it explicitly. Doing so made it seem like we were writing an extra byte and messing things up, because the string write below did not include the \0 which we had to add explicitly. Switch to writing the th_version using size_of() bytes as is used in the pad calculation, to avoid confusion around the header padding. Signed-off-by: John Johansen --- parser/libapparmor_re/aare_rules.cc | 2 +- parser/libapparmor_re/chfa.cc | 9 +++++---- parser/libapparmor_re/chfa.h | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/parser/libapparmor_re/aare_rules.cc b/parser/libapparmor_re/aare_rules.cc index c344117f5..410953cc8 100644 --- a/parser/libapparmor_re/aare_rules.cc +++ b/parser/libapparmor_re/aare_rules.cc @@ -307,7 +307,7 @@ void *aare_rules::create_dfa(size_t *size, int *min_match_len, optflags const &o CHFA chfa(dfa, eq, opts); if (opts.dump & DUMP_DFA_TRANS_TABLE) chfa.dump(cerr); - chfa.flex_table(stream, ""); + chfa.flex_table(stream); } catch(int error) { *size = 0; diff --git a/parser/libapparmor_re/chfa.cc b/parser/libapparmor_re/chfa.cc index 658218958..205c697c2 100644 --- a/parser/libapparmor_re/chfa.cc +++ b/parser/libapparmor_re/chfa.cc @@ -368,7 +368,7 @@ template os << fill64(sizeof(td) + sizeof(*pos) * size); } -void CHFA::flex_table(ostream &os, const char *name) +void CHFA::flex_table(ostream &os) { const char th_version[] = "notflex"; struct table_set_header th = { 0, 0, 0, 0 }; @@ -419,7 +419,8 @@ void CHFA::flex_table(ostream &os, const char *name) /* Write the actual flex parser table. */ /* TODO: add max_oob */ - size_t hsize = pad64(sizeof(th) + sizeof(th_version) + strlen(name) + 1); + // sizeof(th_version) includes trailing \0 + size_t hsize = pad64(sizeof(th) + sizeof(th_version)); th.th_magic = htonl(YYTH_REGEX_MAGIC); th.th_flags = htons(chfaflags); th.th_hsize = htonl(hsize); @@ -432,8 +433,8 @@ void CHFA::flex_table(ostream &os, const char *name) flex_table_size(next_vec.begin(), next_vec.end()) + flex_table_size(check_vec.begin(), check_vec.end())); os.write((char *)&th, sizeof(th)); - os << th_version << (char)0 << name << (char)0; - os << fill64(sizeof(th) + sizeof(th_version) + strlen(name) + 1); + os.write(th_version, sizeof(th_version)); + os << fill64(sizeof(th) + sizeof(th_version)); write_flex_table(os, YYTD_ID_ACCEPT, accept.begin(), accept.end()); write_flex_table(os, YYTD_ID_ACCEPT2, accept2.begin(), accept2.end()); diff --git a/parser/libapparmor_re/chfa.h b/parser/libapparmor_re/chfa.h index 9548080b2..7d6a6f4af 100644 --- a/parser/libapparmor_re/chfa.h +++ b/parser/libapparmor_re/chfa.h @@ -39,7 +39,7 @@ class CHFA { public: CHFA(DFA &dfa, map &eq, optflags const &opts); void dump(ostream & os); - void flex_table(ostream &os, const char *name); + void flex_table(ostream &os); void init_free_list(vector > &free_list, size_t prev, size_t start); bool fits_in(vector > &free_list, size_t base,