diff mbox series

[v5,4/5] bpf: Only enable BPF LSM hooks when an LSM program is attached

Message ID 20230928202410.3765062-5-kpsingh@kernel.org (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series Reduce overhead of LSMs with static calls | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-VM_Test-29 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-30 success Logs for veristat
netdev/tree_selection success Not a local patch, async
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-0 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-1 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-11 fail Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-14 fail Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 fail Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-12 fail Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 fail Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-16 fail Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 fail Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-15 fail Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-28 success Logs for veristat

Commit Message

KP Singh Sept. 28, 2023, 8:24 p.m. UTC 
BPF LSM hooks have side-effects (even when a default value is returned),
as some hooks end up behaving differently due to the very presence of
the hook.

The static keys guarding the BPF LSM hooks are disabled by default and
enabled only when a BPF program is attached implementing the hook
logic. This avoids the issue of the side-effects and also the minor
overhead associated with the empty callback.

security_file_ioctl:
   0xffffffff818f0e30 <+0>:	endbr64
   0xffffffff818f0e34 <+4>:	nopl   0x0(%rax,%rax,1)
   0xffffffff818f0e39 <+9>:	push   %rbp
   0xffffffff818f0e3a <+10>:	push   %r14
   0xffffffff818f0e3c <+12>:	push   %rbx
   0xffffffff818f0e3d <+13>:	mov    %rdx,%rbx
   0xffffffff818f0e40 <+16>:	mov    %esi,%ebp
   0xffffffff818f0e42 <+18>:	mov    %rdi,%r14
   0xffffffff818f0e45 <+21>:	jmp    0xffffffff818f0e57 <security_file_ioctl+39>
   				^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

   Static key enabled for SELinux

   0xffffffff818f0e47 <+23>:	xchg   %ax,%ax
   				^^^^^^^^^^^^^^

   Static key disabled for BPF. This gets patched when a BPF LSM program
   is attached

   0xffffffff818f0e49 <+25>:	xor    %eax,%eax
   0xffffffff818f0e4b <+27>:	xchg   %ax,%ax
   0xffffffff818f0e4d <+29>:	pop    %rbx
   0xffffffff818f0e4e <+30>:	pop    %r14
   0xffffffff818f0e50 <+32>:	pop    %rbp
   0xffffffff818f0e51 <+33>:	cs jmp 0xffffffff82c00000 <__x86_return_thunk>
   0xffffffff818f0e57 <+39>:	endbr64
   0xffffffff818f0e5b <+43>:	mov    %r14,%rdi
   0xffffffff818f0e5e <+46>:	mov    %ebp,%esi
   0xffffffff818f0e60 <+48>:	mov    %rbx,%rdx
   0xffffffff818f0e63 <+51>:	call   0xffffffff819033c0 <selinux_file_ioctl>
   0xffffffff818f0e68 <+56>:	test   %eax,%eax
   0xffffffff818f0e6a <+58>:	jne    0xffffffff818f0e4d <security_file_ioctl+29>
   0xffffffff818f0e6c <+60>:	jmp    0xffffffff818f0e47 <security_file_ioctl+23>
   0xffffffff818f0e6e <+62>:	endbr64
   0xffffffff818f0e72 <+66>:	mov    %r14,%rdi
   0xffffffff818f0e75 <+69>:	mov    %ebp,%esi
   0xffffffff818f0e77 <+71>:	mov    %rbx,%rdx
   0xffffffff818f0e7a <+74>:	call   0xffffffff8141e3b0 <bpf_lsm_file_ioctl>
   0xffffffff818f0e7f <+79>:	test   %eax,%eax
   0xffffffff818f0e81 <+81>:	jne    0xffffffff818f0e4d <security_file_ioctl+29>
   0xffffffff818f0e83 <+83>:	jmp    0xffffffff818f0e49 <security_file_ioctl+25>
   0xffffffff818f0e85 <+85>:	endbr64
   0xffffffff818f0e89 <+89>:	mov    %r14,%rdi
   0xffffffff818f0e8c <+92>:	mov    %ebp,%esi
   0xffffffff818f0e8e <+94>:	mov    %rbx,%rdx
   0xffffffff818f0e91 <+97>:	pop    %rbx
   0xffffffff818f0e92 <+98>:	pop    %r14
   0xffffffff818f0e94 <+100>:	pop    %rbp
   0xffffffff818f0e95 <+101>:	ret

Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
Acked-by: Song Liu <song@kernel.org>
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 include/linux/bpf.h       |  1 +
 include/linux/bpf_lsm.h   |  5 +++++
 include/linux/lsm_hooks.h | 13 ++++++++++++-
 kernel/bpf/trampoline.c   | 29 +++++++++++++++++++++++++++--
 security/bpf/hooks.c      | 25 ++++++++++++++++++++++++-
 security/security.c       |  3 ++-
 6 files changed, 71 insertions(+), 5 deletions(-)

Comments

Jiri Olsa Oct. 5, 2023, 8:09 a.m. UTC | #1 
On Thu, Sep 28, 2023 at 10:24:09PM +0200, KP Singh wrote:

SNIP

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index e97aeda3a86b..df9699bce372 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -13,6 +13,7 @@
>  #include <linux/bpf_verifier.h>
>  #include <linux/bpf_lsm.h>
>  #include <linux/delay.h>
> +#include <linux/bpf_lsm.h>
>  
>  /* dummy _ops. The verifier will operate on target program's ops. */
>  const struct bpf_verifier_ops bpf_extension_verifier_ops = {
> @@ -514,7 +515,7 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
>  {
>  	enum bpf_tramp_prog_type kind;
>  	struct bpf_tramp_link *link_exiting;
> -	int err = 0;
> +	int err = 0, num_lsm_progs = 0;
>  	int cnt = 0, i;
>  
>  	kind = bpf_attach_type_to_tramp(link->link.prog);
> @@ -545,8 +546,14 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
>  			continue;
>  		/* prog already linked */
>  		return -EBUSY;
> +
> +		if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> +			num_lsm_progs++;

this looks wrong, it's never reached.. seems like we should add separate
hlist_for_each_entry loop over trampoline's links for this check/init of
num_lsm_progs ?

jirka

>  	}
>  
> +	if (!num_lsm_progs && link->link.prog->type == BPF_PROG_TYPE_LSM)
> +		bpf_lsm_toggle_hook(tr->func.addr, true);
> +
>  	hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]);
>  	tr->progs_cnt[kind]++;
>  	err = bpf_trampoline_update(tr, true /* lock_direct_mutex */);
> @@ -569,8 +576,10 @@ int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline
>  
>  static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr)
>  {
> +	struct bpf_tramp_link *link_exiting;
>  	enum bpf_tramp_prog_type kind;
> -	int err;
> +	bool lsm_link_found = false;
> +	int err, num_lsm_progs = 0;
>  
>  	kind = bpf_attach_type_to_tramp(link->link.prog);
>  	if (kind == BPF_TRAMP_REPLACE) {
> @@ -580,8 +589,24 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link, struct bpf_
>  		tr->extension_prog = NULL;
>  		return err;
>  	}
> +
> +	if (link->link.prog->type == BPF_PROG_TYPE_LSM) {
> +		hlist_for_each_entry(link_exiting, &tr->progs_hlist[kind],
> +				     tramp_hlist) {
> +			if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> +				num_lsm_progs++;
> +
> +			if (link_exiting->link.prog == link->link.prog)
> +				lsm_link_found = true;
> +		}
> +	}
> +
>  	hlist_del_init(&link->tramp_hlist);
>  	tr->progs_cnt[kind]--;
> +
> +	if (lsm_link_found && num_lsm_progs == 1)
> +		bpf_lsm_toggle_hook(tr->func.addr, false);
> +
>  	return bpf_trampoline_update(tr, true /* lock_direct_mutex */);
>  }
>  
> diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
> index cfaf1d0e6a5f..1957244196d0 100644
> --- a/security/bpf/hooks.c
> +++ b/security/bpf/hooks.c
> @@ -8,7 +8,7 @@
>  
>  static struct security_hook_list bpf_lsm_hooks[] __ro_after_init = {
>  	#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
> -	LSM_HOOK_INIT(NAME, bpf_lsm_##NAME),
> +	LSM_HOOK_INIT_DISABLED(NAME, bpf_lsm_##NAME),
>  	#include <linux/lsm_hook_defs.h>
>  	#undef LSM_HOOK
>  	LSM_HOOK_INIT(inode_free_security, bpf_inode_storage_free),
> @@ -32,3 +32,26 @@ DEFINE_LSM(bpf) = {
>  	.init = bpf_lsm_init,
>  	.blobs = &bpf_lsm_blob_sizes
>  };
> +
> +void bpf_lsm_toggle_hook(void *addr, bool value)
> +{
> +	struct lsm_static_call *scalls;
> +	struct security_hook_list *h;
> +	int i, j;
> +
> +	for (i = 0; i < ARRAY_SIZE(bpf_lsm_hooks); i++) {
> +		h = &bpf_lsm_hooks[i];
> +		scalls = h->scalls;
> +		if (h->hook.lsm_callback == addr)
> +			continue;
> +
> +		for (j = 0; j < MAX_LSM_COUNT; j++) {
> +			if (scalls[j].hl != h)
> +				continue;
> +			if (value)
> +				static_branch_enable(scalls[j].active);
> +			else
> +				static_branch_disable(scalls[j].active);
> +		}
> +	}
> +}
> diff --git a/security/security.c b/security/security.c
> index c2c2cf6b711f..d1ee72e563cc 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -382,7 +382,8 @@ static void __init lsm_static_call_init(struct security_hook_list *hl)
>  			__static_call_update(scall->key, scall->trampoline,
>  					     hl->hook.lsm_callback);
>  			scall->hl = hl;
> -			static_branch_enable(scall->active);
> +			if (hl->default_state)
> +				static_branch_enable(scall->active);
>  			return;
>  		}
>  		scall++;
> -- 
> 2.42.0.582.g8ccd20d70d-goog
> 
>
KP Singh Oct. 5, 2023, 1:26 p.m. UTC | #2 
On Thu, Oct 5, 2023 at 10:09 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Sep 28, 2023 at 10:24:09PM +0200, KP Singh wrote:
>
> SNIP
>
> > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > index e97aeda3a86b..df9699bce372 100644
> > --- a/kernel/bpf/trampoline.c
> > +++ b/kernel/bpf/trampoline.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/bpf_verifier.h>
> >  #include <linux/bpf_lsm.h>
> >  #include <linux/delay.h>
> > +#include <linux/bpf_lsm.h>
> >
> >  /* dummy _ops. The verifier will operate on target program's ops. */
> >  const struct bpf_verifier_ops bpf_extension_verifier_ops = {
> > @@ -514,7 +515,7 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
> >  {
> >       enum bpf_tramp_prog_type kind;
> >       struct bpf_tramp_link *link_exiting;
> > -     int err = 0;
> > +     int err = 0, num_lsm_progs = 0;
> >       int cnt = 0, i;
> >
> >       kind = bpf_attach_type_to_tramp(link->link.prog);
> > @@ -545,8 +546,14 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
> >                       continue;
> >               /* prog already linked */
> >               return -EBUSY;
> > +
> > +             if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> > +                     num_lsm_progs++;
>
> this looks wrong, it's never reached.. seems like we should add separate
> hlist_for_each_entry loop over trampoline's links for this check/init of
> num_lsm_progs ?
>
> jirka

Good catch, I missed this during my rebase, after
https://lore.kernel.org/bpf/20220510205923.3206889-2-kuifeng@fb.com/
this condition is basically never reached. I will do a general loop
over to count LSM programs and toggle the hook to true (and same for
unlink).

- KP

[...]
KP Singh Oct. 5, 2023, 1:27 p.m. UTC | #3 
On Thu, Oct 5, 2023 at 3:26 PM KP Singh <kpsingh@kernel.org> wrote:
>
> On Thu, Oct 5, 2023 at 10:09 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Thu, Sep 28, 2023 at 10:24:09PM +0200, KP Singh wrote:
> >
> > SNIP
> >
> > > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > > index e97aeda3a86b..df9699bce372 100644
> > > --- a/kernel/bpf/trampoline.c
> > > +++ b/kernel/bpf/trampoline.c
> > > @@ -13,6 +13,7 @@
> > >  #include <linux/bpf_verifier.h>
> > >  #include <linux/bpf_lsm.h>
> > >  #include <linux/delay.h>
> > > +#include <linux/bpf_lsm.h>
> > >
> > >  /* dummy _ops. The verifier will operate on target program's ops. */
> > >  const struct bpf_verifier_ops bpf_extension_verifier_ops = {
> > > @@ -514,7 +515,7 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
> > >  {
> > >       enum bpf_tramp_prog_type kind;
> > >       struct bpf_tramp_link *link_exiting;

I think this is a typo here. It should be existing, no?

> > > -     int err = 0;
> > > +     int err = 0, num_lsm_progs = 0;
> > >       int cnt = 0, i;
> > >
> > >       kind = bpf_attach_type_to_tramp(link->link.prog);
> > > @@ -545,8 +546,14 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
> > >                       continue;
> > >               /* prog already linked */
> > >               return -EBUSY;
> > > +
> > > +             if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> > > +                     num_lsm_progs++;
> >
> > this looks wrong, it's never reached.. seems like we should add separate
> > hlist_for_each_entry loop over trampoline's links for this check/init of
> > num_lsm_progs ?
> >
> > jirka
>
> Good catch, I missed this during my rebase, after
> https://lore.kernel.org/bpf/20220510205923.3206889-2-kuifeng@fb.com/
> this condition is basically never reached. I will do a general loop
> over to count LSM programs and toggle the hook to true (and same for
> unlink).
>
> - KP
>
> [...]
Jiri Olsa Oct. 5, 2023, 1:52 p.m. UTC | #4 
On Thu, Oct 05, 2023 at 03:27:35PM +0200, KP Singh wrote:
> On Thu, Oct 5, 2023 at 3:26 PM KP Singh <kpsingh@kernel.org> wrote:
> >
> > On Thu, Oct 5, 2023 at 10:09 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Thu, Sep 28, 2023 at 10:24:09PM +0200, KP Singh wrote:
> > >
> > > SNIP
> > >
> > > > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > > > index e97aeda3a86b..df9699bce372 100644
> > > > --- a/kernel/bpf/trampoline.c
> > > > +++ b/kernel/bpf/trampoline.c
> > > > @@ -13,6 +13,7 @@
> > > >  #include <linux/bpf_verifier.h>
> > > >  #include <linux/bpf_lsm.h>
> > > >  #include <linux/delay.h>
> > > > +#include <linux/bpf_lsm.h>
> > > >
> > > >  /* dummy _ops. The verifier will operate on target program's ops. */
> > > >  const struct bpf_verifier_ops bpf_extension_verifier_ops = {
> > > > @@ -514,7 +515,7 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
> > > >  {
> > > >       enum bpf_tramp_prog_type kind;
> > > >       struct bpf_tramp_link *link_exiting;
> 
> I think this is a typo here. It should be existing, no?

yes, I was wondering about that as well ;-)

jirka

> 
> > > > -     int err = 0;
> > > > +     int err = 0, num_lsm_progs = 0;
> > > >       int cnt = 0, i;
> > > >
> > > >       kind = bpf_attach_type_to_tramp(link->link.prog);
> > > > @@ -545,8 +546,14 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
> > > >                       continue;
> > > >               /* prog already linked */
> > > >               return -EBUSY;
> > > > +
> > > > +             if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> > > > +                     num_lsm_progs++;
> > >
> > > this looks wrong, it's never reached.. seems like we should add separate
> > > hlist_for_each_entry loop over trampoline's links for this check/init of
> > > num_lsm_progs ?
> > >
> > > jirka
> >
> > Good catch, I missed this during my rebase, after
> > https://lore.kernel.org/bpf/20220510205923.3206889-2-kuifeng@fb.com/
> > this condition is basically never reached. I will do a general loop
> > over to count LSM programs and toggle the hook to true (and same for
> > unlink).
> >
> > - KP
> >
> > [...]
KP Singh Oct. 5, 2023, 4:07 p.m. UTC | #5 
On Thu, Oct 5, 2023 at 3:52 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Oct 05, 2023 at 03:27:35PM +0200, KP Singh wrote:
> > On Thu, Oct 5, 2023 at 3:26 PM KP Singh <kpsingh@kernel.org> wrote:
> > >
> > > On Thu, Oct 5, 2023 at 10:09 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > >
> > > > On Thu, Sep 28, 2023 at 10:24:09PM +0200, KP Singh wrote:
> > > >
> > > > SNIP
> > > >
> > > > > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > > > > index e97aeda3a86b..df9699bce372 100644
> > > > > --- a/kernel/bpf/trampoline.c
> > > > > +++ b/kernel/bpf/trampoline.c
> > > > > @@ -13,6 +13,7 @@
> > > > >  #include <linux/bpf_verifier.h>
> > > > >  #include <linux/bpf_lsm.h>
> > > > >  #include <linux/delay.h>
> > > > > +#include <linux/bpf_lsm.h>
> > > > >
> > > > >  /* dummy _ops. The verifier will operate on target program's ops. */
> > > > >  const struct bpf_verifier_ops bpf_extension_verifier_ops = {
> > > > > @@ -514,7 +515,7 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
> > > > >  {
> > > > >       enum bpf_tramp_prog_type kind;
> > > > >       struct bpf_tramp_link *link_exiting;
> >
> > I think this is a typo here. It should be existing, no?
>
> yes, I was wondering about that as well ;-)
>
> jirka
>
> >
> > > > > -     int err = 0;
> > > > > +     int err = 0, num_lsm_progs = 0;
> > > > >       int cnt = 0, i;
> > > > >
> > > > >       kind = bpf_attach_type_to_tramp(link->link.prog);
> > > > > @@ -545,8 +546,14 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
> > > > >                       continue;
> > > > >               /* prog already linked */
> > > > >               return -EBUSY;
> > > > > +
> > > > > +             if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> > > > > +                     num_lsm_progs++;
> > > >
> > > > this looks wrong, it's never reached.. seems like we should add separate
> > > > hlist_for_each_entry loop over trampoline's links for this check/init of
> > > > num_lsm_progs ?
> > > >
> > > > jirka
> > >
> > > Good catch, I missed this during my rebase, after
> > > https://lore.kernel.org/bpf/20220510205923.3206889-2-kuifeng@fb.com/
> > > this condition is basically never reached. I will do a general loop
> > > over to count LSM programs and toggle the hook to true (and same for
> > > unlink).

So, there is something that is unclear about this code, i.e. what
happens when there is an error from bpf_trampoline_update fails and
the link and unlink seem to have different expectations:

* link seems to go back to the linked list and removes the trampoline
and restores the refcount:

[...]
 err = bpf_trampoline_update(tr, true /* lock_direct_mutex */);
 if (err) {
    hlist_del_init(&link->tramp_hlist);
    tr->progs_cnt[kind]--;
  }
 return err;
}


* unlink does restore the side effect (i.e. it does not put the
removed trampoline back and increments the refcount).

hlist_del_init(&link->tramp_hlist);
tr->progs_cnt[kind]--;
return bpf_trampoline_update(tr, true /* lock_direct_mutex */);

However, I think I will make it simpler and enforce the invariant that
if an LSM program is attached, the hook is enabled and vice versa. How
about:

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index df9699bce372..4f31384b5637 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -511,11 +511,30 @@ static enum bpf_tramp_prog_type
bpf_attach_type_to_tramp(struct bpf_prog *prog)
        }
 }

+static void bpf_trampoline_toggle_lsm(struct bpf_trampoline *tr,
+                                     enum bpf_tramp_prog_type kind)
+{
+       struct bpf_tramp_link *link;
+       volatile bool found = false;
+
+       /* Loop through the links and if any LSM program is attached, ensure
+        * that the hook is enabled.
+        */
+       hlist_for_each_entry(link, &tr->progs_hlist[kind], tramp_hlist) {
+               if (link->link.prog->type == BPF_PROG_TYPE_LSM) {
+                       found  = true;
+                       break;
+               }
+       }
+
+       bpf_lsm_toggle_hook(tr->func.addr, found);
+}
+
 static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link,
struct bpf_trampoline *tr)
 {
        enum bpf_tramp_prog_type kind;
        struct bpf_tramp_link *link_exiting;
-       int err = 0, num_lsm_progs = 0;
+       int err = 0;
        int cnt = 0, i;

        kind = bpf_attach_type_to_tramp(link->link.prog);
@@ -547,15 +566,14 @@ static int __bpf_trampoline_link_prog(struct
bpf_tramp_link *link, struct bpf_tr
                /* prog already linked */
                return -EBUSY;

-               if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
-                       num_lsm_progs++;
        }

-       if (!num_lsm_progs && link->link.prog->type == BPF_PROG_TYPE_LSM)
-               bpf_lsm_toggle_hook(tr->func.addr, true);
-
        hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]);
        tr->progs_cnt[kind]++;
+
+       if (link->link.prog->type == BPF_PROG_TYPE_LSM)
+               bpf_trampoline_toggle_lsm(tr, kind);
+
        err = bpf_trampoline_update(tr, true /* lock_direct_mutex */);
        if (err) {
                hlist_del_init(&link->tramp_hlist);
@@ -578,7 +596,6 @@ static int __bpf_trampoline_unlink_prog(struct
bpf_tramp_link *link, struct bpf_
 {
        struct bpf_tramp_link *link_exiting;
        enum bpf_tramp_prog_type kind;
-       bool lsm_link_found = false;
        int err, num_lsm_progs = 0;

        kind = bpf_attach_type_to_tramp(link->link.prog);
@@ -595,18 +612,14 @@ static int __bpf_trampoline_unlink_prog(struct
bpf_tramp_link *link, struct bpf_
                                     tramp_hlist) {
                        if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
                                num_lsm_progs++;
-
-                       if (link_exiting->link.prog == link->link.prog)
-                               lsm_link_found = true;
                }
        }

        hlist_del_init(&link->tramp_hlist);
        tr->progs_cnt[kind]--;

-       if (lsm_link_found && num_lsm_progs == 1)
-               bpf_lsm_toggle_hook(tr->func.addr, false);
-
+       if (link->link.prog->type == BPF_PROG_TYPE_LSM)
+               bpf_trampoline_toggle_lsm(tr, kind);
        return bpf_trampoline_update(tr, true /* lock_direct_mutex */);
 }


- KP
Jiri Olsa Oct. 6, 2023, 7:27 a.m. UTC | #6 
On Thu, Oct 05, 2023 at 06:07:28PM +0200, KP Singh wrote:

SNIP

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index df9699bce372..4f31384b5637 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -511,11 +511,30 @@ static enum bpf_tramp_prog_type
> bpf_attach_type_to_tramp(struct bpf_prog *prog)
>         }
>  }
> 
> +static void bpf_trampoline_toggle_lsm(struct bpf_trampoline *tr,
> +                                     enum bpf_tramp_prog_type kind)
> +{
> +       struct bpf_tramp_link *link;
> +       volatile bool found = false;
> +
> +       /* Loop through the links and if any LSM program is attached, ensure
> +        * that the hook is enabled.
> +        */
> +       hlist_for_each_entry(link, &tr->progs_hlist[kind], tramp_hlist) {
> +               if (link->link.prog->type == BPF_PROG_TYPE_LSM) {
> +                       found  = true;
> +                       break;
> +               }
> +       }
> +
> +       bpf_lsm_toggle_hook(tr->func.addr, found);
> +}
> +
>  static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link,
> struct bpf_trampoline *tr)
>  {
>         enum bpf_tramp_prog_type kind;
>         struct bpf_tramp_link *link_exiting;
> -       int err = 0, num_lsm_progs = 0;
> +       int err = 0;
>         int cnt = 0, i;
> 
>         kind = bpf_attach_type_to_tramp(link->link.prog);
> @@ -547,15 +566,14 @@ static int __bpf_trampoline_link_prog(struct
> bpf_tramp_link *link, struct bpf_tr
>                 /* prog already linked */
>                 return -EBUSY;
> 
> -               if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> -                       num_lsm_progs++;
>         }
> 
> -       if (!num_lsm_progs && link->link.prog->type == BPF_PROG_TYPE_LSM)
> -               bpf_lsm_toggle_hook(tr->func.addr, true);
> -
>         hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]);
>         tr->progs_cnt[kind]++;
> +
> +       if (link->link.prog->type == BPF_PROG_TYPE_LSM)
> +               bpf_trampoline_toggle_lsm(tr, kind);

