apparmor: try to avoid refing the label in apparmor_file_open
authorMateusz Guzik <mjguzik@gmail.com>
Thu, 20 Jun 2024 17:15:27 +0000 (19:15 +0200)
committerJohn Johansen <john.johansen@canonical.com>
Wed, 24 Jul 2024 18:05:14 +0000 (11:05 -0700)
If the label is not stale (which is the common case), the fact that the
passed file object holds a reference can be leverged to avoid the
ref/unref cycle. Doing so reduces performance impact of apparmor on
parallel open() invocations.

When benchmarking on a 24-core vm using will-it-scale's open1_process
("Separate file open"), the results are (ops/s):
before: 6092196
after:  8309726 (+36%)

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
security/apparmor/include/cred.h
security/apparmor/lsm.c

index 58fdc72..7265d2f 100644 (file)
@@ -63,6 +63,26 @@ static inline struct aa_label *aa_get_newest_cred_label(const struct cred *cred)
        return aa_get_newest_label(aa_cred_raw_label(cred));
 }
 
+static inline struct aa_label *aa_get_newest_cred_label_condref(const struct cred *cred,
+                                                               bool *needput)
+{
+       struct aa_label *l = aa_cred_raw_label(cred);
+
+       if (unlikely(label_is_stale(l))) {
+               *needput = true;
+               return aa_get_newest_label(l);
+       }
+
+       *needput = false;
+       return l;
+}
+
+static inline void aa_put_label_condref(struct aa_label *l, bool needput)
+{
+       if (unlikely(needput))
+               aa_put_label(l);
+}
+
 /**
  * aa_current_raw_label - find the current tasks confining label
  *
index 717204f..242d4cf 100644 (file)
@@ -461,6 +461,7 @@ static int apparmor_file_open(struct file *file)
        struct aa_file_ctx *fctx = file_ctx(file);
        struct aa_label *label;
        int error = 0;
+       bool needput;
 
        if (!path_mediated_fs(file->f_path.dentry))
                return 0;
@@ -477,7 +478,7 @@ static int apparmor_file_open(struct file *file)
                return 0;
        }
 
-       label = aa_get_newest_cred_label(file->f_cred);
+       label = aa_get_newest_cred_label_condref(file->f_cred, &needput);
        if (!unconfined(label)) {
                struct mnt_idmap *idmap = file_mnt_idmap(file);
                struct inode *inode = file_inode(file);
@@ -494,7 +495,7 @@ static int apparmor_file_open(struct file *file)
                /* todo cache full allowed permissions set and state */
                fctx->allow = aa_map_file_to_perms(file);
        }
-       aa_put_label(label);
+       aa_put_label_condref(label, needput);
 
        return error;
 }