Skip to contentSkip to conversation

Resolve conflicts in expandattribute.

PATCH Superseded Tri Vo 3 participants 4 messages Mar 16, 2018 – Mar 26, 2018 SELinux security module

lore
Share & tools
lore

Tri Vo

author
#0 lore raw

PATCH v3 1/1 Resolve conflicts in expandattribute.

This commit resolves conflicts in values of expandattribute statements in policy language and expandtypeattribute in CIL.

For example, these statements resolve to false in policy language:
expandattribute hal_audio true;
expandattribute hal_audio false;

Similarly, in CIL these also resolve to false.
(expandtypeattribute (hal_audio) true)
(expandtypeattribute (hal_audio) false)

Motivation
When Android combines multiple .cil files from system.img and vendor.img it's possible to have conflicting expandattribute statements.

This change deals with this scenario by resolving the value of the corresponding expandtypeattribute to false. The rationale behind this override is that true is used for reduce run-time lookups, while false is used for tests which must pass.

---
 checkpolicy/policy_define.c        | 10 ++++++----
 libsepol/cil/src/cil_resolve_ast.c | 21 ++++++---------------
 2 files changed, 12 insertions(+), 19 deletions(-)
checkpolicy/policy_define.c +6 -4
index 2c5db55d..40cc62b0 100644
@@ -1182,10 +1182,6 @@ int expand_attrib(void)
 			goto exit;
 		}
 
-		if (attr->flags & TYPE_FLAGS_EXPAND_ATTR) {
-			yyerror2("%s already has the expandattribute option specified", id);
-			goto exit;
-		}
 		if (ebitmap_set_bit(&attrs, attr->s.value - 1, TRUE)) {
 			yyerror("Out of memory!");
 			goto exit;
@@ -1213,6 +1209,12 @@ int expand_attrib(void)
 		attr = hashtab_search(policydbp->p_types.table,
 				policydbp->sym_val_to_name[SYM_TYPES][i]);
 		attr->flags |= flags;
+		if ((attr->flags & TYPE_FLAGS_EXPAND_ATTR_TRUE) &&
+				(attr->flags & TYPE_FLAGS_EXPAND_ATTR_FALSE)) {
+			yywarn("Expandattribute option was set to both true and false. "
+				"Resolving to false.");
+			attr->flags &= ~TYPE_FLAGS_EXPAND_ATTR_TRUE;
+		}
 	}
 
 	rc = 0;
libsepol/cil/src/cil_resolve_ast.c +6 -15
index d1a5ed87..02259241 100644
@@ -269,9 +269,8 @@ exit:
 	return rc;
 }
 
-int cil_type_used(struct cil_symtab_datum *datum, int used)
+void cil_type_used(struct cil_symtab_datum *datum, int used)
 {
-	int rc = SEPOL_ERR;
 	struct cil_typeattribute *attr = NULL;
 
 	if (FLAVOR(datum) == CIL_TYPEATTRIBUTE) {
@@ -279,16 +278,12 @@ int cil_type_used(struct cil_symtab_datum *datum, int used)
 		attr->used |= used;
 		if ((attr->used & CIL_ATTR_EXPAND_TRUE) &&
 				(attr->used & CIL_ATTR_EXPAND_FALSE)) {
-			cil_log(CIL_ERR, "Conflicting use of expandtypeattribute. "
-					"Expandtypeattribute may be set to true or false "
-					"but not both. \n");
-			goto exit;
+			cil_log(CIL_WARN, "Conflicting use of expandtypeattribute. "
+					"Expandtypeattribute was set to both true or false for %s. "
+					"Resolving to false. \n", attr->datum.name);
+			attr->used &= ~CIL_ATTR_EXPAND_TRUE;
 		}
 	}
-
-	return SEPOL_OK;
-exit:
-	return rc;
 }
 
 int cil_resolve_permissionx(struct cil_tree_node *current, struct cil_permissionx *permx, void *extra_args)
@@ -488,11 +483,7 @@ int cil_resolve_expandtypeattribute(struct cil_tree_node *current, void *extra_a
 			goto exit;
 		}
 		used = expandattr->expand ? CIL_ATTR_EXPAND_TRUE : CIL_ATTR_EXPAND_FALSE;