how about keeping BPF_PROG_TYPE_LSM progs type count of attached programs
in bpf_trampoline and toggle lsm on first coming in and last going out?

also the trampoline attach is actually made in bpf_trampoline_update,
so I wonder it'd make more sense to put it in there, but it's already
complicated, so it actually might be easier in here

jirka

> +
>         err = bpf_trampoline_update(tr, true /* lock_direct_mutex */);
>         if (err) {
>                 hlist_del_init(&link->tramp_hlist);
> @@ -578,7 +596,6 @@ static int __bpf_trampoline_unlink_prog(struct
> bpf_tramp_link *link, struct bpf_
>  {
>         struct bpf_tramp_link *link_exiting;
>         enum bpf_tramp_prog_type kind;
> -       bool lsm_link_found = false;
>         int err, num_lsm_progs = 0;
> 
>         kind = bpf_attach_type_to_tramp(link->link.prog);
> @@ -595,18 +612,14 @@ static int __bpf_trampoline_unlink_prog(struct
> bpf_tramp_link *link, struct bpf_
>                                      tramp_hlist) {
>                         if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
>                                 num_lsm_progs++;
> -
> -                       if (link_exiting->link.prog == link->link.prog)
> -                               lsm_link_found = true;
>                 }
>         }
> 
>         hlist_del_init(&link->tramp_hlist);
>         tr->progs_cnt[kind]--;
> 
> -       if (lsm_link_found && num_lsm_progs == 1)
> -               bpf_lsm_toggle_hook(tr->func.addr, false);
> -
> +       if (link->link.prog->type == BPF_PROG_TYPE_LSM)
> +               bpf_trampoline_toggle_lsm(tr, kind);
>         return bpf_trampoline_update(tr, true /* lock_direct_mutex */);
>  }
> 
> 
> - KP
Jiri Olsa Oct. 6, 2023, 9:05 a.m. UTC | #7 
On Fri, Oct 06, 2023 at 09:27:57AM +0200, Jiri Olsa wrote:

SNIP

> >  static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link,
> > struct bpf_trampoline *tr)
> >  {
> >         enum bpf_tramp_prog_type kind;
> >         struct bpf_tramp_link *link_exiting;
> > -       int err = 0, num_lsm_progs = 0;
> > +       int err = 0;
> >         int cnt = 0, i;
> > 
> >         kind = bpf_attach_type_to_tramp(link->link.prog);
> > @@ -547,15 +566,14 @@ static int __bpf_trampoline_link_prog(struct
> > bpf_tramp_link *link, struct bpf_tr
> >                 /* prog already linked */
> >                 return -EBUSY;
> > 
> > -               if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> > -                       num_lsm_progs++;
> >         }
> > 
> > -       if (!num_lsm_progs && link->link.prog->type == BPF_PROG_TYPE_LSM)
> > -               bpf_lsm_toggle_hook(tr->func.addr, true);
> > -
> >         hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]);
> >         tr->progs_cnt[kind]++;
> > +
> > +       if (link->link.prog->type == BPF_PROG_TYPE_LSM)
> > +               bpf_trampoline_toggle_lsm(tr, kind);
> 
> how about keeping BPF_PROG_TYPE_LSM progs type count of attached programs
> in bpf_trampoline and toggle lsm on first coming in and last going out?

