mbox series

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

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

Message

KP Singh Sept. 22, 2023, 2:55 p.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.

# 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
  bpf: Only enable BPF LSM hooks when an LSM program is attached
  security: Add CONFIG_SECURITY_HOOK_LIKELY

 include/linux/bpf.h       |   1 +
 include/linux/bpf_lsm.h   |   5 +
 include/linux/lsm_count.h | 107 +++++++++++++++++++
 include/linux/lsm_hooks.h |  81 +++++++++++++--
 include/linux/unroll.h    |  36 +++++++
 kernel/bpf/trampoline.c   |  29 +++++-
 security/Kconfig          |  11 ++
 security/bpf/hooks.c      |  25 ++++-
 security/security.c       | 213 +++++++++++++++++++++++++-------------
 9 files changed, 425 insertions(+), 83 deletions(-)
 create mode 100644 include/linux/lsm_count.h
 create mode 100644 include/linux/unroll.h

Comments

Kees Cook Sept. 22, 2023, 3:51 p.m. UTC | #1
On Fri, Sep 22, 2023 at 04:55:00PM +0200, KP Singh wrote:
> # 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

Ship it! ;)

Thanks for continuing to work on this; this is a classic case for
static_call.

-Kees
Mateusz Guzik Sept. 22, 2023, 6:42 p.m. UTC | #2
On Fri, Sep 22, 2023 at 04:55:00PM +0200, 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.
> 

Any plans to further depessimize the state by not calling into these
modules if not configured?

For example Debian has a milipede:
CONFIG_LSM="landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf"

Everything is enabled (but not configured).

In particular tomoyo is quite nasty, rolling with big memsets only to
find it is not even enabled.
KP Singh Sept. 23, 2023, 4:16 p.m. UTC | #3
On Fri, Sep 22, 2023 at 8:42 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
>
> On Fri, Sep 22, 2023 at 04:55:00PM +0200, 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.
> >
>
> Any plans to further depessimize the state by not calling into these
> modules if not configured?
>
> For example Debian has a milipede:
> CONFIG_LSM="landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf"
>
> Everything is enabled (but not configured).

If it's not configured, we won't generate static call slots and even
if they are in the CONFIG_LSM (or lsm=) they are simply ignored.

- KP

>
> In particular tomoyo is quite nasty, rolling with big memsets only to
> find it is not even enabled.
Mateusz Guzik Sept. 23, 2023, 5:13 p.m. UTC | #4
On 9/23/23, KP Singh <kpsingh@kernel.org> wrote:
> On Fri, Sep 22, 2023 at 8:42 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
>>
>> On Fri, Sep 22, 2023 at 04:55:00PM +0200, 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.
>> >
>>
>> Any plans to further depessimize the state by not calling into these
>> modules if not configured?
>>
>> For example Debian has a milipede:
>> CONFIG_LSM="landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf"
>>
>> Everything is enabled (but not configured).
>
> If it's not configured, we won't generate static call slots and even
> if they are in the CONFIG_LSM (or lsm=) they are simply ignored.
>

Maybe there is a terminology mismatch here, so let me be more specific
with tomoyo as an example.

In debian you have:
CONFIG_SECURITY_TOMOYO=y

CONFIG_LSM, as per above, includes it on the list.

At the same time debian does not ship any tooling to configure tomoyo
-- it is compiled into the kernel but not configured to enforce
anything.

On stock kernel this results in tons of calls to
tomoyo_init_request_info, which are quite expensive due to an
avoidable memset thrown in, and which always return
tomoyo_init_request_info.

