[PATCH 1/3] libsepol/cil: Do not copy blockabstracts when inheriting a block

[PATCH 1/3] libsepol/cil: Do not copy blockabstracts when inheriting a block

[James Carter]

Do not copy any blockabstract statements when copying a block to
resolve a blockinherit statement. Inheriting a block from what was
just inherited does not work, so there is no reason to create an
abstract block.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/src/cil_copy_ast.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
index 2fad972c..a4ead9db 100644
--- a/libsepol/cil/src/cil_copy_ast.c
+++ b/libsepol/cil/src/cil_copy_ast.c
@@ -1725,6 +1725,12 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, uint32_t *finished, void
 		copy_func = &cil_copy_block;
 		break;
 	case CIL_BLOCKABSTRACT:
+		if (args->orig_dest->flavor == CIL_BLOCKINHERIT) {
+			/* When inheriting a block, don't copy any blockabstract
+			 * statements. Inheriting a block from a block that was
+			 * just inherited never worked. */
+			return SEPOL_OK;
+		}
 		copy_func = &cil_copy_blockabstract;
 		break;
 	case CIL_BLOCKINHERIT:
-- 
2.31.1

[PATCH 2/3] libsepol/cil: Mark as abstract all sub-blocks of an abstract block

[James Carter]

If a block is marked as abstract, then it will be skipped during
every pass after blockabstracts are resolved (only tunables,
in-befores, and blockinherits are before blockabstracts), so mark
all of its sub-blocks as abstract to reflect their actual status.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/src/cil_resolve_ast.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index e97a9f46..0288b7dc 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -2379,6 +2379,19 @@ exit:
 	return rc;
 }
 
+static void cil_mark_subtree_abstract(struct cil_tree_node *node)
+{
+	struct cil_block *block = node->data;
+
+	block->is_abstract = CIL_TRUE;
+
+	for (node = node->cl_head; node; node = node->next) {
+		if (node->flavor == CIL_BLOCK) {
+			cil_mark_subtree_abstract(node);
+		}
+	}
+}
+
 int cil_resolve_blockabstract(struct cil_tree_node *current, void *extra_args)
 {
 	struct cil_blockabstract *abstract = current->data;
@@ -2398,7 +2411,7 @@ int cil_resolve_blockabstract(struct cil_tree_node *current, void *extra_args)
 		goto exit;
 	}
 
-	((struct cil_block*)block_datum)->is_abstract = CIL_TRUE;
+	cil_mark_subtree_abstract(block_node);
 
 	return SEPOL_OK;
 
-- 
2.31.1

[PATCH 3/3] libsepol/cil: Do not resolve names to declarations in abstract blocks

[James Carter]

Since abstract blocks will not appear in the final policy, do not
resolve names to a declaration inside one.

When resolving blockabstract rules, they must be collected in a list
and processed at the end of the pass because if a parent block is
marked as abstract, then a blockabstract rule for a sub-block will
fail to resolve.

Found by oss-fuzz (#42981)

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/src/cil_resolve_ast.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index 0288b7dc..73115c55 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -65,6 +65,7 @@ struct cil_args_resolve {
 	struct cil_list *sensitivityorder_lists;
 	struct cil_list *in_list_before;
 	struct cil_list *in_list_after;
+	struct cil_list *abstract_blocks;
 };
 
 static struct cil_name * __cil_insert_name(struct cil_db *db, hashtab_key_t key, struct cil_tree_node *ast_node)
@@ -2397,6 +2398,7 @@ int cil_resolve_blockabstract(struct cil_tree_node *current, void *extra_args)
 	struct cil_blockabstract *abstract = current->data;
 	struct cil_symtab_datum *block_datum = NULL;
 	struct cil_tree_node *block_node = NULL;
+	struct cil_args_resolve *args = extra_args;
 	int rc = SEPOL_ERR;
 
 	rc = cil_resolve_name(current, abstract->block_str, CIL_SYM_BLOCKS, extra_args, &block_datum);
@@ -2411,7 +2413,7 @@ int cil_resolve_blockabstract(struct cil_tree_node *current, void *extra_args)
 		goto exit;
 	}
 
-	cil_mark_subtree_abstract(block_node);
+	cil_list_append(args->abstract_blocks, CIL_NODE, block_node);
 
 	return SEPOL_OK;
 
@@ -4097,6 +4099,7 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)
 	extra_args.sensitivityorder_lists = NULL;
 	extra_args.in_list_before = NULL;
 	extra_args.in_list_after = NULL;
+	extra_args.abstract_blocks = NULL;
 
 	cil_list_init(&extra_args.to_destroy, CIL_NODE);
 	cil_list_init(&extra_args.sidorder_lists, CIL_LIST_ITEM);
@@ -4106,6 +4109,7 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)
 	cil_list_init(&extra_args.sensitivityorder_lists, CIL_LIST_ITEM);
 	cil_list_init(&extra_args.in_list_before, CIL_IN);
 	cil_list_init(&extra_args.in_list_after, CIL_IN);