hm we actually allow other tracing program types to attach to bpf_lsm_*
functions, so I wonder we should toggle the lsm hook for each program
type (for bpf_lsm_* trampolines) because they'd expect the hook is called

but I'm not sure it's a valid use case to have like normal fentry program
attached to bpf_lsm_XXX function

jirka

> 
> also the trampoline attach is actually made in bpf_trampoline_update,
> so I wonder it'd make more sense to put it in there, but it's already
> complicated, so it actually might be easier in here
> 
> jirka
> 
> > +
> >         err = bpf_trampoline_update(tr, true /* lock_direct_mutex */);
> >         if (err) {
> >                 hlist_del_init(&link->tramp_hlist);
> > @@ -578,7 +596,6 @@ static int __bpf_trampoline_unlink_prog(struct
> > bpf_tramp_link *link, struct bpf_
> >  {
> >         struct bpf_tramp_link *link_exiting;
> >         enum bpf_tramp_prog_type kind;
> > -       bool lsm_link_found = false;
> >         int err, num_lsm_progs = 0;
> > 
> >         kind = bpf_attach_type_to_tramp(link->link.prog);
> > @@ -595,18 +612,14 @@ static int __bpf_trampoline_unlink_prog(struct
> > bpf_tramp_link *link, struct bpf_
> >                                      tramp_hlist) {
> >                         if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> >                                 num_lsm_progs++;
> > -
> > -                       if (link_exiting->link.prog == link->link.prog)
> > -                               lsm_link_found = true;
> >                 }
> >         }
> > 
> >         hlist_del_init(&link->tramp_hlist);
> >         tr->progs_cnt[kind]--;
> > 
> > -       if (lsm_link_found && num_lsm_progs == 1)
> > -               bpf_lsm_toggle_hook(tr->func.addr, false);
> > -
> > +       if (link->link.prog->type == BPF_PROG_TYPE_LSM)
> > +               bpf_trampoline_toggle_lsm(tr, kind);
> >         return bpf_trampoline_update(tr, true /* lock_direct_mutex */);
> >  }
> > 
> > 
> > - KP
KP Singh Oct. 6, 2023, 10:57 a.m. UTC | #8 
On Fri, Oct 6, 2023 at 11:05 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Fri, Oct 06, 2023 at 09:27:57AM +0200, Jiri Olsa wrote:
>
> SNIP
>
> > >  static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link,
> > > struct bpf_trampoline *tr)
> > >  {
> > >         enum bpf_tramp_prog_type kind;
> > >         struct bpf_tramp_link *link_exiting;
> > > -       int err = 0, num_lsm_progs = 0;
> > > +       int err = 0;
> > >         int cnt = 0, i;
> > >
> > >         kind = bpf_attach_type_to_tramp(link->link.prog);
> > > @@ -547,15 +566,14 @@ static int __bpf_trampoline_link_prog(struct
> > > bpf_tramp_link *link, struct bpf_tr
> > >                 /* prog already linked */
> > >                 return -EBUSY;
> > >
> > > -               if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> > > -                       num_lsm_progs++;
> > >         }
> > >
> > > -       if (!num_lsm_progs && link->link.prog->type == BPF_PROG_TYPE_LSM)
> > > -               bpf_lsm_toggle_hook(tr->func.addr, true);
> > > -
> > >         hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]);
> > >         tr->progs_cnt[kind]++;
> > > +
> > > +       if (link->link.prog->type == BPF_PROG_TYPE_LSM)
> > > +               bpf_trampoline_toggle_lsm(tr, kind);
> >
> > how about keeping BPF_PROG_TYPE_LSM progs type count of attached programs
> > in bpf_trampoline and toggle lsm on first coming in and last going out?
>
> hm we actually allow other tracing program types to attach to bpf_lsm_*
> functions, so I wonder we should toggle the lsm hook for each program
> type (for bpf_lsm_* trampolines) because they'd expect the hook is called

