From: Mateusz Guzik <mjguzik@gmail.com>
To: john.johansen@canonical.com
Cc: paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com,
linux-kernel@vger.kernel.org, apparmor@lists.ubuntu.com,
linux-security-module@vger.kernel.org, Neeraj.Upadhyay@amd.com,
Mateusz Guzik <mjguzik@gmail.com>
Subject: [PATCH] apparmor: try to avoid refing the label in apparmor_file_open
Date: Thu, 20 Jun 2024 15:15:24 +0200 [thread overview]
Message-ID: <20240620131524.156312-1-mjguzik@gmail.com> (raw)
It can be done in the common case.
A 24-way open1_processes from will-it-scale (separate file open) shows:
29.37% [kernel] [k] apparmor_file_open
26.84% [kernel] [k] apparmor_file_alloc_security
26.62% [kernel] [k] apparmor_file_free_security
1.32% [kernel] [k] clear_bhb_loop
apparmor_file_open is eliminated from the profile with the patch.
Throughput (ops/s):
before: 6092196
after: 8309726 (+36%)
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
I think this is a worthwhile touch up regardless of what happens with
label refcouting in the long run. It does not of course does not fully
fix the problem.
I concede the naming is not consistent with other stuff in the file and
I'm not going to argue about it -- happy to name it whatever as long as
the problem is sorted out.
Am I missing something which makes the approach below not work to begin
with?
security/apparmor/include/cred.h | 20 ++++++++++++++++++++
security/apparmor/lsm.c | 5 +++--
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/security/apparmor/include/cred.h b/security/apparmor/include/cred.h
index 58fdc72af664..7265d2f81dd5 100644
--- a/security/apparmor/include/cred.h
+++ b/security/apparmor/include/cred.h
@@ -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
*
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 2cea34657a47..4bf87eac4a56 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -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;
}
--
2.43.0
next reply other threads:[~2024-06-20 13:15 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-20 13:15 Mateusz Guzik [this message]
2024-06-20 16:26 ` [PATCH] apparmor: try to avoid refing the label in apparmor_file_open John Johansen
2024-06-20 16:41 ` Mateusz Guzik
2024-06-20 17:08 ` John Johansen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240620131524.156312-1-mjguzik@gmail.com \
--to=mjguzik@gmail.com \
--cc=Neeraj.Upadhyay@amd.com \
--cc=apparmor@lists.ubuntu.com \
--cc=jmorris@namei.org \
--cc=john.johansen@canonical.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=paul@paul-moore.com \
--cc=serge@hallyn.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).