Provide more descriptive error messages by including the identifier
or other kind of value if available.
Also drop duplicate newlines at the end of messages.
Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
---
checkpolicy/module_compiler.c | 6 +-
checkpolicy/policy_define.c | 123 ++++++++++++++++++++--------------
2 files changed, 76 insertions(+), 53 deletions(-)
diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
index 74a9f93c..119b7e36 100644
--- a/checkpolicy/module_compiler.c
+++ b/checkpolicy/module_compiler.c
@@ -75,7 +75,7 @@ static void print_error_msg(int ret, uint32_t symbol_type)
yyerror2("Could not declare %s here", flavor_str[symbol_type]);
break;
default:
- yyerror("Unknown error");
+ yyerror2("Unknown error %d", ret);
}
}
@@ -86,7 +86,7 @@ int define_policy(int pass, int module_header_given)
if (module_header_given) {
if (policydbp->policy_type != POLICY_MOD) {
yyerror
- ("Module specification found while not building a policy module.\n");
+ ("Module specification found while not building a policy module.");
return -1;
}
@@ -111,7 +111,7 @@ int define_policy(int pass, int module_header_given)
} else {
if (policydbp->policy_type == POLICY_MOD) {
yyerror
- ("Building a policy module, but no module specification found.\n");
+ ("Building a policy module, but no module specification found.");
return -1;
}
}
diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 360cff68..cd49cae3 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -96,6 +96,17 @@ void yyerror2(const char *fmt, ...)
va_end(ap);
}
+__attribute__ ((format(printf, 1, 2)))
+static void yywarn2(const char *fmt, ...)
+{
+ char warnmsg[256];
+ va_list ap;
+ va_start(ap, fmt);
+ vsnprintf(warnmsg, sizeof(warnmsg), fmt, ap);
+ yywarn(warnmsg);
+ va_end(ap);
+}
+
int insert_separator(int push)
{
int error;
@@ -233,7 +244,7 @@ int define_permissive(void)
}
if (t->flavor == TYPE_ATTRIB) {
- yyerror2("attributes may not be permissive: %s\n", type);
+ yyerror2("attributes may not be permissive: %s", type);
rc = -1;
goto out;
}
@@ -520,7 +531,7 @@ int define_common_perms(void)
}
comdatum = hashtab_search(policydbp->p_commons.table, id);
if (comdatum) {
- yyerror2("duplicate declaration for common %s\n", id);
+ yyerror2("duplicate declaration for common %s", id);
free(id);
return -1;
}
@@ -557,8 +568,8 @@ int define_common_perms(void)
perdatum->s.value = comdatum->permissions.nprim + 1;
if (perdatum->s.value > (sizeof(sepol_access_vector_t) * 8)) {
- yyerror
- ("too many permissions to fit in an access vector");
+ yyerror2
+ ("too many permissions (%d) to fit in an access vector", perdatum->s.value);
goto bad_perm;
}
ret = hashtab_insert(comdatum->permissions.table,
@@ -619,12 +630,15 @@ int define_av_perms(int inherits)
yyerror2("class %s is not defined", id);
goto bad;
}
- free(id);
if (cladatum->comdatum || cladatum->permissions.nprim) {
- yyerror("duplicate access vector definition");
- return -1;
+ yyerror2("duplicate access vector definition for class %s", id);
+ goto bad;
}
+
+ free(id);
+ id = NULL;
+
if (symtab_init(&cladatum->permissions, PERM_SYMTAB_SIZE)) {
yyerror("out of memory");
return -1;
@@ -664,8 +678,8 @@ int define_av_perms(int inherits)
perdatum->s.value = ++cladatum->permissions.nprim;
if (perdatum->s.value > (sizeof(sepol_access_vector_t) * 8)) {
- yyerror
- ("too many permissions to fit in an access vector");
+ yyerror2
+ ("too many permissions (%d) to fit in an access vector", perdatum->s.value);
goto bad;
}
if (inherits) {
@@ -737,7 +751,7 @@ int define_sens(void)
return -1;
}
if (id_has_dot(id)) {
- yyerror("sensitivity identifiers may not contain periods");
+ yyerror2("sensitivity identifier %s may not contain periods", id);
goto bad;
}
level = (mls_level_t *) malloc(sizeof(mls_level_t));
@@ -766,11 +780,11 @@ int define_sens(void)
goto bad;
}
case -2:{
- yyerror("duplicate declaration of sensitivity level");
+ yyerror2("duplicate declaration of sensitivity level %s", id);
goto bad;
}
case -1:{
- yyerror("could not declare sensitivity level here");
+ yyerror2("could not declare sensitivity level %s here", id);
goto bad;
}
case 0:
@@ -784,7 +798,7 @@ int define_sens(void)
while ((id = queue_remove(id_queue))) {
if (id_has_dot(id)) {
- yyerror("sensitivity aliases may not contain periods");
+ yyerror2("sensitivity alias %s may not contain periods", id);
free(id);
return -1;
}
@@ -806,13 +820,13 @@ int define_sens(void)
goto bad_alias;
}
case -2:{
- yyerror
- ("duplicate declaration of sensitivity alias");
+ yyerror2
+ ("duplicate declaration of sensitivity alias %s", id);
goto bad_alias;
}
case -1:{
- yyerror
- ("could not declare sensitivity alias here");
+ yyerror2
+ ("could not declare sensitivity alias %s here", id);
goto bad_alias;
}
case 0:
@@ -920,7 +934,7 @@ int define_category(void)
return -1;
}
if (id_has_dot(id)) {
- yyerror("category identifiers may not contain periods");
+ yyerror2("category identifier %s may not contain periods", id);
goto bad;
}
datum = (cat_datum_t *) malloc(sizeof(cat_datum_t));
@@ -938,11 +952,11 @@ int define_category(void)
goto bad;
}
case -2:{
- yyerror("duplicate declaration of category");
+ yyerror2("duplicate declaration of category %s", id);
goto bad;
}
case -1:{
- yyerror("could not declare category here");
+ yyerror2("could not declare category %s here", id);
goto bad;
}
case 0:
@@ -957,7 +971,7 @@ int define_category(void)
while ((id = queue_remove(id_queue))) {
if (id_has_dot(id)) {
- yyerror("category aliases may not contain periods");
+ yyerror2("category alias %s may not contain periods", id);
free(id);
return -1;
}
@@ -980,13 +994,13 @@ int define_category(void)
goto bad_alias;
}
case -2:{
- yyerror
- ("duplicate declaration of category aliases");
+ yyerror2
+ ("duplicate declaration of category alias %s", id);
goto bad_alias;
}
case -1:{
- yyerror
- ("could not declare category aliases here");
+ yyerror2
+ ("could not declare category alias %s here", id);
goto bad_alias;
}
case 0:
@@ -1114,7 +1128,7 @@ int define_level(void)
range_end = cdatum->s.value - 1;
if (range_end < range_start) {
- yyerror2("category range is invalid");
+ yyerror2("category range %d-%d is invalid", range_start, range_end);
free(id);
return -1;
}
@@ -1170,6 +1184,7 @@ int expand_attrib(void)
ebitmap_t attrs;
type_datum_t *attr;
ebitmap_node_t *node;
+ const char *name;
uint32_t i;
int rc = -1;
int flags = 0;
@@ -1222,13 +1237,13 @@ int expand_attrib(void)
}
ebitmap_for_each_positive_bit(&attrs, node, i) {
- attr = hashtab_search(policydbp->p_types.table,
- policydbp->sym_val_to_name[SYM_TYPES][i]);
+ name = policydbp->sym_val_to_name[SYM_TYPES][i];
+ attr = hashtab_search(policydbp->p_types.table, name);
attr->flags |= flags;
if ((attr->flags & TYPE_FLAGS_EXPAND_ATTR_TRUE) &&
(attr->flags & TYPE_FLAGS_EXPAND_ATTR_FALSE)) {
- yywarn("Expandattribute option was set to both true and false. "
- "Resolving to false.");
+ yywarn2("Expandattribute option of attribute %s was set to both true and false; "
+ "Resolving to false.", name);
attr->flags &= ~TYPE_FLAGS_EXPAND_ATTR_TRUE;
}
}
@@ -1247,9 +1262,9 @@ static int add_aliases_to_type(type_datum_t * type)
int ret;
while ((id = queue_remove(id_queue))) {
if (id_has_dot(id)) {
+ yyerror2
+ ("type alias identifier %s may not contain periods", id);
free(id);
- yyerror
- ("type alias identifiers may not contain periods");
return -1;
}
aliasdatum = (type_datum_t *) malloc(sizeof(type_datum_t));
@@ -1274,7 +1289,7 @@ static int add_aliases_to_type(type_datum_t * type)
goto cleanup;
}
case -1:{
- yyerror("could not declare alias here");
+ yyerror2("could not declare alias %s here", id);
goto cleanup;
}
case 0: break;
@@ -1790,8 +1805,8 @@ int define_bool_tunable(int is_tunable)
return -1;
}
if (id_has_dot(id)) {
+ yyerror2("boolean identifier %s may not contain periods", id);
free(id);
- yyerror("boolean identifiers may not contain periods");
return -1;
}
datum = (cond_bool_datum_t *) malloc(sizeof(cond_bool_datum_t));
@@ -1814,7 +1829,7 @@ int define_bool_tunable(int is_tunable)
goto cleanup;
}
case -1:{
- yyerror("could not declare boolean here");
+ yyerror2("could not declare boolean %s here", id);
goto cleanup;
}
case 0:
@@ -1957,7 +1972,8 @@ static int avrule_read_ioctls(struct av_ioctl_range_list **rangehead)
id = queue_remove(id_queue);
r->range.high = (uint16_t) strtoul(id,NULL,0);
if (r->range.high < r->range.low) {
- yyerror("Ioctl ranges must be in ascending order.");
+ yyerror2("Ioctl range %d-%d must be in ascending order.",
+ r->range.low, r->range.high);
return -1;
}
free(id);
@@ -2532,7 +2548,7 @@ int define_te_avtab_extended_perms(int which)
if (strcmp(id,"ioctl") == 0) {
rc = define_te_avtab_ioctl(avrule_template);
} else {
- yyerror("only ioctl extended permissions are supported");
+ yyerror2("only ioctl extended permissions are supported, found %s", id);
rc = -1;
}
@@ -3189,7 +3205,7 @@ int define_role_allow(void)
avrule_t *define_cond_filename_trans(void)
{
yyerror("type transitions with a filename not allowed inside "
- "conditionals\n");
+ "conditionals");
return COND_ERR;
}
@@ -3861,7 +3877,7 @@ int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
f = 0;
expr = define_cond_expr(COND_NOT, expr, 0);
if (!expr) {
- yyerror("unable to invert");
+ yyerror("unable to invert conditional expression");
return -1;
}
}
@@ -4126,7 +4142,7 @@ static int parse_categories(char *id, level_datum_t * levdatum, ebitmap_t * cats
range_end = cdatum->s.value - 1;
if (range_end < range_start) {
- yyerror2("category range is invalid");
+ yyerror2("category range %d-%d is invalid", range_start, range_end);
return -1;
}
} else {
@@ -5102,7 +5118,7 @@ int define_ibendport_context(unsigned int port)
}
if (port > 0xff || port == 0) {
- yyerror("Invalid ibendport port number, should be 0 < port < 256");
+ yyerror2("Invalid ibendport port number %d, should be 0 < port < 256", port);
return -1;
}
@@ -5121,7 +5137,7 @@ int define_ibendport_context(unsigned int port)
}
if (strlen(newc->u.ibendport.dev_name) > IB_DEVICE_NAME_MAX - 1) {
- yyerror("infiniband device name exceeds max length of 63.");
+ yyerror2("infiniband device name %s exceeds max length of 63.", newc->u.ibendport.dev_name);
rc = -1;
goto out;
}
@@ -5248,13 +5264,14 @@ int define_ipv4_node_context(void)
}
rc = inet_pton(AF_INET, id, &addr);
- free(id);
if (rc < 1) {
- yyerror("failed to parse ipv4 address");
+ yyerror2("failed to parse ipv4 address %s", id);
+ free(id);
if (rc == 0)
rc = -1;
goto out;
}
+ free(id);
id = queue_remove(id_queue);
if (!id) {
@@ -5264,14 +5281,16 @@ int define_ipv4_node_context(void)
}
rc = inet_pton(AF_INET, id, &mask);
- free(id);
if (rc < 1) {
- yyerror("failed to parse ipv4 mask");
+ yyerror2("failed to parse ipv4 mask %s", id);
+ free(id);
if (rc == 0)
rc = -1;
goto out;
}
+ free(id);
+
if (mask.s_addr != 0 && ((~mask.s_addr + 1) & ~mask.s_addr) != 0) {
yywarn("ipv4 mask is not contiguous");
}
@@ -5376,14 +5395,16 @@ int define_ipv6_node_context(void)
}
rc = inet_pton(AF_INET6, id, &addr);
- free(id);
if (rc < 1) {
- yyerror("failed to parse ipv6 address");
+ yyerror2("failed to parse ipv6 address %s", id);
+ free(id);
if (rc == 0)
rc = -1;
goto out;
}
+ free(id);
+
id = queue_remove(id_queue);
if (!id) {
yyerror("failed to read ipv6 address");
@@ -5392,14 +5413,16 @@ int define_ipv6_node_context(void)
}
rc = inet_pton(AF_INET6, id, &mask);
- free(id);
if (rc < 1) {
- yyerror("failed to parse ipv6 mask");
+ yyerror2("failed to parse ipv6 mask %s", id);
+ free(id);
if (rc == 0)
rc = -1;
goto out;
}
+ free(id);
+
if (!ipv6_is_mask_contiguous(&mask)) {
yywarn("ipv6 mask is not contiguous");
}
--
2.43.0