Tracing is about tracing, attaching a tracing program to bpf_lsm_ that
changes the actual trace here is not something I would recommend.
Infact, I have used tracing programs to figure out whether bpf_lsm_*
is being called to debug stuff. Tracing users can always attach to
security_* if they like.

- KP

>
> but I'm not sure it's a valid use case to have like normal fentry program
> attached to bpf_lsm_XXX function
>
> jirka
>
> >
> > also the trampoline attach is actually made in bpf_trampoline_update,
> > so I wonder it'd make more sense to put it in there, but it's already
> > complicated, so it actually might be easier in here
> >
> > jirka
> >
> > > +
> > >         err = bpf_trampoline_update(tr, true /* lock_direct_mutex */);
> > >         if (err) {
> > >                 hlist_del_init(&link->tramp_hlist);
> > > @@ -578,7 +596,6 @@ static int __bpf_trampoline_unlink_prog(struct
> > > bpf_tramp_link *link, struct bpf_
> > >  {
> > >         struct bpf_tramp_link *link_exiting;
> > >         enum bpf_tramp_prog_type kind;
> > > -       bool lsm_link_found = false;
> > >         int err, num_lsm_progs = 0;
> > >
> > >         kind = bpf_attach_type_to_tramp(link->link.prog);
> > > @@ -595,18 +612,14 @@ static int __bpf_trampoline_unlink_prog(struct
> > > bpf_tramp_link *link, struct bpf_
> > >                                      tramp_hlist) {
> > >                         if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> > >                                 num_lsm_progs++;
> > > -
> > > -                       if (link_exiting->link.prog == link->link.prog)
> > > -                               lsm_link_found = true;
> > >                 }
> > >         }
> > >
> > >         hlist_del_init(&link->tramp_hlist);
> > >         tr->progs_cnt[kind]--;
> > >
> > > -       if (lsm_link_found && num_lsm_progs == 1)
> > > -               bpf_lsm_toggle_hook(tr->func.addr, false);
> > > -
> > > +       if (link->link.prog->type == BPF_PROG_TYPE_LSM)
> > > +               bpf_trampoline_toggle_lsm(tr, kind);
> > >         return bpf_trampoline_update(tr, true /* lock_direct_mutex */);
> > >  }
> > >
> > >
> > > - KP
>
KP Singh Oct. 6, 2023, 6:32 p.m. UTC | #9 
On Fri, Oct 6, 2023 at 12:57 PM KP Singh <kpsingh@kernel.org> wrote:
>
> On Fri, Oct 6, 2023 at 11:05 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Fri, Oct 06, 2023 at 09:27:57AM +0200, Jiri Olsa wrote:
> >
> > SNIP
> >
> > > >  static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link,
> > > > struct bpf_trampoline *tr)
> > > >  {
> > > >         enum bpf_tramp_prog_type kind;
> > > >         struct bpf_tramp_link *link_exiting;
> > > > -       int err = 0, num_lsm_progs = 0;
> > > > +       int err = 0;
> > > >         int cnt = 0, i;
> > > >
> > > >         kind = bpf_attach_type_to_tramp(link->link.prog);
> > > > @@ -547,15 +566,14 @@ static int __bpf_trampoline_link_prog(struct
> > > > bpf_tramp_link *link, struct bpf_tr
> > > >                 /* prog already linked */
> > > >                 return -EBUSY;
> > > >
> > > > -               if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> > > > -                       num_lsm_progs++;
> > > >         }
> > > >
> > > > -       if (!num_lsm_progs && link->link.prog->type == BPF_PROG_TYPE_LSM)
> > > > -               bpf_lsm_toggle_hook(tr->func.addr, true);
> > > > -
> > > >         hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]);
> > > >         tr->progs_cnt[kind]++;
> > > > +
> > > > +       if (link->link.prog->type == BPF_PROG_TYPE_LSM)
> > > > +               bpf_trampoline_toggle_lsm(tr, kind);
> > >
> > > how about keeping BPF_PROG_TYPE_LSM progs type count of attached programs
> > > in bpf_trampoline and toggle lsm on first coming in and last going out?
> >
> > hm we actually allow other tracing program types to attach to bpf_lsm_*
> > functions, so I wonder we should toggle the lsm hook for each program
> > type (for bpf_lsm_* trampolines) because they'd expect the hook is called
>
> Tracing is about tracing, attaching a tracing program to bpf_lsm_ that
> changes the actual trace here is not something I would recommend.
> Infact, I have used tracing programs to figure out whether bpf_lsm_*
> is being called to debug stuff. Tracing users can always attach to
> security_* if they like.
>

