[PATCH v3] Resolve conflicts in expandattribute.

Discussion:

Tri Vo

2018-03-16 17:55:09 UTC

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.

Signed-off-by: Tri Vo <***@android.com>
---
checkpolicy/policy_define.c | 10 ++++++----
libsepol/cil/src/cil_resolve_ast.c | 21 ++++++---------------
2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 2c5db55d..40cc62b0 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -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;
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index d1a5ed87..02259241 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -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

Tri Vo

2018-03-16 17:59:25 UTC

Permalink

Wait. Let me send this as a separate email.

Post by Tri Vo
This commit resolves conflicts in values of expandattribute statements
in policy language and expandtypeattribute in CIL.
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(-)
diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 2c5db55d..40cc62b0 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -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;
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index d1a5ed87..02259241 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
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;
- 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

Tri Vo

2018-03-16 18:02:52 UTC

Permalink

--
2.16.2.804.g6dcf76e118-goog

Tri Vo

2018-03-16 18:11:36 UTC

Permalink

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)

A warning will be issued on this conflict.

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.

Signed-off-by: Tri Vo <***@android.com>
---
checkpolicy/policy_define.c | 10 ++++++----
libsepol/cil/src/cil_resolve_ast.c | 21 ++++++---------------
2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 2c5db55d..40cc62b0 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -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;
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index d1a5ed87..02259241 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -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

Jeffrey Vander Stoep via Selinux

2018-03-16 18:17:16 UTC

Permalink

Post by Tri Vo
This commit resolves conflicts in values of expandattribute statements
in policy language and expandtypeattribute in CIL.
expandattribute hal_audio true;
expandattribute hal_audio false;
Similarly, in CIL these also resolve to false.
(expandtypeattribute (hal_audio) true)
(expandtypeattribute (hal_audio) false)
A warning will be issued on this conflict.
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(-)
diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 2c5db55d..40cc62b0 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -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;
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index d1a5ed87..02259241 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
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;
- 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

William Roberts

2018-03-16 20:52:04 UTC

Permalink

Acked-by: William Roberts <***@intel.com>

Jeff are you going to merge this?

Post by Tri Vo
---
checkpolicy/policy_define.c | 10 ++++++----
libsepol/cil/src/cil_resolve_ast.c | 21 ++++++---------------
2 files changed, 12 insertions(+), 19 deletions(-)
diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 2c5db55d..40cc62b0 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -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;
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index d1a5ed87..02259241 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
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;
- 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

jwcart2

2018-03-19 18:56:51 UTC

Permalink

--
James Carter <***@tycho.nsa.gov>
National Security Agency

Jeffrey Vander Stoep via Selinux

2018-03-26 19:32:17 UTC

Permalink

Merged.

Post by Tri Vo

option specified", id);

Post by Tri Vo
- 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)) {

Post by Tri Vo
+ yywarn("Expandattribute option was set to both

true and false. "

Post by Tri Vo
+ "Resolving to false.");
+ attr->flags &= ~TYPE_FLAGS_EXPAND_ATTR_TRUE;
+ }
}
rc = 0;
diff --git a/libsepol/cil/src/cil_resolve_ast.c

b/libsepol/cil/src/cil_resolve_ast.c

Post by Tri Vo
index d1a5ed87..02259241 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
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)

Post by Tri Vo
attr->used |= used;
if ((attr->used & CIL_ATTR_EXPAND_TRUE) &&
(attr->used & CIL_ATTR_EXPAND_FALSE)) {
- cil_log(CIL_ERR, "Conflicting use of

expandtypeattribute. "

Post by Tri Vo
- "Expandtypeattribute may be set to

true or false "

Post by Tri Vo
- "but not both. \n");
- goto exit;
+ cil_log(CIL_WARN, "Conflicting use of

expandtypeattribute. "

Post by Tri Vo
+ "Expandtypeattribute was set to

both true or false for %s. "

Post by Tri Vo
+ "Resolving to false. \n", attr->

datum.name);

Post by Tri Vo
+ attr->used &= ~CIL_ATTR_EXPAND_TRUE;
}
}
-
- return SEPOL_OK;
- return rc;
}
int cil_resolve_permissionx(struct cil_tree_node *current, struct

cil_permissionx *permx, void *extra_args)

Post by Tri Vo
@@ -488,11 +483,7 @@ int cil_resolve_expandtypeattribute(struct

cil_tree_node *current, void *extra_a

Post by Tri Vo
goto exit;
}

CIL_ATTR_EXPAND_FALSE;

Post by Tri Vo
- 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);

Post by Tri Vo
}

--
National Security Agency