+	cil_list_init(&extra_args.abstract_blocks, CIL_NODE);
 
 	for (pass = CIL_PASS_TIF; pass < CIL_PASS_NUM; pass++) {
 		extra_args.pass = pass;
@@ -4129,6 +4133,13 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)
 			cil_list_destroy(&extra_args.in_list_after, CIL_FALSE);
 		}
 
+		if (pass == CIL_PASS_BLKABS) {
+			struct cil_list_item *item;
+			cil_list_for_each(item, extra_args.abstract_blocks) {
+				cil_mark_subtree_abstract(item->data);
+			}
+		}
+
 		if (pass == CIL_PASS_BLKIN_LINK) {
 			rc = cil_check_for_bad_inheritance(current);
 			if (rc != SEPOL_OK) {
@@ -4247,6 +4258,7 @@ exit:
 	cil_list_destroy(&extra_args.to_destroy, CIL_FALSE);
 	cil_list_destroy(&extra_args.in_list_before, CIL_FALSE);
 	cil_list_destroy(&extra_args.in_list_after, CIL_FALSE);
+	cil_list_destroy(&extra_args.abstract_blocks, CIL_FALSE);
 
 	return rc;
 }
@@ -4268,9 +4280,13 @@ static int __cil_resolve_name_with_parents(struct cil_tree_node *node, char *nam
 		case CIL_ROOT:
 			goto exit;
 			break;
-		case CIL_BLOCK:
-			symtab = &((struct cil_block*)node->data)->symtab[sym_index];
-			rc = cil_symtab_get_datum(symtab, name, datum);
+		case CIL_BLOCK: {
+			struct cil_block *block = node->data;
+			if (!block->is_abstract) {
+				symtab = &block->symtab[sym_index];
+				rc = cil_symtab_get_datum(symtab, name, datum);
+			}
+		}
 			break;
 		case CIL_BLOCKINHERIT: {
 			struct cil_blockinherit *inherit = node->data;
-- 
2.31.1

Re: [PATCH 1/3] libsepol/cil: Do not copy blockabstracts when inheriting a block

[James Carter]

FYI, I plan on merging this series at the end of the week. If anyone
has any objections, please let me know.
Jim

On Wed, Jan 5, 2022 at 4:16 PM James Carter <jwcart2@gmail.com> wrote:
>
> Do not copy any blockabstract statements when copying a block to
> resolve a blockinherit statement. Inheriting a block from what was
> just inherited does not work, so there is no reason to create an
> abstract block.
>
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
>  libsepol/cil/src/cil_copy_ast.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> index 2fad972c..a4ead9db 100644
> --- a/libsepol/cil/src/cil_copy_ast.c
> +++ b/libsepol/cil/src/cil_copy_ast.c
> @@ -1725,6 +1725,12 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, uint32_t *finished, void
>                 copy_func = &cil_copy_block;
>                 break;
>         case CIL_BLOCKABSTRACT:
> +               if (args->orig_dest->flavor == CIL_BLOCKINHERIT) {
> +                       /* When inheriting a block, don't copy any blockabstract
> +                        * statements. Inheriting a block from a block that was
> +                        * just inherited never worked. */
> +                       return SEPOL_OK;
> +               }
>                 copy_func = &cil_copy_blockabstract;
>                 break;
>         case CIL_BLOCKINHERIT:
> --
> 2.31.1
>

Re: [PATCH 1/3] libsepol/cil: Do not copy blockabstracts when inheriting a block

[James Carter]

On Mon, Feb 7, 2022 at 1:55 PM James Carter <jwcart2@gmail.com> wrote:
>
> FYI, I plan on merging this series at the end of the week. If anyone
> has any objections, please let me know.
> Jim
>
This series has now been merged.
Jim

> On Wed, Jan 5, 2022 at 4:16 PM James Carter <jwcart2@gmail.com> wrote:
> >
> > Do not copy any blockabstract statements when copying a block to
> > resolve a blockinherit statement. Inheriting a block from what was
> > just inherited does not work, so there is no reason to create an
> > abstract block.
> >
> > Signed-off-by: James Carter <jwcart2@gmail.com>
> > ---
> >  libsepol/cil/src/cil_copy_ast.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> > index 2fad972c..a4ead9db 100644
> > --- a/libsepol/cil/src/cil_copy_ast.c
> > +++ b/libsepol/cil/src/cil_copy_ast.c
> > @@ -1725,6 +1725,12 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, uint32_t *finished, void
> >                 copy_func = &cil_copy_block;
> >                 break;
> >         case CIL_BLOCKABSTRACT:
> > +               if (args->orig_dest->flavor == CIL_BLOCKINHERIT) {
> > +                       /* When inheriting a block, don't copy any blockabstract
> > +                        * statements. Inheriting a block from a block that was
> > +                        * just inherited never worked. */
> > +                       return SEPOL_OK;
> > +               }
> >                 copy_func = &cil_copy_blockabstract;
> >                 break;
> >         case CIL_BLOCKINHERIT:
> > --
> > 2.31.1
> >