On 11/14/2017 07:44 PM, Daniel Cashman wrote:
From: Dan Cashman <dcashman@xxxxxxxxxx>
When using cil_db multiple_decls, the different cil_attribute nodes
all point to the same underlying cil_attribute struct. This leads
to problems, though, when modifying the used value in the struct.
__cil_post_db_attr() changes the value of the field to based on
the output of cil_typeattribute_used(), for use later in
cil_typeattribute_to_policydb and cil_typeattribute_to_bitmap, but
due to the multiple declarations, cil_typeattribute_used() could be called
again by a second node. In this second call, the value used is the
modifed value of CIL_TRUE or CIL_FALSE, not the flags actually needed.
This could result in the field being reset again, to an incorrect CIL_FALSE
value. To avoid this, only change the value of the field if it is to
be set to CIL_FALSE. Also update the checks that use this value to
explicitly check for CIL_FALSE, rather than relying on its underlying
value.
Alternatively, a different field could be used rather than overwriting the
'used' field, attrib structs could be un-shared, or duplicate declarations
could just be skipped rather than sticking around in the tree.
You patch does work. I am wondering if it might not be better to have a new
field. I am going to experiment with this today and see how that would work.
Signed-off-by: Daniel Cashman <dcashman@xxxxxxxxxxx>
---
libsepol/cil/src/cil_binary.c | 6 +++---
libsepol/cil/src/cil_post.c | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
index c0ca60f2..e4003c8b 100644
--- a/libsepol/cil/src/cil_binary.c
+++ b/libsepol/cil/src/cil_binary.c
@@ -567,8 +567,8 @@ int cil_typeattribute_to_policydb(policydb_t *pdb, struct cil_typeattribute *cil
char *key = NULL;
type_datum_t *sepol_attr = NULL;
- if (!cil_attr->used) {
- return SEPOL_OK;
+ if (cil_attr->used == CIL_FALSE) {
+ return SEPOL_OK;
}
sepol_attr = cil_malloc(sizeof(*sepol_attr));
@@ -632,7 +632,7 @@ int cil_typeattribute_to_bitmap(policydb_t *pdb, const struct cil_db *db, struct
ebitmap_node_t *tnode;
unsigned int i;
- if (!cil_attr->used) {
+ if (cil_attr->used == CIL_FALSE) {
return SEPOL_OK;
}
I don't think these changes are necessary. It would be a problem if we were
checking specifically for CIL_TRUE because with your patch the used field can
contain other non-false values.
Jim
diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
index 3e013c97..823ecf0f 100644
--- a/libsepol/cil/src/cil_post.c
+++ b/libsepol/cil/src/cil_post.c
@@ -1369,7 +1369,8 @@ static int __cil_post_db_attr_helper(struct cil_tree_node *node, uint32_t *finis
rc = __evaluate_type_expression(attr, db);
if (rc != SEPOL_OK) goto exit;
}
- attr->used = cil_typeattribute_used(attr, db);
+ if (cil_typeattribute_used(attr, db) == CIL_FALSE)
+ attr->used = CIL_FALSE;
break;
}
case CIL_ROLEATTRIBUTE: {
--
James Carter <jwcart2@xxxxxxxxxxxxx>
National Security Agency