Does not look like your patch whacks this problem.
Mateusz Guzik Sept. 23, 2023, 5:15 p.m. UTC | #5
On 9/23/23, Mateusz Guzik <mjguzik@gmail.com> wrote:
> On 9/23/23, KP Singh <kpsingh@kernel.org> wrote:
>> On Fri, Sep 22, 2023 at 8:42 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
>>>
>>> On Fri, Sep 22, 2023 at 04:55:00PM +0200, 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.
>>> >
>>>
>>> Any plans to further depessimize the state by not calling into these
>>> modules if not configured?
>>>
>>> For example Debian has a milipede:
>>> CONFIG_LSM="landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf"
>>>
>>> Everything is enabled (but not configured).
>>
>> If it's not configured, we won't generate static call slots and even
>> if they are in the CONFIG_LSM (or lsm=) they are simply ignored.
>>
>
> Maybe there is a terminology mismatch here, so let me be more specific
> with tomoyo as an example.
>
> In debian you have:
> CONFIG_SECURITY_TOMOYO=y
>
> CONFIG_LSM, as per above, includes it on the list.
>
> At the same time debian does not ship any tooling to configure tomoyo
> -- it is compiled into the kernel but not configured to enforce
> anything.
>
> On stock kernel this results in tons of calls to
> tomoyo_init_request_info, which are quite expensive due to an
> avoidable memset thrown in, and which always return
> tomoyo_init_request_info.
>

Erm, which always return TOMOYO_CONFIG_DISABLED.

> Does not look like your patch whacks this problem.
>

So I am asking if there are plans to make these modules get out of the
way if they have nothing to do, like tomoyo in the example above.

Of course preferably distros would not make these weird configs, but I
suspect this ship has sailed.
Kees Cook Sept. 24, 2023, 2:46 a.m. UTC | #6
On Sat, Sep 23, 2023 at 07:15:05PM +0200, Mateusz Guzik wrote:
> On 9/23/23, Mateusz Guzik <mjguzik@gmail.com> wrote:
> > On 9/23/23, KP Singh <kpsingh@kernel.org> wrote:
> >> On Fri, Sep 22, 2023 at 8:42 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
> >>>
> >>> On Fri, Sep 22, 2023 at 04:55:00PM +0200, 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.
> >>> >
> >>>
> >>> Any plans to further depessimize the state by not calling into these
> >>> modules if not configured?
> >>>
> >>> For example Debian has a milipede:
> >>> CONFIG_LSM="landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf"
> >>>
> >>> Everything is enabled (but not configured).
> >>
> >> If it's not configured, we won't generate static call slots and even
> >> if they are in the CONFIG_LSM (or lsm=) they are simply ignored.
> >>
> >
> > Maybe there is a terminology mismatch here, so let me be more specific
> > with tomoyo as an example.
> >
> > In debian you have:
> > CONFIG_SECURITY_TOMOYO=y
> >
> > CONFIG_LSM, as per above, includes it on the list.
> >
> > At the same time debian does not ship any tooling to configure tomoyo
> > -- it is compiled into the kernel but not configured to enforce
> > anything.
> >
> > On stock kernel this results in tons of calls to
> > tomoyo_init_request_info, which are quite expensive due to an
> > avoidable memset thrown in, and which always return
> > tomoyo_init_request_info.
> >
> 
> Erm, which always return TOMOYO_CONFIG_DISABLED.
> 
> > Does not look like your patch whacks this problem.
> >
> 
> So I am asking if there are plans to make these modules get out of the
> way if they have nothing to do, like tomoyo in the example above.
> 
> Of course preferably distros would not make these weird configs, but I
> suspect this ship has sailed.

This is an artifact of the existing stacking behavior (and solving it,
if needed, can be done in parallel to this series). Specifically it
seems Tomoyo is in the "lsm=" list when it shouldn't be.

That said, I've long advocated[1] for a way to explicitly disable LSMs
without affecting operational ordering. I think it would be very nice to
be able to boot with something like:

lsm=!yama

to disable Yama. Or for your case, "lsm=!tomoyo". Right now, you have to
figure out what the lsm list is, and then create a new one with the
LSM you want disabled removed from the list. i.e. with v6.2 and later
check the boot log, and you'll see:

LSM: initializing lsm=lockdown,capability,landlock,yama,integrity,apparmor

If you wanted to boot with Yama removed, you'd then pass:

	lsm=lockdown,capability,landlock,integrity,apparmor

As a boot param. But I think this is fragile since now any new LSMs will
be by-default disabled once a sysadmin overrides the "lsm" list. Note
that booting with "lsm.debug=1" will show even more details. See commit
86ef3c735ec8 ("LSM: Better reporting of actual LSMs at boot").

