[PATCH 12/15] checkpolicy: provide more descriptive error messages

["Christian Göttsche" <cgzones@googlemail.com>]

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@googlemail.com>
---
 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