Skip to content

Commit

Permalink
apparmor: try to avoid refing the label in apparmor_file_open
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: John Johansen <[email protected]>
  • Loading branch information
mjguzik authored and John Johansen committed Jul 24, 2024
1 parent 4b954a0 commit f4fee21
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
20 changes: 20 additions & 0 deletions security/apparmor/include/cred.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
5 changes: 3 additions & 2 deletions security/apparmor/lsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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;
}
Expand Down

0 comments on commit f4fee21

Please sign in to comment.