apparmor: change fn_label_build() call to not return NULL

Previously fn_label_build() was accepting a NULL which represented
ENOMEM return and ERR_PTR for errors.

Clean this up by requiring the cb fn to return an ERR_PTR or valid
value.

Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>

3 files changed, 35 insertions, 35 deletions

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index c2652165a588..4172dedaba22 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -864,6 +864,15 @@ audit:
 }
 
 /* ensure none ns domain transitions are correctly applied with onexec */
+static struct aa_label *label_merge_wrap(struct aa_label *a, struct aa_label *b,
+					 gfp_t gfp)
+{
+	struct aa_label *label = aa_label_merge(a, b, gfp);
+
+	if (!label)
+		return ERR_PTR(-ENOMEM);
+	return label;
+}
 
 static struct aa_label *handle_onexec(const struct cred *subj_cred,
 				      struct aa_label *label,
@@ -891,12 +900,13 @@ static struct aa_label *handle_onexec(const struct cred *subj_cred,
 		return ERR_PTR(error);
 
 	new = fn_label_build_in_scope(label, profile, GFP_KERNEL,
-			stack ? aa_label_merge(&profile->label, onexec,
-					       GFP_KERNEL)
+			stack ? label_merge_wrap(&profile->label, onexec,
+						 GFP_KERNEL)
 			      : aa_get_newest_label(onexec),
 			profile_transition(subj_cred, profile, bprm,
 					   buffer, cond, unsafe));
-	if (new)
+	AA_BUG(!new);
+	if (!IS_ERR(new))
 		return new;
 
 	/* TODO: get rid of GLOBAL_ROOT_UID */
@@ -905,7 +915,8 @@ static struct aa_label *handle_onexec(const struct cred *subj_cred,
 				      OP_CHANGE_ONEXEC,
 				      AA_MAY_ONEXEC, bprm->filename, NULL,
 				      onexec, GLOBAL_ROOT_UID,
-				      "failed to build target label", -ENOMEM));
+				      "failed to build target label",
+				      PTR_ERR(new)));
 	return ERR_PTR(error);
 }
 
@@ -968,14 +979,10 @@ int apparmor_bprm_creds_for_exec(struct linux_binprm *bprm)
 				profile_transition(subj_cred, profile, bprm,
 						   buffer,
 						   &cond, &unsafe));
-
 	AA_BUG(!new);
 	if (IS_ERR(new)) {
 		error = PTR_ERR(new);
 		goto done;
-	} else if (!new) {
-		error = -ENOMEM;
-		goto done;
 	}
 
 	/* Policy has specified a domain transitions. If no_new_privs and
@@ -1223,12 +1230,10 @@ build:
 				   build_change_hat(subj_cred, profile, name,
 						    sibling),
 				   aa_get_label(&profile->label));
-	if (!new) {
-		info = "label build failed";
-		error = -ENOMEM;
-		goto fail;
-	} /* else if (IS_ERR) build_change_hat has logged error so return new */
 	mutex_unlock(&ns->lock);
+	AA_BUG(!new);
+	/* return new label or error ptr */
+
 	return new;
 }
 
@@ -1556,7 +1561,8 @@ check:
 		new = fn_label_build_in_scope(label, profile, GFP_KERNEL,
 					   aa_get_label(target),
 					   aa_get_label(&profile->label));
-		if (IS_ERR_OR_NULL(new))
+		AA_BUG(!new);
+		if (IS_ERR(new))
 			goto build_fail;
 		/*
 		 * no new privs prevents domain transitions that would
@@ -1580,10 +1586,9 @@ check:
 			goto build_fail;
 		error = aa_replace_current_label(new);
 	} else {
-		if (new) {
-			aa_put_label(new);
-			new = NULL;
-		}
+		/* new will be recomputed so at exec time. So discard */
+		aa_put_label(new);
+		new = NULL;
 
 		/* full transition will be built in exec path */
 		aa_set_current_onexec(target, stack);
diff --git a/security/apparmor/include/lib.h b/security/apparmor/include/lib.h
index 8c6ce8484552..a093304fc998 100644
--- a/security/apparmor/include/lib.h
+++ b/security/apparmor/include/lib.h
@@ -282,7 +282,6 @@ void aa_policy_destroy(struct aa_policy *policy);
  *
  * Returns: new label on success
  *          ERR_PTR if build @FN fails
- *          NULL if label_build fails due to low memory conditions
  *
  * @FN must return a label or ERR_PTR on failure. NULL is not allowed
  */
@@ -298,7 +297,7 @@ void aa_policy_destroy(struct aa_policy *policy);
 		DEFINE_VEC(label, __lvec);				\
 		DEFINE_VEC(profile, __pvec);				\
 		if (vec_setup(label, __lvec, (L)->size, (GFP)))	{	\
-			__new_ = NULL;					\
+			__new_ = ERR_PTR(-ENOMEM);			\
 			goto __done;					\
 		}							\
 		__j = 0;						\
@@ -320,23 +319,24 @@ void aa_policy_destroy(struct aa_policy *policy);
 			if (__count > 1) {				\
 				__new_ = aa_vec_find_or_create_label(__pvec,\
 						     __count, (GFP));	\
-				/* only fails if out of Mem */		\
 				if (!__new_)				\
-					__new_ = NULL;			\
+					__new_ = ERR_PTR(-ENOMEM);	\
 			} else						\
 				__new_ = aa_get_label(&__pvec[0]->label); \
 			vec_cleanup(profile, __pvec, __count);		\
 		} else							\
-			__new_ = NULL;					\
+			__new_ = ERR_PTR(-ENOMEM);			\
 __do_cleanup:								\
 		vec_cleanup(label, __lvec, (L)->size);			\
 	} else {							\
 		(P) = labels_profile(L);				\
 		__new_ = (FN);						\
+		AA_BUG(!__new_);					\
 	}								\
 __done:									\
-	if (!__new_)							\
+	if (PTR_ERR(__new_))						\
 		AA_DEBUG(DEBUG_LABEL, "label build failed\n");		\
+	AA_BUG(!__new_);						\
 	(__new_);							\
 })
 
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 523570aa1a5a..2f5d918832c1 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -735,17 +735,11 @@ int aa_pivotroot(const struct cred *subj_cred, struct aa_label *label,
 			build_pivotroot(subj_cred, profile, new_path,
 					new_buffer,
 					old_path, old_buffer));
-	if (!target) {
-		info = "label build failed";
-		error = -ENOMEM;
-		goto fail;
-	} else if (!IS_ERR(target)) {
+	AA_BUG(!target);
+	if (!IS_ERR(target)) {
 		error = aa_replace_current_label(target);
-		if (error) {
-			/* TODO: audit target */
-			aa_put_label(target);
-			goto out;
-		}
+		if (error)
+			goto fail;
 		aa_put_label(target);
 	} else
 		/* already audited error */
@@ -763,7 +757,8 @@ fail:
 				    NULL /*new_name */,
 				    NULL /* old_name */,
 				    NULL, NULL,
-				    0, NULL, AA_MAY_PIVOTROOT, &nullperms, info,
+				    0, target->hname, AA_MAY_PIVOTROOT, &nullperms, info,
 				    error));
+	aa_put_label(target);
 	goto out;
 }