mbox series

[v12,0/5] Reduce overhead of LSMs with static calls

Message ID 20240516003524.143243-1-kpsingh@kernel.org (mailing list archive)
Headers show
Series Reduce overhead of LSMs with static calls | expand

Message

KP Singh May 16, 2024, 12:35 a.m. UTC
# Background

LSM hooks (callbacks) are currently invoked as indirect function calls. These
callbacks are registered into a linked list at boot time as the order of the
LSMs can be configured on the kernel command line with the "lsm=" command line
parameter.

Indirect function calls have a high overhead due to retpoline mitigation for
various speculative execution attacks.

Retpolines remain relevant even with newer generation CPUs as recently
discovered speculative attacks, like Spectre BHB need Retpolines to mitigate
against branch history injection and still need to be used in combination with
newer mitigation features like eIBRS.

This overhead is especially significant for the "bpf" LSM which allows the user
to implement LSM functionality with eBPF program. In order to facilitate this
the "bpf" LSM provides a default callback for all LSM hooks. When enabled,
the "bpf" LSM incurs an unnecessary / avoidable indirect call. This is
especially bad in OS hot paths (e.g. in the networking stack).
This overhead prevents the adoption of bpf LSM on performance critical
systems, and also, in general, slows down all LSMs.

Since we know the address of the enabled LSM callbacks at compile time and only
the order is determined at boot time, the LSM framework can allocate static
calls for each of the possible LSM callbacks and these calls can be updated once
the order is determined at boot.

This series is a respin of the RFC proposed by Paul Renauld (renauld@google.com)
and Brendan Jackman (jackmanb@google.com) [1]

# Performance improvement

With this patch-set some syscalls with lots of LSM hooks in their path
benefitted at an average of ~3% and I/O and Pipe based system calls benefitting
the most.

Here are the results of the relevant Unixbench system benchmarks with BPF LSM
and SELinux enabled with default policies enabled with and without these
patches.

Benchmark                                               Delta(%): (+ is better)
===============================================================================
Execl Throughput                                             +1.9356
File Write 1024 bufsize 2000 maxblocks                       +6.5953
Pipe Throughput                                              +9.5499
Pipe-based Context Switching                                 +3.0209
Process Creation                                             +2.3246
Shell Scripts (1 concurrent)                                 +1.4975
System Call Overhead                                         +2.7815
System Benchmarks Index Score (Partial Only):                +3.4859

In the best case, some syscalls like eventfd_create benefitted to about ~10%.
The full analysis can be viewed at https://kpsingh.ch/lsm-perf

[1] https://lore.kernel.org/linux-security-module/20200820164753.3256899-1-jackmanb@chromium.org/


# BPF LSM Side effects

Patch 4 of the series also addresses the issues with the side effects of the
default value return values of the BPF LSM callbacks and also removes the
overheads associated with them making it deployable at hyperscale.

# v11 to v12

* Casey's feedback on readability of patch 4
* Tetsuo's catch on the bug in Patch 5
* Changed LSM_HOOK_TOGGLEABLE to LSM_HOOK_RUNTIME, the bikeshed is blue now.
* Also, as security_toggle_hook relies on iterating over a
  flat lsm_static_calls_table, I added the __packed attribute.

# v10 to v11

* bpf_lsm_toggle_hook to security_toggle_hook with LSM_HOOK_TOGGLEABLE
  limiting the hooks as the ones that can be toggled.

# v9 to v10

* Addressed Paul's comments for Patch 3. I did not remove the acks from this one
  as changes were minor.
* Moved BPF LSM specific hook toggling logic bpf_lsm_toggle_hook to s
  security_toggle_hook as a generic API. I removed the Ack's from this patch
  as it's worth another look.
* Refactored the non-standard hooks to use static calls.

# v8 to v9

Paul, I removed the 5th patch about CONFIG_SECURITY_HOOK_LIKELY and went through
all the feedback. I believe it all should be addressed now.
But, please let me know if I missed anything.

The patches are based on https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git
(next branch as of 2024-02-07) and resolved a bunch of conflicts.

I also added Andrii's series ack to indidividual patches.

# v7 to v8

* Addressed Andrii's feedback
* Rebased (this seems to have removed the syscall changes). v7 has the required
  conflict resolution incase the conflicts need to be resolved again.

# v6 -> v7

* Rebased with latest LSM id changes merged

NOTE: The warning shown by the kernel test bot is spurious, there is no flex array
and it seems to come from an older tool chain.

https://lore.kernel.org/bpf/202310111711.wLbijitj-lkp@intel.com/

