apparmor: refactory mount to use check_perms

Move the mount permissions check to use the common backend
aa_check_perms() to check permissions. This will make it so caching,
audit, complain, logic can be handled consistently in a single place.

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

4 files changed, 144 insertions, 168 deletions

diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
index 97d8151b5ff6..5752bb93aaaa 100644
--- a/security/apparmor/audit.c
+++ b/security/apparmor/audit.c
@@ -140,6 +140,17 @@ static void audit_pre(struct audit_buffer *ab, void *va)
 	}
 }
 
+int aa_select_audit_type(u32 denied, const struct aa_perms *perms)
+{
+	if (likely(!denied))
+		return AUDIT_APPARMOR_AUDIT;
+	else if (denied & perms->kill)
+		return AUDIT_APPARMOR_KILL;
+	else if (denied == (denied & perms->complain))
+		return AUDIT_APPARMOR_ALLOWED;
+	return AUDIT_APPARMOR_DENIED;
+}
+
 /**
  * aa_audit_msg - Log a message to the audit subsystem
  * @type: audit type for the message
@@ -153,6 +164,28 @@ void aa_audit_msg(int type, struct apparmor_audit_data *ad,
 	common_lsm_audit(&ad->common, audit_pre, cb);
 }
 
+int aa_audit_perm_error(struct aa_label *label, u32 request, int error,
+			struct apparmor_audit_data *ad,
+			void (*cb)(struct audit_buffer *, void *))
+{
+	int type = aa_select_audit_type(request, &nullperms);
+
+	if (ad) {
+		struct aa_profile *profile;
+		struct label_it i;
+
+		ad->request = request;
+		ad->denied = request;
+		ad->error = error;
+		label_for_each_confined(i, label, profile) {
+			ad->subj_label = &profile->label;
+			aa_audit_msg(type, ad, cb);
+		}
+	}
+
+	return error;
+}
+
 /**
  * aa_audit - Log a profile based audit event to the audit subsystem
  * @type: audit type for the message
diff --git a/security/apparmor/include/audit.h b/security/apparmor/include/audit.h
index da95b5569d68..987ed4441fd3 100644
--- a/security/apparmor/include/audit.h
+++ b/security/apparmor/include/audit.h
@@ -184,6 +184,8 @@ struct apparmor_audit_data {
 		.common.apparmor_audit_data = &NAME,			\
 	};
 
+int aa_select_audit_type(u32 denied, const struct aa_perms *perms);
+
 void aa_audit_msg(int type, struct apparmor_audit_data *ad,
 		  void (*cb) (struct audit_buffer *, void *));
 int aa_audit(int type, struct aa_profile *profile,
@@ -197,6 +199,9 @@ int aa_audit(int type, struct aa_profile *profile,
 	(AD)->error;					\
 })
 
+int aa_audit_perm_error(struct aa_label *label, u32 request, int error,
+			struct apparmor_audit_data *ad,
+			void (*cb)(struct audit_buffer *, void *));
 
 static inline int complain_error(int error)
 {
diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index 0c5c51c326fa..5d33252204fe 100644
--- a/security/apparmor/lib.c
+++ b/security/apparmor/lib.c
@@ -425,7 +425,7 @@ int aa_check_perms(struct aa_profile *profile, struct aa_perms *perms,
 		   u32 request, struct apparmor_audit_data *ad,
 		   void (*cb)(struct audit_buffer *, void *))
 {
-	int type, error;
+	int error;
 	u32 denied = request & (~perms->allow | perms->deny);
 
 	if (likely(!denied)) {
@@ -434,18 +434,10 @@ int aa_check_perms(struct aa_profile *profile, struct aa_perms *perms,
 		if (!request || !ad)
 			return 0;
 
-		type = AUDIT_APPARMOR_AUDIT;
 		error = 0;
 	} else {
 		error = -EACCES;
 
-		if (denied & perms->kill)
-			type = AUDIT_APPARMOR_KILL;
-		else if (denied == (denied & perms->complain))
-			type = AUDIT_APPARMOR_ALLOWED;
-		else
-			type = AUDIT_APPARMOR_DENIED;
-
 		if (denied == (denied & perms->hide))
 			error = -ENOENT;
 
@@ -454,6 +446,8 @@ int aa_check_perms(struct aa_profile *profile, struct aa_perms *perms,
 			return error;
 	}
 
+	int type = aa_select_audit_type(denied, perms);
+
 	if (ad) {
 		ad->subj_label = &profile->label;
 		ad->request = request;
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 537d22eb0303..5aaa4f878d92 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -24,6 +24,10 @@
 #include "include/policy.h"
 
 
+#define DEFINE_AUDIT_MOUNT(NAME, OP, CRED)				\
+	DEFINE_AUDIT_DATA(NAME, LSM_AUDIT_DATA_NONE, AA_CLASS_MOUNT, OP);\
+	NAME.subj_cred = (CRED)
+
 static void audit_mnt_flags(struct audit_buffer *ab, unsigned long flags)
 {
 	if (flags & MS_RDONLY)
@@ -114,78 +118,6 @@ static void audit_cb(struct audit_buffer *ab, void *va)
 }
 
 /**
- * audit_mount - handle the auditing of mount operations
- * @subj_cred: cred of the subject
- * @profile: the profile being enforced  (NOT NULL)
- * @op: operation being mediated (NOT NULL)
- * @name: name of object being mediated (MAYBE NULL)
- * @src_name: src_name of object being mediated (MAYBE_NULL)
- * @type: type of filesystem (MAYBE_NULL)
- * @trans: name of trans (MAYBE NULL)
- * @flags: filesystem independent mount flags
- * @data: filesystem mount flags
- * @request: permissions requested
- * @perms: the permissions computed for the request (NOT NULL)
- * @info: extra information message (MAYBE NULL)
- * @error: 0 if operation allowed else failure error code
- *
- * Returns: %0 or error on failure
- */
-static int audit_mount(const struct cred *subj_cred,
-		       struct aa_profile *profile, const char *op,
-		       const char *name, const char *src_name,
-		       const char *type, const char *trans,
-		       unsigned long flags, const void *data, u32 request,
-		       const struct aa_perms *perms, const char *info,
-		       int error)
-{
-	int audit_type = AUDIT_APPARMOR_AUTO;
-	DEFINE_AUDIT_DATA(ad, LSM_AUDIT_DATA_NONE, AA_CLASS_MOUNT, op);
-
-	if (likely(!error)) {
-		u32 mask = perms->audit;
-
-		if (unlikely(AUDIT_MODE(profile) == AUDIT_ALL))
-			mask = 0xffff;
-
-		/* mask off perms that are not being force audited */
-		request &= mask;
-
-		if (likely(!request))
-			return 0;
-		audit_type = AUDIT_APPARMOR_AUDIT;
-	} else {
-		/* only report permissions that were denied */
-		request = request & ~perms->allow;
-
-		if (request & perms->kill)
-			audit_type = AUDIT_APPARMOR_KILL;
-
-		/* quiet known rejects, assumes quiet and kill do not overlap */
-		if ((request & perms->quiet) &&
-		    AUDIT_MODE(profile) != AUDIT_NOQUIET &&
-		    AUDIT_MODE(profile) != AUDIT_ALL)
-			request &= ~perms->quiet;
-
-		if (!request)
-			return error;
-	}
-
-	ad.subj_cred = subj_cred;
-	ad.name = name;
-	ad.mnt.src_name = src_name;
-	ad.mnt.type = type;
-	ad.mnt.trans = trans;
-	ad.mnt.flags = flags;
-	if (data && (perms->audit & AA_AUDIT_DATA))
-		ad.mnt.data = data;
-	ad.info = info;
-	ad.error = error;
-
-	return aa_audit(audit_type, profile, &ad, audit_cb);
-}
-
-/**
  * match_mnt_flags - Do an ordered match on mount flags
  * @dfa: dfa to match against
  * @state: state to start in
@@ -290,7 +222,6 @@ static int path_flags(struct aa_profile *profile, const struct path *path)
 
 /**
  * match_mnt_path_str - handle path matching for mount
- * @subj_cred: cred of confined subject
  * @profile: the confining profile
  * @mntpath: for the mntpnt (NOT NULL)
  * @buffer: buffer to be used to lookup mntpath
@@ -300,18 +231,19 @@ static int path_flags(struct aa_profile *profile, const struct path *path)
  * @data: fs mount data (MAYBE NULL)
  * @binary: whether @data is binary
  * @devinfo: error str if (IS_ERR(@devname))
+ * @ad: apparmor audit data structure
  *
  * Returns: 0 on success else error
  */