I will rev up with this fix and send it out as I will be unavailable
for the next 2 weeks.

- KP

> - KP
>
> >
> > but I'm not sure it's a valid use case to have like normal fentry program
> > attached to bpf_lsm_XXX function
> >
> > jirka
> >
> > >
> > > also the trampoline attach is actually made in bpf_trampoline_update,
> > > so I wonder it'd make more sense to put it in there, but it's already
> > > complicated, so it actually might be easier in here
> > >
> > > jirka
> > >
> > > > +
> > > >         err = bpf_trampoline_update(tr, true /* lock_direct_mutex */);
> > > >         if (err) {
> > > >                 hlist_del_init(&link->tramp_hlist);
> > > > @@ -578,7 +596,6 @@ static int __bpf_trampoline_unlink_prog(struct
> > > > bpf_tramp_link *link, struct bpf_
> > > >  {
> > > >         struct bpf_tramp_link *link_exiting;
> > > >         enum bpf_tramp_prog_type kind;
> > > > -       bool lsm_link_found = false;
> > > >         int err, num_lsm_progs = 0;
> > > >
> > > >         kind = bpf_attach_type_to_tramp(link->link.prog);
> > > > @@ -595,18 +612,14 @@ static int __bpf_trampoline_unlink_prog(struct
> > > > bpf_tramp_link *link, struct bpf_
> > > >                                      tramp_hlist) {
> > > >                         if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
> > > >                                 num_lsm_progs++;
> > > > -
> > > > -                       if (link_exiting->link.prog == link->link.prog)
> > > > -                               lsm_link_found = true;
> > > >                 }
> > > >         }
> > > >
> > > >         hlist_del_init(&link->tramp_hlist);
> > > >         tr->progs_cnt[kind]--;
> > > >
> > > > -       if (lsm_link_found && num_lsm_progs == 1)
> > > > -               bpf_lsm_toggle_hook(tr->func.addr, false);
> > > > -
> > > > +       if (link->link.prog->type == BPF_PROG_TYPE_LSM)
> > > > +               bpf_trampoline_toggle_lsm(tr, kind);
> > > >         return bpf_trampoline_update(tr, true /* lock_direct_mutex */);
> > > >  }
> > > >
> > > >
> > > > - KP
> >
diff mbox series

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 08b10a422560..9a81e0396aaa 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1159,6 +1159,7 @@  struct bpf_attach_target_info {
 	struct module *tgt_mod;
 	const char *tgt_name;
 	const struct btf_type *tgt_type;
+	bool is_lsm_target;
 };
 
 #define BPF_DISPATCHER_MAX 48 /* Fits in 2048B */
diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
index 1de7ece5d36d..5bbc31ac948c 100644
--- a/include/linux/bpf_lsm.h
+++ b/include/linux/bpf_lsm.h
@@ -29,6 +29,7 @@  int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
 
 bool bpf_lsm_is_sleepable_hook(u32 btf_id);
 bool bpf_lsm_is_trusted(const struct bpf_prog *prog);
+void bpf_lsm_toggle_hook(void *addr, bool value);
 
 static inline struct bpf_storage_blob *bpf_inode(
 	const struct inode *inode)
@@ -78,6 +79,10 @@  static inline void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog,
 {
 }
 
+static inline void bpf_lsm_toggle_hook(void *addr, bool value)
+{
+}
+
 #endif /* CONFIG_BPF_LSM */
 
 #endif /* _LINUX_BPF_LSM_H */
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index c77a1859214d..57ffe4eb6d30 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -97,11 +97,14 @@  struct lsm_static_calls_table {
  * @scalls: The beginning of the array of static calls assigned to this hook.
  * @hook: The callback for the hook.
  * @lsm: The name of the lsm that owns this hook.
+ * @default_state: The state of the LSM hook when initialized. If set to false,
+ * the static key guarding the hook will be set to disabled.
  */
 struct security_hook_list {
 	struct lsm_static_call	*scalls;
 	union security_list_options	hook;
 	const char			*lsm;
+	bool				default_state;
 } __randomize_layout;
 
 /*
@@ -151,7 +154,15 @@  static inline struct xattr *lsm_get_xattr_slot(struct xattr *xattrs,
 #define LSM_HOOK_INIT(NAME, CALLBACK)			\
 	{						\
 		.scalls = static_calls_table.NAME,	\
-		.hook = { .NAME = CALLBACK }		\
+		.hook = { .NAME = CALLBACK },		\
+		.default_state = true			\
+	}
+
+#define LSM_HOOK_INIT_DISABLED(NAME, CALLBACK)		\
+	{						\
+		.scalls = static_calls_table.NAME,	\
+		.hook = { .NAME = CALLBACK },		\
+		.default_state = false			\
 	}
 
 extern char *lsm_names;
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index e97aeda3a86b..df9699bce372 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -13,6 +13,7 @@ 
 #include <linux/bpf_verifier.h>
 #include <linux/bpf_lsm.h>
 #include <linux/delay.h>
+#include <linux/bpf_lsm.h>
 
 /* dummy _ops. The verifier will operate on target program's ops. */
 const struct bpf_verifier_ops bpf_extension_verifier_ops = {
@@ -514,7 +515,7 @@  static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
 {
 	enum bpf_tramp_prog_type kind;
 	struct bpf_tramp_link *link_exiting;
-	int err = 0;
+	int err = 0, num_lsm_progs = 0;
 	int cnt = 0, i;
 
 	kind = bpf_attach_type_to_tramp(link->link.prog);
@@ -545,8 +546,14 @@  static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
 			continue;
 		/* prog already linked */
 		return -EBUSY;
+
+		if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
+			num_lsm_progs++;
 	}
 
+	if (!num_lsm_progs && link->link.prog->type == BPF_PROG_TYPE_LSM)
+		bpf_lsm_toggle_hook(tr->func.addr, true);
+
 	hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]);
 	tr->progs_cnt[kind]++;
 	err = bpf_trampoline_update(tr, true /* lock_direct_mutex */);
@@ -569,8 +576,10 @@  int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline
 
 static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr)
 {
+	struct bpf_tramp_link *link_exiting;
 	enum bpf_tramp_prog_type kind;
-	int err;
+	bool lsm_link_found = false;
+	int err, num_lsm_progs = 0;
 
 	kind = bpf_attach_type_to_tramp(link->link.prog);
 	if (kind == BPF_TRAMP_REPLACE) {
@@ -580,8 +589,24 @@  static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link, struct bpf_
 		tr->extension_prog = NULL;
 		return err;
 	}
+
+	if (link->link.prog->type == BPF_PROG_TYPE_LSM) {
+		hlist_for_each_entry(link_exiting, &tr->progs_hlist[kind],
+				     tramp_hlist) {
+			if (link_exiting->link.prog->type == BPF_PROG_TYPE_LSM)
+				num_lsm_progs++;
+
+			if (link_exiting->link.prog == link->link.prog)
+				lsm_link_found = true;
+		}
+	}
+
 	hlist_del_init(&link->tramp_hlist);
 	tr->progs_cnt[kind]--;
+
+	if (lsm_link_found && num_lsm_progs == 1)
+		bpf_lsm_toggle_hook(tr->func.addr, false);
+
 	return bpf_trampoline_update(tr, true /* lock_direct_mutex */);
 }
 
diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
index cfaf1d0e6a5f..1957244196d0 100644
--- a/security/bpf/hooks.c
+++ b/security/bpf/hooks.c
@@ -8,7 +8,7 @@ 
 
 static struct security_hook_list bpf_lsm_hooks[] __ro_after_init = {
 	#define LSM_HOOK(RET, DEFAULT, NAME, ...) \
-	LSM_HOOK_INIT(NAME, bpf_lsm_##NAME),
+	LSM_HOOK_INIT_DISABLED(NAME, bpf_lsm_##NAME),
 	#include <linux/lsm_hook_defs.h>
 	#undef LSM_HOOK
 	LSM_HOOK_INIT(inode_free_security, bpf_inode_storage_free),
@@ -32,3 +32,26 @@  DEFINE_LSM(bpf) = {
 	.init = bpf_lsm_init,
 	.blobs = &bpf_lsm_blob_sizes
 };
+
+void bpf_lsm_toggle_hook(void *addr, bool value)
+{
+	struct lsm_static_call *scalls;
+	struct security_hook_list *h;
+	int i, j;
+
+	for (i = 0; i < ARRAY_SIZE(bpf_lsm_hooks); i++) {
+		h = &bpf_lsm_hooks[i];
+		scalls = h->scalls;
+		if (h->hook.lsm_callback == addr)
+			continue;
+
+		for (j = 0; j < MAX_LSM_COUNT; j++) {
+			if (scalls[j].hl != h)
+				continue;
+			if (value)
+				static_branch_enable(scalls[j].active);
+			else
+				static_branch_disable(scalls[j].active);
+		}
+	}
+}
diff --git a/security/security.c b/security/security.c
index c2c2cf6b711f..d1ee72e563cc 100644
--- a/security/security.c
+++ b/security/security.c
@@ -382,7 +382,8 @@  static void __init lsm_static_call_init(struct security_hook_list *hl)
 			__static_call_update(scall->key, scall->trampoline,
 					     hl->hook.lsm_callback);
 			scall->hl = hl;
-			static_branch_enable(scall->active);
+			if (hl->default_state)
+				static_branch_enable(scall->active);
 			return;
 		}
 		scall++;