Yep, this works too.  Thank you!

On 11/22/17 8:52 AM, Stephen Smalley wrote:
On Fri, 2017-11-17 at 08:09 -0500, James Carter wrote:
Daniel Cashman <[email protected]> discovered the following:
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.

Add the field "keep" to struct cil_typeattributeset, set its value
using cil_typeattribute_used(), and use it when determining whether
the attribute is to be kept or if it should be expanded.

Signed-off-by: James Carter <[email protected]>
Thanks, applied.

Dan, let us know if this resolves your issue.

---
  libsepol/cil/src/cil.c           | 1 +
  libsepol/cil/src/cil_binary.c    | 8 ++++----
  libsepol/cil/src/cil_internal.h  | 1 +
  libsepol/cil/src/cil_policy.c    | 2 +-
  libsepol/cil/src/cil_post.c      | 2 +-
  libsepol/cil/src/cil_reset_ast.c | 1 +
  6 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
index 3fe68af8..5a64c2bc 100644
--- a/libsepol/cil/src/cil.c
+++ b/libsepol/cil/src/cil.c
@@ -2064,6 +2064,7 @@ void cil_typeattribute_init(struct
cil_typeattribute **attr)
        (*attr)->expr_list = NULL;
        (*attr)->types = NULL;
        (*attr)->used = CIL_FALSE;
+       (*attr)->keep = CIL_FALSE;
  }
void cil_typeattributeset_init(struct cil_typeattributeset
**attrset)
diff --git a/libsepol/cil/src/cil_binary.c
b/libsepol/cil/src/cil_binary.c
index c0ca60f2..431cd9cd 100644
--- a/libsepol/cil/src/cil_binary.c
+++ b/libsepol/cil/src/cil_binary.c
@@ -567,7 +567,7 @@ 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) {
+       if (!cil_attr->keep) {
                return SEPOL_OK;                
        }
@@ -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->keep) {
                return SEPOL_OK;
        }
@@ -1442,7 +1442,7 @@ static int __cil_should_expand_attribute( const
struct cil_db *db, struct cil_sy
attr = (struct cil_typeattribute *)datum; - return !attr->used || (ebitmap_cardinality(attr->types) <
db->attrs_expand_size);
+       return !attr->keep || (ebitmap_cardinality(attr->types) <
db->attrs_expand_size);
  }
int __cil_avrule_to_avtab(policydb_t *pdb, const struct cil_db *db,
struct cil_avrule *cil_avrule, cond_node_t *cond_node, enum
cil_flavor cond_flavor)
@@ -2525,7 +2525,7 @@ int
__cil_constrain_expr_datum_to_sepol_expr(policydb_t *pdb, const
struct cil_d
                        if (rc != SEPOL_OK) {
                                if (FLAVOR(item->data) ==
CIL_TYPEATTRIBUTE) {
                                        struct cil_typeattribute
*attr = item->data;
-                                       if (!attr->used) {
+                                       if (!attr->keep) {
                                                rc = 0;
                                        }
                                }
diff --git a/libsepol/cil/src/cil_internal.h
b/libsepol/cil/src/cil_internal.h
index 136a0049..8393e391 100644
--- a/libsepol/cil/src/cil_internal.h
+++ b/libsepol/cil/src/cil_internal.h
@@ -531,6 +531,7 @@ struct cil_typeattribute {
        struct cil_list *expr_list;
        ebitmap_t *types;
        int used;       // whether or not this attribute was used
in a binary policy rule
+       int keep;
  };
struct cil_typeattributeset {
diff --git a/libsepol/cil/src/cil_policy.c
b/libsepol/cil/src/cil_policy.c
index 6d4987c4..99eb53c2 100644
--- a/libsepol/cil/src/cil_policy.c
+++ b/libsepol/cil/src/cil_policy.c
@@ -1085,7 +1085,7 @@ static void cil_typeattributes_to_policy(FILE
*out, struct cil_list *types, stru
                type = i1->data;
                cil_list_for_each(i2, attributes) {
                        attribute = i2->data;
-                       if (!attribute->used)
+                       if (!attribute->keep)
                                continue;
                        if (ebitmap_get_bit(attribute->types, type-
value)) {
                                if (first) {
diff --git a/libsepol/cil/src/cil_post.c
b/libsepol/cil/src/cil_post.c
index 3e013c97..a2122454 100644
--- a/libsepol/cil/src/cil_post.c
+++ b/libsepol/cil/src/cil_post.c
@@ -1369,7 +1369,7 @@ 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);
+               attr->keep = cil_typeattribute_used(attr, db);
                break;
        }
        case CIL_ROLEATTRIBUTE: {
diff --git a/libsepol/cil/src/cil_reset_ast.c
b/libsepol/cil/src/cil_reset_ast.c
index 8a13a1c1..43e6b88e 100644
--- a/libsepol/cil/src/cil_reset_ast.c
+++ b/libsepol/cil/src/cil_reset_ast.c
@@ -186,6 +186,7 @@ static void cil_reset_typeattr(struct
cil_typeattribute *attr)
                attr->expr_list = NULL;
        }
        attr->used = CIL_FALSE;
+       attr->keep = CIL_FALSE;
  }
static void cil_reset_typeattributeset(struct cil_typeattributeset
*tas)


Reply via email to