On Tue, May 19, 2020 at 10:30 AM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> CIL allows a type to be redeclared when using the multiple declarations
> option ("-m" or "--muliple-decls"), but make it an error for an identifier
> to be declared as both a type and an attribute.
>
> Change the error message so that it always gives the location and flavor
> of both declarations. The flavors will be the same in all other cases,
> but in this case they explain why there is an error even if multiple
> declartions are allowed.
>
> Issue reported by: Topi Miettinen <toiwoton@xxxxxxxxx>
Normally just Reported-by:...
> Fixes: Commit fafe4c212bf6c32c ("libsepol: cil: Add ability to redeclare
> types[attributes]")
Normally just "Fixes: <hash> ("one-liner")", no "Commit".
> Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
> ---
> libsepol/cil/src/cil_build_ast.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> index fcecdc4f..ce2499a1 100644
> --- a/libsepol/cil/src/cil_build_ast.c
> +++ b/libsepol/cil/src/cil_build_ast.c
> @@ -87,7 +87,7 @@ exit:
> * datum, given the new datum and the one already present in a given symtab.
> */
> int cil_is_datum_multiple_decl(__attribute__((unused)) struct cil_symtab_datum *cur,
> - __attribute__((unused)) struct cil_symtab_datum *old,
> + struct cil_symtab_datum *old,
> enum cil_flavor f)
> {
> int rc = CIL_FALSE;
> @@ -95,8 +95,12 @@ int cil_is_datum_multiple_decl(__attribute__((unused)) struct cil_symtab_datum *
> switch (f) {
> case CIL_TYPE:
> case CIL_TYPEATTRIBUTE:
> - /* type and typeattribute statements insert empty datums, ret true */
> - rc = CIL_TRUE;
> + if (!old || f != FLAVOR(old)) {
> + rc = CIL_FALSE;
> + } else {
> + /* type and typeattribute statements insert empty datums */
> + rc = CIL_TRUE;
> + }
> break;
> default:
> break;
> @@ -126,19 +130,20 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
> if (symtab != NULL) {
> rc = cil_symtab_insert(symtab, (hashtab_key_t)key, datum, ast_node);
> if (rc == SEPOL_EEXIST) {
> + rc = cil_symtab_get_datum(symtab, (hashtab_key_t)key, &prev);
> + if (rc != SEPOL_OK) {
> + cil_log(CIL_ERR, "Re-declaration of %s %s, but previous declaration could not be found\n",cil_node_to_string(ast_node), key);
> + goto exit;
> + }
> if (!db->multiple_decls ||
> - cil_symtab_get_datum(symtab, (hashtab_key_t)key, &prev) != SEPOL_OK ||
> !cil_is_datum_multiple_decl(datum, prev, nflavor)) {
> -
> /* multiple_decls not ok, ret error */
> + struct cil_tree_node *node = NODE(prev);
> cil_log(CIL_ERR, "Re-declaration of %s %s\n",
> cil_node_to_string(ast_node), key);
> - if (cil_symtab_get_datum(symtab, key, &datum) == SEPOL_OK) {
> - if (sflavor == CIL_SYM_BLOCKS) {
> - struct cil_tree_node *node = datum->nodes->head->data;
> - cil_tree_log(node, CIL_ERR, "Previous declaration");
> - }
> - }
> + cil_tree_log(node, CIL_ERR, "Previous declaration of %s",
> + cil_node_to_string(node));
> + rc = SEPOL_ERR;
> goto exit;
> }
> /* multiple_decls is enabled and works for this datum type, add node */
> @@ -169,7 +174,7 @@ int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_s
> return SEPOL_OK;
>
> exit:
> - cil_log(CIL_ERR, "Failed to create node\n");
> + cil_log(CIL_INFO, "Failed to create node\n");
Is this useful/meaningful to retain? Seems odd to have an
informational message about a failure to create a node.
