Commit 4a134723 authored by John Johansen's avatar John Johansen
Browse files

apparmor: move check for aa_null file to cover all cases 

files with a dentry pointing aa_null.dentry where already rejected as
part of file_inheritance. Unfortunately the check in
common_file_perm() is insufficient to cover all cases causing
unnecessary audit messages without the original files context.

Eg.
[ 442.886474] audit: type=1400 audit(1704822661.616:329): apparmor="DENIED" operation="file_inherit" class="file" namespace="root//lxd-juju-98527a-0_<var-snap-lxd-common-lxd>" profile="snap.lxd.activate" name="/apparmor/.null" pid=9525 comm="snap-exec"

Further examples of this are in the logs of
https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/2120439
https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1952084
https://bugs.launchpad.net/snapd/+bug/2049099

These messages have no value and should not be sent to the logs.
AppArmor was already filtering the out in some cases but the original
patch did not catch all cases. Fix this by push the existing check
down into two functions that should cover all cases.

Link: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/2122743


Fixes: 192ca6b5 ("apparmor: revalidate files during exec")
Reviewed-by: default avatarGeorgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: default avatarJohn Johansen <john.johansen@canonical.com>
parent e16eee78

security/apparmor/file.c

+10 −2
Original line number Diff line number Diff line
@@ -155,6 +155,10 @@ static int path_name(const char *op, const struct cred *subj_cred, 
	const char *info = NULL;

	int error;



	/* don't reaudit files closed during inheritance */

	if (unlikely(path->dentry == aa_null.dentry))

		error = -EACCES;

	else

		error = aa_path_name(path, flags, buffer, name, &info,

				     labels_profile(label)->disconnected);

	if (error) {
@@ -617,6 +621,10 @@ int aa_file_perm(const char *op, const struct cred *subj_cred, 
	AA_BUG(!label);

	AA_BUG(!file);



	/* don't reaudit files closed during inheritance */

	if (unlikely(file->f_path.dentry == aa_null.dentry))

		return -EACCES;



	fctx = file_ctx(file);



	rcu_read_lock();

security/apparmor/lsm.c

+0 −4
Original line number Diff line number Diff line
@@ -525,10 +525,6 @@ static int common_file_perm(const char *op, struct file *file, u32 mask) 
	struct aa_label *label;

	int error = 0;



	/* don't reaudit files closed during inheritance */

	if (unlikely(file->f_path.dentry == aa_null.dentry))

		return -EACCES;



	label = begin_current_label_crit_section();

	error = aa_file_perm(op, current_cred(), label, file, mask, false);

	end_current_label_crit_section(label);