Re: [apparmor] [PATCH] apparmor: audit_cap dedup based on subj_cred instead of profile

[John Johansen]

On 9/25/24 11:30, Ryan Lee wrote:

The previous audit_cap cache deduping was based on the profile that was
being audited. This could cause confusion due to the deduplication then
occurring across multiple processes, which could happen if multiple
instances of binaries matched the same profile attachment (and thus ran
under the same profile) or a profile was attached to a container and its
processes.

Instead, perform audit_cap deduping over ad->subj_cred, which ensures the
deduping only occurs across a single process, instead of across all
processes that match the current one's profile.

Signed-off-by: Ryan Lee <ryan....@canonical.com>

Acked-by: John Johansen <john.johan...@canoical.com>

I have pulled this into my tree

---
  security/apparmor/capability.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/security/apparmor/capability.c b/security/apparmor/capability.c
index 61d7ab4255b0..3729c7fc86f9 100644
--- a/security/apparmor/capability.c
+++ b/security/apparmor/capability.c
@@ -32,7 +32,7 @@ struct aa_sfs_entry aa_sfs_entry_caps[] = {
  };

struct audit_cache {

-       struct aa_profile *profile;
+       const struct cred *ad_subj_cred;
        /* Capabilities go from 0 to CAP_LAST_CAP */
        u64 ktime_ns_expiration[CAP_LAST_CAP+1];
  };
@@ -95,14 +95,14 @@ static int audit_caps(struct apparmor_audit_data *ad, 
struct aa_profile *profile
        /* Do simple duplicate message elimination */
        ent = &get_cpu_var(audit_cache);
        /* If the capability was never raised the timestamp check would also 
catch that */
-       if (profile == ent->profile && ktime_get_ns() <= 
ent->ktime_ns_expiration[cap]) {
+       if (ad->subj_cred == ent->ad_subj_cred && ktime_get_ns() <= 
ent->ktime_ns_expiration[cap]) {
                put_cpu_var(audit_cache);
                if (COMPLAIN_MODE(profile))
                        return complain_error(error);
                return error;
        } else {
-               aa_put_profile(ent->profile);
-               ent->profile = aa_get_profile(profile);
+               put_cred(ent->ad_subj_cred);
+               ent->ad_subj_cred = get_cred(ad->subj_cred);
                ent->ktime_ns_expiration[cap] = ktime_get_ns() + 
AUDIT_CACHE_TIMEOUT_NS;
        }
        put_cpu_var(audit_cache);