So, if a distro has no support for an LSM but they want it _available_
in the kernel, they should leave it built in, but remove it from the
"lsm=" list. That's a reasonable bug to file against a distro...

-Kees

[1] https://lore.kernel.org/linux-security-module/202210171111.21E3983165@keescook/
Mateusz Guzik Sept. 25, 2023, 8:08 p.m. UTC | #7
On 9/24/23, Kees Cook <keescook@chromium.org> wrote:
> That said, I've long advocated[1] for a way to explicitly disable LSMs
> without affecting operational ordering. I think it would be very nice to
> be able to boot with something like:
>
> lsm=!yama
>
> to disable Yama. Or for your case, "lsm=!tomoyo". Right now, you have to
> figure out what the lsm list is, and then create a new one with the
> LSM you want disabled removed from the list. i.e. with v6.2 and later
> check the boot log, and you'll see:
>
> LSM: initializing lsm=lockdown,capability,landlock,yama,integrity,apparmor
>
> If you wanted to boot with Yama removed, you'd then pass:
>
> 	lsm=lockdown,capability,landlock,integrity,apparmor
>
> As a boot param. But I think this is fragile since now any new LSMs will
> be by-default disabled once a sysadmin overrides the "lsm" list. Note
> that booting with "lsm.debug=1" will show even more details. See commit
> 86ef3c735ec8 ("LSM: Better reporting of actual LSMs at boot").
>
> So, if a distro has no support for an LSM but they want it _available_
> in the kernel, they should leave it built in, but remove it from the
> "lsm=" list. That's a reasonable bug to file against a distro...
>

Maybe I once more expressed myself poorly, I meant to say stock Debian
does not ship any tooling for tomoyo, but the kernel has support
compiled in.

Ultimately, after stacking got implemented, it was inevitable diestros
like Debian will enable whatever modules and expect them to not be a
problem if not configured by userspace.

I don't think any form of messing with CONFIG_LSM is a viable option,
even if you make it a boot param.

What should happen instead is that modules which are not given any
config don't get in the way.
Kees Cook Sept. 25, 2023, 10:02 p.m. UTC | #8
On Mon, Sep 25, 2023 at 10:08:39PM +0200, Mateusz Guzik wrote:
> On 9/24/23, Kees Cook <keescook@chromium.org> wrote:
> > That said, I've long advocated[1] for a way to explicitly disable LSMs
> > without affecting operational ordering. I think it would be very nice to
> > be able to boot with something like:
> >
> > lsm=!yama
> >
> > to disable Yama. Or for your case, "lsm=!tomoyo". Right now, you have to
> > figure out what the lsm list is, and then create a new one with the
> > LSM you want disabled removed from the list. i.e. with v6.2 and later
> > check the boot log, and you'll see:
> >
> > LSM: initializing lsm=lockdown,capability,landlock,yama,integrity,apparmor
> >
> > If you wanted to boot with Yama removed, you'd then pass:
> >
> > 	lsm=lockdown,capability,landlock,integrity,apparmor
> >
> > As a boot param. But I think this is fragile since now any new LSMs will
> > be by-default disabled once a sysadmin overrides the "lsm" list. Note
> > that booting with "lsm.debug=1" will show even more details. See commit
> > 86ef3c735ec8 ("LSM: Better reporting of actual LSMs at boot").
> >
> > So, if a distro has no support for an LSM but they want it _available_
> > in the kernel, they should leave it built in, but remove it from the
> > "lsm=" list. That's a reasonable bug to file against a distro...
> >
> 
> Maybe I once more expressed myself poorly, I meant to say stock Debian
> does not ship any tooling for tomoyo, but the kernel has support
> compiled in.

If there is no tooling Debian should either not build the support into
the kernel or should leave it out of the CONFIG_LSM list.

> Ultimately, after stacking got implemented, it was inevitable diestros
> like Debian will enable whatever modules and expect them to not be a
> problem if not configured by userspace.
> 
> I don't think any form of messing with CONFIG_LSM is a viable option,
> even if you make it a boot param.
> 
> What should happen instead is that modules which are not given any
> config don't get in the way.

Right -- this is an open problem, and I think we can solve it using the
static_call system (much like how the BPF LSM is doing it).

-Kees