* [PATCH 0/8] libsepol/cil: Line mark cleanup and fix
@ 2021-08-10 18:05 James Carter
2021-08-10 18:05 ` [PATCH 1/8] libsepol/cil: Check syntax of src_info statement James Carter
` (7 more replies)
0 siblings, 8 replies; 13+ messages in thread
From: James Carter @ 2021-08-10 18:05 UTC (permalink / raw)
To: selinux; +Cc: James Carter
Patches 1-5 cleanup minor issues with CIL's line marks.
Patches 6-7 fix the way line mark works so that the correct line
numbers will be given for nested line marks.
Patch 8 writes out line mark information when writing out the AST.
James Carter (8):
libsepol/cil: Check syntax of src_info statement
libsepol/cil: Check the token type after getting the next token
libsepol/cil: Check for valid line mark type immediately
libsepol/cil: Push line mark state first when processing a line mark
libsepol/cil: Create common string-to-unsigned-integer functions
libsepol/cil: Add line mark kind and line number to src info
libsepol/cil: Report correct high-level language line numbers
libsepol/cil: When writing AST use line marks for src_info nodes
libsepol/cil/src/cil.c | 70 ++++++++++++++++-
libsepol/cil/src/cil_binary.c | 9 ++-
libsepol/cil/src/cil_build_ast.c | 77 ++++++++++--------
libsepol/cil/src/cil_copy_ast.c | 5 +-
libsepol/cil/src/cil_internal.h | 9 ++-
libsepol/cil/src/cil_parser.c | 131 +++++++++++++++----------------
libsepol/cil/src/cil_tree.c | 53 +++++++++----
libsepol/cil/src/cil_tree.h | 4 +-
libsepol/cil/src/cil_write_ast.c | 21 ++++-
9 files changed, 245 insertions(+), 134 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/8] libsepol/cil: Check syntax of src_info statement
2021-08-10 18:05 [PATCH 0/8] libsepol/cil: Line mark cleanup and fix James Carter
@ 2021-08-10 18:05 ` James Carter
2021-08-10 18:05 ` [PATCH 2/8] libsepol/cil: Check the token type after getting the next token James Carter
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: James Carter @ 2021-08-10 18:05 UTC (permalink / raw)
To: selinux; +Cc: James Carter
Every rule other than src_info has their syntax checked when
building the AST. It wasn't considered necessary for src_info rules
because they were expected to always be generated by the parser and
aren't part of the CIL language. But there is no check preventing
them from occurring in a policy and the secilc fuzzer found some bugs
by using src_info rules in a policy. This caused some syntax checking
to be added. Since the parse AST from secil2tree will contain src_info
rules and since the goal is to be able to compile the output of
secil2tree, it makes sense to check the syntax of src_info rules
in the same way that all of the other rules are checked.
Check the syntax of src_info statements in the same way every other
rule is checked.
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_build_ast.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index 9da90883..5e65a266 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -6075,12 +6075,24 @@ void cil_destroy_mls(struct cil_mls *mls)
int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *ast_node)
{
- /* No need to check syntax, because this is auto generated */
+ int rc = SEPOL_ERR;
+ enum cil_syntax syntax[] = {
+ CIL_SYN_STRING,
+ CIL_SYN_STRING,
+ CIL_SYN_STRING,
+ CIL_SYN_N_LISTS | CIL_SYN_END,
+ CIL_SYN_END
+ };
+ int syntax_len = sizeof(syntax)/sizeof(*syntax);
struct cil_src_info *info = NULL;
- if (parse_current->next == NULL || parse_current->next->next == NULL) {
- cil_tree_log(parse_current, CIL_ERR, "Bad <src_info>");
- return SEPOL_ERR;
+ if (parse_current == NULL || ast_node == NULL) {
+ goto exit;
+ }
+
+ rc = __cil_verify_syntax(parse_current, syntax, syntax_len);
+ if (rc != SEPOL_OK) {
+ goto exit;
}
cil_src_info_init(&info);
@@ -6092,6 +6104,10 @@ int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *
ast_node->flavor = CIL_SRC_INFO;
return SEPOL_OK;
+
+exit:
+ cil_tree_log(parse_current, CIL_ERR, "Bad src info");
+ return rc;
}
void cil_destroy_src_info(struct cil_src_info *info)
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/8] libsepol/cil: Check the token type after getting the next token
2021-08-10 18:05 [PATCH 0/8] libsepol/cil: Line mark cleanup and fix James Carter
2021-08-10 18:05 ` [PATCH 1/8] libsepol/cil: Check syntax of src_info statement James Carter
@ 2021-08-10 18:05 ` James Carter
2021-08-10 18:05 ` [PATCH 3/8] libsepol/cil: Check for valid line mark type immediately James Carter
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: James Carter @ 2021-08-10 18:05 UTC (permalink / raw)
To: selinux; +Cc: James Carter
In add_hll_linemark(), cil_lexer_next() is called and the token
type is not checked after the call for the expected type (SYMBOL).
Check that the token type is SYMBOL after calling cil_lexer_next().
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_parser.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
index fb95f401..fc90caec 100644
--- a/libsepol/cil/src/cil_parser.c
+++ b/libsepol/cil/src/cil_parser.c
@@ -111,6 +111,10 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
unsigned long val;
cil_lexer_next(&tok);
+ if (tok.type != SYMBOL) {
+ cil_log(CIL_ERR, "Invalid line mark syntax\n");
+ goto exit;
+ }
hll_type = cil_strpool_add(tok.value);
if (hll_type == CIL_KEY_HLL_LME) {
if (cil_stack_is_empty(stack)) {
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/8] libsepol/cil: Check for valid line mark type immediately
2021-08-10 18:05 [PATCH 0/8] libsepol/cil: Line mark cleanup and fix James Carter
2021-08-10 18:05 ` [PATCH 1/8] libsepol/cil: Check syntax of src_info statement James Carter
2021-08-10 18:05 ` [PATCH 2/8] libsepol/cil: Check the token type after getting the next token James Carter
@ 2021-08-10 18:05 ` James Carter
2021-08-10 18:05 ` [PATCH 4/8] libsepol/cil: Push line mark state first when processing a line mark James Carter
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: James Carter @ 2021-08-10 18:05 UTC (permalink / raw)
To: selinux; +Cc: James Carter
It clearer to check that the line mark type is a valid option right
after getting the token.
Check that the line mark type is one of the expected values right
awasy.
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_parser.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
index fc90caec..24386f60 100644
--- a/libsepol/cil/src/cil_parser.c
+++ b/libsepol/cil/src/cil_parser.c
@@ -116,6 +116,10 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
goto exit;
}
hll_type = cil_strpool_add(tok.value);
+ if (hll_type != CIL_KEY_HLL_LME && hll_type != CIL_KEY_HLL_LMS && hll_type != CIL_KEY_HLL_LMX) {
+ cil_log(CIL_ERR, "Invalid line mark syntax\n");
+ goto exit;
+ }
if (hll_type == CIL_KEY_HLL_LME) {
if (cil_stack_is_empty(stack)) {
cil_log(CIL_ERR, "Line mark end without start\n");
@@ -134,15 +138,6 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
create_node(&node, *current, tok.line, *hll_lineno, CIL_KEY_SRC_HLL);
insert_node(node, *current);
- if (hll_type == CIL_KEY_HLL_LMS) {
- *hll_expand = 0;
- } else if (hll_type == CIL_KEY_HLL_LMX) {
- *hll_expand = 1;
- } else {
- cil_log(CIL_ERR, "Invalid line mark syntax\n");
- goto exit;
- }
-
cil_lexer_next(&tok);
if (tok.type != SYMBOL) {
cil_log(CIL_ERR, "Invalid line mark syntax\n");
@@ -161,6 +156,7 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
}
#endif
*hll_lineno = val;
+ *hll_expand = (hll_type == CIL_KEY_HLL_LMX) ? 1 : 0;
push_hll_info(stack, *hll_lineno, *hll_expand);
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/8] libsepol/cil: Push line mark state first when processing a line mark
2021-08-10 18:05 [PATCH 0/8] libsepol/cil: Line mark cleanup and fix James Carter
` (2 preceding siblings ...)
2021-08-10 18:05 ` [PATCH 3/8] libsepol/cil: Check for valid line mark type immediately James Carter
@ 2021-08-10 18:05 ` James Carter
2021-08-10 18:05 ` [PATCH 5/8] libsepol/cil: Create common string-to-unsigned-integer functions James Carter
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: James Carter @ 2021-08-10 18:05 UTC (permalink / raw)
To: selinux; +Cc: James Carter
CIL line mark rules are used to annotate the original line and file
of a rule. It is mostly used for neverallow rules that have been
converted to CIL.
Pushing the current line mark state after processing a line mark
section does not make sense since that information is never used.
When the line mark section ends the information is just popped and
discarded. It also makes pop_hll_info() more complicated than it
needs to be.
Push the line mark state first and simplfy pop_hll_info().
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_parser.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
index 24386f60..d36ffc49 100644
--- a/libsepol/cil/src/cil_parser.c
+++ b/libsepol/cil/src/cil_parser.c
@@ -66,19 +66,15 @@ static void push_hll_info(struct cil_stack *stack, uint32_t hll_lineno, uint32_t
static void pop_hll_info(struct cil_stack *stack, uint32_t *hll_lineno, uint32_t *hll_expand)
{
struct cil_stack_item *curr = cil_stack_pop(stack);
- struct cil_stack_item *prev = cil_stack_peek(stack);
- struct hll_info *old;
+ struct hll_info *info;
- free(curr->data);
-
- if (!prev) {
- *hll_lineno = 0;
- *hll_expand = 0;
- } else {
- old = prev->data;
- *hll_lineno = old->hll_lineno;
- *hll_expand = old->hll_expand;
+ if (!curr) {
+ return;
}
+ info = curr->data;
+ *hll_expand = info->hll_expand;
+ *hll_lineno = info->hll_lineno;
+ free(curr->data);
}
static void create_node(struct cil_tree_node **node, struct cil_tree_node *current, uint32_t line, uint32_t hll_line, void *value)
@@ -128,6 +124,8 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
pop_hll_info(stack, hll_lineno, hll_expand);
*current = (*current)->parent;
} else {
+ push_hll_info(stack, *hll_lineno, *hll_expand);
+
create_node(&node, *current, tok.line, *hll_lineno, NULL);
insert_node(node, *current);
*current = node;
@@ -158,8 +156,6 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
*hll_lineno = val;
*hll_expand = (hll_type == CIL_KEY_HLL_LMX) ? 1 : 0;
- push_hll_info(stack, *hll_lineno, *hll_expand);
-
cil_lexer_next(&tok);
if (tok.type != SYMBOL && tok.type != QSTRING) {
cil_log(CIL_ERR, "Invalid line mark syntax\n");
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/8] libsepol/cil: Create common string-to-unsigned-integer functions
2021-08-10 18:05 [PATCH 0/8] libsepol/cil: Line mark cleanup and fix James Carter
` (3 preceding siblings ...)
2021-08-10 18:05 ` [PATCH 4/8] libsepol/cil: Push line mark state first when processing a line mark James Carter
@ 2021-08-10 18:05 ` James Carter
2021-08-10 18:05 ` [PATCH 6/8] libsepol/cil: Add line mark kind and line number to src info James Carter
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: James Carter @ 2021-08-10 18:05 UTC (permalink / raw)
To: selinux; +Cc: James Carter
The functions cil_fill_integer() and cil_fill_integer64() exist in
cil_build_ast.c, but these functions take a node and it would be
better to have a function that can be used in add_hll_linemark()
so that the common functinality is in one place.
Create cil_string_to_uint32() and cil_string_to_uint64() and use
these functions in cil_fill_integer(), cil_fill_integer64(), and
add_hll_linemark().
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil.c | 57 ++++++++++++++++++++++++++++++++
libsepol/cil/src/cil_build_ast.c | 32 ++++--------------
libsepol/cil/src/cil_internal.h | 2 ++
libsepol/cil/src/cil_parser.c | 16 +++------
4 files changed, 69 insertions(+), 38 deletions(-)
diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
index d24c81c8..bdd16eb8 100644
--- a/libsepol/cil/src/cil.c
+++ b/libsepol/cil/src/cil.c
@@ -1997,6 +1997,63 @@ exit:
return SEPOL_ERR;
}
+int cil_string_to_uint32(const char *string, uint32_t *value, int base)
+{
+ unsigned long val;
+ char *end = NULL;
+ int rc = SEPOL_ERR;
+
+ if (string == NULL || value == NULL) {
+ goto exit;
+ }
+
+ errno = 0;
+ val = strtoul(string, &end, base);
+ if (errno != 0 || end == string || *end != '\0') {
+ rc = SEPOL_ERR;
+ goto exit;
+ }
+
+ /* Ensure that the value fits a 32-bit integer without triggering -Wtype-limits */
+#if ULONG_MAX > UINT32_MAX
+ if (val > UINT32_MAX) {
+ rc = SEPOL_ERR;
+ goto exit;
+ }
+#endif
+
+ *value = val;
+
+ return SEPOL_OK;
+
+exit:
+ cil_log(CIL_ERR, "Failed to create uint32_t from string\n");
+ return rc;
+}
+
+int cil_string_to_uint64(const char *string, uint64_t *value, int base)
+{
+ char *end = NULL;
+ int rc = SEPOL_ERR;
+
+ if (string == NULL || value == NULL) {
+ goto exit;
+ }
+
+ errno = 0;
+ *value = strtoull(string, &end, base);
+ if (errno != 0 || end == string || *end != '\0') {
+ rc = SEPOL_ERR;
+ goto exit;
+ }
+
+ return SEPOL_OK;
+
+exit:
+ cil_log(CIL_ERR, "Failed to create uint64_t from string\n");
+ return rc;
+}
+
void cil_sort_init(struct cil_sort **sort)
{
*sort = cil_malloc(sizeof(**sort));
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index 5e65a266..ffbd3082 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -5601,60 +5601,40 @@ void cil_destroy_ipaddr(struct cil_ipaddr *ipaddr)
int cil_fill_integer(struct cil_tree_node *int_node, uint32_t *integer, int base)
{
int rc = SEPOL_ERR;
- char *endptr = NULL;
- unsigned long val;
if (int_node == NULL || int_node->data == NULL || integer == NULL) {
goto exit;
}
- errno = 0;
- val = strtoul(int_node->data, &endptr, base);
- if (errno != 0 || endptr == int_node->data || *endptr != '\0') {
- rc = SEPOL_ERR;
- goto exit;
- }
-
- /* Ensure that the value fits a 32-bit integer without triggering -Wtype-limits */
-#if ULONG_MAX > UINT32_MAX
- if (val > UINT32_MAX) {
- rc = SEPOL_ERR;
+ rc = cil_string_to_uint32(int_node->data, integer, base);
+ if (rc != SEPOL_OK) {
goto exit;
}
-#endif
-
- *integer = val;
return SEPOL_OK;
exit:
- cil_log(CIL_ERR, "Failed to create integer from string\n");
+ cil_log(CIL_ERR, "Failed to fill 32-bit integer\n");
return rc;
}
int cil_fill_integer64(struct cil_tree_node *int_node, uint64_t *integer, int base)
{
int rc = SEPOL_ERR;
- char *endptr = NULL;
- uint64_t val;
if (int_node == NULL || int_node->data == NULL || integer == NULL) {
goto exit;
}
- errno = 0;
- val = strtoull(int_node->data, &endptr, base);
- if (errno != 0 || endptr == int_node->data || *endptr != '\0') {
- rc = SEPOL_ERR;
+ rc = cil_string_to_uint64(int_node->data, integer, base);
+ if (rc != SEPOL_OK) {
goto exit;
}
- *integer = val;
-
return SEPOL_OK;
exit:
- cil_log(CIL_ERR, "Failed to create integer from string\n");
+ cil_log(CIL_ERR, "Failed to fill 64-bit integer\n");
return rc;
}
diff --git a/libsepol/cil/src/cil_internal.h b/libsepol/cil/src/cil_internal.h
index 98e303d1..b9a03a37 100644
--- a/libsepol/cil/src/cil_internal.h
+++ b/libsepol/cil/src/cil_internal.h
@@ -986,6 +986,8 @@ void cil_symtab_array_init(symtab_t symtab[], const int symtab_sizes[CIL_SYM_NUM
void cil_symtab_array_destroy(symtab_t symtab[]);
void cil_destroy_ast_symtabs(struct cil_tree_node *root);
int cil_get_symtab(struct cil_tree_node *ast_node, symtab_t **symtab, enum cil_sym_index sym_index);
+int cil_string_to_uint32(const char *string, uint32_t *value, int base);
+int cil_string_to_uint64(const char *string, uint64_t *value, int base);
void cil_sort_init(struct cil_sort **sort);
void cil_sort_destroy(struct cil_sort **sort);
diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
index d36ffc49..9ca1432e 100644
--- a/libsepol/cil/src/cil_parser.c
+++ b/libsepol/cil/src/cil_parser.c
@@ -103,8 +103,7 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
struct cil_tree_node *node;
struct token tok;
char *hll_file;
- char *end = NULL;
- unsigned long val;
+ int rc;
cil_lexer_next(&tok);
if (tok.type != SYMBOL) {
@@ -142,18 +141,11 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
goto exit;
}
- val = strtoul(tok.value, &end, 10);
- if (errno == ERANGE || *end != '\0') {
- cil_log(CIL_ERR, "Problem parsing line number for line mark\n");
+ rc = cil_string_to_uint32(tok.value, hll_lineno, 10);
+ if (rc != SEPOL_OK) {
goto exit;
}
-#if ULONG_MAX > UINT32_MAX
- if (val > UINT32_MAX) {
- cil_log(CIL_ERR, "Line mark line number > UINT32_MAX\n");
- goto exit;
- }
-#endif
- *hll_lineno = val;
+
*hll_expand = (hll_type == CIL_KEY_HLL_LMX) ? 1 : 0;
cil_lexer_next(&tok);
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/8] libsepol/cil: Add line mark kind and line number to src info
2021-08-10 18:05 [PATCH 0/8] libsepol/cil: Line mark cleanup and fix James Carter
` (4 preceding siblings ...)
2021-08-10 18:05 ` [PATCH 5/8] libsepol/cil: Create common string-to-unsigned-integer functions James Carter
@ 2021-08-10 18:05 ` James Carter
2021-08-16 9:38 ` Nicolas Iooss
2021-08-10 18:05 ` [PATCH 7/8] libsepol/cil: Report correct high-level language line numbers James Carter
2021-08-10 18:05 ` [PATCH 8/8] libsepol/cil: When writing AST use line marks for src_info nodes James Carter
7 siblings, 1 reply; 13+ messages in thread
From: James Carter @ 2021-08-10 18:05 UTC (permalink / raw)
To: selinux; +Cc: James Carter
To be able to write line mark information when writing the AST,
the line mark kind and line number is needed in the src info.
Instead of indicating whether the src info is for CIL or a hll,
differentiate between CIL, a normal hll line mark, and an expanded
hll line mark. Also include the line mark line number in the src
info nodes.
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil.c | 13 +++++++++----
libsepol/cil/src/cil_build_ast.c | 17 +++++++++++++++--
libsepol/cil/src/cil_copy_ast.c | 3 ++-
libsepol/cil/src/cil_internal.h | 7 +++++--
libsepol/cil/src/cil_parser.c | 27 +++++++++++----------------
libsepol/cil/src/cil_tree.c | 2 +-
6 files changed, 43 insertions(+), 26 deletions(-)
diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
index bdd16eb8..caec5dad 100644
--- a/libsepol/cil/src/cil.c
+++ b/libsepol/cil/src/cil.c
@@ -220,7 +220,9 @@ char *CIL_KEY_IOCTL;
char *CIL_KEY_UNORDERED;
char *CIL_KEY_SRC_INFO;
char *CIL_KEY_SRC_CIL;
-char *CIL_KEY_SRC_HLL;
+char *CIL_KEY_SRC_HLL_LMS;
+char *CIL_KEY_SRC_HLL_LMX;
+char *CIL_KEY_SRC_HLL_LME;
static void cil_init_keys(void)
{
@@ -384,8 +386,10 @@ static void cil_init_keys(void)
CIL_KEY_IOCTL = cil_strpool_add("ioctl");
CIL_KEY_UNORDERED = cil_strpool_add("unordered");
CIL_KEY_SRC_INFO = cil_strpool_add("<src_info>");
- CIL_KEY_SRC_CIL = cil_strpool_add("<src_cil>");
- CIL_KEY_SRC_HLL = cil_strpool_add("<src_hll>");
+ CIL_KEY_SRC_CIL = cil_strpool_add("cil");
+ CIL_KEY_SRC_HLL_LMS = cil_strpool_add("lms");
+ CIL_KEY_SRC_HLL_LMX = cil_strpool_add("lmx");
+ CIL_KEY_SRC_HLL_LME = cil_strpool_add("lme");
}
void cil_db_init(struct cil_db **db)
@@ -2881,6 +2885,7 @@ void cil_mls_init(struct cil_mls **mls)
void cil_src_info_init(struct cil_src_info **info)
{
*info = cil_malloc(sizeof(**info));
- (*info)->is_cil = 0;
+ (*info)->kind = NULL;
+ (*info)->hll_line = 0;
(*info)->path = NULL;
}
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index ffbd3082..a0f58b1e 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -6060,6 +6060,7 @@ int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *
CIL_SYN_STRING,
CIL_SYN_STRING,
CIL_SYN_STRING,
+ CIL_SYN_STRING,
CIL_SYN_N_LISTS | CIL_SYN_END,
CIL_SYN_END
};
@@ -6077,8 +6078,19 @@ int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *
cil_src_info_init(&info);
- info->is_cil = (parse_current->next->data == CIL_KEY_SRC_CIL) ? CIL_TRUE : CIL_FALSE;
- info->path = parse_current->next->next->data;
+ info->kind = parse_current->next->data;
+ if (info->kind != CIL_KEY_SRC_CIL && info->kind != CIL_KEY_SRC_HLL_LMS && info->kind != CIL_KEY_SRC_HLL_LMX) {
+ cil_log(CIL_ERR, "Invalid src info kind\n");
+ rc = SEPOL_ERR;
+ goto exit;
+ }
+
+ rc = cil_string_to_uint32(parse_current->next->next->data, &info->hll_line, 10);
+ if (rc != SEPOL_OK) {
+ goto exit;
+ }
+
+ info->path = parse_current->next->next->next->data;
ast_node->data = info;
ast_node->flavor = CIL_SRC_INFO;
@@ -6087,6 +6099,7 @@ int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *
exit:
cil_tree_log(parse_current, CIL_ERR, "Bad src info");
+ cil_destroy_src_info(info);
return rc;
}
diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
index 9c0231f2..02b9828f 100644
--- a/libsepol/cil/src/cil_copy_ast.c
+++ b/libsepol/cil/src/cil_copy_ast.c
@@ -1692,7 +1692,8 @@ int cil_copy_src_info(__attribute__((unused)) struct cil_db *db, void *data, voi
cil_src_info_init(&new);
- new->is_cil = orig->is_cil;
+ new->kind = orig->kind;
+ new->hll_line = orig->hll_line;
new->path = orig->path;
*copy = new;
diff --git a/libsepol/cil/src/cil_internal.h b/libsepol/cil/src/cil_internal.h
index b9a03a37..385677d4 100644
--- a/libsepol/cil/src/cil_internal.h
+++ b/libsepol/cil/src/cil_internal.h
@@ -236,7 +236,9 @@ extern char *CIL_KEY_IOCTL;
extern char *CIL_KEY_UNORDERED;
extern char *CIL_KEY_SRC_INFO;
extern char *CIL_KEY_SRC_CIL;
-extern char *CIL_KEY_SRC_HLL;
+extern char *CIL_KEY_SRC_HLL_LMS;
+extern char *CIL_KEY_SRC_HLL_LMX;
+extern char *CIL_KEY_SRC_HLL_LME;
/*
Symbol Table Array Indices
@@ -963,7 +965,8 @@ struct cil_mls {
};
struct cil_src_info {
- int is_cil;
+ char *kind;
+ uint32_t hll_line;
char *path;
};
diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
index 9ca1432e..842c327c 100644
--- a/libsepol/cil/src/cil_parser.c
+++ b/libsepol/cil/src/cil_parser.c
@@ -44,10 +44,6 @@
#define CIL_PARSER_MAX_EXPR_DEPTH (0x1 << 12)
-char *CIL_KEY_HLL_LMS;
-char *CIL_KEY_HLL_LMX;
-char *CIL_KEY_HLL_LME;
-
struct hll_info {
uint32_t hll_lineno;
uint32_t hll_expand;
@@ -102,7 +98,6 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
char *hll_type;
struct cil_tree_node *node;
struct token tok;
- char *hll_file;
int rc;
cil_lexer_next(&tok);
@@ -111,11 +106,11 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
goto exit;
}
hll_type = cil_strpool_add(tok.value);
- if (hll_type != CIL_KEY_HLL_LME && hll_type != CIL_KEY_HLL_LMS && hll_type != CIL_KEY_HLL_LMX) {
+ if (hll_type != CIL_KEY_SRC_HLL_LME && hll_type != CIL_KEY_SRC_HLL_LMS && hll_type != CIL_KEY_SRC_HLL_LMX) {
cil_log(CIL_ERR, "Invalid line mark syntax\n");
goto exit;
}
- if (hll_type == CIL_KEY_HLL_LME) {
+ if (hll_type == CIL_KEY_SRC_HLL_LME) {
if (cil_stack_is_empty(stack)) {
cil_log(CIL_ERR, "Line mark end without start\n");
goto exit;
@@ -132,7 +127,7 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
create_node(&node, *current, tok.line, *hll_lineno, CIL_KEY_SRC_INFO);
insert_node(node, *current);
- create_node(&node, *current, tok.line, *hll_lineno, CIL_KEY_SRC_HLL);
+ create_node(&node, *current, tok.line, *hll_lineno, hll_type);
insert_node(node, *current);
cil_lexer_next(&tok);
@@ -141,12 +136,15 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
goto exit;
}
+ create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
+ insert_node(node, *current);
+
rc = cil_string_to_uint32(tok.value, hll_lineno, 10);
if (rc != SEPOL_OK) {
goto exit;
}
- *hll_expand = (hll_type == CIL_KEY_HLL_LMX) ? 1 : 0;
+ *hll_expand = (hll_type == CIL_KEY_SRC_HLL_LMX) ? 1 : 0;
cil_lexer_next(&tok);
if (tok.type != SYMBOL && tok.type != QSTRING) {
@@ -159,9 +157,7 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
tok.value = tok.value+1;
}
- hll_file = cil_strpool_add(tok.value);
-
- create_node(&node, *current, tok.line, *hll_lineno, hll_file);
+ create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
insert_node(node, *current);
}
@@ -192,6 +188,9 @@ static void add_cil_path(struct cil_tree_node **current, char *path)
create_node(&node, *current, 0, 0, CIL_KEY_SRC_CIL);
insert_node(node, *current);
+ create_node(&node, *current, 0, 0, "1");
+ insert_node(node, *current);
+
create_node(&node, *current, 0, 0, path);
insert_node(node, *current);
}
@@ -211,10 +210,6 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
struct token tok;
int rc = SEPOL_OK;
- CIL_KEY_HLL_LMS = cil_strpool_add("lms");
- CIL_KEY_HLL_LMX = cil_strpool_add("lmx");
- CIL_KEY_HLL_LME = cil_strpool_add("lme");
-
cil_stack_init(&stack);
cil_lexer_setup(buffer, size);
diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c
index 4cf8dcc8..52b28999 100644
--- a/libsepol/cil/src/cil_tree.c
+++ b/libsepol/cil/src/cil_tree.c
@@ -71,7 +71,7 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **
/* AST */
struct cil_src_info *info = node->data;
*path = info->path;
- *is_cil = info->is_cil;
+ *is_cil = (info->kind == CIL_KEY_SRC_CIL);
return node;
} else {
if (node->flavor == CIL_CALL) {
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 7/8] libsepol/cil: Report correct high-level language line numbers
2021-08-10 18:05 [PATCH 0/8] libsepol/cil: Line mark cleanup and fix James Carter
` (5 preceding siblings ...)
2021-08-10 18:05 ` [PATCH 6/8] libsepol/cil: Add line mark kind and line number to src info James Carter
@ 2021-08-10 18:05 ` James Carter
2021-08-16 9:23 ` Nicolas Iooss
2021-08-10 18:05 ` [PATCH 8/8] libsepol/cil: When writing AST use line marks for src_info nodes James Carter
7 siblings, 1 reply; 13+ messages in thread
From: James Carter @ 2021-08-10 18:05 UTC (permalink / raw)
To: selinux; +Cc: James Carter
CIL supports specifiying the original high-level language file and
line numbers when reporting errors. This is done through line marks
and is mostly used to report the original Refpolicy file and line
number for neverallow rules that have been converted to CIL.
As long as the line mark remain simple, everything works fine, but
the wrong line numbers will be reported with more complex nextings
of line marks.
Example:
;;* lms 100 file01.hll
(type t1a)
(allow t1a self (CLASS (PERM)))
;;* lmx 200 file02.hll
(type t2a)
(allow t2a self (CLASS (PERM)))
;;* lme
(type t1b)
(allow t1b self (CLASS (PERM)))
(allow bad1b self (CLASS (PERM))) ; file01.hll:101 (Should be 106)
;;* lme
The primary problem is that the tree nodes can only store one hll
line number. Instead a number is needed that can be used by any
number of stacked line mark sections. This number would increment
line a normal line number except when in lmx sections (that have
the same line number throughout the section because they represent
an expansion of a line -- like the expansion of a macro call. This
number can go backwards when exiting a lms section within a lmx
section, because line number will increase in the lms section, but
outside the lmx section, the line number did not advance.
This number is called the hll_offset and this is the value that is
now stored in tree nodes instead of the hll line number. To calculate
the hll line number for a rule, a search is made for an ancestor of
the node that is a line mark and the line number for a lms section
is the hll line number stored in the line mark, plus the hll offset
of the rule, minus the hll offset of the line mark node, minus one.
(hll_lineno + hll_offset_rule - hll_offset_lm - 1)
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_binary.c | 9 ++--
libsepol/cil/src/cil_build_ast.c | 4 +-
libsepol/cil/src/cil_copy_ast.c | 2 +-
libsepol/cil/src/cil_parser.c | 74 +++++++++++++++++++-------------
libsepol/cil/src/cil_tree.c | 53 +++++++++++++++--------
libsepol/cil/src/cil_tree.h | 4 +-
6 files changed, 90 insertions(+), 56 deletions(-)
diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
index 2b65c622..43c37fc2 100644
--- a/libsepol/cil/src/cil_binary.c
+++ b/libsepol/cil/src/cil_binary.c
@@ -4480,7 +4480,8 @@ static avrule_t *__cil_init_sepol_avrule(uint32_t kind, struct cil_tree_node *no
avrule_t *avrule;
struct cil_tree_node *source_node;
char *source_path;
- int is_cil;
+ char *lm_kind;
+ uint32_t hll_line;
avrule = cil_malloc(sizeof(avrule_t));
avrule->specified = kind;
@@ -4492,11 +4493,11 @@ static avrule_t *__cil_init_sepol_avrule(uint32_t kind, struct cil_tree_node *no
avrule->source_filename = NULL;
avrule->source_line = node->line;
- source_node = cil_tree_get_next_path(node, &source_path, &is_cil);
+ source_node = cil_tree_get_next_path(node, &lm_kind, &hll_line, &source_path);
if (source_node) {
avrule->source_filename = source_path;
- if (!is_cil) {
- avrule->source_line = node->hll_line;
+ if (lm_kind != CIL_KEY_SRC_CIL) {
+ avrule->source_line = hll_line + node->hll_offset - source_node->hll_offset - 1;
}
}
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index a0f58b1e..a5afc267 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -619,7 +619,7 @@ int cil_gen_perm_nodes(struct cil_db *db, struct cil_tree_node *current_perm, st
cil_tree_node_init(&new_ast);
new_ast->parent = ast_node;
new_ast->line = current_perm->line;
- new_ast->hll_line = current_perm->hll_line;
+ new_ast->hll_offset = current_perm->hll_offset;
rc = cil_gen_perm(db, current_perm, new_ast, flavor, num_perms);
if (rc != SEPOL_OK) {
@@ -6203,7 +6203,7 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f
ast_node->parent = ast_current;
ast_node->line = parse_current->line;
- ast_node->hll_line = parse_current->hll_line;
+ ast_node->hll_offset = parse_current->hll_offset;
if (parse_current->data == CIL_KEY_BLOCK) {
rc = cil_gen_block(db, parse_current, ast_node, 0);
diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
index 02b9828f..34282a92 100644
--- a/libsepol/cil/src/cil_copy_ast.c
+++ b/libsepol/cil/src/cil_copy_ast.c
@@ -2010,7 +2010,7 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
new->parent = parent;
new->line = orig->line;
- new->hll_line = orig->hll_line;
+ new->hll_offset = orig->hll_offset;
new->flavor = orig->flavor;
new->data = data;
diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
index 842c327c..3ccef5d7 100644
--- a/libsepol/cil/src/cil_parser.c
+++ b/libsepol/cil/src/cil_parser.c
@@ -45,21 +45,21 @@
#define CIL_PARSER_MAX_EXPR_DEPTH (0x1 << 12)
struct hll_info {
- uint32_t hll_lineno;
+ uint32_t hll_offset;
uint32_t hll_expand;
};
-static void push_hll_info(struct cil_stack *stack, uint32_t hll_lineno, uint32_t hll_expand)
+static void push_hll_info(struct cil_stack *stack, uint32_t hll_offset, uint32_t hll_expand)
{
struct hll_info *new = cil_malloc(sizeof(*new));
- new->hll_lineno = hll_lineno;
+ new->hll_offset = hll_offset;
new->hll_expand = hll_expand;
cil_stack_push(stack, CIL_NONE, new);
}
-static void pop_hll_info(struct cil_stack *stack, uint32_t *hll_lineno, uint32_t *hll_expand)
+static void pop_hll_info(struct cil_stack *stack, uint32_t *hll_offset, uint32_t *hll_expand)
{
struct cil_stack_item *curr = cil_stack_pop(stack);
struct hll_info *info;
@@ -69,17 +69,17 @@ static void pop_hll_info(struct cil_stack *stack, uint32_t *hll_lineno, uint32_t
}
info = curr->data;
*hll_expand = info->hll_expand;
- *hll_lineno = info->hll_lineno;
+ *hll_offset = info->hll_offset;
free(curr->data);
}
-static void create_node(struct cil_tree_node **node, struct cil_tree_node *current, uint32_t line, uint32_t hll_line, void *value)
+static void create_node(struct cil_tree_node **node, struct cil_tree_node *current, uint32_t line, uint32_t hll_offset, void *value)
{
cil_tree_node_init(node);
(*node)->parent = current;
(*node)->flavor = CIL_NODE;
(*node)->line = line;
- (*node)->hll_line = hll_line;
+ (*node)->hll_offset = hll_offset;
(*node)->data = value;
}
@@ -93,12 +93,12 @@ static void insert_node(struct cil_tree_node *node, struct cil_tree_node *curren
current->cl_tail = node;
}
-static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno, uint32_t *hll_expand, struct cil_stack *stack, char *path)
+static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_offset, uint32_t *hll_expand, struct cil_stack *stack, char *path)
{
char *hll_type;
struct cil_tree_node *node;
struct token tok;
- int rc;
+ uint32_t prev_hll_expand, prev_hll_offset;
cil_lexer_next(&tok);
if (tok.type != SYMBOL) {
@@ -115,19 +115,33 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
cil_log(CIL_ERR, "Line mark end without start\n");
goto exit;
}
- pop_hll_info(stack, hll_lineno, hll_expand);
+ prev_hll_expand = *hll_expand;
+ pop_hll_info(stack, &prev_hll_offset, hll_expand);
+ if (*hll_expand) {
+ /* This is needed when exiting an lms section within an lmx section.
+ * In the lms section, hll_offset will increment and then revert
+ * back to its previous value when going back into the lmx section.
+ */
+ *hll_offset = prev_hll_offset;
+ }
+ if (prev_hll_expand && !*hll_expand) {
+ /* This is needed to count the lme at the end of an lmx section
+ * within an lms section (or within no hll section).
+ */
+ (*hll_offset)++;
+ }
*current = (*current)->parent;
} else {
- push_hll_info(stack, *hll_lineno, *hll_expand);
+ push_hll_info(stack, *hll_offset, *hll_expand);
- create_node(&node, *current, tok.line, *hll_lineno, NULL);
+ create_node(&node, *current, tok.line, *hll_offset, NULL);
insert_node(node, *current);
*current = node;
- create_node(&node, *current, tok.line, *hll_lineno, CIL_KEY_SRC_INFO);
+ create_node(&node, *current, tok.line, *hll_offset, CIL_KEY_SRC_INFO);
insert_node(node, *current);
- create_node(&node, *current, tok.line, *hll_lineno, hll_type);
+ create_node(&node, *current, tok.line, *hll_offset, hll_type);
insert_node(node, *current);
cil_lexer_next(&tok);
@@ -136,16 +150,9 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
goto exit;
}
- create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
+ create_node(&node, *current, tok.line, *hll_offset, cil_strpool_add(tok.value));
insert_node(node, *current);
- rc = cil_string_to_uint32(tok.value, hll_lineno, 10);
- if (rc != SEPOL_OK) {
- goto exit;
- }
-
- *hll_expand = (hll_type == CIL_KEY_SRC_HLL_LMX) ? 1 : 0;
-
cil_lexer_next(&tok);
if (tok.type != SYMBOL && tok.type != QSTRING) {
cil_log(CIL_ERR, "Invalid line mark syntax\n");
@@ -157,8 +164,10 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
tok.value = tok.value+1;
}
- create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
+ create_node(&node, *current, tok.line, *hll_offset, cil_strpool_add(tok.value));
insert_node(node, *current);
+
+ *hll_expand = (hll_type == CIL_KEY_SRC_HLL_LMX) ? 1 : 0;
}
cil_lexer_next(&tok);
@@ -167,6 +176,11 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
goto exit;
}
+ if (!*hll_expand) {
+ /* Need to increment because of the NEWLINE */
+ (*hll_offset)++;
+ }
+
return SEPOL_OK;
exit:
@@ -205,7 +219,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
struct cil_tree_node *current = NULL;
char *path = cil_strpool_add(_path);
struct cil_stack *stack;
- uint32_t hll_lineno = 0;
+ uint32_t hll_offset = 1;
uint32_t hll_expand = 0;
struct token tok;
int rc = SEPOL_OK;
@@ -223,7 +237,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
cil_lexer_next(&tok);
switch (tok.type) {
case HLL_LINEMARK:
- rc = add_hll_linemark(¤t, &hll_lineno, &hll_expand, stack, path);
+ rc = add_hll_linemark(¤t, &hll_offset, &hll_expand, stack, path);
if (rc != SEPOL_OK) {
goto exit;
}
@@ -234,7 +248,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
cil_log(CIL_ERR, "Number of open parenthesis exceeds limit of %d at line %d of %s\n", CIL_PARSER_MAX_EXPR_DEPTH, tok.line, path);
goto exit;
}
- create_node(&node, current, tok.line, hll_lineno, NULL);
+ create_node(&node, current, tok.line, hll_offset, NULL);
insert_node(node, current);
current = node;
break;
@@ -256,12 +270,12 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
goto exit;
}
- create_node(&node, current, tok.line, hll_lineno, cil_strpool_add(tok.value));
+ create_node(&node, current, tok.line, hll_offset, cil_strpool_add(tok.value));
insert_node(node, current);
break;
case NEWLINE :
if (!hll_expand) {
- hll_lineno++;
+ hll_offset++;
}
break;
case COMMENT:
@@ -269,7 +283,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
cil_lexer_next(&tok);
}
if (!hll_expand) {
- hll_lineno++;
+ hll_offset++;
}
if (tok.type != END_OF_FILE) {
break;
@@ -306,7 +320,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
exit:
while (!cil_stack_is_empty(stack)) {
- pop_hll_info(stack, &hll_lineno, &hll_expand);
+ pop_hll_info(stack, &hll_offset, &hll_expand);
}
cil_lexer_destroy();
cil_stack_destroy(&stack);
diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c
index 52b28999..4fdf339d 100644
--- a/libsepol/cil/src/cil_tree.c
+++ b/libsepol/cil/src/cil_tree.c
@@ -50,10 +50,12 @@ __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)
+struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **info_kind, uint32_t *hll_line, char **path)
{
+ int rc;
+
if (!node) {
- return NULL;
+ goto exit;
}
node = node->parent;
@@ -62,16 +64,21 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **
if (node->flavor == CIL_NODE && node->data == NULL) {
if (node->cl_head->data == CIL_KEY_SRC_INFO && node->cl_head->next != NULL && node->cl_head->next->next != NULL) {
/* Parse Tree */
- *path = node->cl_head->next->next->data;
- *is_cil = (node->cl_head->next->data == CIL_KEY_SRC_CIL);
+ *info_kind = node->cl_head->next->data;
+ rc = cil_string_to_uint32(node->cl_head->next->next->data, hll_line, 10);
+ if (rc != SEPOL_OK) {
+ goto exit;
+ }
+ *path = node->cl_head->next->next->next->data;
return node;
}
node = node->parent;
} else if (node->flavor == CIL_SRC_INFO) {
/* AST */
struct cil_src_info *info = node->data;
+ *info_kind = info->kind;
+ *hll_line = info->hll_line;
*path = info->path;
- *is_cil = (info->kind == CIL_KEY_SRC_CIL);
return node;
} else {
if (node->flavor == CIL_CALL) {
@@ -86,17 +93,22 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **
}
}
+exit:
+ *info_kind = NULL;
+ *hll_line = 0;
+ *path = NULL;
return NULL;
}
char *cil_tree_get_cil_path(struct cil_tree_node *node)
{
- char *path = NULL;
- int is_cil;
+ char *info_kind;
+ uint32_t hll_line;
+ char *path;
while (node) {
- node = cil_tree_get_next_path(node, &path, &is_cil);
- if (node && is_cil) {
+ node = cil_tree_get_next_path(node, &info_kind, &hll_line, &path);
+ if (node && info_kind == CIL_KEY_SRC_CIL) {
return path;
}
}
@@ -114,8 +126,7 @@ __attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *n
if (node) {
char *path = NULL;
- int is_cil;
- unsigned hll_line = node->hll_line;
+ uint32_t hll_offset = node->hll_offset;
path = cil_tree_get_cil_path(node);
@@ -124,12 +135,20 @@ __attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *n
}
while (node) {
- node = cil_tree_get_next_path(node, &path, &is_cil);
- if (node && !is_cil) {
+ do {
+ char *info_kind;
+ uint32_t hll_line;
+
+ node = cil_tree_get_next_path(node, &info_kind, &hll_line, &path);
+ if (!node || info_kind == CIL_KEY_SRC_CIL) {
+ break;
+ }
+ if (info_kind == CIL_KEY_SRC_HLL_LMS) {
+ hll_line += hll_offset - node->hll_offset - 1;
+ }
+
cil_log(lvl," from %s:%d", path, hll_line);
- path = NULL;
- hll_line = node->hll_line;
- }
+ } while (1);
}
}
@@ -222,7 +241,7 @@ void cil_tree_node_init(struct cil_tree_node **node)
new_node->next = NULL;
new_node->flavor = CIL_ROOT;
new_node->line = 0;
- new_node->hll_line = 0;
+ new_node->hll_offset = 0;
*node = new_node;
}
diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h
index f4d22071..5a98da55 100644
--- a/libsepol/cil/src/cil_tree.h
+++ b/libsepol/cil/src/cil_tree.h
@@ -46,11 +46,11 @@ struct cil_tree_node {
struct cil_tree_node *next; //Each element in the list points to the next element
enum cil_flavor flavor;
uint32_t line;
- uint32_t hll_line;
+ uint32_t hll_offset;
void *data;
};
-struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **path, int* is_cil);
+struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **info_kind, uint32_t *hll_line, char **path);
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, ...);
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 8/8] libsepol/cil: When writing AST use line marks for src_info nodes
2021-08-10 18:05 [PATCH 0/8] libsepol/cil: Line mark cleanup and fix James Carter
` (6 preceding siblings ...)
2021-08-10 18:05 ` [PATCH 7/8] libsepol/cil: Report correct high-level language line numbers James Carter
@ 2021-08-10 18:05 ` James Carter
7 siblings, 0 replies; 13+ messages in thread
From: James Carter @ 2021-08-10 18:05 UTC (permalink / raw)
To: selinux; +Cc: James Carter
In order to retain as much information as possible, when writing
out the CIL AST, use line mark notation to write out src_info
nodes. This includes using line marks to denote the original CIL
files the AST comes from.
The line numbers will not always be exactly correct because any
blank lines and comments in the original files will not be
represented in the AST.
Line marks are not written for the parse tree because the line
numbers will be widely inaccurate since each token will be on
a different line.
Signed-off-by: James Carter <jwcart2@gmail.com>
---
libsepol/cil/src/cil_write_ast.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/libsepol/cil/src/cil_write_ast.c b/libsepol/cil/src/cil_write_ast.c
index 186070c1..d7f00bcc 100644
--- a/libsepol/cil/src/cil_write_ast.c
+++ b/libsepol/cil/src/cil_write_ast.c
@@ -546,6 +546,18 @@ static const char *macro_param_flavor_to_string(enum cil_flavor flavor)
return str;
}
+void cil_write_src_info_node(FILE *out, struct cil_tree_node *node)
+{
+ struct cil_src_info *info = node->data;
+ if (info->kind == CIL_KEY_SRC_CIL || info->kind == CIL_KEY_SRC_HLL_LMS) {
+ fprintf(out, ";;* lms %u %s\n", info->hll_line, info->path);
+ } else if (info->kind == CIL_KEY_SRC_HLL_LMX) {
+ fprintf(out, ";;* lmx %u %s\n", info->hll_line, info->path);
+ } else {
+ fprintf(out, ";;* <?SRC_INFO_KIND> %u %s\n", info->hll_line, info->path);
+ }
+}
+
void cil_write_ast_node(FILE *out, struct cil_tree_node *node)
{
if (!node->data) {
@@ -1508,8 +1520,10 @@ static int __write_cil_ast_node_helper(struct cil_tree_node *node, uint32_t *fin
{
struct cil_write_ast_args *args = extra_args;
- if (node->flavor == CIL_SRC_INFO)
+ if (node->flavor == CIL_SRC_INFO) {
+ cil_write_src_info_node(args->out, node);
return SEPOL_OK;
+ }
fprintf(args->out, "%*s", args->depth*4, "");
@@ -1539,7 +1553,10 @@ static int __write_cil_ast_last_child_helper(struct cil_tree_node *node, void *e
struct cil_write_ast_args *args = extra_args;
struct cil_tree_node *parent = node->parent;
- if (parent->flavor == CIL_SRC_INFO || parent->flavor == CIL_ROOT) {
+ if (parent->flavor == CIL_ROOT) {
+ return SEPOL_OK;
+ } else if (parent->flavor == CIL_SRC_INFO) {
+ fprintf(args->out, ";;* lme\n");
return SEPOL_OK;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 7/8] libsepol/cil: Report correct high-level language line numbers
2021-08-10 18:05 ` [PATCH 7/8] libsepol/cil: Report correct high-level language line numbers James Carter
@ 2021-08-16 9:23 ` Nicolas Iooss
2021-08-16 14:40 ` James Carter
0 siblings, 1 reply; 13+ messages in thread
From: Nicolas Iooss @ 2021-08-16 9:23 UTC (permalink / raw)
To: James Carter; +Cc: SElinux list
On Tue, Aug 10, 2021 at 8:22 PM James Carter <jwcart2@gmail.com> wrote:
>
> CIL supports specifiying the original high-level language file and
> line numbers when reporting errors. This is done through line marks
> and is mostly used to report the original Refpolicy file and line
> number for neverallow rules that have been converted to CIL.
>
> As long as the line mark remain simple, everything works fine, but
> the wrong line numbers will be reported with more complex nextings
> of line marks.
>
> Example:
> ;;* lms 100 file01.hll
> (type t1a)
> (allow t1a self (CLASS (PERM)))
> ;;* lmx 200 file02.hll
> (type t2a)
> (allow t2a self (CLASS (PERM)))
> ;;* lme
> (type t1b)
> (allow t1b self (CLASS (PERM)))
> (allow bad1b self (CLASS (PERM))) ; file01.hll:101 (Should be 106)
> ;;* lme
>
> The primary problem is that the tree nodes can only store one hll
> line number. Instead a number is needed that can be used by any
> number of stacked line mark sections. This number would increment
> line a normal line number except when in lmx sections (that have
> the same line number throughout the section because they represent
> an expansion of a line -- like the expansion of a macro call. This
> number can go backwards when exiting a lms section within a lmx
> section, because line number will increase in the lms section, but
> outside the lmx section, the line number did not advance.
>
> This number is called the hll_offset and this is the value that is
> now stored in tree nodes instead of the hll line number. To calculate
> the hll line number for a rule, a search is made for an ancestor of
> the node that is a line mark and the line number for a lms section
> is the hll line number stored in the line mark, plus the hll offset
> of the rule, minus the hll offset of the line mark node, minus one.
> (hll_lineno + hll_offset_rule - hll_offset_lm - 1)
>
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
> libsepol/cil/src/cil_binary.c | 9 ++--
> libsepol/cil/src/cil_build_ast.c | 4 +-
> libsepol/cil/src/cil_copy_ast.c | 2 +-
> libsepol/cil/src/cil_parser.c | 74 +++++++++++++++++++-------------
> libsepol/cil/src/cil_tree.c | 53 +++++++++++++++--------
> libsepol/cil/src/cil_tree.h | 4 +-
> 6 files changed, 90 insertions(+), 56 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
> index 2b65c622..43c37fc2 100644
> --- a/libsepol/cil/src/cil_binary.c
> +++ b/libsepol/cil/src/cil_binary.c
> @@ -4480,7 +4480,8 @@ static avrule_t *__cil_init_sepol_avrule(uint32_t kind, struct cil_tree_node *no
> avrule_t *avrule;
> struct cil_tree_node *source_node;
> char *source_path;
> - int is_cil;
> + char *lm_kind;
> + uint32_t hll_line;
>
> avrule = cil_malloc(sizeof(avrule_t));
> avrule->specified = kind;
> @@ -4492,11 +4493,11 @@ static avrule_t *__cil_init_sepol_avrule(uint32_t kind, struct cil_tree_node *no
>
> avrule->source_filename = NULL;
> avrule->source_line = node->line;
> - source_node = cil_tree_get_next_path(node, &source_path, &is_cil);
> + source_node = cil_tree_get_next_path(node, &lm_kind, &hll_line, &source_path);
> if (source_node) {
> avrule->source_filename = source_path;
> - if (!is_cil) {
> - avrule->source_line = node->hll_line;
> + if (lm_kind != CIL_KEY_SRC_CIL) {
> + avrule->source_line = hll_line + node->hll_offset - source_node->hll_offset - 1;
> }
> }
>
> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> index a0f58b1e..a5afc267 100644
> --- a/libsepol/cil/src/cil_build_ast.c
> +++ b/libsepol/cil/src/cil_build_ast.c
> @@ -619,7 +619,7 @@ int cil_gen_perm_nodes(struct cil_db *db, struct cil_tree_node *current_perm, st
> cil_tree_node_init(&new_ast);
> new_ast->parent = ast_node;
> new_ast->line = current_perm->line;
> - new_ast->hll_line = current_perm->hll_line;
> + new_ast->hll_offset = current_perm->hll_offset;
>
> rc = cil_gen_perm(db, current_perm, new_ast, flavor, num_perms);
> if (rc != SEPOL_OK) {
> @@ -6203,7 +6203,7 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f
>
> ast_node->parent = ast_current;
> ast_node->line = parse_current->line;
> - ast_node->hll_line = parse_current->hll_line;
> + ast_node->hll_offset = parse_current->hll_offset;
>
> if (parse_current->data == CIL_KEY_BLOCK) {
> rc = cil_gen_block(db, parse_current, ast_node, 0);
> diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> index 02b9828f..34282a92 100644
> --- a/libsepol/cil/src/cil_copy_ast.c
> +++ b/libsepol/cil/src/cil_copy_ast.c
> @@ -2010,7 +2010,7 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
>
> new->parent = parent;
> new->line = orig->line;
> - new->hll_line = orig->hll_line;
> + new->hll_offset = orig->hll_offset;
> new->flavor = orig->flavor;
> new->data = data;
>
> diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
> index 842c327c..3ccef5d7 100644
> --- a/libsepol/cil/src/cil_parser.c
> +++ b/libsepol/cil/src/cil_parser.c
> @@ -45,21 +45,21 @@
> #define CIL_PARSER_MAX_EXPR_DEPTH (0x1 << 12)
>
> struct hll_info {
> - uint32_t hll_lineno;
> + uint32_t hll_offset;
> uint32_t hll_expand;
> };
>
> -static void push_hll_info(struct cil_stack *stack, uint32_t hll_lineno, uint32_t hll_expand)
> +static void push_hll_info(struct cil_stack *stack, uint32_t hll_offset, uint32_t hll_expand)
> {
> struct hll_info *new = cil_malloc(sizeof(*new));
>
> - new->hll_lineno = hll_lineno;
> + new->hll_offset = hll_offset;
> new->hll_expand = hll_expand;
>
> cil_stack_push(stack, CIL_NONE, new);
> }
>
> -static void pop_hll_info(struct cil_stack *stack, uint32_t *hll_lineno, uint32_t *hll_expand)
> +static void pop_hll_info(struct cil_stack *stack, uint32_t *hll_offset, uint32_t *hll_expand)
> {
> struct cil_stack_item *curr = cil_stack_pop(stack);
> struct hll_info *info;
> @@ -69,17 +69,17 @@ static void pop_hll_info(struct cil_stack *stack, uint32_t *hll_lineno, uint32_t
> }
> info = curr->data;
> *hll_expand = info->hll_expand;
> - *hll_lineno = info->hll_lineno;
> + *hll_offset = info->hll_offset;
> free(curr->data);
> }
>
> -static void create_node(struct cil_tree_node **node, struct cil_tree_node *current, uint32_t line, uint32_t hll_line, void *value)
> +static void create_node(struct cil_tree_node **node, struct cil_tree_node *current, uint32_t line, uint32_t hll_offset, void *value)
> {
> cil_tree_node_init(node);
> (*node)->parent = current;
> (*node)->flavor = CIL_NODE;
> (*node)->line = line;
> - (*node)->hll_line = hll_line;
> + (*node)->hll_offset = hll_offset;
> (*node)->data = value;
> }
>
> @@ -93,12 +93,12 @@ static void insert_node(struct cil_tree_node *node, struct cil_tree_node *curren
> current->cl_tail = node;
> }
>
> -static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno, uint32_t *hll_expand, struct cil_stack *stack, char *path)
> +static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_offset, uint32_t *hll_expand, struct cil_stack *stack, char *path)
> {
> char *hll_type;
> struct cil_tree_node *node;
> struct token tok;
> - int rc;
> + uint32_t prev_hll_expand, prev_hll_offset;
In this function, gcc (in GitHub Actions CI) reports:
../cil/src/cil_parser.c: In function ‘cil_parser’:
../cil/src/cil_parser.c:181:16: error: ‘hll_offset’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
181 | (*hll_offset)++;
| ~~~~~~~~~~~~~^~
../cil/src/cil_parser.c:222:11: note: ‘hll_offset’ was declared here
222 | uint32_t hll_offset = 1;
| ^~~~~~~~~~
In fact the warning is misleading because it comes from
prev_hll_offset not being initialized and gcc having trouble figuring
out it is initialized in calls to pop_hll_info.
To silence this warning, could you initialize prev_hll_offset to some value?
>
> cil_lexer_next(&tok);
> if (tok.type != SYMBOL) {
> @@ -115,19 +115,33 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> cil_log(CIL_ERR, "Line mark end without start\n");
> goto exit;
> }
> - pop_hll_info(stack, hll_lineno, hll_expand);
> + prev_hll_expand = *hll_expand;
> + pop_hll_info(stack, &prev_hll_offset, hll_expand);
> + if (*hll_expand) {
> + /* This is needed when exiting an lms section within an lmx section.
> + * In the lms section, hll_offset will increment and then revert
> + * back to its previous value when going back into the lmx section.
> + */
> + *hll_offset = prev_hll_offset;
> + }
> + if (prev_hll_expand && !*hll_expand) {
> + /* This is needed to count the lme at the end of an lmx section
> + * within an lms section (or within no hll section).
> + */
> + (*hll_offset)++;
> + }
> *current = (*current)->parent;
> } else {
> - push_hll_info(stack, *hll_lineno, *hll_expand);
> + push_hll_info(stack, *hll_offset, *hll_expand);
>
> - create_node(&node, *current, tok.line, *hll_lineno, NULL);
> + create_node(&node, *current, tok.line, *hll_offset, NULL);
> insert_node(node, *current);
> *current = node;
>
> - create_node(&node, *current, tok.line, *hll_lineno, CIL_KEY_SRC_INFO);
> + create_node(&node, *current, tok.line, *hll_offset, CIL_KEY_SRC_INFO);
> insert_node(node, *current);
>
> - create_node(&node, *current, tok.line, *hll_lineno, hll_type);
> + create_node(&node, *current, tok.line, *hll_offset, hll_type);
> insert_node(node, *current);
>
> cil_lexer_next(&tok);
> @@ -136,16 +150,9 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> goto exit;
> }
>
> - create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
> + create_node(&node, *current, tok.line, *hll_offset, cil_strpool_add(tok.value));
> insert_node(node, *current);
>
> - rc = cil_string_to_uint32(tok.value, hll_lineno, 10);
> - if (rc != SEPOL_OK) {
> - goto exit;
> - }
> -
> - *hll_expand = (hll_type == CIL_KEY_SRC_HLL_LMX) ? 1 : 0;
> -
> cil_lexer_next(&tok);
> if (tok.type != SYMBOL && tok.type != QSTRING) {
> cil_log(CIL_ERR, "Invalid line mark syntax\n");
> @@ -157,8 +164,10 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> tok.value = tok.value+1;
> }
>
> - create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
> + create_node(&node, *current, tok.line, *hll_offset, cil_strpool_add(tok.value));
> insert_node(node, *current);
> +
> + *hll_expand = (hll_type == CIL_KEY_SRC_HLL_LMX) ? 1 : 0;
> }
>
> cil_lexer_next(&tok);
> @@ -167,6 +176,11 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> goto exit;
> }
>
> + if (!*hll_expand) {
> + /* Need to increment because of the NEWLINE */
> + (*hll_offset)++;
> + }
> +
> return SEPOL_OK;
>
> exit:
> @@ -205,7 +219,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> struct cil_tree_node *current = NULL;
> char *path = cil_strpool_add(_path);
> struct cil_stack *stack;
> - uint32_t hll_lineno = 0;
> + uint32_t hll_offset = 1;
> uint32_t hll_expand = 0;
> struct token tok;
> int rc = SEPOL_OK;
> @@ -223,7 +237,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> cil_lexer_next(&tok);
> switch (tok.type) {
> case HLL_LINEMARK:
> - rc = add_hll_linemark(¤t, &hll_lineno, &hll_expand, stack, path);
> + rc = add_hll_linemark(¤t, &hll_offset, &hll_expand, stack, path);
> if (rc != SEPOL_OK) {
> goto exit;
> }
> @@ -234,7 +248,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> cil_log(CIL_ERR, "Number of open parenthesis exceeds limit of %d at line %d of %s\n", CIL_PARSER_MAX_EXPR_DEPTH, tok.line, path);
> goto exit;
> }
> - create_node(&node, current, tok.line, hll_lineno, NULL);
> + create_node(&node, current, tok.line, hll_offset, NULL);
> insert_node(node, current);
> current = node;
> break;
> @@ -256,12 +270,12 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> goto exit;
> }
>
> - create_node(&node, current, tok.line, hll_lineno, cil_strpool_add(tok.value));
> + create_node(&node, current, tok.line, hll_offset, cil_strpool_add(tok.value));
> insert_node(node, current);
> break;
> case NEWLINE :
> if (!hll_expand) {
> - hll_lineno++;
> + hll_offset++;
> }
> break;
> case COMMENT:
> @@ -269,7 +283,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> cil_lexer_next(&tok);
> }
> if (!hll_expand) {
> - hll_lineno++;
> + hll_offset++;
> }
> if (tok.type != END_OF_FILE) {
> break;
> @@ -306,7 +320,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
>
> exit:
> while (!cil_stack_is_empty(stack)) {
> - pop_hll_info(stack, &hll_lineno, &hll_expand);
> + pop_hll_info(stack, &hll_offset, &hll_expand);
> }
> cil_lexer_destroy();
> cil_stack_destroy(&stack);
> diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c
> index 52b28999..4fdf339d 100644
> --- a/libsepol/cil/src/cil_tree.c
> +++ b/libsepol/cil/src/cil_tree.c
> @@ -50,10 +50,12 @@ __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)
> +struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **info_kind, uint32_t *hll_line, char **path)
> {
> + int rc;
> +
> if (!node) {
> - return NULL;
> + goto exit;
> }
>
> node = node->parent;
> @@ -62,16 +64,21 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **
> if (node->flavor == CIL_NODE && node->data == NULL) {
> if (node->cl_head->data == CIL_KEY_SRC_INFO && node->cl_head->next != NULL && node->cl_head->next->next != NULL) {
> /* Parse Tree */
> - *path = node->cl_head->next->next->data;
> - *is_cil = (node->cl_head->next->data == CIL_KEY_SRC_CIL);
> + *info_kind = node->cl_head->next->data;
> + rc = cil_string_to_uint32(node->cl_head->next->next->data, hll_line, 10);
> + if (rc != SEPOL_OK) {
> + goto exit;
> + }
> + *path = node->cl_head->next->next->next->data;
> return node;
> }
> node = node->parent;
> } else if (node->flavor == CIL_SRC_INFO) {
> /* AST */
> struct cil_src_info *info = node->data;
> + *info_kind = info->kind;
> + *hll_line = info->hll_line;
> *path = info->path;
> - *is_cil = (info->kind == CIL_KEY_SRC_CIL);
> return node;
> } else {
> if (node->flavor == CIL_CALL) {
> @@ -86,17 +93,22 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **
> }
> }
>
> +exit:
> + *info_kind = NULL;
> + *hll_line = 0;
> + *path = NULL;
> return NULL;
> }
>
> char *cil_tree_get_cil_path(struct cil_tree_node *node)
> {
> - char *path = NULL;
> - int is_cil;
> + char *info_kind;
> + uint32_t hll_line;
> + char *path;
>
> while (node) {
> - node = cil_tree_get_next_path(node, &path, &is_cil);
> - if (node && is_cil) {
> + node = cil_tree_get_next_path(node, &info_kind, &hll_line, &path);
> + if (node && info_kind == CIL_KEY_SRC_CIL) {
> return path;
> }
> }
> @@ -114,8 +126,7 @@ __attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *n
>
> if (node) {
> char *path = NULL;
> - int is_cil;
> - unsigned hll_line = node->hll_line;
> + uint32_t hll_offset = node->hll_offset;
>
> path = cil_tree_get_cil_path(node);
>
> @@ -124,12 +135,20 @@ __attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *n
> }
>
> while (node) {
> - node = cil_tree_get_next_path(node, &path, &is_cil);
> - if (node && !is_cil) {
> + do {
> + char *info_kind;
> + uint32_t hll_line;
> +
> + node = cil_tree_get_next_path(node, &info_kind, &hll_line, &path);
> + if (!node || info_kind == CIL_KEY_SRC_CIL) {
> + break;
> + }
> + if (info_kind == CIL_KEY_SRC_HLL_LMS) {
> + hll_line += hll_offset - node->hll_offset - 1;
> + }
> +
> cil_log(lvl," from %s:%d", path, hll_line);
While modifying this code, it would be nice if this "%d" was modified
to "%u", as hll_line should never be negative.
> - path = NULL;
> - hll_line = node->hll_line;
> - }
> + } while (1);
> }
> }
>
> @@ -222,7 +241,7 @@ void cil_tree_node_init(struct cil_tree_node **node)
> new_node->next = NULL;
> new_node->flavor = CIL_ROOT;
> new_node->line = 0;
> - new_node->hll_line = 0;
> + new_node->hll_offset = 0;
>
> *node = new_node;
> }
> diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h
> index f4d22071..5a98da55 100644
> --- a/libsepol/cil/src/cil_tree.h
> +++ b/libsepol/cil/src/cil_tree.h
> @@ -46,11 +46,11 @@ struct cil_tree_node {
> struct cil_tree_node *next; //Each element in the list points to the next element
> enum cil_flavor flavor;
> uint32_t line;
> - uint32_t hll_line;
> + uint32_t hll_offset;
> void *data;
> };
>
> -struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **path, int* is_cil);
> +struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **info_kind, uint32_t *hll_line, char **path);
> 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, ...);
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/8] libsepol/cil: Add line mark kind and line number to src info
2021-08-10 18:05 ` [PATCH 6/8] libsepol/cil: Add line mark kind and line number to src info James Carter
@ 2021-08-16 9:38 ` Nicolas Iooss
2021-08-16 14:44 ` James Carter
0 siblings, 1 reply; 13+ messages in thread
From: Nicolas Iooss @ 2021-08-16 9:38 UTC (permalink / raw)
To: James Carter; +Cc: SElinux list
On Tue, Aug 10, 2021 at 8:22 PM James Carter <jwcart2@gmail.com> wrote:
>
> To be able to write line mark information when writing the AST,
> the line mark kind and line number is needed in the src info.
>
> Instead of indicating whether the src info is for CIL or a hll,
> differentiate between CIL, a normal hll line mark, and an expanded
> hll line mark. Also include the line mark line number in the src
> info nodes.
>
> Signed-off-by: James Carter <jwcart2@gmail.com>
> ---
> libsepol/cil/src/cil.c | 13 +++++++++----
> libsepol/cil/src/cil_build_ast.c | 17 +++++++++++++++--
> libsepol/cil/src/cil_copy_ast.c | 3 ++-
> libsepol/cil/src/cil_internal.h | 7 +++++--
> libsepol/cil/src/cil_parser.c | 27 +++++++++++----------------
> libsepol/cil/src/cil_tree.c | 2 +-
> 6 files changed, 43 insertions(+), 26 deletions(-)
>
> diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
> index bdd16eb8..caec5dad 100644
> --- a/libsepol/cil/src/cil.c
> +++ b/libsepol/cil/src/cil.c
> @@ -220,7 +220,9 @@ char *CIL_KEY_IOCTL;
> char *CIL_KEY_UNORDERED;
> char *CIL_KEY_SRC_INFO;
> char *CIL_KEY_SRC_CIL;
> -char *CIL_KEY_SRC_HLL;
> +char *CIL_KEY_SRC_HLL_LMS;
> +char *CIL_KEY_SRC_HLL_LMX;
> +char *CIL_KEY_SRC_HLL_LME;
>
> static void cil_init_keys(void)
> {
> @@ -384,8 +386,10 @@ static void cil_init_keys(void)
> CIL_KEY_IOCTL = cil_strpool_add("ioctl");
> CIL_KEY_UNORDERED = cil_strpool_add("unordered");
> CIL_KEY_SRC_INFO = cil_strpool_add("<src_info>");
> - CIL_KEY_SRC_CIL = cil_strpool_add("<src_cil>");
> - CIL_KEY_SRC_HLL = cil_strpool_add("<src_hll>");
> + CIL_KEY_SRC_CIL = cil_strpool_add("cil");
> + CIL_KEY_SRC_HLL_LMS = cil_strpool_add("lms");
> + CIL_KEY_SRC_HLL_LMX = cil_strpool_add("lmx");
> + CIL_KEY_SRC_HLL_LME = cil_strpool_add("lme");
> }
>
> void cil_db_init(struct cil_db **db)
> @@ -2881,6 +2885,7 @@ void cil_mls_init(struct cil_mls **mls)
> void cil_src_info_init(struct cil_src_info **info)
> {
> *info = cil_malloc(sizeof(**info));
> - (*info)->is_cil = 0;
> + (*info)->kind = NULL;
> + (*info)->hll_line = 0;
> (*info)->path = NULL;
> }
> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> index ffbd3082..a0f58b1e 100644
> --- a/libsepol/cil/src/cil_build_ast.c
> +++ b/libsepol/cil/src/cil_build_ast.c
> @@ -6060,6 +6060,7 @@ int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *
> CIL_SYN_STRING,
> CIL_SYN_STRING,
> CIL_SYN_STRING,
> + CIL_SYN_STRING,
> CIL_SYN_N_LISTS | CIL_SYN_END,
> CIL_SYN_END
> };
> @@ -6077,8 +6078,19 @@ int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *
>
> cil_src_info_init(&info);
>
> - info->is_cil = (parse_current->next->data == CIL_KEY_SRC_CIL) ? CIL_TRUE : CIL_FALSE;
> - info->path = parse_current->next->next->data;
> + info->kind = parse_current->next->data;
> + if (info->kind != CIL_KEY_SRC_CIL && info->kind != CIL_KEY_SRC_HLL_LMS && info->kind != CIL_KEY_SRC_HLL_LMX) {
> + cil_log(CIL_ERR, "Invalid src info kind\n");
> + rc = SEPOL_ERR;
> + goto exit;
> + }
> +
> + rc = cil_string_to_uint32(parse_current->next->next->data, &info->hll_line, 10);
> + if (rc != SEPOL_OK) {
> + goto exit;
> + }
> +
> + info->path = parse_current->next->next->next->data;
>
> ast_node->data = info;
> ast_node->flavor = CIL_SRC_INFO;
> @@ -6087,6 +6099,7 @@ int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *
>
> exit:
> cil_tree_log(parse_current, CIL_ERR, "Bad src info");
> + cil_destroy_src_info(info);
> return rc;
> }
>
> diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> index 9c0231f2..02b9828f 100644
> --- a/libsepol/cil/src/cil_copy_ast.c
> +++ b/libsepol/cil/src/cil_copy_ast.c
> @@ -1692,7 +1692,8 @@ int cil_copy_src_info(__attribute__((unused)) struct cil_db *db, void *data, voi
>
> cil_src_info_init(&new);
>
> - new->is_cil = orig->is_cil;
> + new->kind = orig->kind;
> + new->hll_line = orig->hll_line;
> new->path = orig->path;
>
> *copy = new;
> diff --git a/libsepol/cil/src/cil_internal.h b/libsepol/cil/src/cil_internal.h
> index b9a03a37..385677d4 100644
> --- a/libsepol/cil/src/cil_internal.h
> +++ b/libsepol/cil/src/cil_internal.h
> @@ -236,7 +236,9 @@ extern char *CIL_KEY_IOCTL;
> extern char *CIL_KEY_UNORDERED;
> extern char *CIL_KEY_SRC_INFO;
> extern char *CIL_KEY_SRC_CIL;
> -extern char *CIL_KEY_SRC_HLL;
> +extern char *CIL_KEY_SRC_HLL_LMS;
> +extern char *CIL_KEY_SRC_HLL_LMX;
> +extern char *CIL_KEY_SRC_HLL_LME;
>
> /*
> Symbol Table Array Indices
> @@ -963,7 +965,8 @@ struct cil_mls {
> };
>
> struct cil_src_info {
> - int is_cil;
> + char *kind;
> + uint32_t hll_line;
> char *path;
> };
>
> diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
> index 9ca1432e..842c327c 100644
> --- a/libsepol/cil/src/cil_parser.c
> +++ b/libsepol/cil/src/cil_parser.c
> @@ -44,10 +44,6 @@
>
> #define CIL_PARSER_MAX_EXPR_DEPTH (0x1 << 12)
>
> -char *CIL_KEY_HLL_LMS;
> -char *CIL_KEY_HLL_LMX;
> -char *CIL_KEY_HLL_LME;
> -
> struct hll_info {
> uint32_t hll_lineno;
> uint32_t hll_expand;
> @@ -102,7 +98,6 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> char *hll_type;
> struct cil_tree_node *node;
> struct token tok;
> - char *hll_file;
> int rc;
>
> cil_lexer_next(&tok);
> @@ -111,11 +106,11 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> goto exit;
> }
> hll_type = cil_strpool_add(tok.value);
> - if (hll_type != CIL_KEY_HLL_LME && hll_type != CIL_KEY_HLL_LMS && hll_type != CIL_KEY_HLL_LMX) {
> + if (hll_type != CIL_KEY_SRC_HLL_LME && hll_type != CIL_KEY_SRC_HLL_LMS && hll_type != CIL_KEY_SRC_HLL_LMX) {
> cil_log(CIL_ERR, "Invalid line mark syntax\n");
> goto exit;
> }
> - if (hll_type == CIL_KEY_HLL_LME) {
> + if (hll_type == CIL_KEY_SRC_HLL_LME) {
> if (cil_stack_is_empty(stack)) {
> cil_log(CIL_ERR, "Line mark end without start\n");
> goto exit;
> @@ -132,7 +127,7 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> create_node(&node, *current, tok.line, *hll_lineno, CIL_KEY_SRC_INFO);
> insert_node(node, *current);
>
> - create_node(&node, *current, tok.line, *hll_lineno, CIL_KEY_SRC_HLL);
> + create_node(&node, *current, tok.line, *hll_lineno, hll_type);
> insert_node(node, *current);
>
> cil_lexer_next(&tok);
> @@ -141,12 +136,15 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> goto exit;
> }
>
> + create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
> + insert_node(node, *current);
> +
> rc = cil_string_to_uint32(tok.value, hll_lineno, 10);
> if (rc != SEPOL_OK) {
> goto exit;
> }
>
> - *hll_expand = (hll_type == CIL_KEY_HLL_LMX) ? 1 : 0;
> + *hll_expand = (hll_type == CIL_KEY_SRC_HLL_LMX) ? 1 : 0;
>
> cil_lexer_next(&tok);
> if (tok.type != SYMBOL && tok.type != QSTRING) {
> @@ -159,9 +157,7 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> tok.value = tok.value+1;
> }
>
> - hll_file = cil_strpool_add(tok.value);
> -
> - create_node(&node, *current, tok.line, *hll_lineno, hll_file);
> + create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
> insert_node(node, *current);
> }
>
> @@ -192,6 +188,9 @@ static void add_cil_path(struct cil_tree_node **current, char *path)
> create_node(&node, *current, 0, 0, CIL_KEY_SRC_CIL);
> insert_node(node, *current);
>
> + create_node(&node, *current, 0, 0, "1");
> + insert_node(node, *current);
> +
Using this raw "1" here looks strange, while every other case uses
cil_strpool_add indirections. gcc complains about this:
../cil/src/cil_parser.c: In function ‘add_cil_path’:
../cil/src/cil_parser.c:205:44: error: passing argument 5 of
‘create_node’ discards ‘const’ qualifier from pointer target type
[-Werror=discarded-qualifiers]
205 | create_node(&node, *current, 0, 0, "1");
| ^~~
../cil/src/cil_parser.c:76:127: note: expected ‘void *’ but argument
is of type ‘const char *’
76 | tree_node **node, struct cil_tree_node *current, uint32_t
line, uint32_t hll_offset, void *value)
|
I understand that cil_strpool_add is used in order to be able to
quickly compare some strings using raw pointer comparisons. So I do
not know whether a proper fix would consist in using create_note(...,
(void *)"1") or in introducing a char *CIL_KEY_ONE =
cil_strpool_add("1"), or in making (struct cil_tree_node).data const.
> create_node(&node, *current, 0, 0, path);
> insert_node(node, *current);
> }
> @@ -211,10 +210,6 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> struct token tok;
> int rc = SEPOL_OK;
>
> - CIL_KEY_HLL_LMS = cil_strpool_add("lms");
> - CIL_KEY_HLL_LMX = cil_strpool_add("lmx");
> - CIL_KEY_HLL_LME = cil_strpool_add("lme");
> -
> cil_stack_init(&stack);
>
> cil_lexer_setup(buffer, size);
> diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c
> index 4cf8dcc8..52b28999 100644
> --- a/libsepol/cil/src/cil_tree.c
> +++ b/libsepol/cil/src/cil_tree.c
> @@ -71,7 +71,7 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **
> /* AST */
> struct cil_src_info *info = node->data;
> *path = info->path;
> - *is_cil = info->is_cil;
> + *is_cil = (info->kind == CIL_KEY_SRC_CIL);
> return node;
> } else {
> if (node->flavor == CIL_CALL) {
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 7/8] libsepol/cil: Report correct high-level language line numbers
2021-08-16 9:23 ` Nicolas Iooss
@ 2021-08-16 14:40 ` James Carter
0 siblings, 0 replies; 13+ messages in thread
From: James Carter @ 2021-08-16 14:40 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: SElinux list
On Mon, Aug 16, 2021 at 5:23 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Tue, Aug 10, 2021 at 8:22 PM James Carter <jwcart2@gmail.com> wrote:
> >
> > CIL supports specifiying the original high-level language file and
> > line numbers when reporting errors. This is done through line marks
> > and is mostly used to report the original Refpolicy file and line
> > number for neverallow rules that have been converted to CIL.
> >
> > As long as the line mark remain simple, everything works fine, but
> > the wrong line numbers will be reported with more complex nextings
> > of line marks.
> >
> > Example:
> > ;;* lms 100 file01.hll
> > (type t1a)
> > (allow t1a self (CLASS (PERM)))
> > ;;* lmx 200 file02.hll
> > (type t2a)
> > (allow t2a self (CLASS (PERM)))
> > ;;* lme
> > (type t1b)
> > (allow t1b self (CLASS (PERM)))
> > (allow bad1b self (CLASS (PERM))) ; file01.hll:101 (Should be 106)
> > ;;* lme
> >
> > The primary problem is that the tree nodes can only store one hll
> > line number. Instead a number is needed that can be used by any
> > number of stacked line mark sections. This number would increment
> > line a normal line number except when in lmx sections (that have
> > the same line number throughout the section because they represent
> > an expansion of a line -- like the expansion of a macro call. This
> > number can go backwards when exiting a lms section within a lmx
> > section, because line number will increase in the lms section, but
> > outside the lmx section, the line number did not advance.
> >
> > This number is called the hll_offset and this is the value that is
> > now stored in tree nodes instead of the hll line number. To calculate
> > the hll line number for a rule, a search is made for an ancestor of
> > the node that is a line mark and the line number for a lms section
> > is the hll line number stored in the line mark, plus the hll offset
> > of the rule, minus the hll offset of the line mark node, minus one.
> > (hll_lineno + hll_offset_rule - hll_offset_lm - 1)
> >
> > Signed-off-by: James Carter <jwcart2@gmail.com>
> > ---
> > libsepol/cil/src/cil_binary.c | 9 ++--
> > libsepol/cil/src/cil_build_ast.c | 4 +-
> > libsepol/cil/src/cil_copy_ast.c | 2 +-
> > libsepol/cil/src/cil_parser.c | 74 +++++++++++++++++++-------------
> > libsepol/cil/src/cil_tree.c | 53 +++++++++++++++--------
> > libsepol/cil/src/cil_tree.h | 4 +-
> > 6 files changed, 90 insertions(+), 56 deletions(-)
> >
> > diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
> > index 2b65c622..43c37fc2 100644
> > --- a/libsepol/cil/src/cil_binary.c
> > +++ b/libsepol/cil/src/cil_binary.c
> > @@ -4480,7 +4480,8 @@ static avrule_t *__cil_init_sepol_avrule(uint32_t kind, struct cil_tree_node *no
> > avrule_t *avrule;
> > struct cil_tree_node *source_node;
> > char *source_path;
> > - int is_cil;
> > + char *lm_kind;
> > + uint32_t hll_line;
> >
> > avrule = cil_malloc(sizeof(avrule_t));
> > avrule->specified = kind;
> > @@ -4492,11 +4493,11 @@ static avrule_t *__cil_init_sepol_avrule(uint32_t kind, struct cil_tree_node *no
> >
> > avrule->source_filename = NULL;
> > avrule->source_line = node->line;
> > - source_node = cil_tree_get_next_path(node, &source_path, &is_cil);
> > + source_node = cil_tree_get_next_path(node, &lm_kind, &hll_line, &source_path);
> > if (source_node) {
> > avrule->source_filename = source_path;
> > - if (!is_cil) {
> > - avrule->source_line = node->hll_line;
> > + if (lm_kind != CIL_KEY_SRC_CIL) {
> > + avrule->source_line = hll_line + node->hll_offset - source_node->hll_offset - 1;
> > }
> > }
> >
> > diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> > index a0f58b1e..a5afc267 100644
> > --- a/libsepol/cil/src/cil_build_ast.c
> > +++ b/libsepol/cil/src/cil_build_ast.c
> > @@ -619,7 +619,7 @@ int cil_gen_perm_nodes(struct cil_db *db, struct cil_tree_node *current_perm, st
> > cil_tree_node_init(&new_ast);
> > new_ast->parent = ast_node;
> > new_ast->line = current_perm->line;
> > - new_ast->hll_line = current_perm->hll_line;
> > + new_ast->hll_offset = current_perm->hll_offset;
> >
> > rc = cil_gen_perm(db, current_perm, new_ast, flavor, num_perms);
> > if (rc != SEPOL_OK) {
> > @@ -6203,7 +6203,7 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f
> >
> > ast_node->parent = ast_current;
> > ast_node->line = parse_current->line;
> > - ast_node->hll_line = parse_current->hll_line;
> > + ast_node->hll_offset = parse_current->hll_offset;
> >
> > if (parse_current->data == CIL_KEY_BLOCK) {
> > rc = cil_gen_block(db, parse_current, ast_node, 0);
> > diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> > index 02b9828f..34282a92 100644
> > --- a/libsepol/cil/src/cil_copy_ast.c
> > +++ b/libsepol/cil/src/cil_copy_ast.c
> > @@ -2010,7 +2010,7 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
> >
> > new->parent = parent;
> > new->line = orig->line;
> > - new->hll_line = orig->hll_line;
> > + new->hll_offset = orig->hll_offset;
> > new->flavor = orig->flavor;
> > new->data = data;
> >
> > diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
> > index 842c327c..3ccef5d7 100644
> > --- a/libsepol/cil/src/cil_parser.c
> > +++ b/libsepol/cil/src/cil_parser.c
> > @@ -45,21 +45,21 @@
> > #define CIL_PARSER_MAX_EXPR_DEPTH (0x1 << 12)
> >
> > struct hll_info {
> > - uint32_t hll_lineno;
> > + uint32_t hll_offset;
> > uint32_t hll_expand;
> > };
> >
> > -static void push_hll_info(struct cil_stack *stack, uint32_t hll_lineno, uint32_t hll_expand)
> > +static void push_hll_info(struct cil_stack *stack, uint32_t hll_offset, uint32_t hll_expand)
> > {
> > struct hll_info *new = cil_malloc(sizeof(*new));
> >
> > - new->hll_lineno = hll_lineno;
> > + new->hll_offset = hll_offset;
> > new->hll_expand = hll_expand;
> >
> > cil_stack_push(stack, CIL_NONE, new);
> > }
> >
> > -static void pop_hll_info(struct cil_stack *stack, uint32_t *hll_lineno, uint32_t *hll_expand)
> > +static void pop_hll_info(struct cil_stack *stack, uint32_t *hll_offset, uint32_t *hll_expand)
> > {
> > struct cil_stack_item *curr = cil_stack_pop(stack);
> > struct hll_info *info;
> > @@ -69,17 +69,17 @@ static void pop_hll_info(struct cil_stack *stack, uint32_t *hll_lineno, uint32_t
> > }
> > info = curr->data;
> > *hll_expand = info->hll_expand;
> > - *hll_lineno = info->hll_lineno;
> > + *hll_offset = info->hll_offset;
> > free(curr->data);
> > }
> >
> > -static void create_node(struct cil_tree_node **node, struct cil_tree_node *current, uint32_t line, uint32_t hll_line, void *value)
> > +static void create_node(struct cil_tree_node **node, struct cil_tree_node *current, uint32_t line, uint32_t hll_offset, void *value)
> > {
> > cil_tree_node_init(node);
> > (*node)->parent = current;
> > (*node)->flavor = CIL_NODE;
> > (*node)->line = line;
> > - (*node)->hll_line = hll_line;
> > + (*node)->hll_offset = hll_offset;
> > (*node)->data = value;
> > }
> >
> > @@ -93,12 +93,12 @@ static void insert_node(struct cil_tree_node *node, struct cil_tree_node *curren
> > current->cl_tail = node;
> > }
> >
> > -static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno, uint32_t *hll_expand, struct cil_stack *stack, char *path)
> > +static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_offset, uint32_t *hll_expand, struct cil_stack *stack, char *path)
> > {
> > char *hll_type;
> > struct cil_tree_node *node;
> > struct token tok;
> > - int rc;
> > + uint32_t prev_hll_expand, prev_hll_offset;
>
> In this function, gcc (in GitHub Actions CI) reports:
>
> ../cil/src/cil_parser.c: In function ‘cil_parser’:
> ../cil/src/cil_parser.c:181:16: error: ‘hll_offset’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
> 181 | (*hll_offset)++;
> | ~~~~~~~~~~~~~^~
> ../cil/src/cil_parser.c:222:11: note: ‘hll_offset’ was declared here
> 222 | uint32_t hll_offset = 1;
> | ^~~~~~~~~~
>
> In fact the warning is misleading because it comes from
> prev_hll_offset not being initialized and gcc having trouble figuring
> out it is initialized in calls to pop_hll_info.
>
> To silence this warning, could you initialize prev_hll_offset to some value?
>
Yes, I will do that.
Thanks,
Jim
> >
> > cil_lexer_next(&tok);
> > if (tok.type != SYMBOL) {
> > @@ -115,19 +115,33 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> > cil_log(CIL_ERR, "Line mark end without start\n");
> > goto exit;
> > }
> > - pop_hll_info(stack, hll_lineno, hll_expand);
> > + prev_hll_expand = *hll_expand;
> > + pop_hll_info(stack, &prev_hll_offset, hll_expand);
> > + if (*hll_expand) {
> > + /* This is needed when exiting an lms section within an lmx section.
> > + * In the lms section, hll_offset will increment and then revert
> > + * back to its previous value when going back into the lmx section.
> > + */
> > + *hll_offset = prev_hll_offset;
> > + }
> > + if (prev_hll_expand && !*hll_expand) {
> > + /* This is needed to count the lme at the end of an lmx section
> > + * within an lms section (or within no hll section).
> > + */
> > + (*hll_offset)++;
> > + }
> > *current = (*current)->parent;
> > } else {
> > - push_hll_info(stack, *hll_lineno, *hll_expand);
> > + push_hll_info(stack, *hll_offset, *hll_expand);
> >
> > - create_node(&node, *current, tok.line, *hll_lineno, NULL);
> > + create_node(&node, *current, tok.line, *hll_offset, NULL);
> > insert_node(node, *current);
> > *current = node;
> >
> > - create_node(&node, *current, tok.line, *hll_lineno, CIL_KEY_SRC_INFO);
> > + create_node(&node, *current, tok.line, *hll_offset, CIL_KEY_SRC_INFO);
> > insert_node(node, *current);
> >
> > - create_node(&node, *current, tok.line, *hll_lineno, hll_type);
> > + create_node(&node, *current, tok.line, *hll_offset, hll_type);
> > insert_node(node, *current);
> >
> > cil_lexer_next(&tok);
> > @@ -136,16 +150,9 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> > goto exit;
> > }
> >
> > - create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
> > + create_node(&node, *current, tok.line, *hll_offset, cil_strpool_add(tok.value));
> > insert_node(node, *current);
> >
> > - rc = cil_string_to_uint32(tok.value, hll_lineno, 10);
> > - if (rc != SEPOL_OK) {
> > - goto exit;
> > - }
> > -
> > - *hll_expand = (hll_type == CIL_KEY_SRC_HLL_LMX) ? 1 : 0;
> > -
> > cil_lexer_next(&tok);
> > if (tok.type != SYMBOL && tok.type != QSTRING) {
> > cil_log(CIL_ERR, "Invalid line mark syntax\n");
> > @@ -157,8 +164,10 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> > tok.value = tok.value+1;
> > }
> >
> > - create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
> > + create_node(&node, *current, tok.line, *hll_offset, cil_strpool_add(tok.value));
> > insert_node(node, *current);
> > +
> > + *hll_expand = (hll_type == CIL_KEY_SRC_HLL_LMX) ? 1 : 0;
> > }
> >
> > cil_lexer_next(&tok);
> > @@ -167,6 +176,11 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> > goto exit;
> > }
> >
> > + if (!*hll_expand) {
> > + /* Need to increment because of the NEWLINE */
> > + (*hll_offset)++;
> > + }
> > +
> > return SEPOL_OK;
> >
> > exit:
> > @@ -205,7 +219,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> > struct cil_tree_node *current = NULL;
> > char *path = cil_strpool_add(_path);
> > struct cil_stack *stack;
> > - uint32_t hll_lineno = 0;
> > + uint32_t hll_offset = 1;
> > uint32_t hll_expand = 0;
> > struct token tok;
> > int rc = SEPOL_OK;
> > @@ -223,7 +237,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> > cil_lexer_next(&tok);
> > switch (tok.type) {
> > case HLL_LINEMARK:
> > - rc = add_hll_linemark(¤t, &hll_lineno, &hll_expand, stack, path);
> > + rc = add_hll_linemark(¤t, &hll_offset, &hll_expand, stack, path);
> > if (rc != SEPOL_OK) {
> > goto exit;
> > }
> > @@ -234,7 +248,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> > cil_log(CIL_ERR, "Number of open parenthesis exceeds limit of %d at line %d of %s\n", CIL_PARSER_MAX_EXPR_DEPTH, tok.line, path);
> > goto exit;
> > }
> > - create_node(&node, current, tok.line, hll_lineno, NULL);
> > + create_node(&node, current, tok.line, hll_offset, NULL);
> > insert_node(node, current);
> > current = node;
> > break;
> > @@ -256,12 +270,12 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> > goto exit;
> > }
> >
> > - create_node(&node, current, tok.line, hll_lineno, cil_strpool_add(tok.value));
> > + create_node(&node, current, tok.line, hll_offset, cil_strpool_add(tok.value));
> > insert_node(node, current);
> > break;
> > case NEWLINE :
> > if (!hll_expand) {
> > - hll_lineno++;
> > + hll_offset++;
> > }
> > break;
> > case COMMENT:
> > @@ -269,7 +283,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> > cil_lexer_next(&tok);
> > }
> > if (!hll_expand) {
> > - hll_lineno++;
> > + hll_offset++;
> > }
> > if (tok.type != END_OF_FILE) {
> > break;
> > @@ -306,7 +320,7 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> >
> > exit:
> > while (!cil_stack_is_empty(stack)) {
> > - pop_hll_info(stack, &hll_lineno, &hll_expand);
> > + pop_hll_info(stack, &hll_offset, &hll_expand);
> > }
> > cil_lexer_destroy();
> > cil_stack_destroy(&stack);
> > diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c
> > index 52b28999..4fdf339d 100644
> > --- a/libsepol/cil/src/cil_tree.c
> > +++ b/libsepol/cil/src/cil_tree.c
> > @@ -50,10 +50,12 @@ __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)
> > +struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **info_kind, uint32_t *hll_line, char **path)
> > {
> > + int rc;
> > +
> > if (!node) {
> > - return NULL;
> > + goto exit;
> > }
> >
> > node = node->parent;
> > @@ -62,16 +64,21 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **
> > if (node->flavor == CIL_NODE && node->data == NULL) {
> > if (node->cl_head->data == CIL_KEY_SRC_INFO && node->cl_head->next != NULL && node->cl_head->next->next != NULL) {
> > /* Parse Tree */
> > - *path = node->cl_head->next->next->data;
> > - *is_cil = (node->cl_head->next->data == CIL_KEY_SRC_CIL);
> > + *info_kind = node->cl_head->next->data;
> > + rc = cil_string_to_uint32(node->cl_head->next->next->data, hll_line, 10);
> > + if (rc != SEPOL_OK) {
> > + goto exit;
> > + }
> > + *path = node->cl_head->next->next->next->data;
> > return node;
> > }
> > node = node->parent;
> > } else if (node->flavor == CIL_SRC_INFO) {
> > /* AST */
> > struct cil_src_info *info = node->data;
> > + *info_kind = info->kind;
> > + *hll_line = info->hll_line;
> > *path = info->path;
> > - *is_cil = (info->kind == CIL_KEY_SRC_CIL);
> > return node;
> > } else {
> > if (node->flavor == CIL_CALL) {
> > @@ -86,17 +93,22 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **
> > }
> > }
> >
> > +exit:
> > + *info_kind = NULL;
> > + *hll_line = 0;
> > + *path = NULL;
> > return NULL;
> > }
> >
> > char *cil_tree_get_cil_path(struct cil_tree_node *node)
> > {
> > - char *path = NULL;
> > - int is_cil;
> > + char *info_kind;
> > + uint32_t hll_line;
> > + char *path;
> >
> > while (node) {
> > - node = cil_tree_get_next_path(node, &path, &is_cil);
> > - if (node && is_cil) {
> > + node = cil_tree_get_next_path(node, &info_kind, &hll_line, &path);
> > + if (node && info_kind == CIL_KEY_SRC_CIL) {
> > return path;
> > }
> > }
> > @@ -114,8 +126,7 @@ __attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *n
> >
> > if (node) {
> > char *path = NULL;
> > - int is_cil;
> > - unsigned hll_line = node->hll_line;
> > + uint32_t hll_offset = node->hll_offset;
> >
> > path = cil_tree_get_cil_path(node);
> >
> > @@ -124,12 +135,20 @@ __attribute__((format (printf, 3, 4))) void cil_tree_log(struct cil_tree_node *n
> > }
> >
> > while (node) {
> > - node = cil_tree_get_next_path(node, &path, &is_cil);
> > - if (node && !is_cil) {
> > + do {
> > + char *info_kind;
> > + uint32_t hll_line;
> > +
> > + node = cil_tree_get_next_path(node, &info_kind, &hll_line, &path);
> > + if (!node || info_kind == CIL_KEY_SRC_CIL) {
> > + break;
> > + }
> > + if (info_kind == CIL_KEY_SRC_HLL_LMS) {
> > + hll_line += hll_offset - node->hll_offset - 1;
> > + }
> > +
> > cil_log(lvl," from %s:%d", path, hll_line);
>
> While modifying this code, it would be nice if this "%d" was modified
> to "%u", as hll_line should never be negative.
>
> > - path = NULL;
> > - hll_line = node->hll_line;
> > - }
> > + } while (1);
> > }
> > }
> >
> > @@ -222,7 +241,7 @@ void cil_tree_node_init(struct cil_tree_node **node)
> > new_node->next = NULL;
> > new_node->flavor = CIL_ROOT;
> > new_node->line = 0;
> > - new_node->hll_line = 0;
> > + new_node->hll_offset = 0;
> >
> > *node = new_node;
> > }
> > diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h
> > index f4d22071..5a98da55 100644
> > --- a/libsepol/cil/src/cil_tree.h
> > +++ b/libsepol/cil/src/cil_tree.h
> > @@ -46,11 +46,11 @@ struct cil_tree_node {
> > struct cil_tree_node *next; //Each element in the list points to the next element
> > enum cil_flavor flavor;
> > uint32_t line;
> > - uint32_t hll_line;
> > + uint32_t hll_offset;
> > void *data;
> > };
> >
> > -struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **path, int* is_cil);
> > +struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **info_kind, uint32_t *hll_line, char **path);
> > 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, ...);
> >
> > --
> > 2.31.1
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 6/8] libsepol/cil: Add line mark kind and line number to src info
2021-08-16 9:38 ` Nicolas Iooss
@ 2021-08-16 14:44 ` James Carter
0 siblings, 0 replies; 13+ messages in thread
From: James Carter @ 2021-08-16 14:44 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: SElinux list
On Mon, Aug 16, 2021 at 5:39 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Tue, Aug 10, 2021 at 8:22 PM James Carter <jwcart2@gmail.com> wrote:
> >
> > To be able to write line mark information when writing the AST,
> > the line mark kind and line number is needed in the src info.
> >
> > Instead of indicating whether the src info is for CIL or a hll,
> > differentiate between CIL, a normal hll line mark, and an expanded
> > hll line mark. Also include the line mark line number in the src
> > info nodes.
> >
> > Signed-off-by: James Carter <jwcart2@gmail.com>
> > ---
> > libsepol/cil/src/cil.c | 13 +++++++++----
> > libsepol/cil/src/cil_build_ast.c | 17 +++++++++++++++--
> > libsepol/cil/src/cil_copy_ast.c | 3 ++-
> > libsepol/cil/src/cil_internal.h | 7 +++++--
> > libsepol/cil/src/cil_parser.c | 27 +++++++++++----------------
> > libsepol/cil/src/cil_tree.c | 2 +-
> > 6 files changed, 43 insertions(+), 26 deletions(-)
> >
> > diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
> > index bdd16eb8..caec5dad 100644
> > --- a/libsepol/cil/src/cil.c
> > +++ b/libsepol/cil/src/cil.c
> > @@ -220,7 +220,9 @@ char *CIL_KEY_IOCTL;
> > char *CIL_KEY_UNORDERED;
> > char *CIL_KEY_SRC_INFO;
> > char *CIL_KEY_SRC_CIL;
> > -char *CIL_KEY_SRC_HLL;
> > +char *CIL_KEY_SRC_HLL_LMS;
> > +char *CIL_KEY_SRC_HLL_LMX;
> > +char *CIL_KEY_SRC_HLL_LME;
> >
> > static void cil_init_keys(void)
> > {
> > @@ -384,8 +386,10 @@ static void cil_init_keys(void)
> > CIL_KEY_IOCTL = cil_strpool_add("ioctl");
> > CIL_KEY_UNORDERED = cil_strpool_add("unordered");
> > CIL_KEY_SRC_INFO = cil_strpool_add("<src_info>");
> > - CIL_KEY_SRC_CIL = cil_strpool_add("<src_cil>");
> > - CIL_KEY_SRC_HLL = cil_strpool_add("<src_hll>");
> > + CIL_KEY_SRC_CIL = cil_strpool_add("cil");
> > + CIL_KEY_SRC_HLL_LMS = cil_strpool_add("lms");
> > + CIL_KEY_SRC_HLL_LMX = cil_strpool_add("lmx");
> > + CIL_KEY_SRC_HLL_LME = cil_strpool_add("lme");
> > }
> >
> > void cil_db_init(struct cil_db **db)
> > @@ -2881,6 +2885,7 @@ void cil_mls_init(struct cil_mls **mls)
> > void cil_src_info_init(struct cil_src_info **info)
> > {
> > *info = cil_malloc(sizeof(**info));
> > - (*info)->is_cil = 0;
> > + (*info)->kind = NULL;
> > + (*info)->hll_line = 0;
> > (*info)->path = NULL;
> > }
> > diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> > index ffbd3082..a0f58b1e 100644
> > --- a/libsepol/cil/src/cil_build_ast.c
> > +++ b/libsepol/cil/src/cil_build_ast.c
> > @@ -6060,6 +6060,7 @@ int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *
> > CIL_SYN_STRING,
> > CIL_SYN_STRING,
> > CIL_SYN_STRING,
> > + CIL_SYN_STRING,
> > CIL_SYN_N_LISTS | CIL_SYN_END,
> > CIL_SYN_END
> > };
> > @@ -6077,8 +6078,19 @@ int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *
> >
> > cil_src_info_init(&info);
> >
> > - info->is_cil = (parse_current->next->data == CIL_KEY_SRC_CIL) ? CIL_TRUE : CIL_FALSE;
> > - info->path = parse_current->next->next->data;
> > + info->kind = parse_current->next->data;
> > + if (info->kind != CIL_KEY_SRC_CIL && info->kind != CIL_KEY_SRC_HLL_LMS && info->kind != CIL_KEY_SRC_HLL_LMX) {
> > + cil_log(CIL_ERR, "Invalid src info kind\n");
> > + rc = SEPOL_ERR;
> > + goto exit;
> > + }
> > +
> > + rc = cil_string_to_uint32(parse_current->next->next->data, &info->hll_line, 10);
> > + if (rc != SEPOL_OK) {
> > + goto exit;
> > + }
> > +
> > + info->path = parse_current->next->next->next->data;
> >
> > ast_node->data = info;
> > ast_node->flavor = CIL_SRC_INFO;
> > @@ -6087,6 +6099,7 @@ int cil_gen_src_info(struct cil_tree_node *parse_current, struct cil_tree_node *
> >
> > exit:
> > cil_tree_log(parse_current, CIL_ERR, "Bad src info");
> > + cil_destroy_src_info(info);
> > return rc;
> > }
> >
> > diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
> > index 9c0231f2..02b9828f 100644
> > --- a/libsepol/cil/src/cil_copy_ast.c
> > +++ b/libsepol/cil/src/cil_copy_ast.c
> > @@ -1692,7 +1692,8 @@ int cil_copy_src_info(__attribute__((unused)) struct cil_db *db, void *data, voi
> >
> > cil_src_info_init(&new);
> >
> > - new->is_cil = orig->is_cil;
> > + new->kind = orig->kind;
> > + new->hll_line = orig->hll_line;
> > new->path = orig->path;
> >
> > *copy = new;
> > diff --git a/libsepol/cil/src/cil_internal.h b/libsepol/cil/src/cil_internal.h
> > index b9a03a37..385677d4 100644
> > --- a/libsepol/cil/src/cil_internal.h
> > +++ b/libsepol/cil/src/cil_internal.h
> > @@ -236,7 +236,9 @@ extern char *CIL_KEY_IOCTL;
> > extern char *CIL_KEY_UNORDERED;
> > extern char *CIL_KEY_SRC_INFO;
> > extern char *CIL_KEY_SRC_CIL;
> > -extern char *CIL_KEY_SRC_HLL;
> > +extern char *CIL_KEY_SRC_HLL_LMS;
> > +extern char *CIL_KEY_SRC_HLL_LMX;
> > +extern char *CIL_KEY_SRC_HLL_LME;
> >
> > /*
> > Symbol Table Array Indices
> > @@ -963,7 +965,8 @@ struct cil_mls {
> > };
> >
> > struct cil_src_info {
> > - int is_cil;
> > + char *kind;
> > + uint32_t hll_line;
> > char *path;
> > };
> >
> > diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
> > index 9ca1432e..842c327c 100644
> > --- a/libsepol/cil/src/cil_parser.c
> > +++ b/libsepol/cil/src/cil_parser.c
> > @@ -44,10 +44,6 @@
> >
> > #define CIL_PARSER_MAX_EXPR_DEPTH (0x1 << 12)
> >
> > -char *CIL_KEY_HLL_LMS;
> > -char *CIL_KEY_HLL_LMX;
> > -char *CIL_KEY_HLL_LME;
> > -
> > struct hll_info {
> > uint32_t hll_lineno;
> > uint32_t hll_expand;
> > @@ -102,7 +98,6 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> > char *hll_type;
> > struct cil_tree_node *node;
> > struct token tok;
> > - char *hll_file;
> > int rc;
> >
> > cil_lexer_next(&tok);
> > @@ -111,11 +106,11 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> > goto exit;
> > }
> > hll_type = cil_strpool_add(tok.value);
> > - if (hll_type != CIL_KEY_HLL_LME && hll_type != CIL_KEY_HLL_LMS && hll_type != CIL_KEY_HLL_LMX) {
> > + if (hll_type != CIL_KEY_SRC_HLL_LME && hll_type != CIL_KEY_SRC_HLL_LMS && hll_type != CIL_KEY_SRC_HLL_LMX) {
> > cil_log(CIL_ERR, "Invalid line mark syntax\n");
> > goto exit;
> > }
> > - if (hll_type == CIL_KEY_HLL_LME) {
> > + if (hll_type == CIL_KEY_SRC_HLL_LME) {
> > if (cil_stack_is_empty(stack)) {
> > cil_log(CIL_ERR, "Line mark end without start\n");
> > goto exit;
> > @@ -132,7 +127,7 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> > create_node(&node, *current, tok.line, *hll_lineno, CIL_KEY_SRC_INFO);
> > insert_node(node, *current);
> >
> > - create_node(&node, *current, tok.line, *hll_lineno, CIL_KEY_SRC_HLL);
> > + create_node(&node, *current, tok.line, *hll_lineno, hll_type);
> > insert_node(node, *current);
> >
> > cil_lexer_next(&tok);
> > @@ -141,12 +136,15 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> > goto exit;
> > }
> >
> > + create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
> > + insert_node(node, *current);
> > +
> > rc = cil_string_to_uint32(tok.value, hll_lineno, 10);
> > if (rc != SEPOL_OK) {
> > goto exit;
> > }
> >
> > - *hll_expand = (hll_type == CIL_KEY_HLL_LMX) ? 1 : 0;
> > + *hll_expand = (hll_type == CIL_KEY_SRC_HLL_LMX) ? 1 : 0;
> >
> > cil_lexer_next(&tok);
> > if (tok.type != SYMBOL && tok.type != QSTRING) {
> > @@ -159,9 +157,7 @@ static int add_hll_linemark(struct cil_tree_node **current, uint32_t *hll_lineno
> > tok.value = tok.value+1;
> > }
> >
> > - hll_file = cil_strpool_add(tok.value);
> > -
> > - create_node(&node, *current, tok.line, *hll_lineno, hll_file);
> > + create_node(&node, *current, tok.line, *hll_lineno, cil_strpool_add(tok.value));
> > insert_node(node, *current);
> > }
> >
> > @@ -192,6 +188,9 @@ static void add_cil_path(struct cil_tree_node **current, char *path)
> > create_node(&node, *current, 0, 0, CIL_KEY_SRC_CIL);
> > insert_node(node, *current);
> >
> > + create_node(&node, *current, 0, 0, "1");
> > + insert_node(node, *current);
> > +
>
> Using this raw "1" here looks strange, while every other case uses
> cil_strpool_add indirections. gcc complains about this:
>
> ../cil/src/cil_parser.c: In function ‘add_cil_path’:
> ../cil/src/cil_parser.c:205:44: error: passing argument 5 of
> ‘create_node’ discards ‘const’ qualifier from pointer target type
> [-Werror=discarded-qualifiers]
> 205 | create_node(&node, *current, 0, 0, "1");
> | ^~~
> ../cil/src/cil_parser.c:76:127: note: expected ‘void *’ but argument
> is of type ‘const char *’
> 76 | tree_node **node, struct cil_tree_node *current, uint32_t
> line, uint32_t hll_offset, void *value)
> |
>
> I understand that cil_strpool_add is used in order to be able to
> quickly compare some strings using raw pointer comparisons. So I do
> not know whether a proper fix would consist in using create_note(...,
> (void *)"1") or in introducing a char *CIL_KEY_ONE =
> cil_strpool_add("1"), or in making (struct cil_tree_node).data const.
>
I think that I would prefer to go with (void*)"1", since the "1" is
not a keyword of any sort.
Thanks for the review.
Jim
> > create_node(&node, *current, 0, 0, path);
> > insert_node(node, *current);
> > }
> > @@ -211,10 +210,6 @@ int cil_parser(const char *_path, char *buffer, uint32_t size, struct cil_tree *
> > struct token tok;
> > int rc = SEPOL_OK;
> >
> > - CIL_KEY_HLL_LMS = cil_strpool_add("lms");
> > - CIL_KEY_HLL_LMX = cil_strpool_add("lmx");
> > - CIL_KEY_HLL_LME = cil_strpool_add("lme");
> > -
> > cil_stack_init(&stack);
> >
> > cil_lexer_setup(buffer, size);
> > diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c
> > index 4cf8dcc8..52b28999 100644
> > --- a/libsepol/cil/src/cil_tree.c
> > +++ b/libsepol/cil/src/cil_tree.c
> > @@ -71,7 +71,7 @@ struct cil_tree_node *cil_tree_get_next_path(struct cil_tree_node *node, char **
> > /* AST */
> > struct cil_src_info *info = node->data;
> > *path = info->path;
> > - *is_cil = info->is_cil;
> > + *is_cil = (info->kind == CIL_KEY_SRC_CIL);
> > return node;
> > } else {
> > if (node->flavor == CIL_CALL) {
> > --
> > 2.31.1
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-08-16 14:44 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10 18:05 [PATCH 0/8] libsepol/cil: Line mark cleanup and fix James Carter
2021-08-10 18:05 ` [PATCH 1/8] libsepol/cil: Check syntax of src_info statement James Carter
2021-08-10 18:05 ` [PATCH 2/8] libsepol/cil: Check the token type after getting the next token James Carter
2021-08-10 18:05 ` [PATCH 3/8] libsepol/cil: Check for valid line mark type immediately James Carter
2021-08-10 18:05 ` [PATCH 4/8] libsepol/cil: Push line mark state first when processing a line mark James Carter
2021-08-10 18:05 ` [PATCH 5/8] libsepol/cil: Create common string-to-unsigned-integer functions James Carter
2021-08-10 18:05 ` [PATCH 6/8] libsepol/cil: Add line mark kind and line number to src info James Carter
2021-08-16 9:38 ` Nicolas Iooss
2021-08-16 14:44 ` James Carter
2021-08-10 18:05 ` [PATCH 7/8] libsepol/cil: Report correct high-level language line numbers James Carter
2021-08-16 9:23 ` Nicolas Iooss
2021-08-16 14:40 ` James Carter
2021-08-10 18:05 ` [PATCH 8/8] libsepol/cil: When writing AST use line marks for src_info nodes James Carter
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.