apparmor: group dfa policydb unpacking

There are currently three policydb rule groupings (xmatch, file,
policydb) that each do their own slightly different thing. Group them
into a single routine and unify.

This extends/unifies dfa features by
- all dfas are allowed having an optional start field
- all dfas are allowed having a string/transition table

Signed-off-by: John Johansen <john.johansen@canonical.com>

security/apparmor/policy_unpack.c

@@ -648,6 +648,54 @@ fail:
 	return false;
 }
 
+static int unpack_pdb(struct aa_ext *e, struct aa_policydb *policy,
+		      bool required_dfa, bool required_trans,
+		      const char **info)
+{
+	int i;
+
+	policy->dfa = unpack_dfa(e);
+	if (IS_ERR(policy->dfa)) {
+		int error = PTR_ERR(policy->dfa);
+
+		policy->dfa = NULL;
+		*info = "failed to unpack - dfa";
+		return error;
+	} else if (!policy->dfa) {
+		if (required_dfa) {
+			*info = "missing required dfa";
+			return -EPROTO;
+		}
+		goto out;
+	}
+
+	/*
+	 * only unpack the following if a dfa is present
+	 *
+	 * sadly start was given different names for file and policydb
+	 * but since it is optional we can try both
+	 */
+	if (!unpack_u32(e, &policy->start[0], "start"))
+		/* default start state */
+		policy->start[0] = DFA_START;
+	if (!unpack_u32(e, &policy->start[AA_CLASS_FILE], "dfa_start")) {
+		/* default start state for xmatch and file dfa */
+		policy->start[AA_CLASS_FILE] = DFA_START;
+	}	/* setup class index */
+	for (i = AA_CLASS_FILE + 1; i <= AA_CLASS_LAST; i++) {
+		policy->start[i] = aa_dfa_next(policy->dfa, policy->start[0],
+					       i);
+	}
+	if (!unpack_trans_table(e, &policy->trans) && required_trans) {
+		*info = "failed to unpack profile transition table";
+		return -EPROTO;
+	}
+	/* TODO: move compat mapping here, requires dfa merging first */
+
+out:
+	return 0;
+}
+
 static u32 strhash(const void *data, u32 len, u32 seed)
 {
 	const char * const *key = data;
@@ -679,7 +727,7 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
 	struct rhashtable_params params = { 0 };
 	char *key = NULL;
 	struct aa_data *data;
-	int i, error = -EPROTO;
+	int error = -EPROTO;
 	kernel_cap_t tmpcap;
 	u32 tmp;
 
@@ -714,13 +762,10 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
 	(void) unpack_str(e, &profile->attach, "attach");
 
 	/* xmatch is optional and may be NULL */
-	profile->xmatch.dfa = unpack_dfa(e);
+	error = unpack_pdb(e, &profile->xmatch, false, false, &info);
-	if (IS_ERR(profile->xmatch.dfa)) {
+	if (error)
-		error = PTR_ERR(profile->xmatch.dfa);
-		profile->xmatch.dfa = NULL;
-		info = "bad xmatch";
 		goto fail;
-	}
+
 	/* neither xmatch_len not xmatch_perms are optional if xmatch is set */
 	if (profile->xmatch.dfa) {
 		if (!unpack_u32(e, &tmp, NULL)) {
@@ -838,25 +883,16 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
 	if (unpack_nameX(e, AA_STRUCT, "policydb")) {
 		/* generic policy dfa - optional and may be NULL */
 		info = "failed to unpack policydb";
-		profile->policy.dfa = unpack_dfa(e);
+		error = unpack_pdb(e, &profile->policy, true, false, &info);
-		if (IS_ERR(profile->policy.dfa)) {
+		if (error)
-			error = PTR_ERR(profile->policy.dfa);
-			profile->policy.dfa = NULL;
 			goto fail;
-		} else if (!profile->policy.dfa) {
+		/* Fixup: drop when we get rid of start array */
-			error = -EPROTO;
+		if (aa_dfa_next(profile->policy.dfa, profile->policy.start[0],
-			goto fail;
+				AA_CLASS_FILE))
-		}
+			profile->policy.start[AA_CLASS_FILE] =
-		if (!unpack_u32(e, &profile->policy.start[0], "start"))
+			  aa_dfa_next(profile->policy.dfa,
-			/* default start state */
+				      profile->policy.start[0],
-			profile->policy.start[0] = DFA_START;
+				      AA_CLASS_FILE);
-		/* setup class index */
-		for (i = AA_CLASS_FILE; i <= AA_CLASS_LAST; i++) {
-			profile->policy.start[i] =
-				aa_dfa_next(profile->policy.dfa,
-					    profile->policy.start[0],
-					    i);
-		}
 		if (!unpack_nameX(e, AA_STRUCTEND, NULL))
 			goto fail;
 		if (aa_compat_map_policy(&profile->policy, e->version)) {
@@ -867,25 +903,14 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
 		profile->policy.dfa = aa_get_dfa(nulldfa);
 
 	/* get file rules */
-	profile->file.dfa = unpack_dfa(e);
+	error = unpack_pdb(e, &profile->file, false, true, &info);
-	if (IS_ERR(profile->file.dfa)) {
+	if (error) {
-		error = PTR_ERR(profile->file.dfa);
-		profile->file.dfa = NULL;
-		info = "failed to unpack profile file rules";
 		goto fail;
 	} else if (profile->file.dfa) {
-		if (!unpack_u32(e, &profile->file.start[AA_CLASS_FILE],
-				"dfa_start"))
-			/* default start state */
-			profile->file.start[AA_CLASS_FILE] = DFA_START;
 		if (aa_compat_map_file(&profile->file)) {
 			info = "failed to remap file permission table";
 			goto fail;
 		}
-		if (!unpack_trans_table(e, &profile->file.trans)) {
-			info = "failed to unpack profile transition table";
-			goto fail;
-		}
 	} else if (profile->policy.dfa &&
 		   profile->policy.start[AA_CLASS_FILE]) {
 		profile->file.dfa = aa_get_dfa(profile->policy.dfa);