* [PATCH] libsepol/cil: fix compilation with -Wformat-security
@ 2014-09-11 21:05 Nicolas Iooss
2014-09-12 13:18 ` James Carter
0 siblings, 1 reply; 2+ messages in thread
From: Nicolas Iooss @ 2014-09-11 21:05 UTC (permalink / raw)
To: selinux
Once a printf format attribute is added to cil_log, compiling with gcc
-Wformat -Wformat-security -Werror fails with two kind of errors:
* format '%s' expects argument of type 'char *', but argument 3 has type 'void *'
* format '%n' expects argument of type 'int *', but argument 4 has type 'uint32_t'
The second one is quite surprising and it seems %n has been used instead
of %u several times in cil_resolve_ast.c.
Fix these printf-format-check errors.
---
libsepol/cil/include/cil/cil.h | 4 ++++
libsepol/cil/src/cil_build_ast.c | 9 +++++----
libsepol/cil/src/cil_log.h | 4 +---
libsepol/cil/src/cil_resolve_ast.c | 12 ++++++------
libsepol/cil/src/cil_verify.c | 10 +++++-----
5 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/libsepol/cil/include/cil/cil.h b/libsepol/cil/include/cil/cil.h
index 00047a48cbea..902ca3e87b01 100644
--- a/libsepol/cil/include/cil/cil.h
+++ b/libsepol/cil/include/cil/cil.h
@@ -58,6 +58,10 @@ enum cil_log_level {
};
extern void cil_set_log_level(enum cil_log_level lvl);
extern void cil_set_log_handler(void (*handler)(int lvl, char *msg));
+
+#ifdef __GNUC__
+__attribute__ ((format(printf, 2, 3)))
+#endif
extern void cil_log(enum cil_log_level lvl, const char *msg, ...);
extern void cil_set_malloc_error_handler(void (*handler)(void));
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index 945b0a8bf306..3a38559b7c0d 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -2271,7 +2271,7 @@ static int __cil_fill_expr(struct cil_tree_node *current, enum cil_flavor flavor
if (current->cl_head == NULL) {
enum cil_flavor op = __cil_get_expr_operator_flavor(current->data);
if (op != CIL_NONE) {
- cil_log(CIL_ERR,"Operator (%s) not in an expression\n", current->data);
+ cil_log(CIL_ERR, "Operator (%s) not in an expression\n", (char*)current->data);
goto exit;
}
cil_list_append(expr, CIL_STRING, current->data);
@@ -2378,7 +2378,7 @@ static int __cil_fill_constraint_leaf_expr(struct cil_tree_node *current, enum c
leaf_expr_flavor = CIL_LEVEL;
break;
default:
- cil_log(CIL_ERR,"Invalid left operand (%s)\n",current->next->data);
+ cil_log(CIL_ERR, "Invalid left operand (%s)\n", (char*)current->next->data);
goto exit;
}
@@ -2705,7 +2705,7 @@ int cil_gen_condblock(struct cil_db *db, struct cil_tree_node *parse_current, st
exit:
cil_log(CIL_ERR, "Bad %s condition declaration at line %d of %s\n",
- parse_current->data, parse_current->line, parse_current->path);
+ (char*)parse_current->data, parse_current->line, parse_current->path);
cil_destroy_condblock(cb);
return rc;
}
@@ -2765,7 +2765,8 @@ int cil_gen_alias(struct cil_db *db, struct cil_tree_node *parse_current, struct
return SEPOL_OK;
exit:
- cil_log(CIL_ERR, "Bad %s declaration at line %d of %s\n", parse_current->data, parse_current->line, parse_current->path);
+ cil_log(CIL_ERR, "Bad %s declaration at line %d of %s\n",
+ (char*)parse_current->data, parse_current->line, parse_current->path);
cil_destroy_alias(alias);
cil_clear_node(ast_node);
return rc;
diff --git a/libsepol/cil/src/cil_log.h b/libsepol/cil/src/cil_log.h
index 46646c5b329b..9c2ff2e83ebc 100644
--- a/libsepol/cil/src/cil_log.h
+++ b/libsepol/cil/src/cil_log.h
@@ -34,8 +34,6 @@
#define MAX_LOG_SIZE 512
-
-
-void cil_log(enum cil_log_level lvl, const char *msg, ...);
+__attribute__ ((format(printf, 2, 3))) void cil_log(enum cil_log_level lvl, const char *msg, ...);
#endif // CIL_LOG_H_
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index 3b79e90f478b..a36b23808581 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -128,7 +128,7 @@ static int __cil_resolve_perms(symtab_t *class_symtab, symtab_t *common_symtab,
}
}
if (rc != SEPOL_OK) {
- cil_log(CIL_ERR, "Failed to resolve permission %s\n", curr->data);
+ cil_log(CIL_ERR, "Failed to resolve permission %s\n", (char*)curr->data);
goto exit;
}
cil_list_append(*perm_datums, CIL_DATUM, perm_datum);
@@ -2210,7 +2210,7 @@ int cil_resolve_bounds(struct cil_tree_node *current, void *extra_args, enum cil
if (user->bounds != NULL) {
struct cil_tree_node *node = user->bounds->datum.nodes->head->data;
- cil_log(CIL_ERR, "User %s already bound by parent at line %n of %s\n", bounds->child_str, node->line, node->path);
+ cil_log(CIL_ERR, "User %s already bound by parent at line %u of %s\n", bounds->child_str, node->line, node->path);
rc = SEPOL_ERR;
goto exit;
}
@@ -2223,7 +2223,7 @@ int cil_resolve_bounds(struct cil_tree_node *current, void *extra_args, enum cil
if (role->bounds != NULL) {
struct cil_tree_node *node = role->bounds->datum.nodes->head->data;
- cil_log(CIL_ERR, "Role %s already bound by parent at line %n of %s\n", bounds->child_str, node->line, node->path);
+ cil_log(CIL_ERR, "Role %s already bound by parent at line %u of %s\n", bounds->child_str, node->line, node->path);
rc = SEPOL_ERR;
goto exit;
}
@@ -2237,8 +2237,8 @@ int cil_resolve_bounds(struct cil_tree_node *current, void *extra_args, enum cil
if (type->bounds != NULL) {
node = ((struct cil_symtab_datum *)type->bounds)->nodes->head->data;
- cil_log(CIL_ERR, "Type %s already bound by parent at line %n of %s\n", bounds->child_str, node->line, node->path);
- cil_log(CIL_ERR, "Now being bound to parent %s at line %n of %s\n", bounds->parent_str, current->line, current->path);
+ cil_log(CIL_ERR, "Type %s already bound by parent at line %u of %s\n", bounds->child_str, node->line, node->path);
+ cil_log(CIL_ERR, "Now being bound to parent %s at line %u of %s\n", bounds->parent_str, current->line, current->path);
rc = SEPOL_ERR;
goto exit;
}
@@ -2267,7 +2267,7 @@ int cil_resolve_bounds(struct cil_tree_node *current, void *extra_args, enum cil
return SEPOL_OK;
exit:
- cil_log(CIL_ERR, "Bad bounds statement at line %n of %s\n", current->line, current->path);
+ cil_log(CIL_ERR, "Bad bounds statement at line %u of %s\n", current->line, current->path);
return rc;
}
diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
index 995496c5e757..a1576e761f1f 100644
--- a/libsepol/cil/src/cil_verify.c
+++ b/libsepol/cil/src/cil_verify.c
@@ -166,13 +166,13 @@ int cil_verify_expr_syntax(struct cil_tree_node *current, enum cil_flavor op, en
case CIL_EQ:
case CIL_NEQ:
if (expr_flavor != CIL_BOOL && expr_flavor != CIL_TUNABLE ) {
- cil_log(CIL_ERR,"Invalid operator (%s) for set expression\n", current->data);
+ cil_log(CIL_ERR,"Invalid operator (%s) for set expression\n", (char*)current->data);
goto exit;
}
break;
case CIL_ALL:
if (expr_flavor == CIL_BOOL || expr_flavor == CIL_TUNABLE) {
- cil_log(CIL_ERR,"Invalid operator (%s) for boolean or tunable expression\n", current->data);
+ cil_log(CIL_ERR,"Invalid operator (%s) for boolean or tunable expression\n", (char*)current->data);
goto exit;
}
syntax[1] = CIL_SYN_END;
@@ -180,7 +180,7 @@ int cil_verify_expr_syntax(struct cil_tree_node *current, enum cil_flavor op, en
break;
case CIL_RANGE:
if (expr_flavor != CIL_CAT) {
- cil_log(CIL_ERR,"Operator (%s) only valid for catset expression\n", current->data);
+ cil_log(CIL_ERR,"Operator (%s) only valid for catset expression\n", (char*)current->data);
goto exit;
}
syntax[1] = CIL_SYN_STRING;
@@ -192,7 +192,7 @@ int cil_verify_expr_syntax(struct cil_tree_node *current, enum cil_flavor op, en
syntax_len = 2;
break;
default:
- cil_log(CIL_ERR,"Unexpected value (%s) for expression operator\n", current->data);
+ cil_log(CIL_ERR,"Unexpected value (%s) for expression operator\n", (char*)current->data);
goto exit;
}
@@ -298,7 +298,7 @@ int cil_verify_constraint_expr_syntax(struct cil_tree_node *current, enum cil_fl
syntax[2] = CIL_SYN_STRING;
break;
default:
- cil_log(CIL_ERR,"Invalid operator (%s) for constraint expression\n",current->data);
+ cil_log(CIL_ERR, "Invalid operator (%s) for constraint expression\n", (char*)current->data);
goto exit;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] libsepol/cil: fix compilation with -Wformat-security
2014-09-11 21:05 [PATCH] libsepol/cil: fix compilation with -Wformat-security Nicolas Iooss
@ 2014-09-12 13:18 ` James Carter
0 siblings, 0 replies; 2+ messages in thread
From: James Carter @ 2014-09-12 13:18 UTC (permalink / raw)
To: Nicolas Iooss, selinux
Thanks for the patch. I've applied it.
Jim
On 09/11/2014 05:05 PM, Nicolas Iooss wrote:
> Once a printf format attribute is added to cil_log, compiling with gcc
> -Wformat -Wformat-security -Werror fails with two kind of errors:
>
> * format '%s' expects argument of type 'char *', but argument 3 has type 'void *'
> * format '%n' expects argument of type 'int *', but argument 4 has type 'uint32_t'
>
> The second one is quite surprising and it seems %n has been used instead
> of %u several times in cil_resolve_ast.c.
>
> Fix these printf-format-check errors.
> ---
> libsepol/cil/include/cil/cil.h | 4 ++++
> libsepol/cil/src/cil_build_ast.c | 9 +++++----
> libsepol/cil/src/cil_log.h | 4 +---
> libsepol/cil/src/cil_resolve_ast.c | 12 ++++++------
> libsepol/cil/src/cil_verify.c | 10 +++++-----
> 5 files changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/libsepol/cil/include/cil/cil.h b/libsepol/cil/include/cil/cil.h
> index 00047a48cbea..902ca3e87b01 100644
> --- a/libsepol/cil/include/cil/cil.h
> +++ b/libsepol/cil/include/cil/cil.h
> @@ -58,6 +58,10 @@ enum cil_log_level {
> };
> extern void cil_set_log_level(enum cil_log_level lvl);
> extern void cil_set_log_handler(void (*handler)(int lvl, char *msg));
> +
> +#ifdef __GNUC__
> +__attribute__ ((format(printf, 2, 3)))
> +#endif
> extern void cil_log(enum cil_log_level lvl, const char *msg, ...);
>
> extern void cil_set_malloc_error_handler(void (*handler)(void));
> diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
> index 945b0a8bf306..3a38559b7c0d 100644
> --- a/libsepol/cil/src/cil_build_ast.c
> +++ b/libsepol/cil/src/cil_build_ast.c
> @@ -2271,7 +2271,7 @@ static int __cil_fill_expr(struct cil_tree_node *current, enum cil_flavor flavor
> if (current->cl_head == NULL) {
> enum cil_flavor op = __cil_get_expr_operator_flavor(current->data);
> if (op != CIL_NONE) {
> - cil_log(CIL_ERR,"Operator (%s) not in an expression\n", current->data);
> + cil_log(CIL_ERR, "Operator (%s) not in an expression\n", (char*)current->data);
> goto exit;
> }
> cil_list_append(expr, CIL_STRING, current->data);
> @@ -2378,7 +2378,7 @@ static int __cil_fill_constraint_leaf_expr(struct cil_tree_node *current, enum c
> leaf_expr_flavor = CIL_LEVEL;
> break;
> default:
> - cil_log(CIL_ERR,"Invalid left operand (%s)\n",current->next->data);
> + cil_log(CIL_ERR, "Invalid left operand (%s)\n", (char*)current->next->data);
> goto exit;
> }
>
> @@ -2705,7 +2705,7 @@ int cil_gen_condblock(struct cil_db *db, struct cil_tree_node *parse_current, st
>
> exit:
> cil_log(CIL_ERR, "Bad %s condition declaration at line %d of %s\n",
> - parse_current->data, parse_current->line, parse_current->path);
> + (char*)parse_current->data, parse_current->line, parse_current->path);
> cil_destroy_condblock(cb);
> return rc;
> }
> @@ -2765,7 +2765,8 @@ int cil_gen_alias(struct cil_db *db, struct cil_tree_node *parse_current, struct
> return SEPOL_OK;
>
> exit:
> - cil_log(CIL_ERR, "Bad %s declaration at line %d of %s\n", parse_current->data, parse_current->line, parse_current->path);
> + cil_log(CIL_ERR, "Bad %s declaration at line %d of %s\n",
> + (char*)parse_current->data, parse_current->line, parse_current->path);
> cil_destroy_alias(alias);
> cil_clear_node(ast_node);
> return rc;
> diff --git a/libsepol/cil/src/cil_log.h b/libsepol/cil/src/cil_log.h
> index 46646c5b329b..9c2ff2e83ebc 100644
> --- a/libsepol/cil/src/cil_log.h
> +++ b/libsepol/cil/src/cil_log.h
> @@ -34,8 +34,6 @@
>
> #define MAX_LOG_SIZE 512
>
> -
> -
> -void cil_log(enum cil_log_level lvl, const char *msg, ...);
> +__attribute__ ((format(printf, 2, 3))) void cil_log(enum cil_log_level lvl, const char *msg, ...);
>
> #endif // CIL_LOG_H_
> diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
> index 3b79e90f478b..a36b23808581 100644
> --- a/libsepol/cil/src/cil_resolve_ast.c
> +++ b/libsepol/cil/src/cil_resolve_ast.c
> @@ -128,7 +128,7 @@ static int __cil_resolve_perms(symtab_t *class_symtab, symtab_t *common_symtab,
> }
> }
> if (rc != SEPOL_OK) {
> - cil_log(CIL_ERR, "Failed to resolve permission %s\n", curr->data);
> + cil_log(CIL_ERR, "Failed to resolve permission %s\n", (char*)curr->data);
> goto exit;
> }
> cil_list_append(*perm_datums, CIL_DATUM, perm_datum);
> @@ -2210,7 +2210,7 @@ int cil_resolve_bounds(struct cil_tree_node *current, void *extra_args, enum cil
>
> if (user->bounds != NULL) {
> struct cil_tree_node *node = user->bounds->datum.nodes->head->data;
> - cil_log(CIL_ERR, "User %s already bound by parent at line %n of %s\n", bounds->child_str, node->line, node->path);
> + cil_log(CIL_ERR, "User %s already bound by parent at line %u of %s\n", bounds->child_str, node->line, node->path);
> rc = SEPOL_ERR;
> goto exit;
> }
> @@ -2223,7 +2223,7 @@ int cil_resolve_bounds(struct cil_tree_node *current, void *extra_args, enum cil
>
> if (role->bounds != NULL) {
> struct cil_tree_node *node = role->bounds->datum.nodes->head->data;
> - cil_log(CIL_ERR, "Role %s already bound by parent at line %n of %s\n", bounds->child_str, node->line, node->path);
> + cil_log(CIL_ERR, "Role %s already bound by parent at line %u of %s\n", bounds->child_str, node->line, node->path);
> rc = SEPOL_ERR;
> goto exit;
> }
> @@ -2237,8 +2237,8 @@ int cil_resolve_bounds(struct cil_tree_node *current, void *extra_args, enum cil
>
> if (type->bounds != NULL) {
> node = ((struct cil_symtab_datum *)type->bounds)->nodes->head->data;
> - cil_log(CIL_ERR, "Type %s already bound by parent at line %n of %s\n", bounds->child_str, node->line, node->path);
> - cil_log(CIL_ERR, "Now being bound to parent %s at line %n of %s\n", bounds->parent_str, current->line, current->path);
> + cil_log(CIL_ERR, "Type %s already bound by parent at line %u of %s\n", bounds->child_str, node->line, node->path);
> + cil_log(CIL_ERR, "Now being bound to parent %s at line %u of %s\n", bounds->parent_str, current->line, current->path);
> rc = SEPOL_ERR;
> goto exit;
> }
> @@ -2267,7 +2267,7 @@ int cil_resolve_bounds(struct cil_tree_node *current, void *extra_args, enum cil
> return SEPOL_OK;
>
> exit:
> - cil_log(CIL_ERR, "Bad bounds statement at line %n of %s\n", current->line, current->path);
> + cil_log(CIL_ERR, "Bad bounds statement at line %u of %s\n", current->line, current->path);
> return rc;
> }
>
> diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
> index 995496c5e757..a1576e761f1f 100644
> --- a/libsepol/cil/src/cil_verify.c
> +++ b/libsepol/cil/src/cil_verify.c
> @@ -166,13 +166,13 @@ int cil_verify_expr_syntax(struct cil_tree_node *current, enum cil_flavor op, en
> case CIL_EQ:
> case CIL_NEQ:
> if (expr_flavor != CIL_BOOL && expr_flavor != CIL_TUNABLE ) {
> - cil_log(CIL_ERR,"Invalid operator (%s) for set expression\n", current->data);
> + cil_log(CIL_ERR,"Invalid operator (%s) for set expression\n", (char*)current->data);
> goto exit;
> }
> break;
> case CIL_ALL:
> if (expr_flavor == CIL_BOOL || expr_flavor == CIL_TUNABLE) {
> - cil_log(CIL_ERR,"Invalid operator (%s) for boolean or tunable expression\n", current->data);
> + cil_log(CIL_ERR,"Invalid operator (%s) for boolean or tunable expression\n", (char*)current->data);
> goto exit;
> }
> syntax[1] = CIL_SYN_END;
> @@ -180,7 +180,7 @@ int cil_verify_expr_syntax(struct cil_tree_node *current, enum cil_flavor op, en
> break;
> case CIL_RANGE:
> if (expr_flavor != CIL_CAT) {
> - cil_log(CIL_ERR,"Operator (%s) only valid for catset expression\n", current->data);
> + cil_log(CIL_ERR,"Operator (%s) only valid for catset expression\n", (char*)current->data);
> goto exit;
> }
> syntax[1] = CIL_SYN_STRING;
> @@ -192,7 +192,7 @@ int cil_verify_expr_syntax(struct cil_tree_node *current, enum cil_flavor op, en
> syntax_len = 2;
> break;
> default:
> - cil_log(CIL_ERR,"Unexpected value (%s) for expression operator\n", current->data);
> + cil_log(CIL_ERR,"Unexpected value (%s) for expression operator\n", (char*)current->data);
> goto exit;
> }
>
> @@ -298,7 +298,7 @@ int cil_verify_constraint_expr_syntax(struct cil_tree_node *current, enum cil_fl
> syntax[2] = CIL_SYN_STRING;
> break;
> default:
> - cil_log(CIL_ERR,"Invalid operator (%s) for constraint expression\n",current->data);
> + cil_log(CIL_ERR, "Invalid operator (%s) for constraint expression\n", (char*)current->data);
> goto exit;
> }
>
>
--
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-09-12 13:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-11 21:05 [PATCH] libsepol/cil: fix compilation with -Wformat-security Nicolas Iooss
2014-09-12 13:18 ` 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.