-static int match_mnt_path_str(const struct cred *subj_cred,
-			      struct aa_profile *profile,
+static int match_mnt_path_str(struct aa_profile *profile,
 			      const struct path *mntpath, char *buffer,
 			      const char *devname, const char *type,
 			      unsigned long flags, void *data, bool binary,
-			      const char *devinfo)
+			      const char *devinfo,
+			      struct apparmor_audit_data *ad)
 {
 	struct aa_perms perms = { };
-	const char *mntpnt = NULL, *info = NULL;
+	const char *mntpnt = NULL;
 	struct aa_ruleset *rules = profile->label.rules[0];
 	int pos, error;
 
@@ -322,36 +254,37 @@ static int match_mnt_path_str(const struct cred *subj_cred,
 	if (!RULE_MEDIATES(rules, AA_CLASS_MOUNT))
 		return 0;
 
+	ad->mnt.type = type;
+
 	error = aa_path_name(mntpath, path_flags(profile, mntpath), buffer,
-			     &mntpnt, &info, profile->disconnected);
+			     &mntpnt, &ad->info, profile->disconnected);
 	if (error)
-		goto audit;
+		return aa_audit_perm_error(&profile->label, AA_MAY_MOUNT,
+					   error, ad, audit_cb);
+	ad->name = mntpnt;
+
 	if (IS_ERR(devname)) {
 		error = PTR_ERR(devname);
-		devname = NULL;
-		info = devinfo;
-		goto audit;
+		ad->info = devinfo;
+		return aa_audit_perm_error(&profile->label, AA_MAY_MOUNT,
+					   error, ad, audit_cb);
 	}
+	ad->mnt.src_name = devname;
 
-	error = -EACCES;
 	pos = do_match_mnt(rules->policy,
 			   rules->policy->start[AA_CLASS_MOUNT],
 			   mntpnt, devname, type, flags, data, binary, &perms);
-	if (pos) {
-		info = mnt_info_table[pos];
-		goto audit;
-	}
-	error = 0;
+	if (pos)
+		ad->info = mnt_info_table[pos];
 
-audit:
-	return audit_mount(subj_cred, profile, OP_MOUNT, mntpnt, devname,
-			   type, NULL, flags, !binary ? data : NULL,
-			   AA_MAY_MOUNT, &perms, info, error);
+	aa_apply_modes_to_perms(profile, &perms);
+	if (data && !binary && (perms.audit & AA_AUDIT_DATA))
+		ad->mnt.data = data;
+	return aa_check_perms(profile, &perms, AA_MAY_MOUNT, ad, audit_cb);
 }
 
 /**
  * match_mnt - handle path matching for mount
- * @subj_cred: cred of the subject
  * @profile: the confining profile
  * @path: for the mntpnt (NOT NULL)
  * @buffer: buffer to be used to lookup mntpath
@@ -361,14 +294,14 @@ audit:
  * @flags: mount flags to match
  * @data: fs mount data (MAYBE NULL)
  * @binary: whether @data is binary
+ * @ad: apparmor audit data structure
  *
  * Returns: 0 on success else error
  */
