| Message ID | 1461075965-17161-4-git-send-email-jwcart2@tycho.nsa.gov (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
On 04/19/2016 10:26 AM, James Carter wrote: > Provide more detailed log messages containing all relevant CIL and > high-level language source file information through cil_tree_log(). > > cil_tree_log() uses two new functions: cil_tree_get_next_path() and > cil_tree_get_cil_path(). > > cil_tree_get_next_path() traverses up the parse tree or AST until > it finds the next CIL or high-level language source information nodes. > It will return the path and whether or not the path is for a CIL file. > > cil_tree_get_cil_path() uses cil_tree_get_next_path() to return > the CIL path. > > Example cil_tree_log() message: > Problem at line 21 of policy.cil (from line 11 of foo.hll) (from line 2 of bar.hll) > > Signed-off-by: James Carter <jwcart2@tycho.nsa.gov> > --- > libsepol/cil/src/cil_tree.c | 76 +++++++++++++++++++++++++++++++++++++++++++++ > libsepol/cil/src/cil_tree.h | 4 +++ > 2 files changed, 80 insertions(+) > > diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c > index 6e56dd1..1ccf9f5 100644 > --- a/libsepol/cil/src/cil_tree.c > +++ b/libsepol/cil/src/cil_tree.c > @@ -59,6 +59,82 @@ __attribute__((noreturn)) __attribute__((format (printf, 1, 2))) void cil_tree_e > exit(1); > } > > +struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **path, int* is_cil) > +{ > + if (!node) { > + return NULL; > + } > + > + node = node->parent; > + > + while (node) { > + if (node->flavor == CIL_SRC_INFO) { > + /* AST */ > + struct cil_src_info *info = node->data; > + *path = info->path; > + *is_cil = info->is_cil; > + return node; > + } else if (node->flavor == CIL_NODE && node->data == NULL) { > + if (node->cl_head->data == CIL_KEY_SRC_INFO) { > + /* Parse Tree */ > + *path = node->cl_head->next->next->data; > + *is_cil = (node->cl_head->next->data == CIL_KEY_SRC_CIL); > + return node; > + } > + } > + node = node->parent; > + } > + > + return NULL; > +} > + > +char *cil_tree_get_cil_path(struct cil_tree_node *node) > +{ > + char *path = NULL; > + int is_cil; > + > + while (node) { > + node = cil_tree_get_next_path(node, &path, &is_cil); > + if (node && is_cil) { > + return path; > + } > + } > + > + return NULL; > +} Do we need to take into account things like copies from macro calls and block inherits here? For example, say you inherit a block from another CIL file. Right now, it looks like this just traverses up the current node parents until it reaches a node with src_cil node, which isn't necessarily the CIL file the blockinherit content came from. So the path/line for a CIL statment will look like is was from what it was copied into rather than where it was copied from. > + > +__attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *node, enum cil_log_level lvl, const char* msg, ...) > +{ > + va_list ap; > + > + va_start(ap, msg); > + cil_vlog(lvl, msg, ap); > + va_end(ap); > + > + if (node) { > + char *path = NULL; > + int is_cil; > + unsigned hll_line = node->hll_line; > + > + path = cil_tree_get_cil_path(node); > + > + if (path != NULL) { In what cases would path be NULL? Seems like there should always be a cil path. > + cil_log(lvl, " at line %d of %s", node->line, path); > + } > + > + while (node) { > + node = cil_tree_get_next_path(node, &path, &is_cil); > + if (node && !is_cil) { > + cil_log(lvl," (from line %d of %s)", hll_line, path); Minor, but it might be worth condensing this a bit. If there were multiple levels of HLL includes (which I could easily imagine in some complex HLLs) this could get pretty long and difficult to read. Perhaps just something like this? foo.cil:10 > foo.hll:11 > bar.hll:12 vs at line 10 of foo.cil (from line 12 of foo.hll) (from line 12 of bar.hll) > + path = NULL; > + hll_line = node->hll_line; > + } > + } > + } > + > + cil_log(lvl,"\n"); > +} > + > int cil_tree_init(struct cil_tree **tree) > { > struct cil_tree *new_tree = cil_malloc(sizeof(*new_tree)); > diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h > index 43d6b98..318eecd 100644 > --- a/libsepol/cil/src/cil_tree.h > +++ b/libsepol/cil/src/cil_tree.h > @@ -51,6 +51,10 @@ struct cil_tree_node { > void *data; > }; > > +struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **path, int* is_cil); > +char *cil_tree_get_cil_path(struct cil_tree_node *node); > +__attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *node, enum cil_log_level lvl, const char* msg, ...); > + > int cil_tree_init(struct cil_tree **tree); > void cil_tree_destroy(struct cil_tree **tree); > void cil_tree_subtree_destroy(struct cil_tree_node *node); >
On 04/20/2016 10:47 AM, Steve Lawrence wrote: > On 04/19/2016 10:26 AM, James Carter wrote: >> Provide more detailed log messages containing all relevant CIL and >> high-level language source file information through cil_tree_log(). >> >> cil_tree_log() uses two new functions: cil_tree_get_next_path() and >> cil_tree_get_cil_path(). >> >> cil_tree_get_next_path() traverses up the parse tree or AST until >> it finds the next CIL or high-level language source information nodes. >> It will return the path and whether or not the path is for a CIL file. >> >> cil_tree_get_cil_path() uses cil_tree_get_next_path() to return >> the CIL path. >> >> Example cil_tree_log() message: >> Problem at line 21 of policy.cil (from line 11 of foo.hll) (from line 2 of bar.hll) >> >> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov> >> --- >> libsepol/cil/src/cil_tree.c | 76 +++++++++++++++++++++++++++++++++++++++++++++ >> libsepol/cil/src/cil_tree.h | 4 +++ >> 2 files changed, 80 insertions(+) >> >> diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c >> index 6e56dd1..1ccf9f5 100644 >> --- a/libsepol/cil/src/cil_tree.c >> +++ b/libsepol/cil/src/cil_tree.c >> @@ -59,6 +59,82 @@ __attribute__((noreturn)) __attribute__((format (printf, 1, 2))) void cil_tree_e >> exit(1); >> } >> >> +struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **path, int* is_cil) >> +{ >> + if (!node) { >> + return NULL; >> + } >> + >> + node = node->parent; >> + >> + while (node) { >> + if (node->flavor == CIL_SRC_INFO) { >> + /* AST */ >> + struct cil_src_info *info = node->data; >> + *path = info->path; >> + *is_cil = info->is_cil; >> + return node; >> + } else if (node->flavor == CIL_NODE && node->data == NULL) { >> + if (node->cl_head->data == CIL_KEY_SRC_INFO) { >> + /* Parse Tree */ >> + *path = node->cl_head->next->next->data; >> + *is_cil = (node->cl_head->next->data == CIL_KEY_SRC_CIL); >> + return node; >> + } >> + } >> + node = node->parent; >> + } >> + >> + return NULL; >> +} >> + >> +char *cil_tree_get_cil_path(struct cil_tree_node *node) >> +{ >> + char *path = NULL; >> + int is_cil; >> + >> + while (node) { >> + node = cil_tree_get_next_path(node, &path, &is_cil); >> + if (node && is_cil) { >> + return path; >> + } >> + } >> + >> + return NULL; >> +} > > Do we need to take into account things like copies from macro calls and > block inherits here? For example, say you inherit a block from another > CIL file. Right now, it looks like this just traverses up the current > node parents until it reaches a node with src_cil node, which isn't > necessarily the CIL file the blockinherit content came from. So the > path/line for a CIL statment will look like is was from what it was > copied into rather than where it was copied from. > You are right. We do need to take these into account. >> + >> +__attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *node, enum cil_log_level lvl, const char* msg, ...) >> +{ >> + va_list ap; >> + >> + va_start(ap, msg); >> + cil_vlog(lvl, msg, ap); >> + va_end(ap); >> + >> + if (node) { >> + char *path = NULL; >> + int is_cil; >> + unsigned hll_line = node->hll_line; >> + >> + path = cil_tree_get_cil_path(node); >> + >> + if (path != NULL) { > > In what cases would path be NULL? Seems like there should always be a > cil path. The root node does not have a path. I currently print it out just to help distinguish between multiple traces. Like: neverallow check failed at line 66 of hll_test.cil (neverallow TYPE self (CLASS (PERM1))) <root> blockinherit at line 49 of hll_test.cil (from line 500 of e.hll) allow at line 43 of hll_test.cil (from line 402 of e.hll) (allow TYPE self (CLASS (PERM1))) <root> block at line 53 of hll_test.cil (from line 600 of f.hll) blockinherit at line 54 of hll_test.cil (from line 601 of f.hll) allow at line 43 of hll_test.cil (from line 402 of f.hll) (allow TYPE self (CLASS (PERM1))) > >> + cil_log(lvl, " at line %d of %s", node->line, path); >> + } >> + >> + while (node) { >> + node = cil_tree_get_next_path(node, &path, &is_cil); >> + if (node && !is_cil) { >> + cil_log(lvl," (from line %d of %s)", hll_line, path); > > Minor, but it might be worth condensing this a bit. If there were > multiple levels of HLL includes (which I could easily imagine in some > complex HLLs) this could get pretty long and difficult to read. Perhaps > just something like this? > > foo.cil:10 > foo.hll:11 > bar.hll:12 > > vs > > at line 10 of foo.cil (from line 12 of foo.hll) (from line 12 of bar.hll) > That's a good idea. >> + path = NULL; >> + hll_line = node->hll_line; >> + } >> + } >> + } >> + >> + cil_log(lvl,"\n"); >> +} >> + >> int cil_tree_init(struct cil_tree **tree) >> { >> struct cil_tree *new_tree = cil_malloc(sizeof(*new_tree)); >> diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h >> index 43d6b98..318eecd 100644 >> --- a/libsepol/cil/src/cil_tree.h >> +++ b/libsepol/cil/src/cil_tree.h >> @@ -51,6 +51,10 @@ struct cil_tree_node { >> void *data; >> }; >> >> +struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **path, int* is_cil); >> +char *cil_tree_get_cil_path(struct cil_tree_node *node); >> +__attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *node, enum cil_log_level lvl, const char* msg, ...); >> + >> int cil_tree_init(struct cil_tree **tree); >> void cil_tree_destroy(struct cil_tree **tree); >> void cil_tree_subtree_destroy(struct cil_tree_node *node); >>
diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c index 6e56dd1..1ccf9f5 100644 --- a/libsepol/cil/src/cil_tree.c +++ b/libsepol/cil/src/cil_tree.c @@ -59,6 +59,82 @@ __attribute__((noreturn)) __attribute__((format (printf, 1, 2))) void cil_tree_e exit(1); } +struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **path, int* is_cil) +{ + if (!node) { + return NULL; + } + + node = node->parent; + + while (node) { + if (node->flavor == CIL_SRC_INFO) { + /* AST */ + struct cil_src_info *info = node->data; + *path = info->path; + *is_cil = info->is_cil; + return node; + } else if (node->flavor == CIL_NODE && node->data == NULL) { + if (node->cl_head->data == CIL_KEY_SRC_INFO) { + /* Parse Tree */ + *path = node->cl_head->next->next->data; + *is_cil = (node->cl_head->next->data == CIL_KEY_SRC_CIL); + return node; + } + } + node = node->parent; + } + + return NULL; +} + +char *cil_tree_get_cil_path(struct cil_tree_node *node) +{ + char *path = NULL; + int is_cil; + + while (node) { + node = cil_tree_get_next_path(node, &path, &is_cil); + if (node && is_cil) { + return path; + } + } + + return NULL; +} + +__attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *node, enum cil_log_level lvl, const char* msg, ...) +{ + va_list ap; + + va_start(ap, msg); + cil_vlog(lvl, msg, ap); + va_end(ap); + + if (node) { + char *path = NULL; + int is_cil; + unsigned hll_line = node->hll_line; + + path = cil_tree_get_cil_path(node); + + if (path != NULL) { + cil_log(lvl, " at line %d of %s", node->line, path); + } + + while (node) { + node = cil_tree_get_next_path(node, &path, &is_cil); + if (node && !is_cil) { + cil_log(lvl," (from line %d of %s)", hll_line, path); + path = NULL; + hll_line = node->hll_line; + } + } + } + + cil_log(lvl,"\n"); +} + int cil_tree_init(struct cil_tree **tree) { struct cil_tree *new_tree = cil_malloc(sizeof(*new_tree)); diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h index 43d6b98..318eecd 100644 --- a/libsepol/cil/src/cil_tree.h +++ b/libsepol/cil/src/cil_tree.h @@ -51,6 +51,10 @@ struct cil_tree_node { void *data; }; +struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **path, int* is_cil); +char *cil_tree_get_cil_path(struct cil_tree_node *node); +__attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *node, enum cil_log_level lvl, const char* msg, ...); + int cil_tree_init(struct cil_tree **tree); void cil_tree_destroy(struct cil_tree **tree); void cil_tree_subtree_destroy(struct cil_tree_node *node);
Provide more detailed log messages containing all relevant CIL and high-level language source file information through cil_tree_log(). cil_tree_log() uses two new functions: cil_tree_get_next_path() and cil_tree_get_cil_path(). cil_tree_get_next_path() traverses up the parse tree or AST until it finds the next CIL or high-level language source information nodes. It will return the path and whether or not the path is for a CIL file. cil_tree_get_cil_path() uses cil_tree_get_next_path() to return the CIL path. Example cil_tree_log() message: Problem at line 21 of policy.cil (from line 11 of foo.hll) (from line 2 of bar.hll) Signed-off-by: James Carter <jwcart2@tycho.nsa.gov> --- libsepol/cil/src/cil_tree.c | 76 +++++++++++++++++++++++++++++++++++++++++++++ libsepol/cil/src/cil_tree.h | 4 +++ 2 files changed, 80 insertions(+)