From: KP Singh <kpsingh@kernel.org>
To: bpf@vger.kernel.org, linux-security-module@vger.kernel.org
Cc: paul@paul-moore.com, keescook@chromium.org,
casey@schaufler-ca.com, song@kernel.org, daniel@iogearbox.net,
ast@kernel.org, pabeni@redhat.com, andrii@kernel.org,
kpsingh@kernel.org, Jiri Olsa <jolsa@kernel.org>
Subject: [PATCH v9 4/4] bpf: Only enable BPF LSM hooks when an LSM program is attached
Date: Wed, 7 Feb 2024 13:49:18 +0100 [thread overview]
Message-ID: <20240207124918.3498756-5-kpsingh@kernel.org> (raw)
In-Reply-To: <20240207124918.3498756-1-kpsingh@kernel.org>
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>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
include/linux/bpf_lsm.h | 5 +++++
include/linux/lsm_hooks.h | 13 ++++++++++++-
kernel/bpf/trampoline.c | 24 ++++++++++++++++++++++++
security/bpf/hooks.c | 25 ++++++++++++++++++++++++-
security/security.c | 3 ++-
5 files changed, 67 insertions(+), 3 deletions(-)
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 ba63d8b54448..e95f0a5cb409 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -110,11 +110,14 @@ struct lsm_id {
* @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 struct lsm_id *lsmid;
+ bool default_enabled;
} __randomize_layout;
/*
@@ -164,7 +167,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_enabled = true \
+ }
+
+#define LSM_HOOK_INIT_DISABLED(NAME, CALLBACK) \
+ { \
+ .scalls = static_calls_table.NAME, \
+ .hook = { .NAME = CALLBACK }, \
+ .default_enabled = false \
}
extern char *lsm_names;
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index d382f5ebe06c..5281c3338e19 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 = {
@@ -521,6 +522,21 @@ 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;
+ bool found = false;
+
+ 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;
@@ -560,6 +576,10 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_tr
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);
@@ -593,6 +613,10 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link, struct bpf_
}
hlist_del_init(&link->tramp_hlist);
tr->progs_cnt[kind]--;
+
+ if (link->link.prog->type == BPF_PROG_TYPE_LSM)
+ bpf_trampoline_toggle_lsm(tr, kind);
+
return bpf_trampoline_update(tr, true /* lock_direct_mutex */);
}
diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
index 57b9ffd53c98..38bedab2b4f9 100644
--- a/security/bpf/hooks.c
+++ b/security/bpf/hooks.c
@@ -9,7 +9,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),
@@ -39,3 +39,26 @@ DEFINE_LSM(bpf) = {
.init = bpf_lsm_init,
.blobs = &bpf_lsm_blob_sizes
};
+
+void bpf_lsm_toggle_hook(void *addr, bool enable)
+{
+ 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];
+ if (h->hook.lsm_callback != addr)
+ continue;
+
+ for (j = 0; j < MAX_LSM_COUNT; j++) {
+ scalls = &h->scalls[j];
+ if (scalls->hl != &bpf_lsm_hooks[i])
+ continue;
+ if (enable)
+ static_branch_enable(scalls->active);
+ else
+ static_branch_disable(scalls->active);
+ }
+ }
+}
diff --git a/security/security.c b/security/security.c
index e05d2157c95a..40d83da87f68 100644
--- a/security/security.c
+++ b/security/security.c
@@ -406,7 +406,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_enabled)
+ static_branch_enable(scall->active);
return;
}
scall++;
--
2.43.0.594.gd9cf4e227d-goog
next prev parent reply other threads:[~2024-02-07 12:49 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-07 12:49 [PATCH v9 0/4] Reduce overhead of LSMs with static calls KP Singh
2024-02-07 12:49 ` [PATCH v9 1/4] kernel: Add helper macros for loop unrolling KP Singh
2024-02-07 12:49 ` [PATCH v9 2/4] security: Count the LSMs enabled at compile time KP Singh
2024-02-07 12:49 ` [PATCH v9 3/4] security: Replace indirect LSM hook calls with static calls KP Singh
2024-04-11 0:38 ` Paul Moore
2024-04-11 7:12 ` KP Singh
2024-04-12 15:39 ` Paul Moore
2024-02-07 12:49 ` KP Singh [this message]
2024-04-11 0:38 ` [PATCH v9 4/4] bpf: Only enable BPF LSM hooks when an LSM program is attached Paul Moore
2024-05-05 16:25 ` KP Singh
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=20240207124918.3498756-5-kpsingh@kernel.org \
--to=kpsingh@kernel.org \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=casey@schaufler-ca.com \
--cc=daniel@iogearbox.net \
--cc=jolsa@kernel.org \
--cc=keescook@chromium.org \
--cc=linux-security-module@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=paul@paul-moore.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).