On Fri, 2017-11-17 at 08:09 -0500, James Carter wrote:
> Daniel Cashman <dcashman@xxxxxxxxxxx> 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 <jwcart2@xxxxxxxxxxxxx>
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)