# v5 -> v6

* Fix a bug in BPF LSM hook toggle logic.

# v4 -> v5

* Rebase to linux-next/master
* Fixed the case where MAX_LSM_COUNT comes to zero when just CONFIG_SECURITY
  is compiled in without any other LSM enabled as reported here:

  https://lore.kernel.org/bpf/202309271206.d7fb60f9-oliver.sang@intel.com

# v3 -> v4

* Refactor LSM count macros to use COUNT_ARGS
* Change CONFIG_SECURITY_HOOK_LIKELY likely's default value to be based on
  the LSM enabled and have it depend on CONFIG_EXPERT. There are a lot of subtle
  options behind CONFIG_EXPERT and this should, hopefully alleviate concerns
  about yet another knob.
* __randomize_layout for struct lsm_static_call and, in addition to the cover
  letter add performance numbers to 3rd patch and some minor commit message
  updates.
* Rebase to linux-next.

# v2 -> v3

* Fixed a build issue on archs which don't have static calls and enable
  CONFIG_SECURITY.
* Updated the LSM_COUNT macros based on Andrii's suggestions.
* Changed the security_ prefix to lsm_prefix based on Casey's suggestion.
* Inlined static_branch_maybe into lsm_for_each_hook on Kees' feedback.

# v1 -> v2 (based on linux-next, next-20230614)

* Incorporated suggestions from Kees
* Changed the way MAX_LSMs are counted from a binary based generator to a clever header.
* Add CONFIG_SECURITY_HOOK_LIKELY to configure the likelihood of LSM hooks.


KP Singh (5):
  kernel: Add helper macros for loop unrolling
  security: Count the LSMs enabled at compile time
  security: Replace indirect LSM hook calls with static calls
  security: Update non standard hooks to use static calls
  bpf: Only enable BPF LSM hooks when an LSM program is attached

 include/linux/args.h      |   6 +-
 include/linux/lsm_count.h | 128 ++++++++++++
 include/linux/lsm_hooks.h | 100 +++++++++-
 include/linux/unroll.h    |  36 ++++
 kernel/bpf/trampoline.c   |  40 +++-
 security/bpf/hooks.c      |   2 +-
 security/security.c       | 403 ++++++++++++++++++++++++++------------
 7 files changed, 568 insertions(+), 147 deletions(-)
 create mode 100644 include/linux/lsm_count.h
 create mode 100644 include/linux/unroll.h

Comments

Tetsuo Handa May 18, 2024, 6:01 a.m. UTC | #1
On 2024/05/16 9:35, KP Singh wrote:
> Since we know the address of the enabled LSM callbacks at compile time and only
> the order is determined at boot time, the LSM framework can allocate static
> calls for each of the possible LSM callbacks and these calls can be updated once
> the order is determined at boot.

I don't like this assumption. None of built-in LSMs is used by (or affordable for)
my customers. There is a reality that only out-of-tree security modules which the
distributor (namely, Red Hat) cannot support (and therefore cannot be built into
RHEL kernels) are used by (or affordable for) such customers.

Therefore, without giving room for allowing such security modules to load after
boot, I consider this change a regression.
Kees Cook June 6, 2024, 3:58 p.m. UTC | #2
On Thu, May 16, 2024 at 02:35:19AM +0200, KP Singh wrote:
> This series is a respin of the RFC proposed by Paul Renauld (renauld@google.com)
> and Brendan Jackman (jackmanb@google.com) [1]
> 
> # Performance improvement
> 
> With this patch-set some syscalls with lots of LSM hooks in their path
> benefitted at an average of ~3% and I/O and Pipe based system calls benefitting
> the most.

Hi Paul,

With the merge window closed now, can we please land this in -next so we
can get any glitches found/hammered out with maximal time before the
next merge window?

-Kees
Paul Moore June 6, 2024, 4:36 p.m. UTC | #3
On Thu, Jun 6, 2024 at 11:58 AM Kees Cook <kees@kernel.org> wrote:
> On Thu, May 16, 2024 at 02:35:19AM +0200, KP Singh wrote:
> > This series is a respin of the RFC proposed by Paul Renauld (renauld@google.com)
> > and Brendan Jackman (jackmanb@google.com) [1]
> >
> > # Performance improvement
> >
> > With this patch-set some syscalls with lots of LSM hooks in their path
> > benefitted at an average of ~3% and I/O and Pipe based system calls benefitting
> > the most.
>
> Hi Paul,
>
> With the merge window closed now, can we please land this in -next so we
> can get any glitches found/hammered out with maximal time before the
> next merge window?