-		rc = cil_type_used(attr_datum, used);
-		if (rc != SEPOL_OK) {
-			goto exit;
-		}
-
+		cil_type_used(attr_datum, used);
 		cil_list_append(expandattr->attr_datums, CIL_TYPE, attr_datum);
 	}
 
Signature · 1 line
2.16.2.804.g6dcf76e118-goog
author
#1 lore raw

Wait. Let me send this as a separate email.

Quoted 106 lines · Tri Vo

This commit resolves conflicts in values of expandattribute statements in policy language and expandtypeattribute in CIL.

For example, these statements resolve to false in policy language:
expandattribute hal_audio true;
expandattribute hal_audio false;

Similarly, in CIL these also resolve to false.
(expandtypeattribute (hal_audio) true)
(expandtypeattribute (hal_audio) false)

Motivation
When Android combines multiple .cil files from system.img and vendor.img it's possible to have conflicting expandattribute statements.

This change deals with this scenario by resolving the value of the corresponding expandtypeattribute to false. The rationale behind this override is that true is used for reduce run-time lookups, while false is used for tests which must pass.

---
 checkpolicy/policy_define.c        | 10 ++++++----
 libsepol/cil/src/cil_resolve_ast.c | 21 ++++++---------------
 2 files changed, 12 insertions(+), 19 deletions(-)
checkpolicy/policy_define.c +6 -4
index 2c5db55d..40cc62b0 100644
@@ -1182,10 +1182,6 @@ int expand_attrib(void)
                        goto exit;
                }
 
-               if (attr->flags & TYPE_FLAGS_EXPAND_ATTR) {
-                       yyerror2("%s already has the expandattribute option specified", id);
-                       goto exit;
-               }
                if (ebitmap_set_bit(&attrs, attr->s.value - 1, TRUE)) {
                        yyerror("Out of memory!");
                        goto exit;
@@ -1213,6 +1209,12 @@ int expand_attrib(void)
                attr = hashtab_search(policydbp->p_types.table,
                                policydbp->sym_val_to_name[SYM_TYPES][i]);
                attr->flags |= flags;
+               if ((attr->flags & TYPE_FLAGS_EXPAND_ATTR_TRUE) &&
+                               (attr->flags & TYPE_FLAGS_EXPAND_ATTR_FALSE)) {
+                       yywarn("Expandattribute option was set to both true and false. "
+                               "Resolving to false.");
+                       attr->flags &= ~TYPE_FLAGS_EXPAND_ATTR_TRUE;
+               }
        }
 
        rc = 0;
libsepol/cil/src/cil_resolve_ast.c +6 -16
index d1a5ed87..02259241 100644
@@ -269,9 +269,8 @@ exit:
        return rc;
 }
 
-int cil_type_used(struct cil_symtab_datum *datum, int used)
+void cil_type_used(struct cil_symtab_datum *datum, int used)
 {
-       int rc = SEPOL_ERR;
        struct cil_typeattribute *attr = NULL;
 
        if (FLAVOR(datum) == CIL_TYPEATTRIBUTE) {
@@ -279,16 +278,12 @@ int cil_type_used(struct cil_symtab_datum *datum, int used)
                attr->used |= used;
                if ((attr->used & CIL_ATTR_EXPAND_TRUE) &&
                                (attr->used & CIL_ATTR_EXPAND_FALSE)) {
-                       cil_log(CIL_ERR, "Conflicting use of expandtypeattribute. "
-                                       "Expandtypeattribute may be set to true or false "
-                                       "but not both. \n");
-                       goto exit;
+                       cil_log(CIL_WARN, "Conflicting use of expandtypeattribute. "
+                                       "Expandtypeattribute was set to both true or false for %s. "
+                                       "Resolving to false. \n", attr->datum.name);
+                       attr->used &= ~CIL_ATTR_EXPAND_TRUE;
                }
        }
-
-       return SEPOL_OK;
-exit:
-       return rc;
 }
 
 int cil_resolve_permissionx(struct cil_tree_node *current, struct cil_permissionx *permx, void *extra_args)
