From: John Johansen Date: Sat, 13 Sep 2025 09:22:21 +0000 (-0700) Subject: apparmor: move check for aa_null file to cover all cases X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4a134723f9f1ad2f3621566259db673350d19cb1;p=thirdparty%2Flinux.git 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_" 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: 192ca6b55a86 ("apparmor: revalidate files during exec") Reviewed-by: Georgia Garcia Signed-off-by: John Johansen --- diff --git a/security/apparmor/file.c b/security/apparmor/file.c index b69fece45ade..c751f2774c59 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -155,8 +155,12 @@ static int path_name(const char *op, const struct cred *subj_cred, const char *info = NULL; int error; - error = aa_path_name(path, flags, buffer, name, &info, - labels_profile(label)->disconnected); + /* 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) { fn_for_each_confined(label, profile, aa_audit_file(subj_cred, @@ -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(); diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index f47d60d8c40a..8d5d9a966b71 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -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);