All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] Resolve conflicts in expandattribute.
@ 2018-03-16 18:02 Tri Vo
  0 siblings, 0 replies; 8+ messages in thread
From: Tri Vo @ 2018-03-16 18:02 UTC (permalink / raw)
  To: selinux; +Cc: jeffv, dcashman, sspatil, bill.c.roberts, jwcart2, 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.

Signed-off-by: Tri Vo <trong@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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re:  [PATCH v3] Resolve conflicts in expandattribute.
  2018-03-19 18:56 ` jwcart2
@ 2018-03-26 19:32   ` Jeffrey Vander Stoep
  0 siblings, 0 replies; 8+ messages in thread
From: Jeffrey Vander Stoep @ 2018-03-26 19:32 UTC (permalink / raw)
  To: jwcart2; +Cc: Tri Vo, selinux, dcashman, sspatil, bill.c.roberts

[-- Attachment #1: Type: text/plain, Size: 4934 bytes --]

Merged.

On Mon, Mar 19, 2018 at 11:55 AM jwcart2 <jwcart2@tycho.nsa.gov> wrote:

> On 03/16/2018 01:55 PM, Tri Vo wrote:
> > 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 <trong@android.com>
>
> Acked-by: James Carter <jwcart2@tycho.nsa.gov>
>
> > ---
> >   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);
> >       }
> >
> >
>
>
> --
> James Carter <jwcart2@tycho.nsa.gov>
> National Security Agency
>

[-- Attachment #2: Type: text/html, Size: 6738 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re:  [PATCH v3] Resolve conflicts in expandattribute.
  2018-03-16 17:55 Tri Vo
  2018-03-16 17:59 ` Tri Vo
@ 2018-03-19 18:56 ` jwcart2
  2018-03-26 19:32   ` Jeffrey Vander Stoep
  1 sibling, 1 reply; 8+ messages in thread
From: jwcart2 @ 2018-03-19 18:56 UTC (permalink / raw)
  To: Tri Vo, selinux; +Cc: jeffv, dcashman, sspatil, bill.c.roberts

On 03/16/2018 01:55 PM, Tri Vo wrote:
> 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 <trong@android.com>

Acked-by: James Carter <jwcart2@tycho.nsa.gov>

> ---
>   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);
>   	}
>   
> 


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] Resolve conflicts in expandattribute.
  2018-03-16 18:17 ` Jeffrey Vander Stoep
@ 2018-03-16 20:52   ` William Roberts
  0 siblings, 0 replies; 8+ messages in thread
From: William Roberts @ 2018-03-16 20:52 UTC (permalink / raw)
  To: Jeffrey Vander Stoep
  Cc: Tri Vo, SELinux, Daniel Cashman, Sandeep Patil, James Carter

On Fri, Mar 16, 2018 at 11:17 AM, Jeffrey Vander Stoep <jeffv@google.com> wrote:
> On Fri, Mar 16, 2018 at 11:11 AM, Tri Vo <trong@android.com> wrote:
>> 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 <trong@android.com>
>
> Acked-by: Jeff Vander Stoep <jeffv@google.com>

Acked-by: William Roberts <william.c.roberts@intel.com>

Jeff are you going to merge this?

>
>> ---
>>  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
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] Resolve conflicts in expandattribute.
  2018-03-16 18:11 Tri Vo
@ 2018-03-16 18:17 ` Jeffrey Vander Stoep
  2018-03-16 20:52   ` William Roberts
  0 siblings, 1 reply; 8+ messages in thread
From: Jeffrey Vander Stoep @ 2018-03-16 18:17 UTC (permalink / raw)
  To: Tri Vo
  Cc: SELinux, Daniel Cashman, Sandeep Patil, William Roberts, James Carter

On Fri, Mar 16, 2018 at 11:11 AM, Tri Vo <trong@android.com> wrote:
> 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 <trong@android.com>

Acked-by: Jeff Vander Stoep <jeffv@google.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
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3] Resolve conflicts in expandattribute.
@ 2018-03-16 18:11 Tri Vo
  2018-03-16 18:17 ` Jeffrey Vander Stoep
  0 siblings, 1 reply; 8+ messages in thread
From: Tri Vo @ 2018-03-16 18:11 UTC (permalink / raw)
  To: selinux; +Cc: jeffv, dcashman, sspatil, bill.c.roberts, jwcart2, 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)

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 <trong@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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] Resolve conflicts in expandattribute.
  2018-03-16 17:55 Tri Vo
@ 2018-03-16 17:59 ` Tri Vo
  2018-03-19 18:56 ` jwcart2
  1 sibling, 0 replies; 8+ messages in thread
From: Tri Vo @ 2018-03-16 17:59 UTC (permalink / raw)
  To: selinux
  Cc: Jeffrey Vander Stoep, Daniel Cashman, Sandeep Patil,
	William Roberts, jwcart2, Tri Vo

Wait. Let me send this as a separate email.

On Fri, Mar 16, 2018 at 10:55 AM, Tri Vo <trong@android.com> wrote:
> 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 <trong@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
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3] Resolve conflicts in expandattribute.
@ 2018-03-16 17:55 Tri Vo
  2018-03-16 17:59 ` Tri Vo
  2018-03-19 18:56 ` jwcart2
  0 siblings, 2 replies; 8+ messages in thread
From: Tri Vo @ 2018-03-16 17:55 UTC (permalink / raw)
  To: selinux; +Cc: jeffv, dcashman, sspatil, bill.c.roberts, jwcart2, 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.

Signed-off-by: Tri Vo <trong@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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-03-26 19:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-16 18:02 [PATCH v3] Resolve conflicts in expandattribute Tri Vo
  -- strict thread matches above, loose matches on Subject: below --
2018-03-16 18:11 Tri Vo
2018-03-16 18:17 ` Jeffrey Vander Stoep
2018-03-16 20:52   ` William Roberts
2018-03-16 17:55 Tri Vo
2018-03-16 17:59 ` Tri Vo
2018-03-19 18:56 ` jwcart2
2018-03-26 19:32   ` Jeffrey Vander Stoep

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.