@@ -488,11 +483,7 @@ int cil_resolve_expandtypeattribute(struct cil_tree_node *current, void *extra_a
                        goto exit;
                }
                used = expandattr->expand ? CIL_ATTR_EXPAND_TRUE : CIL_ATTR_EXPAND_FALSE;
-               rc = cil_type_used(attr_datum, used);
-               if (rc != SEPOL_OK) {
-                       goto exit;
-               }
-
+               cil_type_used(attr_datum, used);
                cil_list_append(expandattr->attr_datums, CIL_TYPE, attr_datum);
        }
 
--

2.16.2.804.g6dcf76e118-goog

Acked-by on v3 1/1
#2 lore raw
In reply to Tri Vo
Quoted 15 lines · Tri Vo

This commit resolves conflicts in values of expandattribute statements in policy language and expandtypeattribute in CIL.

For example, these statements resolve to false in policy language:
expandattribute hal_audio true;
expandattribute hal_audio false;

Similarly, in CIL these also resolve to false.
(expandtypeattribute (hal_audio) true)
(expandtypeattribute (hal_audio) false)

Motivation
When Android combines multiple .cil files from system.img and vendor.img it's possible to have conflicting expandattribute statements.

Quoted 6 lines

This change deals with this scenario by resolving the value of the corresponding expandtypeattribute to false. The rationale behind this override is that true is used for reduce run-time lookups, while false is used for tests which must pass.

Quoted 83 lines
---
  checkpolicy/policy_define.c        | 10 ++++++----
  libsepol/cil/src/cil_resolve_ast.c | 21 ++++++---------------
  2 files changed, 12 insertions(+), 19 deletions(-)
checkpolicy/policy_define.c +6 -4
index 2c5db55d..40cc62b0 100644
@@ -1182,10 +1182,6 @@ int expand_attrib(void)
  			goto exit;
  		}
 
-		if (attr->flags & TYPE_FLAGS_EXPAND_ATTR) {
-			yyerror2("%s already has the expandattribute option specified", id);
-			goto exit;
-		}
  		if (ebitmap_set_bit(&attrs, attr->s.value - 1, TRUE)) {
  			yyerror("Out of memory!");
  			goto exit;
@@ -1213,6 +1209,12 @@ int expand_attrib(void)
  		attr = hashtab_search(policydbp->p_types.table,
  				policydbp->sym_val_to_name[SYM_TYPES][i]);
  		attr->flags |= flags;
+		if ((attr->flags & TYPE_FLAGS_EXPAND_ATTR_TRUE) &&
+				(attr->flags & TYPE_FLAGS_EXPAND_ATTR_FALSE)) {
+			yywarn("Expandattribute option was set to both true and false. "
+				"Resolving to false.");
+			attr->flags &= ~TYPE_FLAGS_EXPAND_ATTR_TRUE;
+		}
  	}
 
  	rc = 0;
libsepol/cil/src/cil_resolve_ast.c +6 -15
index d1a5ed87..02259241 100644
@@ -269,9 +269,8 @@ exit:
  	return rc;
  }
 
-int cil_type_used(struct cil_symtab_datum *datum, int used)
+void cil_type_used(struct cil_symtab_datum *datum, int used)
  {
-	int rc = SEPOL_ERR;
  	struct cil_typeattribute *attr = NULL;
 
  	if (FLAVOR(datum) == CIL_TYPEATTRIBUTE) {
@@ -279,16 +278,12 @@ int cil_type_used(struct cil_symtab_datum *datum, int used)
  		attr->used |= used;
  		if ((attr->used & CIL_ATTR_EXPAND_TRUE) &&
  				(attr->used & CIL_ATTR_EXPAND_FALSE)) {
-			cil_log(CIL_ERR, "Conflicting use of expandtypeattribute. "
-					"Expandtypeattribute may be set to true or false "
-					"but not both. \n");
-			goto exit;
+			cil_log(CIL_WARN, "Conflicting use of expandtypeattribute. "
+					"Expandtypeattribute was set to both true or false for %s. "
+					"Resolving to false. \n", attr->datum.name);
+			attr->used &= ~CIL_ATTR_EXPAND_TRUE;
  		}
  	}
-
-	return SEPOL_OK;
-exit:
-	return rc;
  }
 
  int cil_resolve_permissionx(struct cil_tree_node *current, struct cil_permissionx *permx, void *extra_args)
