From: James Carter <jwcart2@tycho.nsa.gov>
To: Steve Lawrence <slawrence@tresys.com>,
SELinux List <selinux@tycho.nsa.gov>
Subject: Re: [PATCH v2] libsepol/cil: improve recursion detection
Date: Tue, 15 Sep 2015 12:53:09 -0400 [thread overview]
Message-ID: <55F84CF5.1030007@tycho.nsa.gov> (raw)
In-Reply-To: <1442333155-8215-1-git-send-email-slawrence@tresys.com>
On 09/15/2015 12:05 PM, Steve Lawrence wrote:
> Add support for detecting recursive blockinherits, and print a trace of
> the detected loop. Output will look something like this upon detection:
>
> Recursive blockinherit found:
> test.cil:42: block a
> test.cil:43: blockinherit b
> test.cil:36: block b
> test.cil:37: blockinherit c
> test.cil:39: block c
> test.cil:40: blockinherit a
>
> Additionally, improve support for detecting recursive macros/calls. Due
> to the way calls are copied, the existing code only detected recursion
> with call depth of three or more. Smaller depths, like
>
> (macro m ()
> (call m))
>
> were not detected and caused a segfault. The callstack that was used for
> this was not sufficient, so that is removed and replaced with a method
> similar to the block recursion detection. A similar trace is also
> displayed for recursive macros/calls.
>
> Also, cleanup sidorder, classorder, catorder, sensorder, and in lists at
> the end of resolve, fixing a potential memory leak if errors occur
> during resolve.
>
> Signed-off-by: Steve Lawrence <slawrence@tresys.com>
Looks good. Applied.
Thanks,
Jim
> ---
>
> Changes since v2:
> - Move the recursive print loops into separate functions to make for
> more clear code
> - Change the recursive print loops to terminate on a known node rather
> than looping until the node is NULL
>
> libsepol/cil/src/cil_resolve_ast.c | 199 ++++++++++++++++++++++++++++++-------
> 1 file changed, 161 insertions(+), 38 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
> index 0df5c63..e332cbd 100644
> --- a/libsepol/cil/src/cil_resolve_ast.c
> +++ b/libsepol/cil/src/cil_resolve_ast.c
> @@ -52,7 +52,6 @@ struct cil_args_resolve {
> enum cil_pass pass;
> uint32_t *changed;
> char *last_resolved_name;
> - struct cil_tree_node *callstack;
> struct cil_tree_node *optstack;
> struct cil_tree_node *boolif;
> struct cil_tree_node *macro;
> @@ -2210,6 +2209,73 @@ exit:
> return rc;
> }
>
> +void cil_print_recursive_blockinherit(struct cil_tree_node *bi_node, struct cil_tree_node *terminating_node)
> +{
> + struct cil_list *trace = NULL;
> + struct cil_list_item *item = NULL;
> + struct cil_tree_node *curr = NULL;
> +
> + cil_list_init(&trace, CIL_NODE);
> +
> + for (curr = bi_node; curr != terminating_node; curr = curr->parent) {
> + if (curr->flavor == CIL_BLOCK) {
> + cil_list_prepend(trace, CIL_NODE, curr);
> + } else {
> + if (curr != bi_node) {
> + cil_list_prepend(trace, CIL_NODE, NODE(((struct cil_blockinherit *)curr->data)->block));
> + }
> + cil_list_prepend(trace, CIL_NODE, curr);
> + }
> + }
> + cil_list_prepend(trace, CIL_NODE, terminating_node);
> +
> + cil_list_for_each(item, trace) {
> + curr = item->data;
> + cil_log(CIL_ERR, " %s:%d: ", curr->path, curr->line);
> +
> + if (curr->flavor == CIL_BLOCK) {
> + cil_log(CIL_ERR, "block %s\n", DATUM(curr->data)->name);
> + } else {
> + cil_log(CIL_ERR, "blockinherit %s\n", ((struct cil_blockinherit *)curr->data)->block_str);
> + }
> + }
> +
> + cil_list_destroy(&trace, CIL_FALSE);
> +}
> +
> +int cil_check_recursive_blockinherit(struct cil_tree_node *bi_node)
> +{
> + struct cil_tree_node *curr = NULL;
> + struct cil_blockinherit *bi = NULL;
> + struct cil_block *block = NULL;
> + int rc = SEPOL_ERR;
> +
> + bi = bi_node->data;
> +
> + for (curr = bi_node->parent; curr != NULL; curr = curr->parent) {
> + if (curr->flavor != CIL_BLOCK) {
> + continue;
> + }
> +
> + block = curr->data;
> +
> + if (block != bi->block) {
> + continue;
> + }
> +
> + cil_log(CIL_ERR, "Recursive blockinherit found:\n");
> + cil_print_recursive_blockinherit(bi_node, curr);
> +
> + rc = SEPOL_ERR;
> + goto exit;
> + }
> +
> + rc = SEPOL_OK;
> +
> +exit:
> + return rc;
> +}
> +
> int cil_resolve_blockinherit_copy(struct cil_tree_node *current, void *extra_args)
> {
> struct cil_block *block = current->data;
> @@ -2233,6 +2299,11 @@ int cil_resolve_blockinherit_copy(struct cil_tree_node *current, void *extra_arg
> }
>
> cil_list_for_each(item, block->bi_nodes) {
> + rc = cil_check_recursive_blockinherit(item->data);
> + if (rc != SEPOL_OK) {
> + goto exit;
> + }
> +
> rc = cil_copy_ast(db, current, item->data);
> if (rc != SEPOL_OK) {
> cil_log(CIL_ERR, "Failed to copy block contents into blockinherit\n");
> @@ -2500,6 +2571,80 @@ exit:
> return rc;
> }
>
> +void cil_print_recursive_call(struct cil_tree_node *call_node, struct cil_tree_node *terminating_node)
> +{
> + struct cil_list *trace = NULL;
> + struct cil_list_item * item = NULL;
> + struct cil_tree_node *curr = NULL;
> +
> + cil_list_init(&trace, CIL_NODE);
> +
> + for (curr = call_node; curr != terminating_node; curr = curr->parent) {
> + if (curr->flavor == CIL_CALL) {
> + if (curr != call_node) {
> + cil_list_prepend(trace, CIL_NODE, NODE(((struct cil_call *)curr->data)->macro));
> + }
> + cil_list_prepend(trace, CIL_NODE, curr);
> + }
> + }
> +
> + if (terminating_node->flavor == CIL_MACRO) {
> + cil_list_prepend(trace, CIL_NODE, terminating_node);
> + } else {
> + cil_list_prepend(trace, CIL_NODE, NODE(((struct cil_call *)terminating_node->data)->macro));
> + }
> +
> + cil_list_for_each(item, trace) {
> + curr = item->data;
> + cil_log(CIL_ERR, " %s:%d: ", curr->path, curr->line);
> +
> + if (curr->flavor == CIL_MACRO) {
> + cil_log(CIL_ERR, "macro %s\n", DATUM(curr->data)->name);
> + } else {
> + cil_log(CIL_ERR, "call %s\n", ((struct cil_call *)curr->data)->macro_str);
> + }
> + }
> +
> + cil_list_destroy(&trace, CIL_FALSE);
> +}
> +
> +int cil_check_recursive_call(struct cil_tree_node *call_node, struct cil_tree_node *macro_node)
> +{
> + struct cil_tree_node *curr = NULL;
> + struct cil_call * call = NULL;
> + int rc = SEPOL_ERR;
> +
> + for (curr = call_node; curr != NULL; curr = curr->parent) {
> + if (curr->flavor == CIL_CALL) {
> + if (curr == call_node) {
> + continue;
> + }
> +
> + call = curr->data;
> + if (call->macro != macro_node->data) {
> + continue;
> + }
> + } else if (curr->flavor == CIL_MACRO) {
> + if (curr != macro_node) {
> + rc = SEPOL_OK;
> + goto exit;
> + }
> + } else {
> + continue;
> + }
> +
> + cil_log(CIL_ERR, "Recursive macro call found:\n");
> + cil_print_recursive_call(call_node, curr);
> +
> + rc = SEPOL_ERR;
> + goto exit;
> + }
> +
> + rc = SEPOL_OK;
> +exit:
> + return rc;
> +}
> +
> int cil_resolve_call1(struct cil_tree_node *current, void *extra_args)
> {
> struct cil_call *new_call = current->data;
> @@ -2736,6 +2881,12 @@ int cil_resolve_call1(struct cil_tree_node *current, void *extra_args)
>
> if (new_call->copied == 0) {
> new_call->copied = 1;
> +
> + rc = cil_check_recursive_call(current, macro_node);
> + if (rc != SEPOL_OK) {
> + goto exit;
> + }
> +
> rc = cil_copy_ast(db, macro_node, current);
> if (rc != SEPOL_OK) {
> cil_log(CIL_ERR, "Failed to copy macro, rc: %d\n", rc);
> @@ -3509,7 +3660,6 @@ int __cil_resolve_ast_first_child_helper(struct cil_tree_node *current, void *ex
> {
> int rc = SEPOL_ERR;
> struct cil_args_resolve *args = extra_args;
> - struct cil_tree_node *callstack = NULL;
> struct cil_tree_node *optstack = NULL;
> struct cil_tree_node *parent = NULL;
> struct cil_tree_node *blockstack = NULL;
> @@ -3519,36 +3669,18 @@ int __cil_resolve_ast_first_child_helper(struct cil_tree_node *current, void *ex
> goto exit;
> }
>
> - callstack = args->callstack;
> optstack = args->optstack;
> parent = current->parent;
> blockstack = args->blockstack;
>
> - if (parent->flavor == CIL_CALL || parent->flavor == CIL_OPTIONAL || parent->flavor == CIL_BLOCK) {
> + if (parent->flavor == CIL_OPTIONAL || parent->flavor == CIL_BLOCK) {
> /* push this node onto a stack */
> cil_tree_node_init(&new);
>
> new->data = parent->data;
> new->flavor = parent->flavor;
>
> - if (parent->flavor == CIL_CALL) {
> - if (callstack != NULL) {
> - struct cil_tree_node *curr = NULL;
> - struct cil_call *new_call = new->data;
> - for (curr = callstack->cl_head; curr != NULL;
> - curr = curr->cl_head) {
> - struct cil_call *curr_call = curr->data;
> - if (curr_call->macro == new_call->macro) {
> - cil_log(CIL_ERR, "Recursive macro call found\n");
> - rc = SEPOL_ERR;
> - goto exit;
> - }
> - }
> - callstack->parent = new;
> - new->cl_head = callstack;
> - }
> - args->callstack = new;
> - } else if (parent->flavor == CIL_OPTIONAL) {
> + if (parent->flavor == CIL_OPTIONAL) {
> if (optstack != NULL) {
> optstack->parent = new;
> new->cl_head = optstack;
> @@ -3587,15 +3719,7 @@ int __cil_resolve_ast_last_child_helper(struct cil_tree_node *current, void *ext
>
> parent = current->parent;
>
> - if (parent->flavor == CIL_CALL) {
> - /* pop off the stack */
> - struct cil_tree_node *callstack = args->callstack;
> - args->callstack = callstack->cl_head;
> - if (callstack->cl_head) {
> - callstack->cl_head->parent = NULL;
> - }
> - free(callstack);
> - } else if (parent->flavor == CIL_MACRO) {
> + if (parent->flavor == CIL_MACRO) {
> args->macro = NULL;
> } else if (parent->flavor == CIL_OPTIONAL) {
> struct cil_tree_node *optstack;
> @@ -3645,7 +3769,6 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)
> extra_args.pass = pass;
> extra_args.changed = &changed;
> extra_args.last_resolved_name = NULL;
> - extra_args.callstack = NULL;
> extra_args.optstack = NULL;
> extra_args.boolif= NULL;
> extra_args.macro = NULL;
> @@ -3737,12 +3860,6 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)
>
> /* reset the arguments */
> changed = 0;
> - while (extra_args.callstack != NULL) {
> - struct cil_tree_node *curr = extra_args.callstack;
> - struct cil_tree_node *next = curr->cl_head;
> - free(curr);
> - extra_args.callstack = next;
> - }
> while (extra_args.optstack != NULL) {
> struct cil_tree_node *curr = extra_args.optstack;
> struct cil_tree_node *next = curr->cl_head;
> @@ -3764,6 +3881,12 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)
>
> rc = SEPOL_OK;
> exit:
> + __cil_ordered_lists_destroy(&extra_args.sidorder_lists);
> + __cil_ordered_lists_destroy(&extra_args.classorder_lists);
> + __cil_ordered_lists_destroy(&extra_args.catorder_lists);
> + __cil_ordered_lists_destroy(&extra_args.sensitivityorder_lists);
> + cil_list_destroy(&extra_args.in_list, CIL_FALSE);
> +
> return rc;
> }
>
>
--
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency
prev parent reply other threads:[~2015-09-15 16:53 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-15 16:05 [PATCH v2] libsepol/cil: improve recursion detection Steve Lawrence
2015-09-15 16:53 ` James Carter [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55F84CF5.1030007@tycho.nsa.gov \
--to=jwcart2@tycho.nsa.gov \
--cc=selinux@tycho.nsa.gov \
--cc=slawrence@tresys.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.