-static int match_mnt(const struct cred *subj_cred,
-		     struct aa_profile *profile, const struct path *path,
+static int match_mnt(struct aa_profile *profile, const struct path *path,
 		     char *buffer, const struct path *devpath, char *devbuffer,
 		     const char *type, unsigned long flags, void *data,
-		     bool binary)
+		     bool binary, struct apparmor_audit_data *ad)
 {
 	const char *devname = NULL, *info = NULL;
 	struct aa_ruleset *rules = profile->label.rules[0];
@@ -388,8 +321,8 @@ static int match_mnt(const struct cred *subj_cred,
 			devname = ERR_PTR(error);
 	}
 
-	return match_mnt_path_str(subj_cred, profile, path, buffer, devname,
-				  type, flags, data, binary, info);
+	return match_mnt_path_str(profile, path, buffer, devname,
+				  type, flags, data, binary, info, ad);
 }
 
 int aa_remount(const struct cred *subj_cred,
@@ -400,6 +333,8 @@ int aa_remount(const struct cred *subj_cred,
 	char *buffer = NULL;
 	bool binary;
 	int error;
+	DEFINE_AUDIT_MOUNT(ad, OP_MOUNT, subj_cred);
+	ad.mnt.flags = flags;
 
 	AA_BUG(!label);
 	AA_BUG(!path);
@@ -410,9 +345,8 @@ int aa_remount(const struct cred *subj_cred,
 	if (!buffer)
 		return -ENOMEM;
 	error = fn_for_each_confined(label, profile,
-			match_mnt(subj_cred, profile, path, buffer, NULL,
-				  NULL, NULL,
-				  flags, data, binary));
+			match_mnt(profile, path, buffer, NULL, NULL, NULL,
+				  flags, data, binary, &ad));
 	aa_put_buffer(buffer);
 
 	return error;
@@ -426,6 +360,7 @@ int aa_bind_mount(const struct cred *subj_cred,
 	char *buffer = NULL, *old_buffer = NULL;
 	struct path old_path;
 	int error;
+	DEFINE_AUDIT_MOUNT(ad, OP_MOUNT, subj_cred);
 
 	AA_BUG(!label);
 	AA_BUG(!path);
@@ -434,10 +369,12 @@ int aa_bind_mount(const struct cred *subj_cred,
 		return -EINVAL;
 
 	flags &= MS_REC | MS_BIND;
+	ad.mnt.flags = flags;
 
 	error = kern_path(dev_name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &old_path);
 	if (error)
-		return error;
+		return aa_audit_perm_error(label, AA_MAY_MOUNT, error, &ad,
+			audit_cb);
 
 	buffer = aa_get_buffer(false);
 	old_buffer = aa_get_buffer(false);
@@ -446,8 +383,8 @@ int aa_bind_mount(const struct cred *subj_cred,
 		goto out;
 
 	error = fn_for_each_confined(label, profile,
-			match_mnt(subj_cred, profile, path, buffer, &old_path,
-				  old_buffer, NULL, flags, NULL, false));
+			match_mnt(profile, path, buffer, &old_path,
+				  old_buffer, NULL, flags, NULL, false, &ad));
 out:
 	aa_put_buffer(buffer);
 	aa_put_buffer(old_buffer);
@@ -463,6 +400,7 @@ int aa_mount_change_type(const struct cred *subj_cred,
 	struct aa_profile *profile;
 	char *buffer = NULL;
 	int error;
+	DEFINE_AUDIT_MOUNT(ad, OP_MOUNT, subj_cred);
 
 	AA_BUG(!label);
 	AA_BUG(!path);
@@ -470,14 +408,14 @@ int aa_mount_change_type(const struct cred *subj_cred,
 	/* These are the flags allowed by do_change_type() */
 	flags &= (MS_REC | MS_SILENT | MS_SHARED | MS_PRIVATE | MS_SLAVE |
 		  MS_UNBINDABLE);
+	ad.mnt.flags = flags;
 
 	buffer = aa_get_buffer(false);
 	if (!buffer)
 		return -ENOMEM;
 	error = fn_for_each_confined(label, profile,
-			match_mnt(subj_cred, profile, path, buffer, NULL,
-				  NULL, NULL,
-				  flags, NULL, false));
+			match_mnt(profile, path, buffer, NULL, NULL, NULL,
+				  flags, NULL, false, &ad));
 	aa_put_buffer(buffer);
 
 	return error;
@@ -490,6 +428,8 @@ int aa_move_mount(const struct cred *subj_cred,
 	struct aa_profile *profile;
 	char *to_buffer = NULL, *from_buffer = NULL;
 	int error;
+	DEFINE_AUDIT_MOUNT(ad, OP_MOUNT, subj_cred);
+	ad.mnt.flags = MS_MOVE;
 
 	AA_BUG(!label);
 	AA_BUG(!from_path);
@@ -505,9 +445,9 @@ int aa_move_mount(const struct cred *subj_cred,
 		/* moving a mount detached from the namespace */
 		from_path = NULL;
 	error = fn_for_each_confined(label, profile,
-			match_mnt(subj_cred, profile, to_path, to_buffer,
-				  from_path, from_buffer,
-				  NULL, MS_MOVE, NULL, false));
+			match_mnt(profile, to_path, to_buffer, from_path,
+				  from_buffer, NULL, MS_MOVE, NULL, false,
+				  &ad));
 out:
 	aa_put_buffer(to_buffer);
 	aa_put_buffer(from_buffer);
@@ -543,6 +483,8 @@ int aa_new_mount(const struct cred *subj_cred, struct aa_label *label,
 	int error;
 	int requires_dev = 0;
 	struct path tmp_path, *dev_path = NULL;
+	DEFINE_AUDIT_MOUNT(ad, OP_MOUNT, subj_cred);
+	ad.mnt.flags = flags;
 
 	AA_BUG(!label);
 	AA_BUG(!path);
@@ -580,14 +522,14 @@ int aa_new_mount(const struct cred *subj_cred, struct aa_label *label,
 			goto out;
 		}
 		error = fn_for_each_confined(label, profile,
-				match_mnt(subj_cred, profile, path, buffer,
-					  dev_path, dev_buffer,
-				  type, flags, data, binary));
+				match_mnt(profile, path, buffer, dev_path,
+					  dev_buffer, type, flags, data,
+					  binary, &ad));
 	} else {
 		error = fn_for_each_confined(label, profile,
-				match_mnt_path_str(subj_cred, profile, path,
-					buffer, dev_name,
-					type, flags, data, binary, NULL));
+				match_mnt_path_str(profile, path, buffer,
+						   dev_name, type, flags, data,
+						   binary, NULL, &ad));
 	}
 
 out:
@@ -599,13 +541,12 @@ out:
 	return error;
 }
 
-static int profile_umount(const struct cred *subj_cred,
-			  struct aa_profile *profile, const struct path *path,
-			  char *buffer)
+static int profile_umount(struct aa_profile *profile, const struct path *path,
+			  char *buffer, struct apparmor_audit_data *ad)
 {
 	struct aa_ruleset *rules = profile->label.rules[0];
 	struct aa_perms perms = { };
-	const char *name = NULL, *info = NULL;
+	const char *name = NULL;
 	aa_state_t state;
 	int error;
 
@@ -615,22 +556,23 @@ static int profile_umount(const struct cred *subj_cred,
 	if (!RULE_MEDIATES(rules, AA_CLASS_MOUNT))
 		return 0;
 
+	/* TODO: lift path_name, need to separate profile path_flags from
+	 * the lookup
+	 */
 	error = aa_path_name(path, path_flags(profile, path), buffer, &name,
-			     &info, profile->disconnected);
+			     &ad->info, profile->disconnected);
 	if (error)
-		goto audit;
+		return aa_audit_perm_error(&profile->label, AA_MAY_UMOUNT,
+					   error, ad, audit_cb);
 
+	ad->name = name;
 	state = aa_dfa_match(rules->policy->dfa,
 			     rules->policy->start[AA_CLASS_MOUNT],
 			     name);
 	perms = *aa_lookup_perms(rules->policy, state);
-	if (AA_MAY_UMOUNT & ~perms.allow)
-		error = -EACCES;
 
-audit:
-	return audit_mount(subj_cred, profile, OP_UMOUNT, name, NULL, NULL,
-			   NULL, 0, NULL,
-			   AA_MAY_UMOUNT, &perms, info, error);
+	aa_apply_modes_to_perms(profile, &perms);
+	return aa_check_perms(profile, &perms, AA_MAY_UMOUNT, ad, audit_cb);
 }
 
 int aa_umount(const struct cred *subj_cred, struct aa_label *label,
@@ -640,6 +582,7 @@ int aa_umount(const struct cred *subj_cred, struct aa_label *label,
 	char *buffer = NULL;
 	int error;
 	struct path path = { .mnt = mnt, .dentry = mnt->mnt_root };
+	DEFINE_AUDIT_MOUNT(ad, OP_UMOUNT, subj_cred);
 
 	AA_BUG(!label);
 	AA_BUG(!mnt);
@@ -649,7 +592,7 @@ int aa_umount(const struct cred *subj_cred, struct aa_label *label,
 		return -ENOMEM;
 
 	error = fn_for_each_confined(label, profile,
-			profile_umount(subj_cred, profile, &path, buffer));
+			profile_umount(profile, &path, buffer, &ad));
 	aa_put_buffer(buffer);
 
 	return error;
@@ -659,16 +602,15 @@ int aa_umount(const struct cred *subj_cred, struct aa_label *label,
  *
  * Returns: label for transition or ERR_PTR. Does not return NULL
  */
-static struct aa_label *build_pivotroot(const struct cred *subj_cred,
-					struct aa_profile *profile,
+static struct aa_label *build_pivotroot(struct aa_profile *profile,
 					const struct path *new_path,
 					char *new_buffer,
 					const struct path *old_path,
-					char *old_buffer)
+					char *old_buffer,
+					struct apparmor_audit_data *ad)
 {
 	struct aa_ruleset *rules = profile->label.rules[0];
-	const char *old_name, *new_name = NULL, *info = NULL;
-	const char *trans_name = NULL;
+	const char *old_name, *new_name = NULL;
 	struct aa_perms perms = { };
 	aa_state_t state;
 	int error;
@@ -682,36 +624,40 @@ static struct aa_label *build_pivotroot(const struct cred *subj_cred,
 		return aa_get_newest_label(&profile->label);
 
 	error = aa_path_name(old_path, path_flags(profile, old_path),
-			     old_buffer, &old_name, &info,
+			     old_buffer, &old_name, &ad->info,
 			     profile->disconnected);
 	if (error)
-		goto audit;
+		goto err;
+	ad->mnt.src_name = old_name;
 	error = aa_path_name(new_path, path_flags(profile, new_path),
-			     new_buffer, &new_name, &info,
+			     new_buffer, &new_name, &ad->info,
 			     profile->disconnected);
 	if (error)
-		goto audit;
+		goto err;
+	ad->name = new_name;
 
-	error = -EACCES;
 	state = aa_dfa_match(rules->policy->dfa,
 			     rules->policy->start[AA_CLASS_MOUNT],
 			     new_name);
 	state = aa_dfa_null_transition(rules->policy->dfa, state);
 	state = aa_dfa_match(rules->policy->dfa, state, old_name);
 	perms = *aa_lookup_perms(rules->policy, state);
+	/* todo: allow pivotroot to specify a transition other than profile */
+	ad->mnt.trans = profile->label.hname;
 
-	if (AA_MAY_PIVOTROOT & perms.allow)
-		error = 0;
+	aa_apply_modes_to_perms(profile, &perms);
+	error = aa_check_perms(profile, &perms, AA_MAY_PIVOTROOT, ad, audit_cb);
 
-audit:
-	error = audit_mount(subj_cred, profile, OP_PIVOTROOT, new_name,
-			    old_name,
-			    NULL, trans_name, 0, NULL, AA_MAY_PIVOTROOT,
-			    &perms, info, error);
+out:
 	if (error)
 		return ERR_PTR(error);
 
 	return aa_get_newest_label(&profile->label);
+
+err:
+	error = aa_audit_perm_error(&profile->label, AA_MAY_PIVOTROOT, error,
+				    ad, audit_cb);
+	goto out;
 }
 
 int aa_pivotroot(const struct cred *subj_cred, struct aa_label *label,
@@ -720,8 +666,9 @@ int aa_pivotroot(const struct cred *subj_cred, struct aa_label *label,
 {
 	struct aa_profile *profile;
 	struct aa_label *target = NULL;
-	char *old_buffer = NULL, *new_buffer = NULL, *info = NULL;
+	char *old_buffer = NULL, *new_buffer = NULL;
 	int error;
+	DEFINE_AUDIT_MOUNT(ad, OP_PIVOTROOT, subj_cred);
 
 	AA_BUG(!label);
 	AA_BUG(!old_path);
@@ -733,9 +680,8 @@ int aa_pivotroot(const struct cred *subj_cred, struct aa_label *label,
 	if (!old_buffer || !new_buffer)
 		goto out;
 	target = fn_label_build(label, profile, GFP_KERNEL,
-			build_pivotroot(subj_cred, profile, new_path,
-					new_buffer,
-					old_path, old_buffer));
+			build_pivotroot(profile, new_path, new_buffer,
+					old_path, old_buffer, &ad));
 	AA_BUG(!target);
 	if (!IS_ERR(target)) {
 		error = aa_replace_current_label(target);
@@ -743,7 +689,7 @@ int aa_pivotroot(const struct cred *subj_cred, struct aa_label *label,
 			goto fail;
 		aa_put_label(target);
 	} else
-		/* already audited error */
+		/* already audited error in build_pivotroot */
 		error = PTR_ERR(target);
 out:
 	aa_put_buffer(old_buffer);
@@ -752,14 +698,12 @@ out:
 	return error;
 
 fail:
-	/* TODO: add back in auditing of new_name and old_name */
-	error = fn_for_each(label, profile,
-			audit_mount(subj_cred, profile, OP_PIVOTROOT,
-				    NULL /*new_name */,
-				    NULL /* old_name */,
-				    NULL, NULL,
-				    0, target->hname, AA_MAY_PIVOTROOT, &nullperms, info,
-				    error));
+	/* TODO: add back in auditing of new_name and old_name,
+	 * needs lifting of name lookup out of profile cb
+	 */
+	ad.mnt.trans = target->hname;
+	error = aa_audit_perm_error(label, AA_MAY_PIVOTROOT, error, &ad,
+				    audit_cb);
 	aa_put_label(target);
 	goto out;
 }