@@ -488,11 +483,7 @@ int cil_resolve_expandtypeattribute(struct cil_tree_node *current, void *extra_a
  			goto exit;
  		}
  		used = expandattr->expand ? CIL_ATTR_EXPAND_TRUE : CIL_ATTR_EXPAND_FALSE;
-		rc = cil_type_used(attr_datum, used);
-		if (rc != SEPOL_OK) {
-			goto exit;
-		}
-
+		cil_type_used(attr_datum, used);
  		cil_list_append(expandattr->attr_datums, CIL_TYPE, attr_datum);
  	}
 
Signature · 2 lines
James Carter <[email protected]>
National Security Agency
#3 lore raw

Merged.

Quoted 128 lines · On Mon, Mar 19, 2018 at 11:55 AM jwcart2 <[email protected]> wrote:
Quoted 15 lines · Tri Vo

This commit resolves conflicts in values of expandattribute statements in policy language and expandtypeattribute in CIL.

For example, these statements resolve to false in policy language:
expandattribute hal_audio true;
expandattribute hal_audio false;

Similarly, in CIL these also resolve to false.
(expandtypeattribute (hal_audio) true)
(expandtypeattribute (hal_audio) false)

Motivation
When Android combines multiple .cil files from system.img and vendor.img it's possible to have conflicting expandattribute statements.

Quoted 6 lines

This change deals with this scenario by resolving the value of the corresponding expandtypeattribute to false. The rationale behind this override is that true is used for reduce run-time lookups, while false is used for tests which must pass.

Quoted 10 lines
---
  checkpolicy/policy_define.c        | 10 ++++++----
  libsepol/cil/src/cil_resolve_ast.c | 21 ++++++---------------
  2 files changed, 12 insertions(+), 19 deletions(-)
checkpolicy/policy_define.c +0 -0
index 2c5db55d..40cc62b0 100644

checkpolicy/policy_define.c int expand_attrib(void) · line 1186

Quoted 5 lines
checkpolicy/policy_define.c +0 -2
@@ -1182,10 +1182,6 @@ int expand_attrib(void)
                      goto exit;
              }
 
