[PATCH bpf-next 4/4] bpf: Only enable BPF LSM hooks when an LSM program is attached
KP Singh
kpsingh at kernel.org
Thu Jan 19 23:10:33 UTC 2023
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:
0xffffffff814f0e90 <+0>: endbr64
0xffffffff814f0e94 <+4>: push %rbp
0xffffffff814f0e95 <+5>: push %r14
0xffffffff814f0e97 <+7>: push %rbx
0xffffffff814f0e98 <+8>: mov %rdx,%rbx
0xffffffff814f0e9b <+11>: mov %esi,%ebp
0xffffffff814f0e9d <+13>: mov %rdi,%r14
>>> 0xffffffff814f0ea0 <+16>: jmp 0xffffffff814f0eb1 <security_file_ioctl+33>
Static key enabled for SELinux
0xffffffff814f0ea2 <+18>: xchg %ax,%ax
Static key disabled for BPF. This gets patched to a jmp when a BPF
LSM program is attached.
0xffffffff814f0ea4 <+20>: xor %eax,%eax
0xffffffff814f0ea6 <+22>: xchg %ax,%ax
0xffffffff814f0ea8 <+24>: pop %rbx
0xffffffff814f0ea9 <+25>: pop %r14
0xffffffff814f0eab <+27>: pop %rbp
0xffffffff814f0eac <+28>: jmp 0xffffffff81f767c4 <__x86_return_thunk>
0xffffffff814f0eb1 <+33>: endbr64
0xffffffff814f0eb5 <+37>: mov %r14,%rdi
0xffffffff814f0eb8 <+40>: mov %ebp,%esi
0xffffffff814f0eba <+42>: mov %rbx,%rdx
0xffffffff814f0ebd <+45>: call 0xffffffff814fe8e0 <selinux_file_ioctl>
0xffffffff814f0ec2 <+50>: test %eax,%eax
0xffffffff814f0ec4 <+52>: jne 0xffffffff814f0ea8 <security_file_ioctl+24>
0xffffffff814f0ec6 <+54>: jmp 0xffffffff814f0ea2 <security_file_ioctl+18>
0xffffffff814f0ec8 <+56>: endbr64
0xffffffff814f0ecc <+60>: mov %r14,%rdi
0xffffffff814f0ecf <+63>: mov %ebp,%esi
0xffffffff814f0ed1 <+65>: mov %rbx,%rdx
0xffffffff814f0ed4 <+68>: call 0xffffffff8123b890 <bpf_lsm_file_ioctl>
0xffffffff814f0ed9 <+73>: test %eax,%eax
0xffffffff814f0edb <+75>: jne 0xffffffff814f0ea8 <security_file_ioctl+24>
0xffffffff814f0edd <+77>: jmp 0xffffffff814f0ea4 <security_file_ioctl+20>
0xffffffff814f0edf <+79>: endbr64
0xffffffff814f0ee3 <+83>: mov %r14,%rdi
0xffffffff814f0ee6 <+86>: mov %ebp,%esi
0xffffffff814f0ee8 <+88>: mov %rbx,%rdx
0xffffffff814f0eeb <+91>: pop %rbx
0xffffffff814f0eec <+92>: pop %r14
0xffffffff814f0eee <+94>: pop %rbp
0xffffffff814f0eef <+95>: ret
0xffffffff814f0ef0 <+96>: int3
0xffffffff814f0ef1 <+97>: int3
0xffffffff814f0ef2 <+98>: int3
0xffffffff814f0ef3 <+99>: int3
Signed-off-by: KP Singh <kpsingh at kernel.org>
---
include/linux/bpf.h | 1 +
include/linux/bpf_lsm.h | 1 +
include/linux/lsm_hooks.h | 13 ++++++++++++-
kernel/bpf/trampoline.c | 29 +++++++++++++++++++++++++++--
security/bpf/hooks.c | 26 +++++++++++++++++++++++++-
security/security.c | 3 ++-
6 files changed, 68 insertions(+), 5 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index ae7771c7d750..4008f4a00851 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1049,6 +1049,7 @@ struct bpf_attach_target_info {
long tgt_addr;
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..cce615af93d5 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)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index c82d15a4ef50..5e85d3340a07 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1716,11 +1716,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;
/*
@@ -1751,7 +1754,15 @@ struct lsm_blob_sizes {
#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 d0ed7d6f5eec..9789ecf6f29c 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -14,6 +14,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 = {
@@ -536,7 +537,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);
@@ -567,8 +568,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 */);
@@ -591,8 +598,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) {
@@ -602,8 +611,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 e5971fa74fd7..a39799e9f648 100644
--- a/security/bpf/hooks.c
+++ b/security/bpf/hooks.c
@@ -8,7 +8,7 @@
static struct security_hook_list bpf_lsm_hooks[] __lsm_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,27 @@ DEFINE_LSM(bpf) = {
.init = bpf_lsm_init,
.blobs = &bpf_lsm_blob_sizes
};
+
+void bpf_lsm_toggle_hook(void *addr, bool value)
+{
+ struct security_hook_list *h;
+ struct lsm_static_call *scalls;
+ 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_key_enable(scalls[j].enabled_key);
+ else
+ static_key_disable(scalls[j].enabled_key);
+ }
+ }
+}
diff --git a/security/security.c b/security/security.c
index e54d5ba187d1..f74135349429 100644
--- a/security/security.c
+++ b/security/security.c
@@ -366,7 +366,8 @@ static void __init lsm_static_call_init(struct security_hook_list *hl)
if (!scall->hl) {
__static_call_update(scall->key, scall->trampoline, hl->hook.lsm_callback);
scall->hl = hl;
- static_key_enable(scall->enabled_key);
+ if (hl->default_state)
+ static_key_enable(scall->enabled_key);
return;
}
scall++;
--
2.39.0.246.g2a6d74b583-goog
More information about the Linux-security-module-archive
mailing list