In checkpolicy, sensitivities that have aliases will temporarily
share the mls_level_t structure until a level statement defines the
categories for the level and the alias is updated to have its own
mls_level_t structure. Currently, this does not cause a problem
because checkpolicy does very little clean-up before exiting when
an error is detected. But if the policydb is destroyed before exiting
due to an error after a sensitivity and its alias is declared, but
before a level statement involving either of them, then a double
free of the shared mls_level_t will occur.
The defined field of the level_datum_t is set after a level statement
is processed for the level_datum_t. This means that we know the alias
has its own mls_level_t if the defined field is set. This means that
the defined field can be used to determine whether or not the
mls_level_t pointed to by an alias level_datum_t should be destroyed.
Since the defined field is not set when reading or expanding a policy,
update libsepol to set the defined field.
Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
---
checkpolicy/policy_define.c | 11 +++++++----
libsepol/src/expand.c | 1 +
libsepol/src/policydb.c | 7 +++++--
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 260e609d..542bb978 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -1006,9 +1006,10 @@ static int clone_level(hashtab_key_t key __attribute__ ((unused)), hashtab_datum
mls_level_t *level = (mls_level_t *) arg, *newlevel;
if (levdatum->level == level) {
- levdatum->defined = 1;
- if (!levdatum->isalias)
+ if (!levdatum->isalias) {
+ levdatum->defined = 1;
return 0;
+ }
newlevel = (mls_level_t *) malloc(sizeof(mls_level_t));
if (!newlevel)
return -1;
@@ -1017,6 +1018,7 @@ static int clone_level(hashtab_key_t key __attribute__ ((unused)), hashtab_datum
return -1;
}
levdatum->level = newlevel;
+ levdatum->defined = 1;
}
return 0;
}
@@ -1057,8 +1059,6 @@ int define_level(void)
}
free(id);
- levdatum->defined = 1;
-
while ((id = queue_remove(id_queue))) {
cat_datum_t *cdatum;
int range_start, range_end, i;
@@ -1121,6 +1121,9 @@ int define_level(void)
free(id);
}
+ if (!levdatum->isalias)
+ levdatum->defined = 1;
+
if (hashtab_map
(policydbp->p_levels.table, clone_level, levdatum->level)) {
yyerror("out of memory");
diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index e63414b1..0e16c502 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -1191,6 +1191,7 @@ static int sens_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
goto out_of_mem;
}
new_level->isalias = level->isalias;
+ new_level->defined = 1;
state->out->p_levels.nprim++;
if (hashtab_insert(state->out->p_levels.table,
diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index f10a8a95..0c950bf1 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -1390,8 +1390,10 @@ static int sens_destroy(hashtab_key_t key, hashtab_datum_t datum, void *p
if (key)
free(key);
levdatum = (level_datum_t *) datum;
- mls_level_destroy(levdatum->level);
- free(levdatum->level);
+ if (!levdatum->isalias || levdatum->defined) {
+ mls_level_destroy(levdatum->level);
+ free(levdatum->level);
+ }
level_datum_destroy(levdatum);
free(levdatum);
return 0;
@@ -3357,6 +3359,7 @@ static int sens_read(policydb_t * p
goto bad;
levdatum->isalias = le32_to_cpu(buf[1]);
+ levdatum->defined = 1;
levdatum->level = malloc(sizeof(mls_level_t));
if (!levdatum->level || mls_read_level(levdatum->level, fp))
--
2.43.0