-             if (attr->flags & TYPE_FLAGS_EXPAND_ATTR) {
-                     yyerror2("%s already has the expandattribute

option specified", id);

checkpolicy/policy_define.c int expand_attrib(void) · line 1213

Quoted 11 lines
checkpolicy/policy_define.c +2 -2
-                     goto exit;
-             }
              if (ebitmap_set_bit(&attrs, attr->s.value - 1, TRUE)) {
                      yyerror("Out of memory!");
                      goto exit;
@@ -1213,6 +1209,12 @@ int expand_attrib(void)
              attr = hashtab_search(policydbp->p_types.table,
                              policydbp->sym_val_to_name[SYM_TYPES][i]);
              attr->flags |= flags;
+             if ((attr->flags & TYPE_FLAGS_EXPAND_ATTR_TRUE) &&
+                             (attr->flags &

TYPE_FLAGS_EXPAND_ATTR_FALSE)) {

checkpolicy/policy_define.c int expand_attrib(void) · line 1214

Quoted 1 line
checkpolicy/policy_define.c +1 -0
+                     yywarn("Expandattribute option was set to both

true and false. "

Review

Quoted 7 lines
checkpolicy/policy_define.c +3 -0
+                             "Resolving to false.");
+                     attr->flags &= ~TYPE_FLAGS_EXPAND_ATTR_TRUE;
+             }
      }
 
      rc = 0;
(quoted diff) +0 -0

b/libsepol/cil/src/cil_resolve_ast.c

libsepol/cil/src/cil_resolve_ast.c int cil_type_used(struct cil_symtab_datum *datum,

Quoted 15 lines

index d1a5ed87..02259241 100644

libsepol/cil/src/cil_resolve_ast.c +1 -2
@@ -269,9 +269,8 @@ exit:
      return rc;
  }
 
-int cil_type_used(struct cil_symtab_datum *datum, int used)
+void cil_type_used(struct cil_symtab_datum *datum, int used)
  {
-     int rc = SEPOL_ERR;
      struct cil_typeattribute *attr = NULL;
 
      if (FLAVOR(datum) == CIL_TYPEATTRIBUTE) {
@@ -279,16 +278,12 @@ int cil_type_used(struct cil_symtab_datum *datum,

int used)

libsepol/cil/src/cil_resolve_ast.c int cil_type_used(struct cil_symtab_datum *datum, int used)

Quoted 4 lines
libsepol/cil/src/cil_resolve_ast.c +0 -1
              attr->used |= used;
              if ((attr->used & CIL_ATTR_EXPAND_TRUE) &&
                              (attr->used & CIL_ATTR_EXPAND_FALSE)) {
-                     cil_log(CIL_ERR, "Conflicting use of

expandtypeattribute. "

libsepol/cil/src/cil_resolve_ast.c int cil_type_used(struct cil_symtab_datum *datum, · line 283

Quoted 1 line
libsepol/cil/src/cil_resolve_ast.c +0 -1
-                                     "Expandtypeattribute may be set to

true or false "

libsepol/cil/src/cil_resolve_ast.c int cil_type_used(struct cil_symtab_datum *datum, int used)

Quoted 3 lines
libsepol/cil/src/cil_resolve_ast.c +1 -2
-                                     "but not both. \n");
-                     goto exit;
+                     cil_log(CIL_WARN, "Conflicting use of

expandtypeattribute. "

libsepol/cil/src/cil_resolve_ast.c int cil_type_used(struct cil_symtab_datum *datum, · line 282

Quoted 1 line
libsepol/cil/src/cil_resolve_ast.c +1 -0
+                                     "Expandtypeattribute was set to

both true or false for %s. "

libsepol/cil/src/cil_resolve_ast.c int cil_type_used(struct cil_symtab_datum *datum, · line 283

Quoted 1 line
libsepol/cil/src/cil_resolve_ast.c +1 -0
+                                     "Resolving to false. \n", attr->

datum.name);

libsepol/cil/src/cil_resolve_ast.c int cil_type_used(struct cil_symtab_datum *datum, int used)

Quoted 10 lines
libsepol/cil/src/cil_resolve_ast.c +1 -4
+                     attr->used &= ~CIL_ATTR_EXPAND_TRUE;
              }
      }
-
-     return SEPOL_OK;
-exit:
-     return rc;
  }
 
  int cil_resolve_permissionx(struct cil_tree_node *current, struct

cil_permissionx *permx, void *extra_args)

libsepol/cil/src/cil_resolve_ast.c int cil_resolve_expandtypeattribute(struct

Quoted 1 line
libsepol/cil/src/cil_resolve_ast.c +0 -0
@@ -488,11 +483,7 @@ int cil_resolve_expandtypeattribute(struct

cil_tree_node *current, void *extra_a

libsepol/cil/src/cil_resolve_ast.c int cil_resolve_expandtypeattribute(struct · line 485

Quoted 3 lines
libsepol/cil/src/cil_resolve_ast.c +0 -0
                      goto exit;
              }
              used = expandattr->expand ? CIL_ATTR_EXPAND_TRUE :

CIL_ATTR_EXPAND_FALSE;

libsepol/cil/src/cil_resolve_ast.c int cil_resolve_expandtypeattribute(struct cil_tree_node *current, void *extra_a

Quoted 7 lines
libsepol/cil/src/cil_resolve_ast.c +1 -5
-             rc = cil_type_used(attr_datum, used);
-             if (rc != SEPOL_OK) {
-                     goto exit;
-             }
-
+             cil_type_used(attr_datum, used);
              cil_list_append(expandattr->attr_datums, CIL_TYPE,

attr_datum);

libsepol/cil/src/cil_resolve_ast.c int cil_resolve_expandtypeattribute(struct · line 489

Quoted 3 lines
libsepol/cil/src/cil_resolve_ast.c +0 -0
      }
 

--
James Carter <[email protected]>
National Security Agency