From: Jiri Olsa <olsajiri@gmail.com>
To: KP Singh <kpsingh@kernel.org>
Cc: linux-security-module@vger.kernel.org, bpf@vger.kernel.org,
paul@paul-moore.com, keescook@chromium.org,
casey@schaufler-ca.com, song@kernel.org, daniel@iogearbox.net,
ast@kernel.org, renauld@google.com
Subject: Re: [PATCH v5 4/5] bpf: Only enable BPF LSM hooks when an LSM program is attached
Date: Thu, 5 Oct 2023 10:09:47 +0200 [thread overview]
Message-ID: <ZR5vSyyNGBb8TvNH@krava> (raw)
In-Reply-To: <20230928202410.3765062-5-kpsingh@kernel.org>
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
>
>
next prev parent reply other threads:[~2023-10-05 8:09 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-28 20:24 [PATCH v5 0/5] Reduce overhead of LSMs with static calls KP Singh
2023-09-28 20:24 ` [PATCH v5 1/5] kernel: Add helper macros for loop unrolling KP Singh
2023-09-28 20:24 ` [PATCH v5 2/5] security: Count the LSMs enabled at compile time KP Singh
2023-09-29 0:37 ` Kees Cook
2023-09-28 20:24 ` [PATCH v5 3/5] security: Replace indirect LSM hook calls with static calls KP Singh
2023-09-30 16:13 ` kernel test robot
2023-09-30 20:40 ` Kees Cook
2023-10-04 0:09 ` KP Singh
2023-09-28 20:24 ` [PATCH v5 4/5] bpf: Only enable BPF LSM hooks when an LSM program is attached KP Singh
2023-10-05 8:09 ` Jiri Olsa [this message]
2023-10-05 13:26 ` KP Singh
2023-10-05 13:27 ` KP Singh
2023-10-05 13:52 ` Jiri Olsa
2023-10-05 16:07 ` KP Singh
2023-10-06 7:27 ` Jiri Olsa
2023-10-06 9:05 ` Jiri Olsa
2023-10-06 10:57 ` KP Singh
2023-10-06 18:32 ` KP Singh
2023-09-28 20:24 ` [PATCH v5 5/5] security: Add CONFIG_SECURITY_HOOK_LIKELY KP Singh
2023-09-29 0:38 ` Kees Cook
2023-09-29 0:41 ` [PATCH v5 0/5] Reduce overhead of LSMs with static calls Kees Cook
2023-10-02 11:06 ` Paolo Abeni
2023-10-02 11:09 ` KP Singh
2023-10-02 13:27 ` Paolo Abeni
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=ZR5vSyyNGBb8TvNH@krava \
--to=olsajiri@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=casey@schaufler-ca.com \
--cc=daniel@iogearbox.net \
--cc=keescook@chromium.org \
--cc=kpsingh@kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=paul@paul-moore.com \
--cc=renauld@google.com \
--cc=song@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.