[apparmor] [PATCH 3/4] apparmor: include conflicting attachment info for confined ix/ux fallback

[Ryan Lee]

Instead of silently overwriting the conflicting profile attachment string,
include that information in the ix/ux fallback string that gets set as info
instead. Also add a warning print if some other info is set that would be
overwritten by the ix/ux fallback string or by the profile not found error.

Signed-off-by: Ryan Lee <ryan.lee at canonical.com>
---
 security/apparmor/domain.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index a73307ee1c7f..e8cd9badfb54 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -30,6 +30,10 @@
 #include "include/policy_ns.h"
 
 static const char * const CONFLICTING_ATTACH_STR = "conflicting profile attachments";
+static const char * const CONFLICTING_ATTACH_STR_IX =
+	"conflicting profile attachments - ix fallback";
+static const char * const CONFLICTING_ATTACH_STR_UX =
+	"conflicting profile attachments - ux fallback";
 
 /**
  * may_change_ptraced_domain - check if can change profile on ptraced task
@@ -577,6 +579,8 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
 	struct aa_label *stack = NULL;
 	struct aa_ns *ns = profile->ns;
 	u32 xtype = xindex & AA_X_TYPE_MASK;
+	/* Used for info checks during fallback handling */
+	const char *old_info = NULL;
 
 	switch (xtype) {
 	case AA_X_NONE:
@@ -612,12 +616,32 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
 			/* (p|c|n)ix - don't change profile but do
 			 * use the newest version
 			 */
-			*info = "ix fallback";
+			if (*info == CONFLICTING_ATTACH_STR) {
+				*info = CONFLICTING_ATTACH_STR_IX;
+			} else {
+				old_info = *info;
+				*info = "ix fallback";
+			}
 			/* no profile && no error */
 			new = aa_get_newest_label(&profile->label);
 		} else if (xindex & AA_X_UNCONFINED) {
 			new = aa_get_newest_label(ns_unconfined(profile->ns));
-			*info = "ux fallback";
+			if (*info == CONFLICTING_ATTACH_STR) {
+				*info = CONFLICTING_ATTACH_STR_UX;
+			} else {
+				old_info = *info;
+				*info = "ux fallback";
+			}
+		}
+		/* We set old_info on the code paths above where overwriting
+		 * could have happened, so now check if info was set by
+		 * find_attach as well (i.e. whether we actually overwrote)
+		 * and warn accordingly.
+		 */
+		if (old_info && old_info != CONFLICTING_ATTACH_STR) {
+			pr_warn_ratelimited(
+				"AppArmor: find_attach (from profile %s) audit info \"%s\" dropped",
+				profile->base.hname, old_info);
 		}
 	}
 
@@ -703,6 +727,11 @@ static struct aa_label *profile_transition(const struct cred *subj_cred,
 			/* hack ix fallback - improve how this is detected */
 			goto audit;
 		} else if (!new) {
+			if (info) {
+				pr_warn_ratelimited(
+					"AppArmor: profile_transition (from profile %s) audit info \"%s\" dropped on missing transition",
+					profile->base.hname, info);
+			}
 			info = "profile transition not found";
 			/* remove MAY_EXEC to audit as failure or complaint */
 			perms.allow &= ~MAY_EXEC;
-- 
2.43.0