summaryrefslogtreecommitdiff
path: root/security
diff options
context:
space:
mode:
authorPaul Moore <paul@paul-moore.com>2023-03-07 14:55:01 -0500
committerPaul Moore <paul@paul-moore.com>2023-03-08 11:48:27 -0500
commitf62ca0b6e31d82e0622a8e31ce5562e80edf6c3c (patch)
treed8c7e86a5ca644d30cf5b9edf683c3db545c5092 /security
parentfe15c26ee26efa11741a7b632e9f23b01aca4cc6 (diff)
selinux: uninline unlikely parts of avc_has_perm_noaudit()
This is based on earlier patch posted to the list by Linus, his commit description read: "avc_has_perm_noaudit()is one of those hot functions that end up being used by almost all filesystem operations (through "avc_has_perm()") and it's intended to be cheap enough to inline. However, it turns out that the unlikely parts of it (where it doesn't find an existing avc node) need a fair amount of stack space for the automatic replacement node, so if it were to be inlined (at least clang does not) it would just use stack space unnecessarily. So split the unlikely part out of it, and mark that part noinline. That improves the actual likely part." The basic idea behind the patch was reasonable, but there were minor nits (double indenting, etc.) and the RCU read lock unlock/re-lock in avc_compute_av() began to look even more ugly. This patch builds on Linus' first effort by cleaning things up a bit and removing the RCU unlock/lock dance in avc_compute_av(). Removing the RCU lock dance in avc_compute_av() is safe as there are currently two callers of avc_compute_av(): avc_has_perm_noaudit() and avc_has_extended_perms(). The first caller in avc_has_perm_noaudit() does not require a RCU lock as there is no avc_node to protect so the RCU lock can be dropped before calling avc_compute_av(). The second caller, avc_has_extended_perms(), is similar in that there is no avc_node that requires RCU protection, but the code is simplified by holding the RCU look around the avc_compute_av() call, and given that we enter a RCU critical section in security_compute_av() (called from av_compute_av()) the impact will likely be unnoticeable. It is also worth noting that avc_has_extended_perms() is only called from the SELinux ioctl() access control hook at the moment. Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Paul Moore <paul@paul-moore.com>
Diffstat (limited to 'security')
-rw-r--r--security/selinux/avc.c85
1 files changed, 57 insertions, 28 deletions
diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index 9a43af0ebd7d..5ce3ad451665 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -988,25 +988,26 @@ int avc_ss_reset(struct selinux_avc *avc, u32 seqno)
return rc;
}
-/*
- * Slow-path helper function for avc_has_perm_noaudit,
- * when the avc_node lookup fails. We get called with
- * the RCU read lock held, and need to return with it
- * still held, but drop if for the security compute.
+/**
+ * avc_compute_av - Add an entry to the AVC based on the security policy
+ * @state: SELinux state pointer
+ * @ssid: subject
+ * @tsid: object/target
+ * @tclass: object class
+ * @avd: access vector decision
+ * @xp_node: AVC extended permissions node
*
- * Don't inline this, since it's the slow-path and just
- * results in a bigger stack frame.
+ * Slow-path helper function for avc_has_perm_noaudit, when the avc_node lookup
+ * fails. Don't inline this, since it's the slow-path and just results in a
+ * bigger stack frame.
*/
-static noinline
-struct avc_node *avc_compute_av(struct selinux_state *state,
- u32 ssid, u32 tsid,
- u16 tclass, struct av_decision *avd,
- struct avc_xperms_node *xp_node)
+static noinline struct avc_node *avc_compute_av(struct selinux_state *state,
+ u32 ssid, u32 tsid, u16 tclass,
+ struct av_decision *avd,
+ struct avc_xperms_node *xp_node)
{
- rcu_read_unlock();
INIT_LIST_HEAD(&xp_node->xpd_head);
security_compute_av(state, ssid, tsid, tclass, avd, &xp_node->xp);
- rcu_read_lock();
return avc_insert(state->avc, ssid, tsid, tclass, avd, xp_node);
}
@@ -1113,6 +1114,36 @@ decision:
}
/**
+ * avc_perm_nonode - Add an entry to the AVC
+ * @state: SELinux state pointer
+ * @ssid: subject
+ * @tsid: object/target
+ * @tclass: object class
+ * @requested: requested permissions
+ * @flags: AVC flags
+ * @avd: access vector decision
+ *
+ * This is the "we have no node" part of avc_has_perm_noaudit(), which is
+ * unlikely and needs extra stack space for the new node that we generate, so
+ * don't inline it.
+ */
+static noinline int avc_perm_nonode(struct selinux_state *state,
+ u32 ssid, u32 tsid, u16 tclass,
+ u32 requested, unsigned int flags,
+ struct av_decision *avd)
+{
+ u32 denied;
+ struct avc_xperms_node xp_node;
+
+ avc_compute_av(state, ssid, tsid, tclass, avd, &xp_node);
+ denied = requested & ~(avd->allowed);
+ if (unlikely(denied))
+ return avc_denied(state, ssid, tsid, tclass, requested, 0, 0,
+ flags, avd);
+ return 0;
+}
+
+/**
* avc_has_perm_noaudit - Check permissions but perform no auditing.
* @state: SELinux state
* @ssid: source security identifier
@@ -1139,29 +1170,27 @@ inline int avc_has_perm_noaudit(struct selinux_state *state,
unsigned int flags,
struct av_decision *avd)
{
- struct avc_node *node;
- struct avc_xperms_node xp_node;
- int rc = 0;
u32 denied;
+ struct avc_node *node;
if (WARN_ON(!requested))
return -EACCES;
rcu_read_lock();
-
node = avc_lookup(state->avc, ssid, tsid, tclass);
- if (unlikely(!node))
- avc_compute_av(state, ssid, tsid, tclass, avd, &xp_node);
- else
- memcpy(avd, &node->ae.avd, sizeof(*avd));
+ if (unlikely(!node)) {
+ rcu_read_unlock();
+ return avc_perm_nonode(state, ssid, tsid, tclass, requested,
+ flags, avd);
+ }
+ denied = requested & ~node->ae.avd.allowed;
+ memcpy(avd, &node->ae.avd, sizeof(*avd));
+ rcu_read_unlock();
- denied = requested & ~(avd->allowed);
if (unlikely(denied))
- rc = avc_denied(state, ssid, tsid, tclass, requested, 0, 0,
- flags, avd);
-
- rcu_read_unlock();
- return rc;
+ return avc_denied(state, ssid, tsid, tclass, requested, 0, 0,
+ flags, avd);
+ return 0;
}
/**