It's in the queue, I've been swamped lately as you'll likely notice I
haven't really had a chance to process patches yet during this cycle.

We've also been over this soo many times that I've lost count - you
know this is on my list, I've mentioned it multiple times that this is
now high on the todo list, and your continued nagging does nothing
other than make me want to look at other patches first.  You're not
helping your cause Kees.
Kees Cook June 6, 2024, 6:07 p.m. UTC | #4
On Thu, Jun 06, 2024 at 12:36:03PM -0400, Paul Moore wrote:
> It's in the queue, I've been swamped lately as you'll likely notice I
> haven't really had a chance to process patches yet during this cycle.

I get that you're busy, and I understand that situation -- I get swamped
too. The latency on LSM patch review has been very high lately, and I'm
hoping there's some way we could help. I assume other folks could jump
in and help with the queue[1], but I'm not sure if that would satisfy
your requirements? Other subsystems have been switching more and more to
a group maintainership system, etc. Are there other people you'd be
comfortable sharing the load with? (Currently James and Serge are also
listed as "M:" in MAINTAINERS, but I don't think the 3 of you have a
shared git.kernel.org tree, for example...)

And yes, there are a lot of patches up for review. I'm antsy about this
series in particular because I'm worried inaction is going to create
larger problems for the LSM as a whole. We've already had Linus drop
in and tell us to do better; I'd really like to avoid having him make
unilateral changes to the LSM, especially when we have a solution on
deck that has been reviewed by many people.

I will go back to being anxious/patient... ;)

-Kees

[1] https://patchwork.kernel.org/project/linux-security-module/list/
Paul Moore June 6, 2024, 8:07 p.m. UTC | #5
On Thu, Jun 6, 2024 at 2:07 PM Kees Cook <kees@kernel.org> wrote:
> On Thu, Jun 06, 2024 at 12:36:03PM -0400, Paul Moore wrote:
> > It's in the queue, I've been swamped lately as you'll likely notice I
> > haven't really had a chance to process patches yet during this cycle.
>
> I get that you're busy, and I understand that situation -- I get swamped
> too. The latency on LSM patch review has been very high lately, and I'm
> hoping there's some way we could help. I assume other folks could jump
> in and help with the queue[1], but I'm not sure if that would satisfy
> your requirements? Other subsystems have been switching more and more to
> a group maintainership system, etc. Are there other people you'd be
> comfortable sharing the load with? (Currently James and Serge are also
> listed as "M:" in MAINTAINERS, but I don't think the 3 of you have a
> shared git.kernel.org tree, for example...)

We already have a fairly distributed model with each LSM sending
directly to Linus; the issue we are seeing now is an odd combination
of multiple things: 1) $DAYJOB has had a sudden explosion of high
priority internal work which has siphoned away a good chunk of my time
2) we have seen an historically unusual amount of development at the
LSM layer itself (as opposed to the individual LSMs) 3) we had a long
holiday weekend here in the US and I decided to do what normal people
do and not spend all weekend arguing with people on the Internet.
Without going into details on #1, let's just say it is transient and
not something I expect to be a long term issue (if it does become a
long term issue I'll start looking for a new $DAYJOB).  I suspect
issue #2 is a byproduct of some of the efforts around reinvigorating
LSM development, clearing up some long standing warts, etc. and also
not something I expect to last for an extended period of time.  Issue
#3 is a direct result of ... well, far too many threads like this,
both from well and poorly intentioned authors.

As an aside, things like this typically work better if you have
another email setup so you can do the good-cop/bad-cop bit more
convincingly.

> And yes, there are a lot of patches up for review. I'm antsy about this
> series in particular because I'm worried inaction is going to create
> larger problems for the LSM as a whole. We've already had Linus drop
> in and tell us to do better; I'd really like to avoid having him make
> unilateral changes to the LSM, especially when we have a solution on
> deck that has been reviewed by many people.

You'll note that I'm one of those "many people" that has reviewed it
(and had my feedback ignored for several rounds).  I simply haven't
had the chance to review the latest revision.

In my opinion the LSM's largest issue has nothing to do with code, and
everything to do with the mentality that a hardware flaw is a LSM bug.
If that same mentality wants to ignore decades of work, in a construct
it insisted on, and break the user experience for billions of users
(and entire usage scenarios) in order to satisfy some misdirected
need, then I seriously doubt one patchset is going to solve anything.

> I will go back to being anxious/patient... ;)

"do better" ;)