* [PATCH bpf-next v5 0/8] bpf: cgroup_sock lsm flavor
@ 2022-04-19 19:00 Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 1/8] bpf: add bpf_func_t and trampoline helpers Stanislav Fomichev
` (7 more replies)
0 siblings, 8 replies; 15+ messages in thread
From: Stanislav Fomichev @ 2022-04-19 19:00 UTC (permalink / raw)
To: netdev, bpf
Cc: ast, daniel, andrii, Stanislav Fomichev, kafai, kpsingh, jakub
This series implements new lsm flavor for attaching per-cgroup programs to
existing lsm hooks. The cgroup is taken out of 'current', unless
the first argument of the hook is 'struct socket'. In this case,
the cgroup association is taken out of socket. The attachment
looks like a regular per-cgroup attachment: we add new BPF_LSM_CGROUP
attach type which, together with attach_btf_id, signals per-cgroup lsm.
Behind the scenes, we allocate trampoline shim program and
attach to lsm. This program looks up cgroup from current/socket
and runs cgroup's effective prog array. The rest of the per-cgroup BPF
stays the same: hierarchy, local storage, retval conventions
(return 1 == success).
Current limitations:
* haven't considered sleepable bpf; can be extended later on
* not sure the verifier does the right thing with null checks;
see latest selftest for details
* total of 10 (global) per-cgroup LSM attach points; this bloats
bpf_cgroup a bit
Cc: ast@kernel.org
Cc: daniel@iogearbox.net
Cc: kafai@fb.com
Cc: kpsingh@kernel.org
Cc: jakub@cloudflare.com
v5:
- __cgroup_bpf_run_lsm_socket remove NULL sock/sk checks (Martin KaFai Lau)
- __cgroup_bpf_run_lsm_{socket,current} s/prog/shim_prog/ (Martin)
- make sure bpf_lsm_find_cgroup_shim works for hooks without args (Martin)
- __cgroup_bpf_attach make sure attach_btf_id is the same when replacing (Martin)
- call bpf_cgroup_lsm_shim_release only for LSM_CGROUP (Martin)
- drop BPF_LSM_CGROUP from bpf_attach_type_to_tramp (Martin)
- drop jited check from cgroup_shim_find (Martin)
- new patch to convert cgroup_bpf to hlist_node (Jakub Sitnicki)
- new shim flavor for 'struct sock' + list of exceptions (Martin)
v4:
- fix build when jit is on but syscall is off
v3:
- add BPF_LSM_CGROUP to bpftool
- use simple int instead of refcnt_t (to avoid use-after-free
false positive)
v2:
- addressed build bot failures
Stanislav Fomichev (8):
bpf: add bpf_func_t and trampoline helpers
bpf: convert cgroup_bpf.progs to hlist
bpf: per-cgroup lsm flavor
bpf: minimize number of allocated lsm slots per program
bpf: allow writing to a subset of sock fields from lsm progtype
libbpf: add lsm_cgoup_sock type
selftests/bpf: lsm_cgroup functional test
selftests/bpf: verify lsm_cgroup struct sock access
include/linux/bpf-cgroup-defs.h | 12 +-
include/linux/bpf-cgroup.h | 9 +-
include/linux/bpf.h | 24 +-
include/linux/bpf_lsm.h | 8 +
include/uapi/linux/bpf.h | 1 +
kernel/bpf/bpf_lsm.c | 116 ++++++
kernel/bpf/btf.c | 11 +
kernel/bpf/cgroup.c | 361 +++++++++++++++---
kernel/bpf/syscall.c | 10 +
kernel/bpf/trampoline.c | 206 ++++++++--
kernel/bpf/verifier.c | 4 +-
tools/bpf/bpftool/common.c | 1 +
tools/include/uapi/linux/bpf.h | 1 +
tools/lib/bpf/libbpf.c | 2 +
.../selftests/bpf/prog_tests/lsm_cgroup.c | 213 +++++++++++
.../testing/selftests/bpf/progs/lsm_cgroup.c | 126 ++++++
tools/testing/selftests/bpf/test_verifier.c | 54 ++-
.../selftests/bpf/verifier/lsm_cgroup.c | 34 ++
18 files changed, 1102 insertions(+), 91 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
create mode 100644 tools/testing/selftests/bpf/progs/lsm_cgroup.c
create mode 100644 tools/testing/selftests/bpf/verifier/lsm_cgroup.c
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next v5 1/8] bpf: add bpf_func_t and trampoline helpers
2022-04-19 19:00 [PATCH bpf-next v5 0/8] bpf: cgroup_sock lsm flavor Stanislav Fomichev
@ 2022-04-19 19:00 ` Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 2/8] bpf: convert cgroup_bpf.progs to hlist Stanislav Fomichev
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Stanislav Fomichev @ 2022-04-19 19:00 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, andrii, Stanislav Fomichev
I'll be adding lsm cgroup specific helpers that grab
trampoline mutex.
No functional changes.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/linux/bpf.h | 11 ++++---
kernel/bpf/trampoline.c | 63 +++++++++++++++++++++++------------------
2 files changed, 40 insertions(+), 34 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 7bf441563ffc..ab2ba5f4bc9c 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -52,6 +52,8 @@ typedef u64 (*bpf_callback_t)(u64, u64, u64, u64, u64);
typedef int (*bpf_iter_init_seq_priv_t)(void *private_data,
struct bpf_iter_aux_info *aux);
typedef void (*bpf_iter_fini_seq_priv_t)(void *private_data);
+typedef unsigned int (*bpf_func_t)(const void *,
+ const struct bpf_insn *);
struct bpf_iter_seq_info {
const struct seq_operations *seq_ops;
bpf_iter_init_seq_priv_t init_seq_private;
@@ -798,8 +800,7 @@ struct bpf_dispatcher {
static __always_inline __nocfi unsigned int bpf_dispatcher_nop_func(
const void *ctx,
const struct bpf_insn *insnsi,
- unsigned int (*bpf_func)(const void *,
- const struct bpf_insn *))
+ bpf_func_t bpf_func)
{
return bpf_func(ctx, insnsi);
}
@@ -827,8 +828,7 @@ int arch_prepare_bpf_dispatcher(void *image, s64 *funcs, int num_funcs);
noinline __nocfi unsigned int bpf_dispatcher_##name##_func( \
const void *ctx, \
const struct bpf_insn *insnsi, \
- unsigned int (*bpf_func)(const void *, \
- const struct bpf_insn *)) \
+ bpf_func_t bpf_func) \
{ \
return bpf_func(ctx, insnsi); \
} \
@@ -839,8 +839,7 @@ int arch_prepare_bpf_dispatcher(void *image, s64 *funcs, int num_funcs);
unsigned int bpf_dispatcher_##name##_func( \
const void *ctx, \
const struct bpf_insn *insnsi, \
- unsigned int (*bpf_func)(const void *, \
- const struct bpf_insn *)); \
+ bpf_func_t bpf_func); \
extern struct bpf_dispatcher bpf_dispatcher_##name;
#define BPF_DISPATCHER_FUNC(name) bpf_dispatcher_##name##_func
#define BPF_DISPATCHER_PTR(name) (&bpf_dispatcher_##name)
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index ada97751ae1b..0c4fd194e801 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -407,42 +407,34 @@ static enum bpf_tramp_prog_type bpf_attach_type_to_tramp(struct bpf_prog *prog)
}
}
-int bpf_trampoline_link_prog(struct bpf_prog *prog, struct bpf_trampoline *tr)
+static int __bpf_trampoline_link_prog(struct bpf_prog *prog,
+ struct bpf_trampoline *tr)
{
enum bpf_tramp_prog_type kind;
int err = 0;
int cnt;
kind = bpf_attach_type_to_tramp(prog);
- mutex_lock(&tr->mutex);
- if (tr->extension_prog) {
+ if (tr->extension_prog)
/* cannot attach fentry/fexit if extension prog is attached.
* cannot overwrite extension prog either.
*/
- err = -EBUSY;
- goto out;
- }
+ return -EBUSY;
+
cnt = tr->progs_cnt[BPF_TRAMP_FENTRY] + tr->progs_cnt[BPF_TRAMP_FEXIT];
if (kind == BPF_TRAMP_REPLACE) {
/* Cannot attach extension if fentry/fexit are in use. */
- if (cnt) {
- err = -EBUSY;
- goto out;
- }
+ if (cnt)
+ return -EBUSY;
tr->extension_prog = prog;
- err = bpf_arch_text_poke(tr->func.addr, BPF_MOD_JUMP, NULL,
- prog->bpf_func);
- goto out;
- }
- if (cnt >= BPF_MAX_TRAMP_PROGS) {
- err = -E2BIG;
- goto out;
+ return bpf_arch_text_poke(tr->func.addr, BPF_MOD_JUMP, NULL,
+ prog->bpf_func);
}
- if (!hlist_unhashed(&prog->aux->tramp_hlist)) {
+ if (cnt >= BPF_MAX_TRAMP_PROGS)
+ return -E2BIG;
+ if (!hlist_unhashed(&prog->aux->tramp_hlist))
/* prog already linked */
- err = -EBUSY;
- goto out;
- }
+ return -EBUSY;
hlist_add_head(&prog->aux->tramp_hlist, &tr->progs_hlist[kind]);
tr->progs_cnt[kind]++;
err = bpf_trampoline_update(tr);
@@ -450,30 +442,45 @@ int bpf_trampoline_link_prog(struct bpf_prog *prog, struct bpf_trampoline *tr)
hlist_del_init(&prog->aux->tramp_hlist);
tr->progs_cnt[kind]--;
}
-out:
+ return err;
+}
+
+int bpf_trampoline_link_prog(struct bpf_prog *prog, struct bpf_trampoline *tr)
+{
+ int err;
+
+ mutex_lock(&tr->mutex);
+ err = __bpf_trampoline_link_prog(prog, tr);
mutex_unlock(&tr->mutex);
return err;
}
-/* bpf_trampoline_unlink_prog() should never fail. */
-int bpf_trampoline_unlink_prog(struct bpf_prog *prog, struct bpf_trampoline *tr)
+static int __bpf_trampoline_unlink_prog(struct bpf_prog *prog,
+ struct bpf_trampoline *tr)
{
enum bpf_tramp_prog_type kind;
int err;
kind = bpf_attach_type_to_tramp(prog);
- mutex_lock(&tr->mutex);
if (kind == BPF_TRAMP_REPLACE) {
WARN_ON_ONCE(!tr->extension_prog);
err = bpf_arch_text_poke(tr->func.addr, BPF_MOD_JUMP,
tr->extension_prog->bpf_func, NULL);
tr->extension_prog = NULL;
- goto out;
+ return err;
}
hlist_del_init(&prog->aux->tramp_hlist);
tr->progs_cnt[kind]--;
- err = bpf_trampoline_update(tr);
-out:
+ return bpf_trampoline_update(tr);
+}
+
+/* bpf_trampoline_unlink_prog() should never fail. */
+int bpf_trampoline_unlink_prog(struct bpf_prog *prog, struct bpf_trampoline *tr)
+{
+ int err;
+
+ mutex_lock(&tr->mutex);
+ err = __bpf_trampoline_unlink_prog(prog, tr);
mutex_unlock(&tr->mutex);
return err;
}
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH bpf-next v5 2/8] bpf: convert cgroup_bpf.progs to hlist
2022-04-19 19:00 [PATCH bpf-next v5 0/8] bpf: cgroup_sock lsm flavor Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 1/8] bpf: add bpf_func_t and trampoline helpers Stanislav Fomichev
@ 2022-04-19 19:00 ` Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor Stanislav Fomichev
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Stanislav Fomichev @ 2022-04-19 19:00 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, andrii, Stanislav Fomichev, Jakub Sitnicki
This lets us reclaim some space to be used by new cgroup lsm slots.
Before:
struct cgroup_bpf {
struct bpf_prog_array * effective[23]; /* 0 184 */
/* --- cacheline 2 boundary (128 bytes) was 56 bytes ago --- */
struct list_head progs[23]; /* 184 368 */
/* --- cacheline 8 boundary (512 bytes) was 40 bytes ago --- */
u32 flags[23]; /* 552 92 */
/* XXX 4 bytes hole, try to pack */
/* --- cacheline 10 boundary (640 bytes) was 8 bytes ago --- */
struct list_head storages; /* 648 16 */
struct bpf_prog_array * inactive; /* 664 8 */
struct percpu_ref refcnt; /* 672 16 */
struct work_struct release_work; /* 688 32 */
/* size: 720, cachelines: 12, members: 7 */
/* sum members: 716, holes: 1, sum holes: 4 */
/* last cacheline: 16 bytes */
};
After:
struct cgroup_bpf {
struct bpf_prog_array * effective[23]; /* 0 184 */
/* --- cacheline 2 boundary (128 bytes) was 56 bytes ago --- */
struct hlist_head progs[23]; /* 184 184 */
/* --- cacheline 5 boundary (320 bytes) was 48 bytes ago --- */
u8 flags[23]; /* 368 23 */
/* XXX 1 byte hole, try to pack */
/* --- cacheline 6 boundary (384 bytes) was 8 bytes ago --- */
struct list_head storages; /* 392 16 */
struct bpf_prog_array * inactive; /* 408 8 */
struct percpu_ref refcnt; /* 416 16 */
struct work_struct release_work; /* 432 72 */
/* size: 504, cachelines: 8, members: 7 */
/* sum members: 503, holes: 1, sum holes: 1 */
/* last cacheline: 56 bytes */
};
Suggested-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/linux/bpf-cgroup-defs.h | 4 +-
include/linux/bpf-cgroup.h | 2 +-
kernel/bpf/cgroup.c | 72 +++++++++++++++++++--------------
3 files changed, 45 insertions(+), 33 deletions(-)
diff --git a/include/linux/bpf-cgroup-defs.h b/include/linux/bpf-cgroup-defs.h
index 695d1224a71b..5d268e76d8e6 100644
--- a/include/linux/bpf-cgroup-defs.h
+++ b/include/linux/bpf-cgroup-defs.h
@@ -47,8 +47,8 @@ struct cgroup_bpf {
* have either zero or one element
* when BPF_F_ALLOW_MULTI the list can have up to BPF_CGROUP_MAX_PROGS
*/
- struct list_head progs[MAX_CGROUP_BPF_ATTACH_TYPE];
- u32 flags[MAX_CGROUP_BPF_ATTACH_TYPE];
+ struct hlist_head progs[MAX_CGROUP_BPF_ATTACH_TYPE];
+ u8 flags[MAX_CGROUP_BPF_ATTACH_TYPE];
/* list of cgroup shared storages */
struct list_head storages;
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 88a51b242adc..69b611f7b2c7 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -95,7 +95,7 @@ struct bpf_cgroup_link {
};
struct bpf_prog_list {
- struct list_head node;
+ struct hlist_node node;
struct bpf_prog *prog;
struct bpf_cgroup_link *link;
struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE];
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 0cb6211fcb58..aaf9e36f2736 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -184,11 +184,12 @@ static void cgroup_bpf_release(struct work_struct *work)
mutex_lock(&cgroup_mutex);
for (atype = 0; atype < ARRAY_SIZE(cgrp->bpf.progs); atype++) {
- struct list_head *progs = &cgrp->bpf.progs[atype];
- struct bpf_prog_list *pl, *pltmp;
+ struct hlist_head *progs = &cgrp->bpf.progs[atype];
+ struct bpf_prog_list *pl;
+ struct hlist_node *pltmp;
- list_for_each_entry_safe(pl, pltmp, progs, node) {
- list_del(&pl->node);
+ hlist_for_each_entry_safe(pl, pltmp, progs, node) {
+ hlist_del(&pl->node);
if (pl->prog)
bpf_prog_put(pl->prog);
if (pl->link)
@@ -244,12 +245,12 @@ static struct bpf_prog *prog_list_prog(struct bpf_prog_list *pl)
/* count number of elements in the list.
* it's slow but the list cannot be long
*/
-static u32 prog_list_length(struct list_head *head)
+static u32 prog_list_length(struct hlist_head *head)
{
struct bpf_prog_list *pl;
u32 cnt = 0;
- list_for_each_entry(pl, head, node) {
+ hlist_for_each_entry(pl, head, node) {
if (!prog_list_prog(pl))
continue;
cnt++;
@@ -318,7 +319,7 @@ static int compute_effective_progs(struct cgroup *cgrp,
if (cnt > 0 && !(p->bpf.flags[atype] & BPF_F_ALLOW_MULTI))
continue;
- list_for_each_entry(pl, &p->bpf.progs[atype], node) {
+ hlist_for_each_entry(pl, &p->bpf.progs[atype], node) {
if (!prog_list_prog(pl))
continue;
@@ -369,7 +370,7 @@ int cgroup_bpf_inherit(struct cgroup *cgrp)
cgroup_bpf_get(p);
for (i = 0; i < NR; i++)
- INIT_LIST_HEAD(&cgrp->bpf.progs[i]);
+ INIT_HLIST_HEAD(&cgrp->bpf.progs[i]);
INIT_LIST_HEAD(&cgrp->bpf.storages);
@@ -445,7 +446,7 @@ static int update_effective_progs(struct cgroup *cgrp,
#define BPF_CGROUP_MAX_PROGS 64
-static struct bpf_prog_list *find_attach_entry(struct list_head *progs,
+static struct bpf_prog_list *find_attach_entry(struct hlist_head *progs,
struct bpf_prog *prog,
struct bpf_cgroup_link *link,
struct bpf_prog *replace_prog,
@@ -455,12 +456,12 @@ static struct bpf_prog_list *find_attach_entry(struct list_head *progs,
/* single-attach case */
if (!allow_multi) {
- if (list_empty(progs))
+ if (hlist_empty(progs))
return NULL;
- return list_first_entry(progs, typeof(*pl), node);
+ return hlist_entry(progs->first, typeof(*pl), node);
}
- list_for_each_entry(pl, progs, node) {
+ hlist_for_each_entry(pl, progs, node) {
if (prog && pl->prog == prog && prog != replace_prog)
/* disallow attaching the same prog twice */
return ERR_PTR(-EINVAL);
@@ -471,7 +472,7 @@ static struct bpf_prog_list *find_attach_entry(struct list_head *progs,
/* direct prog multi-attach w/ replacement case */
if (replace_prog) {
- list_for_each_entry(pl, progs, node) {
+ hlist_for_each_entry(pl, progs, node) {
if (pl->prog == replace_prog)
/* a match found */
return pl;
@@ -507,7 +508,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
enum cgroup_bpf_attach_type atype;
struct bpf_prog_list *pl;
- struct list_head *progs;
+ struct hlist_head *progs;
int err;
if (((flags & BPF_F_ALLOW_OVERRIDE) && (flags & BPF_F_ALLOW_MULTI)) ||
@@ -530,7 +531,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
if (!hierarchy_allows_attach(cgrp, atype))
return -EPERM;
- if (!list_empty(progs) && cgrp->bpf.flags[atype] != saved_flags)
+ if (!hlist_empty(progs) && cgrp->bpf.flags[atype] != saved_flags)
/* Disallow attaching non-overridable on top
* of existing overridable in this cgroup.
* Disallow attaching multi-prog if overridable or none
@@ -552,12 +553,22 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
if (pl) {
old_prog = pl->prog;
} else {
+ struct hlist_node *last = NULL;
+
pl = kmalloc(sizeof(*pl), GFP_KERNEL);
if (!pl) {
bpf_cgroup_storages_free(new_storage);
return -ENOMEM;
}
- list_add_tail(&pl->node, progs);
+ if (hlist_empty(progs))
+ hlist_add_head(&pl->node, progs);
+ else
+ hlist_for_each(last, progs) {
+ if (last->next)
+ continue;
+ hlist_add_behind(&pl->node, last);
+ break;
+ }
}
pl->prog = prog;
@@ -583,7 +594,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
}
bpf_cgroup_storages_free(new_storage);
if (!old_prog) {
- list_del(&pl->node);
+ hlist_del(&pl->node);
kfree(pl);
}
return err;
@@ -614,7 +625,7 @@ static void replace_effective_prog(struct cgroup *cgrp,
struct cgroup_subsys_state *css;
struct bpf_prog_array *progs;
struct bpf_prog_list *pl;
- struct list_head *head;
+ struct hlist_head *head;
struct cgroup *cg;
int pos;
@@ -630,7 +641,7 @@ static void replace_effective_prog(struct cgroup *cgrp,
continue;
head = &cg->bpf.progs[atype];
- list_for_each_entry(pl, head, node) {
+ hlist_for_each_entry(pl, head, node) {
if (!prog_list_prog(pl))
continue;
if (pl->link == link)
@@ -664,7 +675,7 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
enum cgroup_bpf_attach_type atype;
struct bpf_prog *old_prog;
struct bpf_prog_list *pl;
- struct list_head *progs;
+ struct hlist_head *progs;
bool found = false;
atype = to_cgroup_bpf_attach_type(link->type);
@@ -676,7 +687,7 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
if (link->link.prog->type != new_prog->type)
return -EINVAL;
- list_for_each_entry(pl, progs, node) {
+ hlist_for_each_entry(pl, progs, node) {
if (pl->link == link) {
found = true;
break;
@@ -715,7 +726,7 @@ static int cgroup_bpf_replace(struct bpf_link *link, struct bpf_prog *new_prog,
return ret;
}
-static struct bpf_prog_list *find_detach_entry(struct list_head *progs,
+static struct bpf_prog_list *find_detach_entry(struct hlist_head *progs,
struct bpf_prog *prog,
struct bpf_cgroup_link *link,
bool allow_multi)
@@ -723,14 +734,14 @@ static struct bpf_prog_list *find_detach_entry(struct list_head *progs,
struct bpf_prog_list *pl;
if (!allow_multi) {
- if (list_empty(progs))
+ if (hlist_empty(progs))
/* report error when trying to detach and nothing is attached */
return ERR_PTR(-ENOENT);
/* to maintain backward compatibility NONE and OVERRIDE cgroups
* allow detaching with invalid FD (prog==NULL) in legacy mode
*/
- return list_first_entry(progs, typeof(*pl), node);
+ return hlist_entry(progs->first, typeof(*pl), node);
}
if (!prog && !link)
@@ -740,7 +751,7 @@ static struct bpf_prog_list *find_detach_entry(struct list_head *progs,
return ERR_PTR(-EINVAL);
/* find the prog or link and detach it */
- list_for_each_entry(pl, progs, node) {
+ hlist_for_each_entry(pl, progs, node) {
if (pl->prog == prog && pl->link == link)
return pl;
}
@@ -764,7 +775,7 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
enum cgroup_bpf_attach_type atype;
struct bpf_prog *old_prog;
struct bpf_prog_list *pl;
- struct list_head *progs;
+ struct hlist_head *progs;
u32 flags;
int err;
@@ -793,9 +804,10 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
goto cleanup;
/* now can actually delete it from this cgroup list */
- list_del(&pl->node);
+ hlist_del(&pl->node);
+
kfree(pl);
- if (list_empty(progs))
+ if (hlist_empty(progs))
/* last program was detached, reset flags to zero */
cgrp->bpf.flags[atype] = 0;
if (old_prog)
@@ -829,7 +841,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
enum bpf_attach_type type = attr->query.attach_type;
enum cgroup_bpf_attach_type atype;
struct bpf_prog_array *effective;
- struct list_head *progs;
+ struct hlist_head *progs;
struct bpf_prog *prog;
int cnt, ret = 0, i;
u32 flags;
@@ -868,7 +880,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
u32 id;
i = 0;
- list_for_each_entry(pl, progs, node) {
+ hlist_for_each_entry(pl, progs, node) {
prog = prog_list_prog(pl);
id = prog->aux->id;
if (copy_to_user(prog_ids + i, &id, sizeof(id)))
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor
2022-04-19 19:00 [PATCH bpf-next v5 0/8] bpf: cgroup_sock lsm flavor Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 1/8] bpf: add bpf_func_t and trampoline helpers Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 2/8] bpf: convert cgroup_bpf.progs to hlist Stanislav Fomichev
@ 2022-04-19 19:00 ` Stanislav Fomichev
2022-04-26 6:27 ` Martin KaFai Lau
2022-04-27 0:10 ` Martin KaFai Lau
2022-04-19 19:00 ` [PATCH bpf-next v5 4/8] bpf: minimize number of allocated lsm slots per program Stanislav Fomichev
` (4 subsequent siblings)
7 siblings, 2 replies; 15+ messages in thread
From: Stanislav Fomichev @ 2022-04-19 19:00 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, andrii, Stanislav Fomichev
Allow attaching to lsm hooks in the cgroup context.
Attaching to per-cgroup LSM works exactly like attaching
to other per-cgroup hooks. New BPF_LSM_CGROUP is added
to trigger new mode; the actual lsm hook we attach to is
signaled via existing attach_btf_id.
For the hooks that have 'struct socket' as its first argument,
we use the cgroup associated with that socket. For the rest,
we use 'current' cgroup (this is all on default hierarchy == v2 only).
Note that for the hooks that work on 'struct sock' we still
take the cgroup from 'current' because most of the time,
the 'sock' argument is not properly initialized.
Behind the scenes, we allocate a shim program that is attached
to the trampoline and runs cgroup effective BPF programs array.
This shim has some rudimentary ref counting and can be shared
between several programs attaching to the same per-cgroup lsm hook.
Note that this patch bloats cgroup size because we add 211
cgroup_bpf_attach_type(s) for simplicity sake. This will be
addressed in the subsequent patch.
Also note that we only add non-sleepable flavor for now. To enable
sleepable use-cases, BPF_PROG_RUN_ARRAY_CG has to grab trace rcu,
shim programs have to be freed via trace rcu, cgroup_bpf.effective
should be also trace-rcu-managed + maybe some other changes that
I'm not aware of.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/linux/bpf-cgroup-defs.h | 6 +
include/linux/bpf-cgroup.h | 7 ++
include/linux/bpf.h | 13 +++
include/linux/bpf_lsm.h | 14 +++
include/uapi/linux/bpf.h | 1 +
kernel/bpf/bpf_lsm.c | 63 +++++++++++
kernel/bpf/btf.c | 11 ++
kernel/bpf/cgroup.c | 189 ++++++++++++++++++++++++++++++--
kernel/bpf/syscall.c | 10 ++
kernel/bpf/trampoline.c | 145 ++++++++++++++++++++++++
kernel/bpf/verifier.c | 1 +
tools/include/uapi/linux/bpf.h | 1 +
12 files changed, 449 insertions(+), 12 deletions(-)
diff --git a/include/linux/bpf-cgroup-defs.h b/include/linux/bpf-cgroup-defs.h
index 5d268e76d8e6..d5a70a35dace 100644
--- a/include/linux/bpf-cgroup-defs.h
+++ b/include/linux/bpf-cgroup-defs.h
@@ -10,6 +10,8 @@
struct bpf_prog_array;
+#define CGROUP_LSM_NUM 211 /* will be addressed in the next patch */
+
enum cgroup_bpf_attach_type {
CGROUP_BPF_ATTACH_TYPE_INVALID = -1,
CGROUP_INET_INGRESS = 0,
@@ -35,6 +37,10 @@ enum cgroup_bpf_attach_type {
CGROUP_INET4_GETSOCKNAME,
CGROUP_INET6_GETSOCKNAME,
CGROUP_INET_SOCK_RELEASE,
+#ifdef CONFIG_BPF_LSM
+ CGROUP_LSM_START,
+ CGROUP_LSM_END = CGROUP_LSM_START + CGROUP_LSM_NUM - 1,
+#endif
MAX_CGROUP_BPF_ATTACH_TYPE
};
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 69b611f7b2c7..2a3482161ca2 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -23,6 +23,13 @@ struct ctl_table;
struct ctl_table_header;
struct task_struct;
+unsigned int __cgroup_bpf_run_lsm_sock(const void *ctx,
+ const struct bpf_insn *insn);
+unsigned int __cgroup_bpf_run_lsm_socket(const void *ctx,
+ const struct bpf_insn *insn);
+unsigned int __cgroup_bpf_run_lsm_current(const void *ctx,
+ const struct bpf_insn *insn);
+
#ifdef CONFIG_CGROUP_BPF
#define CGROUP_ATYPE(type) \
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index ab2ba5f4bc9c..20328c8fe24f 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -980,6 +980,7 @@ struct bpf_prog_aux {
u64 load_time; /* ns since boottime */
u32 verified_insns;
struct bpf_map *cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE];
+ int cgroup_atype; /* enum cgroup_bpf_attach_type */
char name[BPF_OBJ_NAME_LEN];
#ifdef CONFIG_SECURITY
void *security;
@@ -1117,6 +1118,9 @@ struct bpf_dummy_ops {
int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,
union bpf_attr __user *uattr);
#endif
+int bpf_trampoline_link_cgroup_shim(struct bpf_prog *prog,
+ struct bpf_attach_target_info *tgt_info);
+void bpf_trampoline_unlink_cgroup_shim(struct bpf_prog *prog);
#else
static inline const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id)
{
@@ -1140,6 +1144,14 @@ static inline int bpf_struct_ops_map_sys_lookup_elem(struct bpf_map *map,
{
return -EINVAL;
}
+static inline int bpf_trampoline_link_cgroup_shim(struct bpf_prog *prog,
+ struct bpf_attach_target_info *tgt_info)
+{
+ return -EOPNOTSUPP;
+}
+static inline void bpf_trampoline_unlink_cgroup_shim(struct bpf_prog *prog)
+{
+}
#endif
struct bpf_array {
@@ -2282,6 +2294,7 @@ void *bpf_arch_text_copy(void *dst, void *src, size_t len);
struct btf_id_set;
bool btf_id_set_contains(const struct btf_id_set *set, u32 id);
+int btf_id_set_index(const struct btf_id_set *set, u32 id);
#define MAX_BPRINTF_VARARGS 12
diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
index 479c101546ad..7f0e59f5f9be 100644
--- a/include/linux/bpf_lsm.h
+++ b/include/linux/bpf_lsm.h
@@ -42,6 +42,9 @@ extern const struct bpf_func_proto bpf_inode_storage_get_proto;
extern const struct bpf_func_proto bpf_inode_storage_delete_proto;
void bpf_inode_storage_free(struct inode *inode);
+int bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func);
+int bpf_lsm_hook_idx(u32 btf_id);
+
#else /* !CONFIG_BPF_LSM */
static inline bool bpf_lsm_is_sleepable_hook(u32 btf_id)
@@ -65,6 +68,17 @@ static inline void bpf_inode_storage_free(struct inode *inode)
{
}
+static inline int bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog,
+ bpf_func_t *bpf_func)
+{
+ return -ENOENT;
+}
+
+static inline int bpf_lsm_hook_idx(u32 btf_id)
+{
+ return -EINVAL;
+}
+
#endif /* CONFIG_BPF_LSM */
#endif /* _LINUX_BPF_LSM_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d14b10b85e51..bbe48a2dd852 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -998,6 +998,7 @@ enum bpf_attach_type {
BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
BPF_PERF_EVENT,
BPF_TRACE_KPROBE_MULTI,
+ BPF_LSM_CGROUP,
__MAX_BPF_ATTACH_TYPE
};
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index 064eccba641d..2161cba1fe0c 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -16,6 +16,7 @@
#include <linux/bpf_local_storage.h>
#include <linux/btf_ids.h>
#include <linux/ima.h>
+#include <linux/bpf-cgroup.h>
/* For every LSM hook that allows attachment of BPF programs, declare a nop
* function where a BPF program can be attached.
@@ -35,6 +36,68 @@ BTF_SET_START(bpf_lsm_hooks)
#undef LSM_HOOK
BTF_SET_END(bpf_lsm_hooks)
+/* List of LSM hooks that should operate on 'current' cgroup regardless
+ * of function signature.
+ */
+BTF_SET_START(bpf_lsm_current_hooks)
+/* operate on freshly allocated sk without any cgroup association */
+BTF_ID(func, bpf_lsm_sk_alloc_security)
+BTF_ID(func, bpf_lsm_sk_free_security)
+BTF_SET_END(bpf_lsm_current_hooks)
+
+int bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog,
+ bpf_func_t *bpf_func)
+{
+ const struct btf_type *first_arg_type;
+ const struct btf_type *sock_type;
+ const struct btf_type *sk_type;
+ const struct btf *btf_vmlinux;
+ const struct btf_param *args;
+ s32 type_id;
+
+ if (!prog->aux->attach_func_proto ||
+ !btf_type_is_func_proto(prog->aux->attach_func_proto))
+ return -EINVAL;
+
+ if (btf_type_vlen(prog->aux->attach_func_proto) < 1 ||
+ btf_id_set_contains(&bpf_lsm_current_hooks,
+ prog->aux->attach_btf_id)) {
+ *bpf_func = __cgroup_bpf_run_lsm_current;
+ return 0;
+ }
+
+ args = btf_params(prog->aux->attach_func_proto);
+
+ btf_vmlinux = bpf_get_btf_vmlinux();
+ if (!btf_vmlinux)
+ return -EINVAL;
+
+ type_id = btf_find_by_name_kind(btf_vmlinux, "socket", BTF_KIND_STRUCT);
+ if (type_id < 0)
+ return -EINVAL;
+ sock_type = btf_type_by_id(btf_vmlinux, type_id);
+
+ type_id = btf_find_by_name_kind(btf_vmlinux, "sock", BTF_KIND_STRUCT);
+ if (type_id < 0)
+ return -EINVAL;
+ sk_type = btf_type_by_id(btf_vmlinux, type_id);
+
+ first_arg_type = btf_type_resolve_ptr(btf_vmlinux, args[0].type, NULL);
+ if (first_arg_type == sock_type)
+ *bpf_func = __cgroup_bpf_run_lsm_socket;
+ else if (first_arg_type == sk_type)
+ *bpf_func = __cgroup_bpf_run_lsm_sock;
+ else
+ *bpf_func = __cgroup_bpf_run_lsm_current;
+
+ return 0;
+}
+
+int bpf_lsm_hook_idx(u32 btf_id)
+{
+ return btf_id_set_index(&bpf_lsm_hooks, btf_id);
+}
+
int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
const struct bpf_prog *prog)
{
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 0918a39279f6..4199de31f49c 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4971,6 +4971,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
if (arg == nr_args) {
switch (prog->expected_attach_type) {
+ case BPF_LSM_CGROUP:
case BPF_LSM_MAC:
case BPF_TRACE_FEXIT:
/* When LSM programs are attached to void LSM hooks
@@ -6396,6 +6397,16 @@ static int btf_id_cmp_func(const void *a, const void *b)
return *pa - *pb;
}
+int btf_id_set_index(const struct btf_id_set *set, u32 id)
+{
+ const u32 *p;
+
+ p = bsearch(&id, set->ids, set->cnt, sizeof(u32), btf_id_cmp_func);
+ if (!p)
+ return -1;
+ return p - set->ids;
+}
+
bool btf_id_set_contains(const struct btf_id_set *set, u32 id)
{
return bsearch(&id, set->ids, set->cnt, sizeof(u32), btf_id_cmp_func) != NULL;
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index aaf9e36f2736..ba0e0c7a661d 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -14,6 +14,9 @@
#include <linux/string.h>
#include <linux/bpf.h>
#include <linux/bpf-cgroup.h>
+#include <linux/btf_ids.h>
+#include <linux/bpf_lsm.h>
+#include <linux/bpf_verifier.h>
#include <net/sock.h>
#include <net/bpf_sk_storage.h>
@@ -88,6 +91,85 @@ bpf_prog_run_array_cg(const struct cgroup_bpf *cgrp,
return run_ctx.retval;
}
+unsigned int __cgroup_bpf_run_lsm_sock(const void *ctx,
+ const struct bpf_insn *insn)
+{
+ const struct bpf_prog *shim_prog;
+ struct sock *sk;
+ struct cgroup *cgrp;
+ int ret = 0;
+ u64 *regs;
+
+ regs = (u64 *)ctx;
+ sk = (void *)(unsigned long)regs[BPF_REG_0];
+ /*shim_prog = container_of(insn, struct bpf_prog, insnsi);*/
+ shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
+
+ cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
+ if (likely(cgrp))
+ ret = bpf_prog_run_array_cg(&cgrp->bpf,
+ shim_prog->aux->cgroup_atype,
+ ctx, bpf_prog_run, 0);
+ return ret;
+}
+
+unsigned int __cgroup_bpf_run_lsm_socket(const void *ctx,
+ const struct bpf_insn *insn)
+{
+ const struct bpf_prog *shim_prog;
+ struct socket *sock;
+ struct cgroup *cgrp;
+ int ret = 0;
+ u64 *regs;
+
+ regs = (u64 *)ctx;
+ sock = (void *)(unsigned long)regs[BPF_REG_0];
+ /*shim_prog = container_of(insn, struct bpf_prog, insnsi);*/
+ shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
+
+ cgrp = sock_cgroup_ptr(&sock->sk->sk_cgrp_data);
+ if (likely(cgrp))
+ ret = bpf_prog_run_array_cg(&cgrp->bpf,
+ shim_prog->aux->cgroup_atype,
+ ctx, bpf_prog_run, 0);
+ return ret;
+}
+
+unsigned int __cgroup_bpf_run_lsm_current(const void *ctx,
+ const struct bpf_insn *insn)
+{
+ const struct bpf_prog *shim_prog;
+ struct cgroup *cgrp;
+ int ret = 0;
+
+ if (unlikely(!current))
+ return 0;
+
+ /*shim_prog = container_of(insn, struct bpf_prog, insnsi);*/
+ shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
+
+ rcu_read_lock();
+ cgrp = task_dfl_cgroup(current);
+ if (likely(cgrp))
+ ret = bpf_prog_run_array_cg(&cgrp->bpf,
+ shim_prog->aux->cgroup_atype,
+ ctx, bpf_prog_run, 0);
+ rcu_read_unlock();
+ return ret;
+}
+
+#ifdef CONFIG_BPF_LSM
+static enum cgroup_bpf_attach_type bpf_lsm_attach_type_get(u32 attach_btf_id)
+{
+ return CGROUP_LSM_START + bpf_lsm_hook_idx(attach_btf_id);
+}
+#else
+static enum cgroup_bpf_attach_type bpf_lsm_attach_type_get(u32 attach_btf_id)
+{
+ return -EOPNOTSUPP;
+}
+#endif /* CONFIG_BPF_LSM */
+
void cgroup_bpf_offline(struct cgroup *cgrp)
{
cgroup_get(cgrp);
@@ -155,6 +237,14 @@ static void bpf_cgroup_storages_link(struct bpf_cgroup_storage *storages[],
bpf_cgroup_storage_link(storages[stype], cgrp, attach_type);
}
+static void bpf_cgroup_storages_unlink(struct bpf_cgroup_storage *storages[])
+{
+ enum bpf_cgroup_storage_type stype;
+
+ for_each_cgroup_storage_type(stype)
+ bpf_cgroup_storage_unlink(storages[stype]);
+}
+
/* Called when bpf_cgroup_link is auto-detached from dying cgroup.
* It drops cgroup and bpf_prog refcounts, and marks bpf_link as defunct. It
* doesn't free link memory, which will eventually be done by bpf_link's
@@ -166,6 +256,16 @@ static void bpf_cgroup_link_auto_detach(struct bpf_cgroup_link *link)
link->cgroup = NULL;
}
+static void bpf_cgroup_lsm_shim_release(struct bpf_prog *prog,
+ enum cgroup_bpf_attach_type atype)
+{
+ if (prog->aux->cgroup_atype < CGROUP_LSM_START ||
+ prog->aux->cgroup_atype > CGROUP_LSM_END)
+ return;
+
+ bpf_trampoline_unlink_cgroup_shim(prog);
+}
+
/**
* cgroup_bpf_release() - put references of all bpf programs and
* release all cgroup bpf data
@@ -190,10 +290,18 @@ static void cgroup_bpf_release(struct work_struct *work)
hlist_for_each_entry_safe(pl, pltmp, progs, node) {
hlist_del(&pl->node);
- if (pl->prog)
+ if (pl->prog) {
+ if (atype == BPF_LSM_CGROUP)
+ bpf_cgroup_lsm_shim_release(pl->prog,
+ atype);
bpf_prog_put(pl->prog);
- if (pl->link)
+ }
+ if (pl->link) {
+ if (atype == BPF_LSM_CGROUP)
+ bpf_cgroup_lsm_shim_release(pl->link->link.prog,
+ atype);
bpf_cgroup_link_auto_detach(pl->link);
+ }
kfree(pl);
static_branch_dec(&cgroup_bpf_enabled_key[atype]);
}
@@ -506,6 +614,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
struct bpf_prog *old_prog = NULL;
struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
+ struct bpf_attach_target_info tgt_info = {};
enum cgroup_bpf_attach_type atype;
struct bpf_prog_list *pl;
struct hlist_head *progs;
@@ -522,9 +631,35 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
/* replace_prog implies BPF_F_REPLACE, and vice versa */
return -EINVAL;
- atype = to_cgroup_bpf_attach_type(type);
- if (atype < 0)
- return -EINVAL;
+ if (type == BPF_LSM_CGROUP) {
+ struct bpf_prog *p = prog ? : link->link.prog;
+
+ if (replace_prog) {
+ /* Reusing shim from the original program.
+ */
+ if (replace_prog->aux->attach_btf_id !=
+ p->aux->attach_btf_id)
+ return -EINVAL;
+
+ atype = replace_prog->aux->cgroup_atype;
+ } else {
+ err = bpf_check_attach_target(NULL, p, NULL,
+ p->aux->attach_btf_id,
+ &tgt_info);
+ if (err)
+ return -EINVAL;
+
+ atype = bpf_lsm_attach_type_get(p->aux->attach_btf_id);
+ if (atype < 0)
+ return atype;
+ }
+
+ p->aux->cgroup_atype = atype;
+ } else {
+ atype = to_cgroup_bpf_attach_type(type);
+ if (atype < 0)
+ return -EINVAL;
+ }
progs = &cgrp->bpf.progs[atype];
@@ -580,13 +715,26 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
if (err)
goto cleanup;
+ bpf_cgroup_storages_link(new_storage, cgrp, type);
+
+ if (type == BPF_LSM_CGROUP && !old_prog) {
+ struct bpf_prog *p = prog ? : link->link.prog;
+
+ err = bpf_trampoline_link_cgroup_shim(p, &tgt_info);
+ if (err)
+ goto cleanup_trampoline;
+ }
+
if (old_prog)
bpf_prog_put(old_prog);
else
static_branch_inc(&cgroup_bpf_enabled_key[atype]);
- bpf_cgroup_storages_link(new_storage, cgrp, type);
+
return 0;
+cleanup_trampoline:
+ bpf_cgroup_storages_unlink(new_storage);
+
cleanup:
if (old_prog) {
pl->prog = old_prog;
@@ -678,9 +826,13 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
struct hlist_head *progs;
bool found = false;
- atype = to_cgroup_bpf_attach_type(link->type);
- if (atype < 0)
- return -EINVAL;
+ if (link->type == BPF_LSM_CGROUP) {
+ atype = link->link.prog->aux->cgroup_atype;
+ } else {
+ atype = to_cgroup_bpf_attach_type(link->type);
+ if (atype < 0)
+ return -EINVAL;
+ }
progs = &cgrp->bpf.progs[atype];
@@ -696,6 +848,9 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
if (!found)
return -ENOENT;
+ if (link->type == BPF_LSM_CGROUP)
+ new_prog->aux->cgroup_atype = atype;
+
old_prog = xchg(&link->link.prog, new_prog);
replace_effective_prog(cgrp, atype, link);
bpf_prog_put(old_prog);
@@ -779,9 +934,15 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
u32 flags;
int err;
- atype = to_cgroup_bpf_attach_type(type);
- if (atype < 0)
- return -EINVAL;
+ if (type == BPF_LSM_CGROUP) {
+ struct bpf_prog *p = prog ? : link->link.prog;
+
+ atype = p->aux->cgroup_atype;
+ } else {
+ atype = to_cgroup_bpf_attach_type(type);
+ if (atype < 0)
+ return -EINVAL;
+ }
progs = &cgrp->bpf.progs[atype];
flags = cgrp->bpf.flags[atype];
@@ -803,6 +964,10 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
if (err)
goto cleanup;
+ if (type == BPF_LSM_CGROUP)
+ bpf_cgroup_lsm_shim_release(prog ? : link->link.prog,
+ atype);
+
/* now can actually delete it from this cgroup list */
hlist_del(&pl->node);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index e9621cfa09f2..d94f4951154e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3139,6 +3139,11 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
return prog->enforce_expected_attach_type &&
prog->expected_attach_type != attach_type ?
-EINVAL : 0;
+ case BPF_PROG_TYPE_LSM:
+ if (prog->expected_attach_type != BPF_LSM_CGROUP)
+ return -EINVAL;
+ return 0;
+
default:
return 0;
}
@@ -3194,6 +3199,8 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
return BPF_PROG_TYPE_SK_LOOKUP;
case BPF_XDP:
return BPF_PROG_TYPE_XDP;
+ case BPF_LSM_CGROUP:
+ return BPF_PROG_TYPE_LSM;
default:
return BPF_PROG_TYPE_UNSPEC;
}
@@ -3247,6 +3254,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
case BPF_PROG_TYPE_CGROUP_SOCKOPT:
case BPF_PROG_TYPE_CGROUP_SYSCTL:
case BPF_PROG_TYPE_SOCK_OPS:
+ case BPF_PROG_TYPE_LSM:
ret = cgroup_bpf_prog_attach(attr, ptype, prog);
break;
default:
@@ -3284,6 +3292,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
case BPF_PROG_TYPE_CGROUP_SOCKOPT:
case BPF_PROG_TYPE_CGROUP_SYSCTL:
case BPF_PROG_TYPE_SOCK_OPS:
+ case BPF_PROG_TYPE_LSM:
return cgroup_bpf_prog_detach(attr, ptype);
default:
return -EINVAL;
@@ -4317,6 +4326,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
case BPF_PROG_TYPE_CGROUP_DEVICE:
case BPF_PROG_TYPE_CGROUP_SYSCTL:
case BPF_PROG_TYPE_CGROUP_SOCKOPT:
+ case BPF_PROG_TYPE_LSM:
ret = cgroup_bpf_link_attach(attr, prog);
break;
case BPF_PROG_TYPE_TRACING:
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 0c4fd194e801..c76dfa4ea2d9 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -11,6 +11,8 @@
#include <linux/rcupdate_wait.h>
#include <linux/module.h>
#include <linux/static_call.h>
+#include <linux/bpf_verifier.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 = {
@@ -485,6 +487,149 @@ int bpf_trampoline_unlink_prog(struct bpf_prog *prog, struct bpf_trampoline *tr)
return err;
}
+#if defined(CONFIG_BPF_JIT) && defined(CONFIG_BPF_SYSCALL)
+static struct bpf_prog *cgroup_shim_alloc(const struct bpf_prog *prog,
+ bpf_func_t bpf_func)
+{
+ struct bpf_prog *p;
+
+ p = bpf_prog_alloc(1, 0);
+ if (!p)
+ return NULL;
+
+ p->jited = false;
+ p->bpf_func = bpf_func;
+
+ p->aux->cgroup_atype = prog->aux->cgroup_atype;
+ p->aux->attach_func_proto = prog->aux->attach_func_proto;
+ p->aux->attach_btf_id = prog->aux->attach_btf_id;
+ p->aux->attach_btf = prog->aux->attach_btf;
+ btf_get(p->aux->attach_btf);
+ p->type = BPF_PROG_TYPE_LSM;
+ p->expected_attach_type = BPF_LSM_MAC;
+ bpf_prog_inc(p);
+
+ return p;
+}
+
+static struct bpf_prog *cgroup_shim_find(struct bpf_trampoline *tr,
+ bpf_func_t bpf_func)
+{
+ const struct bpf_prog_aux *aux;
+ int kind;
+
+ for (kind = 0; kind < BPF_TRAMP_MAX; kind++) {
+ hlist_for_each_entry(aux, &tr->progs_hlist[kind], tramp_hlist) {
+ struct bpf_prog *p = aux->prog;
+
+ if (p->bpf_func == bpf_func)
+ return p;
+ }
+ }
+
+ return NULL;
+}
+
+int bpf_trampoline_link_cgroup_shim(struct bpf_prog *prog,
+ struct bpf_attach_target_info *tgt_info)
+{
+ struct bpf_prog *shim_prog = NULL;
+ struct bpf_trampoline *tr;
+ bpf_func_t bpf_func;
+ u64 key;
+ int err;
+
+ key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf,
+ prog->aux->attach_btf_id);
+
+ err = bpf_lsm_find_cgroup_shim(prog, &bpf_func);
+ if (err)
+ return err;
+
+ tr = bpf_trampoline_get(key, tgt_info);
+ if (!tr)
+ return -ENOMEM;
+
+ mutex_lock(&tr->mutex);
+
+ shim_prog = cgroup_shim_find(tr, bpf_func);
+ if (shim_prog) {
+ /* Reusing existing shim attached by the other program.
+ */
+ bpf_prog_inc(shim_prog);
+ mutex_unlock(&tr->mutex);
+ return 0;
+ }
+
+ /* Allocate and install new shim.
+ */
+
+ shim_prog = cgroup_shim_alloc(prog, bpf_func);
+ if (!shim_prog) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ err = __bpf_trampoline_link_prog(shim_prog, tr);
+ if (err)
+ goto out;
+
+ mutex_unlock(&tr->mutex);
+
+ return 0;
+out:
+ if (shim_prog)
+ bpf_prog_put(shim_prog);
+
+ mutex_unlock(&tr->mutex);
+ return err;
+}
+
+void bpf_trampoline_unlink_cgroup_shim(struct bpf_prog *prog)
+{
+ struct bpf_prog *shim_prog;
+ struct bpf_trampoline *tr;
+ bpf_func_t bpf_func;
+ u64 key;
+ int err;
+
+ key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf,
+ prog->aux->attach_btf_id);
+
+ err = bpf_lsm_find_cgroup_shim(prog, &bpf_func);
+ if (err)
+ return;
+
+ tr = bpf_trampoline_lookup(key);
+ if (!tr)
+ return;
+
+ mutex_lock(&tr->mutex);
+
+ shim_prog = cgroup_shim_find(tr, bpf_func);
+ if (shim_prog) {
+ /* We use shim_prog refcnt for tracking whether to
+ * remove the shim program from the trampoline.
+ * Trampoline's mutex is held while refcnt is
+ * added/subtracted so we don't need to care about
+ * potential races.
+ */
+
+ if (atomic64_read(&shim_prog->aux->refcnt) == 1)
+ WARN_ON_ONCE(__bpf_trampoline_unlink_prog(shim_prog, tr));
+
+ bpf_prog_put(shim_prog);
+ }
+
+ mutex_unlock(&tr->mutex);
+
+ bpf_trampoline_put(tr); /* bpf_trampoline_lookup */
+
+ if (shim_prog)
+ bpf_trampoline_put(tr);
+}
+#endif
+
struct bpf_trampoline *bpf_trampoline_get(u64 key,
struct bpf_attach_target_info *tgt_info)
{
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9c1a02b82ecd..cc84954846d7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14197,6 +14197,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
fallthrough;
case BPF_MODIFY_RETURN:
case BPF_LSM_MAC:
+ case BPF_LSM_CGROUP:
case BPF_TRACE_FENTRY:
case BPF_TRACE_FEXIT:
if (!btf_type_is_func(t)) {
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index d14b10b85e51..bbe48a2dd852 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -998,6 +998,7 @@ enum bpf_attach_type {
BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
BPF_PERF_EVENT,
BPF_TRACE_KPROBE_MULTI,
+ BPF_LSM_CGROUP,
__MAX_BPF_ATTACH_TYPE
};
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH bpf-next v5 4/8] bpf: minimize number of allocated lsm slots per program
2022-04-19 19:00 [PATCH bpf-next v5 0/8] bpf: cgroup_sock lsm flavor Stanislav Fomichev
` (2 preceding siblings ...)
2022-04-19 19:00 ` [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor Stanislav Fomichev
@ 2022-04-19 19:00 ` Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 5/8] bpf: allow writing to a subset of sock fields from lsm progtype Stanislav Fomichev
` (3 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Stanislav Fomichev @ 2022-04-19 19:00 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, andrii, Stanislav Fomichev
Previous patch adds 1:1 mapping between all 211 LSM hooks
and bpf_cgroup program array. Instead of reserving a slot per
possible hook, reserve 10 slots per cgroup for lsm programs.
Those slots are dynamically allocated on demand and reclaimed.
It should be possible to eventually extend this idea to all hooks if
the memory consumption is unacceptable and shrink overall effective
programs array.
struct cgroup_bpf {
struct bpf_prog_array * effective[33]; /* 0 264 */
/* --- cacheline 4 boundary (256 bytes) was 8 bytes ago --- */
struct hlist_head progs[33]; /* 264 264 */
/* --- cacheline 8 boundary (512 bytes) was 16 bytes ago --- */
u8 flags[33]; /* 528 33 */
/* XXX 7 bytes hole, try to pack */
struct list_head storages; /* 568 16 */
/* --- cacheline 9 boundary (576 bytes) was 8 bytes ago --- */
struct bpf_prog_array * inactive; /* 584 8 */
struct percpu_ref refcnt; /* 592 16 */
struct work_struct release_work; /* 608 72 */
/* size: 680, cachelines: 11, members: 7 */
/* sum members: 673, holes: 1, sum holes: 7 */
/* last cacheline: 40 bytes */
};
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/linux/bpf-cgroup-defs.h | 4 +-
include/linux/bpf_lsm.h | 6 --
kernel/bpf/bpf_lsm.c | 5 --
kernel/bpf/cgroup.c | 108 +++++++++++++++++++++++++++-----
4 files changed, 96 insertions(+), 27 deletions(-)
diff --git a/include/linux/bpf-cgroup-defs.h b/include/linux/bpf-cgroup-defs.h
index d5a70a35dace..603f0a51eb3f 100644
--- a/include/linux/bpf-cgroup-defs.h
+++ b/include/linux/bpf-cgroup-defs.h
@@ -10,7 +10,9 @@
struct bpf_prog_array;
-#define CGROUP_LSM_NUM 211 /* will be addressed in the next patch */
+/* Maximum number of concurrently attachable per-cgroup LSM hooks.
+ */
+#define CGROUP_LSM_NUM 10
enum cgroup_bpf_attach_type {
CGROUP_BPF_ATTACH_TYPE_INVALID = -1,
diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
index 7f0e59f5f9be..613de44aa429 100644
--- a/include/linux/bpf_lsm.h
+++ b/include/linux/bpf_lsm.h
@@ -43,7 +43,6 @@ extern const struct bpf_func_proto bpf_inode_storage_delete_proto;
void bpf_inode_storage_free(struct inode *inode);
int bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func);
-int bpf_lsm_hook_idx(u32 btf_id);
#else /* !CONFIG_BPF_LSM */
@@ -74,11 +73,6 @@ static inline int bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog,
return -ENOENT;
}
-static inline int bpf_lsm_hook_idx(u32 btf_id)
-{
- return -EINVAL;
-}
-
#endif /* CONFIG_BPF_LSM */
#endif /* _LINUX_BPF_LSM_H */
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index 2161cba1fe0c..7f8a631da90f 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -93,11 +93,6 @@ int bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog,
return 0;
}
-int bpf_lsm_hook_idx(u32 btf_id)
-{
- return btf_id_set_index(&bpf_lsm_hooks, btf_id);
-}
-
int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
const struct bpf_prog *prog)
{
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index ba0e0c7a661d..539d17f7c9b2 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -106,10 +106,13 @@ unsigned int __cgroup_bpf_run_lsm_sock(const void *ctx,
shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
- if (likely(cgrp))
+ if (likely(cgrp)) {
+ rcu_read_lock(); /* See bpf_lsm_attach_type_get(). */
ret = bpf_prog_run_array_cg(&cgrp->bpf,
shim_prog->aux->cgroup_atype,
ctx, bpf_prog_run, 0);
+ rcu_read_unlock();
+ }
return ret;
}
@@ -128,10 +131,13 @@ unsigned int __cgroup_bpf_run_lsm_socket(const void *ctx,
shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
cgrp = sock_cgroup_ptr(&sock->sk->sk_cgrp_data);
- if (likely(cgrp))
+ if (likely(cgrp)) {
+ rcu_read_lock(); /* See bpf_lsm_attach_type_get(). */
ret = bpf_prog_run_array_cg(&cgrp->bpf,
shim_prog->aux->cgroup_atype,
ctx, bpf_prog_run, 0);
+ rcu_read_unlock();
+ }
return ret;
}
@@ -148,7 +154,7 @@ unsigned int __cgroup_bpf_run_lsm_current(const void *ctx,
/*shim_prog = container_of(insn, struct bpf_prog, insnsi);*/
shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
- rcu_read_lock();
+ rcu_read_lock(); /* See bpf_lsm_attach_type_get(). */
cgrp = task_dfl_cgroup(current);
if (likely(cgrp))
ret = bpf_prog_run_array_cg(&cgrp->bpf,
@@ -159,15 +165,68 @@ unsigned int __cgroup_bpf_run_lsm_current(const void *ctx,
}
#ifdef CONFIG_BPF_LSM
+/* Readers are protected by rcu+synchronize_rcu.
+ * Writers are protected by cgroup_mutex.
+ */
+int cgroup_lsm_atype_usecnt[CGROUP_LSM_NUM];
+u32 cgroup_lsm_atype_btf_id[CGROUP_LSM_NUM];
+
static enum cgroup_bpf_attach_type bpf_lsm_attach_type_get(u32 attach_btf_id)
{
- return CGROUP_LSM_START + bpf_lsm_hook_idx(attach_btf_id);
+ int i;
+
+ WARN_ON_ONCE(!mutex_is_locked(&cgroup_mutex));
+
+ for (i = 0; i < ARRAY_SIZE(cgroup_lsm_atype_btf_id); i++) {
+ if (cgroup_lsm_atype_btf_id[i] != attach_btf_id)
+ continue;
+
+ cgroup_lsm_atype_usecnt[i]++;
+ return CGROUP_LSM_START + i;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(cgroup_lsm_atype_usecnt); i++) {
+ if (cgroup_lsm_atype_usecnt[i] != 0)
+ continue;
+
+ cgroup_lsm_atype_btf_id[i] = attach_btf_id;
+ cgroup_lsm_atype_usecnt[i] = 1;
+ return CGROUP_LSM_START + i;
+ }
+
+ return -E2BIG;
+}
+
+static void bpf_lsm_attach_type_put(u32 attach_btf_id)
+{
+ int i;
+
+ WARN_ON_ONCE(!mutex_is_locked(&cgroup_mutex));
+
+ for (i = 0; i < ARRAY_SIZE(cgroup_lsm_atype_btf_id); i++) {
+ if (cgroup_lsm_atype_btf_id[i] != attach_btf_id)
+ continue;
+
+ if (--cgroup_lsm_atype_usecnt[i] <= 0) {
+ /* Wait for any existing users to finish.
+ */
+ synchronize_rcu();
+ WARN_ON_ONCE(cgroup_lsm_atype_usecnt[i] < 0);
+ }
+ return;
+ }
+
+ WARN_ON_ONCE(1);
}
#else
static enum cgroup_bpf_attach_type bpf_lsm_attach_type_get(u32 attach_btf_id)
{
return -EOPNOTSUPP;
}
+
+static void bpf_lsm_attach_type_put(u32 attach_btf_id)
+{
+}
#endif /* CONFIG_BPF_LSM */
void cgroup_bpf_offline(struct cgroup *cgrp)
@@ -264,6 +323,7 @@ static void bpf_cgroup_lsm_shim_release(struct bpf_prog *prog,
return;
bpf_trampoline_unlink_cgroup_shim(prog);
+ bpf_lsm_attach_type_put(prog->aux->attach_btf_id);
}
/**
@@ -663,27 +723,37 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
progs = &cgrp->bpf.progs[atype];
- if (!hierarchy_allows_attach(cgrp, atype))
- return -EPERM;
+ if (!hierarchy_allows_attach(cgrp, atype)) {
+ err = -EPERM;
+ goto cleanup_attach_type;
+ }
- if (!hlist_empty(progs) && cgrp->bpf.flags[atype] != saved_flags)
+ if (!hlist_empty(progs) && cgrp->bpf.flags[atype] != saved_flags) {
/* Disallow attaching non-overridable on top
* of existing overridable in this cgroup.
* Disallow attaching multi-prog if overridable or none
*/
- return -EPERM;
+ err = -EPERM;
+ goto cleanup_attach_type;
+ }
- if (prog_list_length(progs) >= BPF_CGROUP_MAX_PROGS)
- return -E2BIG;
+ if (prog_list_length(progs) >= BPF_CGROUP_MAX_PROGS) {
+ err = -E2BIG;
+ goto cleanup_attach_type;
+ }
pl = find_attach_entry(progs, prog, link, replace_prog,
flags & BPF_F_ALLOW_MULTI);
- if (IS_ERR(pl))
- return PTR_ERR(pl);
+ if (IS_ERR(pl)) {
+ err = PTR_ERR(pl);
+ goto cleanup_attach_type;
+ }
if (bpf_cgroup_storages_alloc(storage, new_storage, type,
- prog ? : link->link.prog, cgrp))
- return -ENOMEM;
+ prog ? : link->link.prog, cgrp)) {
+ err = -ENOMEM;
+ goto cleanup_attach_type;
+ }
if (pl) {
old_prog = pl->prog;
@@ -693,7 +763,8 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
pl = kmalloc(sizeof(*pl), GFP_KERNEL);
if (!pl) {
bpf_cgroup_storages_free(new_storage);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto cleanup_attach_type;
}
if (hlist_empty(progs))
hlist_add_head(&pl->node, progs);
@@ -745,6 +816,13 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
hlist_del(&pl->node);
kfree(pl);
}
+
+cleanup_attach_type:
+ if (type == BPF_LSM_CGROUP) {
+ struct bpf_prog *p = prog ? : link->link.prog;
+
+ bpf_lsm_attach_type_put(p->aux->attach_btf_id);
+ }
return err;
}
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH bpf-next v5 5/8] bpf: allow writing to a subset of sock fields from lsm progtype
2022-04-19 19:00 [PATCH bpf-next v5 0/8] bpf: cgroup_sock lsm flavor Stanislav Fomichev
` (3 preceding siblings ...)
2022-04-19 19:00 ` [PATCH bpf-next v5 4/8] bpf: minimize number of allocated lsm slots per program Stanislav Fomichev
@ 2022-04-19 19:00 ` Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 6/8] libbpf: add lsm_cgoup_sock type Stanislav Fomichev
` (2 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Stanislav Fomichev @ 2022-04-19 19:00 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, andrii, Stanislav Fomichev
For now, allow only the obvious ones, like sk_priority and sk_mark.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
kernel/bpf/bpf_lsm.c | 58 +++++++++++++++++++++++++++++++++++++++++++
kernel/bpf/verifier.c | 3 ++-
2 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index 7f8a631da90f..d49663685946 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -301,7 +301,65 @@ bool bpf_lsm_is_sleepable_hook(u32 btf_id)
const struct bpf_prog_ops lsm_prog_ops = {
};
+static int lsm_btf_struct_access(struct bpf_verifier_log *log,
+ const struct btf *btf,
+ const struct btf_type *t, int off,
+ int size, enum bpf_access_type atype,
+ u32 *next_btf_id,
+ enum bpf_type_flag *flag)
+{
+ const struct btf_type *sock_type;
+ struct btf *btf_vmlinux;
+ s32 type_id;
+ size_t end;
+
+ if (atype == BPF_READ)
+ return btf_struct_access(log, btf, t, off, size, atype, next_btf_id,
+ flag);
+
+ btf_vmlinux = bpf_get_btf_vmlinux();
+ if (!btf_vmlinux) {
+ bpf_log(log, "no vmlinux btf\n");
+ return -EOPNOTSUPP;
+ }
+
+ type_id = btf_find_by_name_kind(btf_vmlinux, "sock", BTF_KIND_STRUCT);
+ if (type_id < 0) {
+ bpf_log(log, "'struct sock' not found in vmlinux btf\n");
+ return -EINVAL;
+ }
+
+ sock_type = btf_type_by_id(btf_vmlinux, type_id);
+
+ if (t != sock_type) {
+ bpf_log(log, "only 'struct sock' writes are supported\n");
+ return -EACCES;
+ }
+
+ switch (off) {
+ case bpf_ctx_range(struct sock, sk_priority):
+ end = offsetofend(struct sock, sk_priority);
+ break;
+ case bpf_ctx_range(struct sock, sk_mark):
+ end = offsetofend(struct sock, sk_mark);
+ break;
+ default:
+ bpf_log(log, "no write support to 'struct sock' at off %d\n", off);
+ return -EACCES;
+ }
+
+ if (off + size > end) {
+ bpf_log(log,
+ "write access at off %d with size %d beyond the member of 'struct sock' ended at %zu\n",
+ off, size, end);
+ return -EACCES;
+ }
+
+ return NOT_INIT;
+}
+
const struct bpf_verifier_ops lsm_verifier_ops = {
.get_func_proto = bpf_lsm_func_proto,
.is_valid_access = btf_ctx_access,
+ .btf_struct_access = lsm_btf_struct_access,
};
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index cc84954846d7..0d6d5be30a36 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12872,7 +12872,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
insn->code = BPF_LDX | BPF_PROBE_MEM |
BPF_SIZE((insn)->code);
env->prog->aux->num_exentries++;
- } else if (resolve_prog_type(env->prog) != BPF_PROG_TYPE_STRUCT_OPS) {
+ } else if (resolve_prog_type(env->prog) != BPF_PROG_TYPE_STRUCT_OPS &&
+ resolve_prog_type(env->prog) != BPF_PROG_TYPE_LSM) {
verbose(env, "Writes through BTF pointers are not allowed\n");
return -EINVAL;
}
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH bpf-next v5 6/8] libbpf: add lsm_cgoup_sock type
2022-04-19 19:00 [PATCH bpf-next v5 0/8] bpf: cgroup_sock lsm flavor Stanislav Fomichev
` (4 preceding siblings ...)
2022-04-19 19:00 ` [PATCH bpf-next v5 5/8] bpf: allow writing to a subset of sock fields from lsm progtype Stanislav Fomichev
@ 2022-04-19 19:00 ` Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 7/8] selftests/bpf: lsm_cgroup functional test Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 8/8] selftests/bpf: verify lsm_cgroup struct sock access Stanislav Fomichev
7 siblings, 0 replies; 15+ messages in thread
From: Stanislav Fomichev @ 2022-04-19 19:00 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, andrii, Stanislav Fomichev
lsm_cgroup/ is the prefix for BPF_LSM_CGROUP.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/bpf/bpftool/common.c | 1 +
tools/lib/bpf/libbpf.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index c740142c24d8..a7a913784c47 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -66,6 +66,7 @@ const char * const attach_type_name[__MAX_BPF_ATTACH_TYPE] = {
[BPF_TRACE_FEXIT] = "fexit",
[BPF_MODIFY_RETURN] = "mod_ret",
[BPF_LSM_MAC] = "lsm_mac",
+ [BPF_LSM_CGROUP] = "lsm_cgroup",
[BPF_SK_LOOKUP] = "sk_lookup",
[BPF_TRACE_ITER] = "trace_iter",
[BPF_XDP_DEVMAP] = "xdp_devmap",
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index bf4f7ac54ebf..912cfa13814d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8701,6 +8701,7 @@ static const struct bpf_sec_def section_defs[] = {
SEC_DEF("freplace/", EXT, 0, SEC_ATTACH_BTF, attach_trace),
SEC_DEF("lsm/", LSM, BPF_LSM_MAC, SEC_ATTACH_BTF, attach_lsm),
SEC_DEF("lsm.s/", LSM, BPF_LSM_MAC, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_lsm),
+ SEC_DEF("lsm_cgroup/", LSM, BPF_LSM_CGROUP, SEC_ATTACH_BTF),
SEC_DEF("iter/", TRACING, BPF_TRACE_ITER, SEC_ATTACH_BTF, attach_iter),
SEC_DEF("iter.s/", TRACING, BPF_TRACE_ITER, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_iter),
SEC_DEF("syscall", SYSCALL, 0, SEC_SLEEPABLE),
@@ -9122,6 +9123,7 @@ void btf_get_kernel_prefix_kind(enum bpf_attach_type attach_type,
*kind = BTF_KIND_TYPEDEF;
break;
case BPF_LSM_MAC:
+ case BPF_LSM_CGROUP:
*prefix = BTF_LSM_PREFIX;
*kind = BTF_KIND_FUNC;
break;
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH bpf-next v5 7/8] selftests/bpf: lsm_cgroup functional test
2022-04-19 19:00 [PATCH bpf-next v5 0/8] bpf: cgroup_sock lsm flavor Stanislav Fomichev
` (5 preceding siblings ...)
2022-04-19 19:00 ` [PATCH bpf-next v5 6/8] libbpf: add lsm_cgoup_sock type Stanislav Fomichev
@ 2022-04-19 19:00 ` Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 8/8] selftests/bpf: verify lsm_cgroup struct sock access Stanislav Fomichev
7 siblings, 0 replies; 15+ messages in thread
From: Stanislav Fomichev @ 2022-04-19 19:00 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, andrii, Stanislav Fomichev
Functional test that exercises the following:
1. apply default sk_priority policy
2. permit TX-only AF_PACKET socket
3. cgroup attach/detach/replace
4. reusing trampoline shim
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../selftests/bpf/prog_tests/lsm_cgroup.c | 213 ++++++++++++++++++
.../testing/selftests/bpf/progs/lsm_cgroup.c | 126 +++++++++++
2 files changed, 339 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
create mode 100644 tools/testing/selftests/bpf/progs/lsm_cgroup.c
diff --git a/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c b/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
new file mode 100644
index 000000000000..fbd6ed29f7d4
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <test_progs.h>
+
+#include "lsm_cgroup.skel.h"
+#include "cgroup_helpers.h"
+#include "network_helpers.h"
+
+void test_lsm_cgroup(void)
+{
+ DECLARE_LIBBPF_OPTS(bpf_prog_attach_opts, attach_opts);
+ DECLARE_LIBBPF_OPTS(bpf_link_update_opts, update_opts);
+ int cgroup_fd, cgroup_fd2, err, fd, prio;
+ int listen_fd, client_fd, accepted_fd;
+ struct lsm_cgroup *skel = NULL;
+ int post_create_prog_fd2 = -1;
+ int post_create_prog_fd = -1;
+ int bind_link_fd2 = -1;
+ int bind_prog_fd2 = -1;
+ int alloc_prog_fd = -1;
+ int bind_prog_fd = -1;
+ int bind_link_fd = -1;
+ int clone_prog_fd = -1;
+ socklen_t socklen;
+
+ cgroup_fd = test__join_cgroup("/sock_policy");
+ if (!ASSERT_GE(cgroup_fd, 0, "join_cgroup"))
+ goto close_skel;
+
+ cgroup_fd2 = create_and_get_cgroup("/sock_policy2");
+ if (!ASSERT_GE(cgroup_fd2, 0, "create second cgroup"))
+ goto close_skel;
+
+ skel = lsm_cgroup__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ goto close_cgroup;
+
+ post_create_prog_fd = bpf_program__fd(skel->progs.socket_post_create);
+ post_create_prog_fd2 = bpf_program__fd(skel->progs.socket_post_create2);
+ bind_prog_fd = bpf_program__fd(skel->progs.socket_bind);
+ bind_prog_fd2 = bpf_program__fd(skel->progs.socket_bind2);
+ alloc_prog_fd = bpf_program__fd(skel->progs.socket_alloc);
+ clone_prog_fd = bpf_program__fd(skel->progs.socket_clone);
+
+ err = bpf_prog_attach(alloc_prog_fd, cgroup_fd, BPF_LSM_CGROUP, 0);
+ if (!ASSERT_OK(err, "attach alloc_prog_fd"))
+ goto detach_cgroup;
+
+ err = bpf_prog_attach(clone_prog_fd, cgroup_fd, BPF_LSM_CGROUP, 0);
+ if (!ASSERT_OK(err, "attach clone_prog_fd"))
+ goto detach_cgroup;
+
+ /* Make sure replacing works.
+ */
+
+ err = bpf_prog_attach(post_create_prog_fd, cgroup_fd,
+ BPF_LSM_CGROUP, 0);
+ if (!ASSERT_OK(err, "attach post_create_prog_fd"))
+ goto close_cgroup;
+
+ attach_opts.replace_prog_fd = post_create_prog_fd;
+ err = bpf_prog_attach_opts(post_create_prog_fd2, cgroup_fd,
+ BPF_LSM_CGROUP, &attach_opts);
+ if (!ASSERT_OK(err, "prog replace post_create_prog_fd"))
+ goto detach_cgroup;
+
+ /* Try the same attach/replace via link API.
+ */
+
+ bind_link_fd = bpf_link_create(bind_prog_fd, cgroup_fd,
+ BPF_LSM_CGROUP, NULL);
+ if (!ASSERT_GE(bind_link_fd, 0, "link create bind_prog_fd"))
+ goto detach_cgroup;
+
+ update_opts.old_prog_fd = bind_prog_fd;
+ update_opts.flags = BPF_F_REPLACE;
+
+ err = bpf_link_update(bind_link_fd, bind_prog_fd2, &update_opts);
+ if (!ASSERT_OK(err, "link update bind_prog_fd"))
+ goto detach_cgroup;
+
+ /* Attach another instance of bind program to another cgroup.
+ * This should trigger the reuse of the trampoline shim (two
+ * programs attaching to the same btf_id).
+ */
+
+ bind_link_fd2 = bpf_link_create(bind_prog_fd2, cgroup_fd2,
+ BPF_LSM_CGROUP, NULL);
+ if (!ASSERT_GE(bind_link_fd2, 0, "link create bind_prog_fd2"))
+ goto detach_cgroup;
+
+ /* AF_UNIX is prohibited.
+ */
+
+ fd = socket(AF_UNIX, SOCK_STREAM, 0);
+ ASSERT_LT(fd, 0, "socket(AF_UNIX)");
+
+ /* AF_INET6 gets default policy (sk_priority).
+ */
+
+ fd = socket(AF_INET6, SOCK_STREAM, 0);
+ if (!ASSERT_GE(fd, 0, "socket(SOCK_STREAM)"))
+ goto detach_cgroup;
+
+ prio = 0;
+ socklen = sizeof(prio);
+ ASSERT_GE(getsockopt(fd, SOL_SOCKET, SO_PRIORITY, &prio, &socklen), 0,
+ "getsockopt");
+ ASSERT_EQ(prio, 123, "sk_priority");
+
+ close(fd);
+
+ /* TX-only AF_PACKET is allowed.
+ */
+
+ ASSERT_LT(socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL)), 0,
+ "socket(AF_PACKET, ..., ETH_P_ALL)");
+
+ fd = socket(AF_PACKET, SOCK_RAW, 0);
+ ASSERT_GE(fd, 0, "socket(AF_PACKET, ..., 0)");
+
+ /* TX-only AF_PACKET can not be rebound.
+ */
+
+ struct sockaddr_ll sa = {
+ .sll_family = AF_PACKET,
+ .sll_protocol = htons(ETH_P_ALL),
+ };
+ ASSERT_LT(bind(fd, (struct sockaddr *)&sa, sizeof(sa)), 0,
+ "bind(ETH_P_ALL)");
+
+ close(fd);
+
+ /* Trigger passive open.
+ */
+
+ listen_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0);
+ ASSERT_GE(listen_fd, 0, "start_server");
+ client_fd = connect_to_fd(listen_fd, 0);
+ ASSERT_GE(client_fd, 0, "connect_to_fd");
+ accepted_fd = accept(listen_fd, NULL, NULL);
+ ASSERT_GE(accepted_fd, 0, "accept");
+
+ prio = 0;
+ socklen = sizeof(prio);
+ ASSERT_GE(getsockopt(accepted_fd, SOL_SOCKET, SO_PRIORITY, &prio, &socklen), 0,
+ "getsockopt");
+ ASSERT_EQ(prio, 234, "sk_priority");
+
+ /* These are replaced and never called. */
+ ASSERT_EQ(skel->bss->called_socket_post_create, 0, "called_create");
+ ASSERT_EQ(skel->bss->called_socket_bind, 0, "called_bind");
+
+ /* AF_INET6+SOCK_STREAM
+ * AF_PACKET+SOCK_RAW
+ * listen_fd
+ * client_fd
+ * accepted_fd
+ */
+ ASSERT_EQ(skel->bss->called_socket_post_create2, 5, "called_create2");
+
+ /* start_server
+ * bind(ETH_P_ALL)
+ */
+ ASSERT_EQ(skel->bss->called_socket_bind2, 2, "called_bind2");
+ /* Single accept(). */
+ ASSERT_EQ(skel->bss->called_socket_clone, 1, "called_clone");
+
+ /* AF_UNIX+SOCK_STREAM (failed)
+ * AF_INET6+SOCK_STREAM
+ * AF_PACKET+SOCK_RAW (failed)
+ * AF_PACKET+SOCK_RAW
+ * listen_fd
+ * client_fd
+ * accepted_fd
+ */
+ ASSERT_EQ(skel->bss->called_socket_alloc, 7, "called_alloc");
+
+ /* Make sure other cgroup doesn't trigger the programs.
+ */
+
+ if (!ASSERT_OK(join_cgroup(""), "join root cgroup"))
+ goto detach_cgroup;
+
+ fd = socket(AF_INET6, SOCK_STREAM, 0);
+ if (!ASSERT_GE(fd, 0, "socket(SOCK_STREAM)"))
+ goto detach_cgroup;
+
+ prio = 0;
+ socklen = sizeof(prio);
+ ASSERT_GE(getsockopt(fd, SOL_SOCKET, SO_PRIORITY, &prio, &socklen), 0,
+ "getsockopt");
+ ASSERT_EQ(prio, 0, "sk_priority");
+
+ close(fd);
+
+detach_cgroup:
+ ASSERT_GE(bpf_prog_detach2(post_create_prog_fd2, cgroup_fd,
+ BPF_LSM_CGROUP), 0, "detach_create");
+ close(bind_link_fd);
+ /* Don't close bind_link_fd2, exercise cgroup release cleanup. */
+ ASSERT_GE(bpf_prog_detach2(alloc_prog_fd, cgroup_fd,
+ BPF_LSM_CGROUP), 0, "detach_alloc");
+ ASSERT_GE(bpf_prog_detach2(clone_prog_fd, cgroup_fd,
+ BPF_LSM_CGROUP), 0, "detach_clone");
+
+close_cgroup:
+ close(cgroup_fd);
+close_skel:
+ lsm_cgroup__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/lsm_cgroup.c b/tools/testing/selftests/bpf/progs/lsm_cgroup.c
new file mode 100644
index 000000000000..70f4ffd90d49
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/lsm_cgroup.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+#ifndef AF_PACKET
+#define AF_PACKET 17
+#endif
+
+#ifndef AF_UNIX
+#define AF_UNIX 1
+#endif
+
+#ifndef EPERM
+#define EPERM 1
+#endif
+
+int called_socket_post_create;
+int called_socket_post_create2;
+int called_socket_bind;
+int called_socket_bind2;
+int called_socket_alloc;
+int called_socket_clone;
+
+static __always_inline int real_create(struct socket *sock, int family,
+ int protocol)
+{
+ struct sock *sk;
+
+ /* Reject non-tx-only AF_PACKET.
+ */
+ if (family == AF_PACKET && protocol != 0)
+ return 0; /* EPERM */
+
+ sk = sock->sk;
+ if (!sk)
+ return 1;
+
+ /* The rest of the sockets get default policy.
+ */
+ sk->sk_priority = 123;
+ return 1;
+}
+
+/* __cgroup_bpf_run_lsm_socket */
+SEC("lsm_cgroup/socket_post_create")
+int BPF_PROG(socket_post_create, struct socket *sock, int family,
+ int type, int protocol, int kern)
+{
+ called_socket_post_create++;
+ return real_create(sock, family, protocol);
+}
+
+/* __cgroup_bpf_run_lsm_socket */
+SEC("lsm_cgroup/socket_post_create")
+int BPF_PROG(socket_post_create2, struct socket *sock, int family,
+ int type, int protocol, int kern)
+{
+ called_socket_post_create2++;
+ return real_create(sock, family, protocol);
+}
+
+static __always_inline int real_bind(struct socket *sock,
+ struct sockaddr *address,
+ int addrlen)
+{
+ struct sockaddr_ll sa = {};
+
+ if (sock->sk->__sk_common.skc_family != AF_PACKET)
+ return 1;
+
+ if (sock->sk->sk_kern_sock)
+ return 1;
+
+ bpf_probe_read_kernel(&sa, sizeof(sa), address);
+ if (sa.sll_protocol)
+ return 0; /* EPERM */
+
+ return 1;
+}
+
+/* __cgroup_bpf_run_lsm_socket */
+SEC("lsm_cgroup/socket_bind")
+int BPF_PROG(socket_bind, struct socket *sock, struct sockaddr *address,
+ int addrlen)
+{
+ called_socket_bind++;
+ return real_bind(sock, address, addrlen);
+}
+
+/* __cgroup_bpf_run_lsm_socket */
+SEC("lsm_cgroup/socket_bind")
+int BPF_PROG(socket_bind2, struct socket *sock, struct sockaddr *address,
+ int addrlen)
+{
+ called_socket_bind2++;
+ return real_bind(sock, address, addrlen);
+}
+
+/* __cgroup_bpf_run_lsm_current (via bpf_lsm_current_hooks) */
+SEC("lsm_cgroup/sk_alloc_security")
+int BPF_PROG(socket_alloc, struct sock *sk, int family, gfp_t priority)
+{
+ called_socket_alloc++;
+ if (family == AF_UNIX)
+ return 0; /* EPERM */
+ return 1;
+}
+
+/* __cgroup_bpf_run_lsm_sock */
+SEC("lsm_cgroup/inet_csk_clone")
+int BPF_PROG(socket_clone, struct sock *newsk, const struct request_sock *req)
+{
+ called_socket_clone++;
+
+ if (!newsk)
+ return 1;
+
+ /* Accepted request sockets get a different priority.
+ */
+ newsk->sk_priority = 234;
+ return 1;
+}
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH bpf-next v5 8/8] selftests/bpf: verify lsm_cgroup struct sock access
2022-04-19 19:00 [PATCH bpf-next v5 0/8] bpf: cgroup_sock lsm flavor Stanislav Fomichev
` (6 preceding siblings ...)
2022-04-19 19:00 ` [PATCH bpf-next v5 7/8] selftests/bpf: lsm_cgroup functional test Stanislav Fomichev
@ 2022-04-19 19:00 ` Stanislav Fomichev
7 siblings, 0 replies; 15+ messages in thread
From: Stanislav Fomichev @ 2022-04-19 19:00 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, andrii, Stanislav Fomichev
sk_priority & sk_mark are writable, the rest is readonly.
Add new ldx_offset fixups to lookup the offset of struct field.
Allow using test.kfunc regardless of prog_type.
One interesting thing here is that the verifier doesn't
really force me to add NULL checks anywhere :-/
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_verifier.c | 54 ++++++++++++++++++-
.../selftests/bpf/verifier/lsm_cgroup.c | 34 ++++++++++++
2 files changed, 87 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/verifier/lsm_cgroup.c
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index a2cd236c32eb..d6bc55c54aaa 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -75,6 +75,12 @@ struct kfunc_btf_id_pair {
int insn_idx;
};
+struct ldx_offset {
+ const char *strct;
+ const char *field;
+ int insn_idx;
+};
+
struct bpf_test {
const char *descr;
struct bpf_insn insns[MAX_INSNS];
@@ -102,6 +108,7 @@ struct bpf_test {
int fixup_map_ringbuf[MAX_FIXUPS];
int fixup_map_timer[MAX_FIXUPS];
struct kfunc_btf_id_pair fixup_kfunc_btf_id[MAX_FIXUPS];
+ struct ldx_offset fixup_ldx[MAX_FIXUPS];
/* Expected verifier log output for result REJECT or VERBOSE_ACCEPT.
* Can be a tab-separated sequence of expected strings. An empty string
* means no log verification.
@@ -755,6 +762,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
int *fixup_map_ringbuf = test->fixup_map_ringbuf;
int *fixup_map_timer = test->fixup_map_timer;
struct kfunc_btf_id_pair *fixup_kfunc_btf_id = test->fixup_kfunc_btf_id;
+ struct ldx_offset *fixup_ldx = test->fixup_ldx;
if (test->fill_helper) {
test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
@@ -967,6 +975,50 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
fixup_kfunc_btf_id++;
} while (fixup_kfunc_btf_id->kfunc);
}
+
+ if (fixup_ldx->strct) {
+ const struct btf_member *memb;
+ const struct btf_type *tp;
+ const char *name;
+ struct btf *btf;
+ int btf_id;
+ int off;
+ int i;
+
+ btf = btf__load_vmlinux_btf();
+
+ do {
+ off = -1;
+ if (!btf)
+ goto next_ldx;
+
+ btf_id = btf__find_by_name_kind(btf,
+ fixup_ldx->strct,
+ BTF_KIND_STRUCT);
+ if (btf_id < 0)
+ goto next_ldx;
+
+ tp = btf__type_by_id(btf, btf_id);
+ memb = btf_members(tp);
+
+ for (i = 0; i < btf_vlen(tp); i++) {
+ name = btf__name_by_offset(btf,
+ memb->name_off);
+ if (strcmp(fixup_ldx->field, name) == 0) {
+ off = memb->offset / 8;
+ break;
+ }
+ memb++;
+ }
+
+next_ldx:
+ prog[fixup_ldx->insn_idx].off = off;
+ fixup_ldx++;
+
+ } while (fixup_ldx->strct);
+
+ btf__free(btf);
+ }
}
struct libcap {
@@ -1131,7 +1183,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
opts.log_level = 4;
opts.prog_flags = pflags;
- if (prog_type == BPF_PROG_TYPE_TRACING && test->kfunc) {
+ if (test->kfunc) {
int attach_btf_id;
attach_btf_id = libbpf_find_vmlinux_btf_id(test->kfunc,
diff --git a/tools/testing/selftests/bpf/verifier/lsm_cgroup.c b/tools/testing/selftests/bpf/verifier/lsm_cgroup.c
new file mode 100644
index 000000000000..af0efe783511
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/lsm_cgroup.c
@@ -0,0 +1,34 @@
+#define SK_WRITABLE_FIELD(tp, field, size, res) \
+{ \
+ .descr = field, \
+ .insns = { \
+ /* r1 = *(u64 *)(r1 + 0) */ \
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, 0), \
+ /* r1 = *(u64 *)(r1 + offsetof(struct socket, sk)) */ \
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, 0), \
+ /* r2 = *(u64 *)(r1 + offsetof(struct sock, <field>)) */ \
+ BPF_LDX_MEM(size, BPF_REG_2, BPF_REG_1, 0), \
+ /* *(u64 *)(r1 + offsetof(struct sock, <field>)) = r2 */ \
+ BPF_STX_MEM(size, BPF_REG_1, BPF_REG_2, 0), \
+ BPF_MOV64_IMM(BPF_REG_0, 1), \
+ BPF_EXIT_INSN(), \
+ }, \
+ .result = res, \
+ .errstr = res ? "no write support to 'struct sock' at off" : "", \
+ .prog_type = BPF_PROG_TYPE_LSM, \
+ .expected_attach_type = BPF_LSM_CGROUP, \
+ .kfunc = "socket_post_create", \
+ .fixup_ldx = { \
+ { "socket", "sk", 1 }, \
+ { tp, field, 2 }, \
+ { tp, field, 3 }, \
+ }, \
+}
+
+SK_WRITABLE_FIELD("sock_common", "skc_family", BPF_H, REJECT),
+SK_WRITABLE_FIELD("sock", "sk_sndtimeo", BPF_DW, REJECT),
+SK_WRITABLE_FIELD("sock", "sk_priority", BPF_W, ACCEPT),
+SK_WRITABLE_FIELD("sock", "sk_mark", BPF_W, ACCEPT),
+SK_WRITABLE_FIELD("sock", "sk_pacing_rate", BPF_DW, REJECT),
+
+#undef SK_WRITABLE_FIELD
--
2.36.0.rc0.470.gd361397f0d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor
2022-04-19 19:00 ` [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor Stanislav Fomichev
@ 2022-04-26 6:27 ` Martin KaFai Lau
2022-04-26 6:30 ` Martin KaFai Lau
2022-04-26 22:44 ` Stanislav Fomichev
2022-04-27 0:10 ` Martin KaFai Lau
1 sibling, 2 replies; 15+ messages in thread
From: Martin KaFai Lau @ 2022-04-26 6:27 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, bpf, ast, daniel, andrii
On Tue, Apr 19, 2022 at 12:00:48PM -0700, Stanislav Fomichev wrote:
> Allow attaching to lsm hooks in the cgroup context.
>
> Attaching to per-cgroup LSM works exactly like attaching
> to other per-cgroup hooks. New BPF_LSM_CGROUP is added
> to trigger new mode; the actual lsm hook we attach to is
> signaled via existing attach_btf_id.
>
> For the hooks that have 'struct socket' as its first argument,
> we use the cgroup associated with that socket. For the rest,
> we use 'current' cgroup (this is all on default hierarchy == v2 only).
> Note that for the hooks that work on 'struct sock' we still
> take the cgroup from 'current' because most of the time,
> the 'sock' argument is not properly initialized.
This paragraph is out-dated.
> Behind the scenes, we allocate a shim program that is attached
> to the trampoline and runs cgroup effective BPF programs array.
> This shim has some rudimentary ref counting and can be shared
> between several programs attaching to the same per-cgroup lsm hook.
>
> Note that this patch bloats cgroup size because we add 211
> cgroup_bpf_attach_type(s) for simplicity sake. This will be
> addressed in the subsequent patch.
>
> Also note that we only add non-sleepable flavor for now. To enable
> sleepable use-cases, BPF_PROG_RUN_ARRAY_CG has to grab trace rcu,
s/BPF_PROG_RUN_ARRAY_CG/bpf_prog_run_array_cg/
> shim programs have to be freed via trace rcu, cgroup_bpf.effective
> should be also trace-rcu-managed + maybe some other changes that
> I'm not aware of.
>
[ ... ]
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index 064eccba641d..2161cba1fe0c 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -16,6 +16,7 @@
> #include <linux/bpf_local_storage.h>
> #include <linux/btf_ids.h>
> #include <linux/ima.h>
> +#include <linux/bpf-cgroup.h>
>
> /* For every LSM hook that allows attachment of BPF programs, declare a nop
> * function where a BPF program can be attached.
> @@ -35,6 +36,68 @@ BTF_SET_START(bpf_lsm_hooks)
> #undef LSM_HOOK
> BTF_SET_END(bpf_lsm_hooks)
>
> +/* List of LSM hooks that should operate on 'current' cgroup regardless
> + * of function signature.
> + */
> +BTF_SET_START(bpf_lsm_current_hooks)
> +/* operate on freshly allocated sk without any cgroup association */
> +BTF_ID(func, bpf_lsm_sk_alloc_security)
> +BTF_ID(func, bpf_lsm_sk_free_security)
> +BTF_SET_END(bpf_lsm_current_hooks)
> +
> +int bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog,
> + bpf_func_t *bpf_func)
> +{
> + const struct btf_type *first_arg_type;
> + const struct btf_type *sock_type;
> + const struct btf_type *sk_type;
> + const struct btf *btf_vmlinux;
> + const struct btf_param *args;
> + s32 type_id;
> +
> + if (!prog->aux->attach_func_proto ||
> + !btf_type_is_func_proto(prog->aux->attach_func_proto))
> + return -EINVAL;
> +
> + if (btf_type_vlen(prog->aux->attach_func_proto) < 1 ||
> + btf_id_set_contains(&bpf_lsm_current_hooks,
> + prog->aux->attach_btf_id)) {
> + *bpf_func = __cgroup_bpf_run_lsm_current;
> + return 0;
> + }
> +
> + args = btf_params(prog->aux->attach_func_proto);
> +
> + btf_vmlinux = bpf_get_btf_vmlinux();
> + if (!btf_vmlinux)
Remove this check and other similar checks because the btf_vmlinux has
been successfully parsed during the prog load time.
> + return -EINVAL;
> +
> + type_id = btf_find_by_name_kind(btf_vmlinux, "socket", BTF_KIND_STRUCT);
> + if (type_id < 0)
> + return -EINVAL;
> + sock_type = btf_type_by_id(btf_vmlinux, type_id);
> +
> + type_id = btf_find_by_name_kind(btf_vmlinux, "sock", BTF_KIND_STRUCT);
> + if (type_id < 0)
> + return -EINVAL;
> + sk_type = btf_type_by_id(btf_vmlinux, type_id);
> +
> + first_arg_type = btf_type_resolve_ptr(btf_vmlinux, args[0].type, NULL);
> + if (first_arg_type == sock_type)
> + *bpf_func = __cgroup_bpf_run_lsm_socket;
> + else if (first_arg_type == sk_type)
> + *bpf_func = __cgroup_bpf_run_lsm_sock;
> + else
> + *bpf_func = __cgroup_bpf_run_lsm_current;
> +
> + return 0;
> +}
> +
> +int bpf_lsm_hook_idx(u32 btf_id)
> +{
> + return btf_id_set_index(&bpf_lsm_hooks, btf_id);
> +}
> +
> int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
> const struct bpf_prog *prog)
> {
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 0918a39279f6..4199de31f49c 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -4971,6 +4971,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>
> if (arg == nr_args) {
> switch (prog->expected_attach_type) {
> + case BPF_LSM_CGROUP:
> case BPF_LSM_MAC:
> case BPF_TRACE_FEXIT:
> /* When LSM programs are attached to void LSM hooks
> @@ -6396,6 +6397,16 @@ static int btf_id_cmp_func(const void *a, const void *b)
> return *pa - *pb;
> }
>
> +int btf_id_set_index(const struct btf_id_set *set, u32 id)
> +{
> + const u32 *p;
> +
> + p = bsearch(&id, set->ids, set->cnt, sizeof(u32), btf_id_cmp_func);
> + if (!p)
> + return -1;
> + return p - set->ids;
> +}
> +
> bool btf_id_set_contains(const struct btf_id_set *set, u32 id)
> {
> return bsearch(&id, set->ids, set->cnt, sizeof(u32), btf_id_cmp_func) != NULL;
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index aaf9e36f2736..ba0e0c7a661d 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -14,6 +14,9 @@
> #include <linux/string.h>
> #include <linux/bpf.h>
> #include <linux/bpf-cgroup.h>
> +#include <linux/btf_ids.h>
> +#include <linux/bpf_lsm.h>
> +#include <linux/bpf_verifier.h>
> #include <net/sock.h>
> #include <net/bpf_sk_storage.h>
>
> @@ -88,6 +91,85 @@ bpf_prog_run_array_cg(const struct cgroup_bpf *cgrp,
> return run_ctx.retval;
> }
>
> +unsigned int __cgroup_bpf_run_lsm_sock(const void *ctx,
> + const struct bpf_insn *insn)
> +{
> + const struct bpf_prog *shim_prog;
> + struct sock *sk;
> + struct cgroup *cgrp;
> + int ret = 0;
> + u64 *regs;
> +
> + regs = (u64 *)ctx;
> + sk = (void *)(unsigned long)regs[BPF_REG_0];
> + /*shim_prog = container_of(insn, struct bpf_prog, insnsi);*/
> + shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
> +
> + cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
> + if (likely(cgrp))
> + ret = bpf_prog_run_array_cg(&cgrp->bpf,
> + shim_prog->aux->cgroup_atype,
> + ctx, bpf_prog_run, 0);
> + return ret;
> +}
> +
> +unsigned int __cgroup_bpf_run_lsm_socket(const void *ctx,
> + const struct bpf_insn *insn)
> +{
> + const struct bpf_prog *shim_prog;
> + struct socket *sock;
> + struct cgroup *cgrp;
> + int ret = 0;
> + u64 *regs;
> +
> + regs = (u64 *)ctx;
> + sock = (void *)(unsigned long)regs[BPF_REG_0];
> + /*shim_prog = container_of(insn, struct bpf_prog, insnsi);*/
> + shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
> +
> + cgrp = sock_cgroup_ptr(&sock->sk->sk_cgrp_data);
> + if (likely(cgrp))
> + ret = bpf_prog_run_array_cg(&cgrp->bpf,
> + shim_prog->aux->cgroup_atype,
> + ctx, bpf_prog_run, 0);
> + return ret;
> +}
> +
> +unsigned int __cgroup_bpf_run_lsm_current(const void *ctx,
> + const struct bpf_insn *insn)
> +{
> + const struct bpf_prog *shim_prog;
> + struct cgroup *cgrp;
> + int ret = 0;
> +
> + if (unlikely(!current))
> + return 0;
> +
> + /*shim_prog = container_of(insn, struct bpf_prog, insnsi);*/
> + shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
> +
> + rcu_read_lock();
> + cgrp = task_dfl_cgroup(current);
> + if (likely(cgrp))
> + ret = bpf_prog_run_array_cg(&cgrp->bpf,
> + shim_prog->aux->cgroup_atype,
> + ctx, bpf_prog_run, 0);
> + rcu_read_unlock();
> + return ret;
> +}
> +
> +#ifdef CONFIG_BPF_LSM
> +static enum cgroup_bpf_attach_type bpf_lsm_attach_type_get(u32 attach_btf_id)
> +{
> + return CGROUP_LSM_START + bpf_lsm_hook_idx(attach_btf_id);
> +}
> +#else
> +static enum cgroup_bpf_attach_type bpf_lsm_attach_type_get(u32 attach_btf_id)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_BPF_LSM */
> +
> void cgroup_bpf_offline(struct cgroup *cgrp)
> {
> cgroup_get(cgrp);
> @@ -155,6 +237,14 @@ static void bpf_cgroup_storages_link(struct bpf_cgroup_storage *storages[],
> bpf_cgroup_storage_link(storages[stype], cgrp, attach_type);
> }
>
> +static void bpf_cgroup_storages_unlink(struct bpf_cgroup_storage *storages[])
> +{
> + enum bpf_cgroup_storage_type stype;
> +
> + for_each_cgroup_storage_type(stype)
> + bpf_cgroup_storage_unlink(storages[stype]);
> +}
> +
> /* Called when bpf_cgroup_link is auto-detached from dying cgroup.
> * It drops cgroup and bpf_prog refcounts, and marks bpf_link as defunct. It
> * doesn't free link memory, which will eventually be done by bpf_link's
> @@ -166,6 +256,16 @@ static void bpf_cgroup_link_auto_detach(struct bpf_cgroup_link *link)
> link->cgroup = NULL;
> }
>
> +static void bpf_cgroup_lsm_shim_release(struct bpf_prog *prog,
> + enum cgroup_bpf_attach_type atype)
> +{
> + if (prog->aux->cgroup_atype < CGROUP_LSM_START ||
> + prog->aux->cgroup_atype > CGROUP_LSM_END)
These checks are unnecessary. cgroup_atype was set by the kernel
during attach and it could not be invalid during detach.
Remove this helper and directly call bpf_trampoline_unlink_cgroup_shim(prog)
instead.
> + return;
> +
> + bpf_trampoline_unlink_cgroup_shim(prog);
> +}
> +
> /**
> * cgroup_bpf_release() - put references of all bpf programs and
> * release all cgroup bpf data
> @@ -190,10 +290,18 @@ static void cgroup_bpf_release(struct work_struct *work)
>
> hlist_for_each_entry_safe(pl, pltmp, progs, node) {
> hlist_del(&pl->node);
> - if (pl->prog)
> + if (pl->prog) {
> + if (atype == BPF_LSM_CGROUP)
> + bpf_cgroup_lsm_shim_release(pl->prog,
> + atype);
> bpf_prog_put(pl->prog);
> - if (pl->link)
> + }
> + if (pl->link) {
> + if (atype == BPF_LSM_CGROUP)
> + bpf_cgroup_lsm_shim_release(pl->link->link.prog,
> + atype);
> bpf_cgroup_link_auto_detach(pl->link);
> + }
> kfree(pl);
> static_branch_dec(&cgroup_bpf_enabled_key[atype]);
> }
> @@ -506,6 +614,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> struct bpf_prog *old_prog = NULL;
> struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> + struct bpf_attach_target_info tgt_info = {};
> enum cgroup_bpf_attach_type atype;
> struct bpf_prog_list *pl;
> struct hlist_head *progs;
> @@ -522,9 +631,35 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> /* replace_prog implies BPF_F_REPLACE, and vice versa */
> return -EINVAL;
>
> - atype = to_cgroup_bpf_attach_type(type);
> - if (atype < 0)
> - return -EINVAL;
> + if (type == BPF_LSM_CGROUP) {
> + struct bpf_prog *p = prog ? : link->link.prog;
> +
> + if (replace_prog) {
> + /* Reusing shim from the original program.
> + */
> + if (replace_prog->aux->attach_btf_id !=
> + p->aux->attach_btf_id)
> + return -EINVAL;
> +
> + atype = replace_prog->aux->cgroup_atype;
> + } else {
> + err = bpf_check_attach_target(NULL, p, NULL,
> + p->aux->attach_btf_id,
> + &tgt_info);
> + if (err)
> + return -EINVAL;
> +
> + atype = bpf_lsm_attach_type_get(p->aux->attach_btf_id);
> + if (atype < 0)
> + return atype;
> + }
> +
> + p->aux->cgroup_atype = atype;
> + } else {
> + atype = to_cgroup_bpf_attach_type(type);
> + if (atype < 0)
> + return -EINVAL;
> + }
>
> progs = &cgrp->bpf.progs[atype];
>
> @@ -580,13 +715,26 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> if (err)
> goto cleanup;
>
> + bpf_cgroup_storages_link(new_storage, cgrp, type);
It looks everything is ready for the cgrp local_storage.
How about also allowing bpf_get_local_storage() for BPF_LSM_CGROUP?
> +
> + if (type == BPF_LSM_CGROUP && !old_prog) {
> + struct bpf_prog *p = prog ? : link->link.prog;
> +
> + err = bpf_trampoline_link_cgroup_shim(p, &tgt_info);
> + if (err)
> + goto cleanup_trampoline;
> + }
> +
> if (old_prog)
> bpf_prog_put(old_prog);
> else
> static_branch_inc(&cgroup_bpf_enabled_key[atype]);
> - bpf_cgroup_storages_link(new_storage, cgrp, type);
> +
> return 0;
>
> +cleanup_trampoline:
> + bpf_cgroup_storages_unlink(new_storage);
> +
> cleanup:
> if (old_prog) {
> pl->prog = old_prog;
> @@ -678,9 +826,13 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
> struct hlist_head *progs;
> bool found = false;
>
> - atype = to_cgroup_bpf_attach_type(link->type);
> - if (atype < 0)
> - return -EINVAL;
> + if (link->type == BPF_LSM_CGROUP) {
> + atype = link->link.prog->aux->cgroup_atype;
> + } else {
> + atype = to_cgroup_bpf_attach_type(link->type);
> + if (atype < 0)
> + return -EINVAL;
> + }
>
> progs = &cgrp->bpf.progs[atype];
>
> @@ -696,6 +848,9 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
> if (!found)
> return -ENOENT;
>
> + if (link->type == BPF_LSM_CGROUP)
> + new_prog->aux->cgroup_atype = atype;
Does it also need to check attach_btf_id between the
new_prog and the old prog?
> +
> old_prog = xchg(&link->link.prog, new_prog);
> replace_effective_prog(cgrp, atype, link);
> bpf_prog_put(old_prog);
> @@ -779,9 +934,15 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
> u32 flags;
> int err;
>
> - atype = to_cgroup_bpf_attach_type(type);
> - if (atype < 0)
> - return -EINVAL;
> + if (type == BPF_LSM_CGROUP) {
> + struct bpf_prog *p = prog ? : link->link.prog;
> +
> + atype = p->aux->cgroup_atype;
> + } else {
> + atype = to_cgroup_bpf_attach_type(type);
> + if (atype < 0)
> + return -EINVAL;
> + }
>
> progs = &cgrp->bpf.progs[atype];
> flags = cgrp->bpf.flags[atype];
> @@ -803,6 +964,10 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
> if (err)
> goto cleanup;
>
> + if (type == BPF_LSM_CGROUP)
> + bpf_cgroup_lsm_shim_release(prog ? : link->link.prog,
> + atype);
After looking at find_detach_entry(),
the pl->prog may not be the same as the prog or link->link.prog here.
> +
> /* now can actually delete it from this cgroup list */
> hlist_del(&pl->node);
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index e9621cfa09f2..d94f4951154e 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3139,6 +3139,11 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> return prog->enforce_expected_attach_type &&
> prog->expected_attach_type != attach_type ?
> -EINVAL : 0;
> + case BPF_PROG_TYPE_LSM:
> + if (prog->expected_attach_type != BPF_LSM_CGROUP)
> + return -EINVAL;
> + return 0;
> +
> default:
> return 0;
> }
> @@ -3194,6 +3199,8 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
> return BPF_PROG_TYPE_SK_LOOKUP;
> case BPF_XDP:
> return BPF_PROG_TYPE_XDP;
> + case BPF_LSM_CGROUP:
> + return BPF_PROG_TYPE_LSM;
> default:
> return BPF_PROG_TYPE_UNSPEC;
> }
> @@ -3247,6 +3254,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> case BPF_PROG_TYPE_CGROUP_SYSCTL:
> case BPF_PROG_TYPE_SOCK_OPS:
> + case BPF_PROG_TYPE_LSM:
> ret = cgroup_bpf_prog_attach(attr, ptype, prog);
> break;
> default:
> @@ -3284,6 +3292,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
> case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> case BPF_PROG_TYPE_CGROUP_SYSCTL:
> case BPF_PROG_TYPE_SOCK_OPS:
> + case BPF_PROG_TYPE_LSM:
> return cgroup_bpf_prog_detach(attr, ptype);
> default:
> return -EINVAL;
> @@ -4317,6 +4326,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> case BPF_PROG_TYPE_CGROUP_DEVICE:
> case BPF_PROG_TYPE_CGROUP_SYSCTL:
> case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> + case BPF_PROG_TYPE_LSM:
> ret = cgroup_bpf_link_attach(attr, prog);
> break;
> case BPF_PROG_TYPE_TRACING:
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 0c4fd194e801..c76dfa4ea2d9 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -11,6 +11,8 @@
> #include <linux/rcupdate_wait.h>
> #include <linux/module.h>
> #include <linux/static_call.h>
> +#include <linux/bpf_verifier.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 = {
> @@ -485,6 +487,149 @@ int bpf_trampoline_unlink_prog(struct bpf_prog *prog, struct bpf_trampoline *tr)
> return err;
> }
>
> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_BPF_SYSCALL)
> +static struct bpf_prog *cgroup_shim_alloc(const struct bpf_prog *prog,
> + bpf_func_t bpf_func)
> +{
> + struct bpf_prog *p;
> +
> + p = bpf_prog_alloc(1, 0);
> + if (!p)
> + return NULL;
> +
> + p->jited = false;
> + p->bpf_func = bpf_func;
> +
> + p->aux->cgroup_atype = prog->aux->cgroup_atype;
> + p->aux->attach_func_proto = prog->aux->attach_func_proto;
> + p->aux->attach_btf_id = prog->aux->attach_btf_id;
> + p->aux->attach_btf = prog->aux->attach_btf;
> + btf_get(p->aux->attach_btf);
> + p->type = BPF_PROG_TYPE_LSM;
> + p->expected_attach_type = BPF_LSM_MAC;
> + bpf_prog_inc(p);
> +
> + return p;
> +}
> +
> +static struct bpf_prog *cgroup_shim_find(struct bpf_trampoline *tr,
> + bpf_func_t bpf_func)
> +{
> + const struct bpf_prog_aux *aux;
> + int kind;
> +
> + for (kind = 0; kind < BPF_TRAMP_MAX; kind++) {
> + hlist_for_each_entry(aux, &tr->progs_hlist[kind], tramp_hlist) {
> + struct bpf_prog *p = aux->prog;
> +
> + if (p->bpf_func == bpf_func)
> + return p;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +int bpf_trampoline_link_cgroup_shim(struct bpf_prog *prog,
> + struct bpf_attach_target_info *tgt_info)
> +{
> + struct bpf_prog *shim_prog = NULL;
> + struct bpf_trampoline *tr;
> + bpf_func_t bpf_func;
> + u64 key;
> + int err;
> +
> + key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf,
> + prog->aux->attach_btf_id);
> +
> + err = bpf_lsm_find_cgroup_shim(prog, &bpf_func);
> + if (err)
> + return err;
> +
> + tr = bpf_trampoline_get(key, tgt_info);
> + if (!tr)
> + return -ENOMEM;
> +
> + mutex_lock(&tr->mutex);
> +
> + shim_prog = cgroup_shim_find(tr, bpf_func);
> + if (shim_prog) {
> + /* Reusing existing shim attached by the other program.
> + */
nit. Avoid extra line in '*/' for one liner comment. Didn't
see this convention in other bpf codes.
Also, how about the earlier comment regarding to another __bpf_prog_enter
and __bpf_prog_exit for BPF_LSM_CGROUP that don't do the active counts
and stats collection ?
> + bpf_prog_inc(shim_prog);
> + mutex_unlock(&tr->mutex);
> + return 0;
> + }
> +
> + /* Allocate and install new shim.
> + */
Same here.
> +
> + shim_prog = cgroup_shim_alloc(prog, bpf_func);
> + if (!shim_prog) {
> + err = -ENOMEM;
> + goto out;
> + }
> +
> + err = __bpf_trampoline_link_prog(shim_prog, tr);
> + if (err)
> + goto out;
> +
> + mutex_unlock(&tr->mutex);
> +
> + return 0;
> +out:
> + if (shim_prog)
> + bpf_prog_put(shim_prog);
> +
> + mutex_unlock(&tr->mutex);
> + return err;
> +}
> +
> +void bpf_trampoline_unlink_cgroup_shim(struct bpf_prog *prog)
> +{
> + struct bpf_prog *shim_prog;
> + struct bpf_trampoline *tr;
> + bpf_func_t bpf_func;
> + u64 key;
> + int err;
> +
> + key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf,
> + prog->aux->attach_btf_id);
> +
> + err = bpf_lsm_find_cgroup_shim(prog, &bpf_func);
> + if (err)
> + return;
> +
> + tr = bpf_trampoline_lookup(key);
> + if (!tr)
> + return;
> +
> + mutex_lock(&tr->mutex);
> +
> + shim_prog = cgroup_shim_find(tr, bpf_func);
> + if (shim_prog) {
> + /* We use shim_prog refcnt for tracking whether to
> + * remove the shim program from the trampoline.
> + * Trampoline's mutex is held while refcnt is
> + * added/subtracted so we don't need to care about
> + * potential races.
> + */
> +
> + if (atomic64_read(&shim_prog->aux->refcnt) == 1)
> + WARN_ON_ONCE(__bpf_trampoline_unlink_prog(shim_prog, tr));
> +
> + bpf_prog_put(shim_prog);
> + }
> +
> + mutex_unlock(&tr->mutex);
> +
> + bpf_trampoline_put(tr); /* bpf_trampoline_lookup */
> +
> + if (shim_prog)
> + bpf_trampoline_put(tr);
> +}
> +#endif
> +
> struct bpf_trampoline *bpf_trampoline_get(u64 key,
> struct bpf_attach_target_info *tgt_info)
> {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 9c1a02b82ecd..cc84954846d7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -14197,6 +14197,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> fallthrough;
> case BPF_MODIFY_RETURN:
> case BPF_LSM_MAC:
> + case BPF_LSM_CGROUP:
> case BPF_TRACE_FENTRY:
> case BPF_TRACE_FEXIT:
> if (!btf_type_is_func(t)) {
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index d14b10b85e51..bbe48a2dd852 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -998,6 +998,7 @@ enum bpf_attach_type {
> BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
> BPF_PERF_EVENT,
> BPF_TRACE_KPROBE_MULTI,
> + BPF_LSM_CGROUP,
> __MAX_BPF_ATTACH_TYPE
> };
>
> --
> 2.36.0.rc0.470.gd361397f0d-goog
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor
2022-04-26 6:27 ` Martin KaFai Lau
@ 2022-04-26 6:30 ` Martin KaFai Lau
2022-04-26 22:44 ` Stanislav Fomichev
1 sibling, 0 replies; 15+ messages in thread
From: Martin KaFai Lau @ 2022-04-26 6:30 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, bpf, ast, daniel, andrii
On Mon, Apr 25, 2022 at 11:27:08PM -0700, Martin KaFai Lau wrote:
> On Tue, Apr 19, 2022 at 12:00:48PM -0700, Stanislav Fomichev wrote:
> > Allow attaching to lsm hooks in the cgroup context.
> >
> > Attaching to per-cgroup LSM works exactly like attaching
> > to other per-cgroup hooks. New BPF_LSM_CGROUP is added
> > to trigger new mode; the actual lsm hook we attach to is
> > signaled via existing attach_btf_id.
> >
> > For the hooks that have 'struct socket' as its first argument,
> > we use the cgroup associated with that socket. For the rest,
> > we use 'current' cgroup (this is all on default hierarchy == v2 only).
> > Note that for the hooks that work on 'struct sock' we still
> > take the cgroup from 'current' because most of the time,
> > the 'sock' argument is not properly initialized.
> This paragraph is out-dated.
>
> > Behind the scenes, we allocate a shim program that is attached
> > to the trampoline and runs cgroup effective BPF programs array.
> > This shim has some rudimentary ref counting and can be shared
> > between several programs attaching to the same per-cgroup lsm hook.
> >
> > Note that this patch bloats cgroup size because we add 211
> > cgroup_bpf_attach_type(s) for simplicity sake. This will be
> > addressed in the subsequent patch.
> >
> > Also note that we only add non-sleepable flavor for now. To enable
> > sleepable use-cases, BPF_PROG_RUN_ARRAY_CG has to grab trace rcu,
> s/BPF_PROG_RUN_ARRAY_CG/bpf_prog_run_array_cg/
>
> > shim programs have to be freed via trace rcu, cgroup_bpf.effective
> > should be also trace-rcu-managed + maybe some other changes that
> > I'm not aware of.
Will continue the review tomorrow. thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor
2022-04-26 6:27 ` Martin KaFai Lau
2022-04-26 6:30 ` Martin KaFai Lau
@ 2022-04-26 22:44 ` Stanislav Fomichev
1 sibling, 0 replies; 15+ messages in thread
From: Stanislav Fomichev @ 2022-04-26 22:44 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: netdev, bpf, ast, daniel, andrii
On Mon, Apr 25, 2022 at 11:27 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Tue, Apr 19, 2022 at 12:00:48PM -0700, Stanislav Fomichev wrote:
> > Allow attaching to lsm hooks in the cgroup context.
> >
> > Attaching to per-cgroup LSM works exactly like attaching
> > to other per-cgroup hooks. New BPF_LSM_CGROUP is added
> > to trigger new mode; the actual lsm hook we attach to is
> > signaled via existing attach_btf_id.
> >
> > For the hooks that have 'struct socket' as its first argument,
> > we use the cgroup associated with that socket. For the rest,
> > we use 'current' cgroup (this is all on default hierarchy == v2 only).
> > Note that for the hooks that work on 'struct sock' we still
> > take the cgroup from 'current' because most of the time,
> > the 'sock' argument is not properly initialized.
> This paragraph is out-dated.
Ack, will update that last part about sock, thanks!
> > Behind the scenes, we allocate a shim program that is attached
> > to the trampoline and runs cgroup effective BPF programs array.
> > This shim has some rudimentary ref counting and can be shared
> > between several programs attaching to the same per-cgroup lsm hook.
> >
> > Note that this patch bloats cgroup size because we add 211
> > cgroup_bpf_attach_type(s) for simplicity sake. This will be
> > addressed in the subsequent patch.
> >
> > Also note that we only add non-sleepable flavor for now. To enable
> > sleepable use-cases, BPF_PROG_RUN_ARRAY_CG has to grab trace rcu,
> s/BPF_PROG_RUN_ARRAY_CG/bpf_prog_run_array_cg/
Sure, thanks!
> > shim programs have to be freed via trace rcu, cgroup_bpf.effective
> > should be also trace-rcu-managed + maybe some other changes that
> > I'm not aware of.
> >
>
> [ ... ]
>
> > diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> > index 064eccba641d..2161cba1fe0c 100644
> > --- a/kernel/bpf/bpf_lsm.c
> > +++ b/kernel/bpf/bpf_lsm.c
> > @@ -16,6 +16,7 @@
> > #include <linux/bpf_local_storage.h>
> > #include <linux/btf_ids.h>
> > #include <linux/ima.h>
> > +#include <linux/bpf-cgroup.h>
> >
> > /* For every LSM hook that allows attachment of BPF programs, declare a nop
> > * function where a BPF program can be attached.
> > @@ -35,6 +36,68 @@ BTF_SET_START(bpf_lsm_hooks)
> > #undef LSM_HOOK
> > BTF_SET_END(bpf_lsm_hooks)
> >
> > +/* List of LSM hooks that should operate on 'current' cgroup regardless
> > + * of function signature.
> > + */
> > +BTF_SET_START(bpf_lsm_current_hooks)
> > +/* operate on freshly allocated sk without any cgroup association */
> > +BTF_ID(func, bpf_lsm_sk_alloc_security)
> > +BTF_ID(func, bpf_lsm_sk_free_security)
> > +BTF_SET_END(bpf_lsm_current_hooks)
> > +
> > +int bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog,
> > + bpf_func_t *bpf_func)
> > +{
> > + const struct btf_type *first_arg_type;
> > + const struct btf_type *sock_type;
> > + const struct btf_type *sk_type;
> > + const struct btf *btf_vmlinux;
> > + const struct btf_param *args;
> > + s32 type_id;
> > +
> > + if (!prog->aux->attach_func_proto ||
> > + !btf_type_is_func_proto(prog->aux->attach_func_proto))
> > + return -EINVAL;
> > +
> > + if (btf_type_vlen(prog->aux->attach_func_proto) < 1 ||
> > + btf_id_set_contains(&bpf_lsm_current_hooks,
> > + prog->aux->attach_btf_id)) {
> > + *bpf_func = __cgroup_bpf_run_lsm_current;
> > + return 0;
> > + }
> > +
> > + args = btf_params(prog->aux->attach_func_proto);
> > +
> > + btf_vmlinux = bpf_get_btf_vmlinux();
> > + if (!btf_vmlinux)
> Remove this check and other similar checks because the btf_vmlinux has
> been successfully parsed during the prog load time.
Good point, will remove.
> > + return -EINVAL;
> > +
> > + type_id = btf_find_by_name_kind(btf_vmlinux, "socket", BTF_KIND_STRUCT);
> > + if (type_id < 0)
> > + return -EINVAL;
> > + sock_type = btf_type_by_id(btf_vmlinux, type_id);
> > +
> > + type_id = btf_find_by_name_kind(btf_vmlinux, "sock", BTF_KIND_STRUCT);
> > + if (type_id < 0)
> > + return -EINVAL;
> > + sk_type = btf_type_by_id(btf_vmlinux, type_id);
> > +
> > + first_arg_type = btf_type_resolve_ptr(btf_vmlinux, args[0].type, NULL);
> > + if (first_arg_type == sock_type)
> > + *bpf_func = __cgroup_bpf_run_lsm_socket;
> > + else if (first_arg_type == sk_type)
> > + *bpf_func = __cgroup_bpf_run_lsm_sock;
> > + else
> > + *bpf_func = __cgroup_bpf_run_lsm_current;
> > +
> > + return 0;
> > +}
> > +
> > +int bpf_lsm_hook_idx(u32 btf_id)
> > +{
> > + return btf_id_set_index(&bpf_lsm_hooks, btf_id);
> > +}
> > +
> > int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
> > const struct bpf_prog *prog)
> > {
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 0918a39279f6..4199de31f49c 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -4971,6 +4971,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> >
> > if (arg == nr_args) {
> > switch (prog->expected_attach_type) {
> > + case BPF_LSM_CGROUP:
> > case BPF_LSM_MAC:
> > case BPF_TRACE_FEXIT:
> > /* When LSM programs are attached to void LSM hooks
> > @@ -6396,6 +6397,16 @@ static int btf_id_cmp_func(const void *a, const void *b)
> > return *pa - *pb;
> > }
> >
> > +int btf_id_set_index(const struct btf_id_set *set, u32 id)
> > +{
> > + const u32 *p;
> > +
> > + p = bsearch(&id, set->ids, set->cnt, sizeof(u32), btf_id_cmp_func);
> > + if (!p)
> > + return -1;
> > + return p - set->ids;
> > +}
> > +
> > bool btf_id_set_contains(const struct btf_id_set *set, u32 id)
> > {
> > return bsearch(&id, set->ids, set->cnt, sizeof(u32), btf_id_cmp_func) != NULL;
> > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> > index aaf9e36f2736..ba0e0c7a661d 100644
> > --- a/kernel/bpf/cgroup.c
> > +++ b/kernel/bpf/cgroup.c
> > @@ -14,6 +14,9 @@
> > #include <linux/string.h>
> > #include <linux/bpf.h>
> > #include <linux/bpf-cgroup.h>
> > +#include <linux/btf_ids.h>
> > +#include <linux/bpf_lsm.h>
> > +#include <linux/bpf_verifier.h>
> > #include <net/sock.h>
> > #include <net/bpf_sk_storage.h>
> >
> > @@ -88,6 +91,85 @@ bpf_prog_run_array_cg(const struct cgroup_bpf *cgrp,
> > return run_ctx.retval;
> > }
> >
> > +unsigned int __cgroup_bpf_run_lsm_sock(const void *ctx,
> > + const struct bpf_insn *insn)
> > +{
> > + const struct bpf_prog *shim_prog;
> > + struct sock *sk;
> > + struct cgroup *cgrp;
> > + int ret = 0;
> > + u64 *regs;
> > +
> > + regs = (u64 *)ctx;
> > + sk = (void *)(unsigned long)regs[BPF_REG_0];
> > + /*shim_prog = container_of(insn, struct bpf_prog, insnsi);*/
> > + shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
> > +
> > + cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
> > + if (likely(cgrp))
> > + ret = bpf_prog_run_array_cg(&cgrp->bpf,
> > + shim_prog->aux->cgroup_atype,
> > + ctx, bpf_prog_run, 0);
> > + return ret;
> > +}
> > +
> > +unsigned int __cgroup_bpf_run_lsm_socket(const void *ctx,
> > + const struct bpf_insn *insn)
> > +{
> > + const struct bpf_prog *shim_prog;
> > + struct socket *sock;
> > + struct cgroup *cgrp;
> > + int ret = 0;
> > + u64 *regs;
> > +
> > + regs = (u64 *)ctx;
> > + sock = (void *)(unsigned long)regs[BPF_REG_0];
> > + /*shim_prog = container_of(insn, struct bpf_prog, insnsi);*/
> > + shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
> > +
> > + cgrp = sock_cgroup_ptr(&sock->sk->sk_cgrp_data);
> > + if (likely(cgrp))
> > + ret = bpf_prog_run_array_cg(&cgrp->bpf,
> > + shim_prog->aux->cgroup_atype,
> > + ctx, bpf_prog_run, 0);
> > + return ret;
> > +}
> > +
> > +unsigned int __cgroup_bpf_run_lsm_current(const void *ctx,
> > + const struct bpf_insn *insn)
> > +{
> > + const struct bpf_prog *shim_prog;
> > + struct cgroup *cgrp;
> > + int ret = 0;
> > +
> > + if (unlikely(!current))
> > + return 0;
> > +
> > + /*shim_prog = container_of(insn, struct bpf_prog, insnsi);*/
> > + shim_prog = (const struct bpf_prog *)((void *)insn - offsetof(struct bpf_prog, insnsi));
> > +
> > + rcu_read_lock();
> > + cgrp = task_dfl_cgroup(current);
> > + if (likely(cgrp))
> > + ret = bpf_prog_run_array_cg(&cgrp->bpf,
> > + shim_prog->aux->cgroup_atype,
> > + ctx, bpf_prog_run, 0);
> > + rcu_read_unlock();
> > + return ret;
> > +}
> > +
> > +#ifdef CONFIG_BPF_LSM
> > +static enum cgroup_bpf_attach_type bpf_lsm_attach_type_get(u32 attach_btf_id)
> > +{
> > + return CGROUP_LSM_START + bpf_lsm_hook_idx(attach_btf_id);
> > +}
> > +#else
> > +static enum cgroup_bpf_attach_type bpf_lsm_attach_type_get(u32 attach_btf_id)
> > +{
> > + return -EOPNOTSUPP;
> > +}
> > +#endif /* CONFIG_BPF_LSM */
> > +
> > void cgroup_bpf_offline(struct cgroup *cgrp)
> > {
> > cgroup_get(cgrp);
> > @@ -155,6 +237,14 @@ static void bpf_cgroup_storages_link(struct bpf_cgroup_storage *storages[],
> > bpf_cgroup_storage_link(storages[stype], cgrp, attach_type);
> > }
> >
> > +static void bpf_cgroup_storages_unlink(struct bpf_cgroup_storage *storages[])
> > +{
> > + enum bpf_cgroup_storage_type stype;
> > +
> > + for_each_cgroup_storage_type(stype)
> > + bpf_cgroup_storage_unlink(storages[stype]);
> > +}
> > +
> > /* Called when bpf_cgroup_link is auto-detached from dying cgroup.
> > * It drops cgroup and bpf_prog refcounts, and marks bpf_link as defunct. It
> > * doesn't free link memory, which will eventually be done by bpf_link's
> > @@ -166,6 +256,16 @@ static void bpf_cgroup_link_auto_detach(struct bpf_cgroup_link *link)
> > link->cgroup = NULL;
> > }
> >
> > +static void bpf_cgroup_lsm_shim_release(struct bpf_prog *prog,
> > + enum cgroup_bpf_attach_type atype)
> > +{
> > + if (prog->aux->cgroup_atype < CGROUP_LSM_START ||
> > + prog->aux->cgroup_atype > CGROUP_LSM_END)
> These checks are unnecessary. cgroup_atype was set by the kernel
> during attach and it could not be invalid during detach.
>
> Remove this helper and directly call bpf_trampoline_unlink_cgroup_shim(prog)
> instead.
True. I'll remove the checks, I'll still leave
bpf_cgroup_lsm_shim_release around because in the next patch I add
bpf_lsm_attach_type_put here (seems than copy-pasting
bpf_lsm_attach_type_put elsewhere?)
> > + return;
> > +
> > + bpf_trampoline_unlink_cgroup_shim(prog);
> > +}
> > +
> > /**
> > * cgroup_bpf_release() - put references of all bpf programs and
> > * release all cgroup bpf data
> > @@ -190,10 +290,18 @@ static void cgroup_bpf_release(struct work_struct *work)
> >
> > hlist_for_each_entry_safe(pl, pltmp, progs, node) {
> > hlist_del(&pl->node);
> > - if (pl->prog)
> > + if (pl->prog) {
> > + if (atype == BPF_LSM_CGROUP)
> > + bpf_cgroup_lsm_shim_release(pl->prog,
> > + atype);
> > bpf_prog_put(pl->prog);
> > - if (pl->link)
> > + }
> > + if (pl->link) {
> > + if (atype == BPF_LSM_CGROUP)
> > + bpf_cgroup_lsm_shim_release(pl->link->link.prog,
> > + atype);
> > bpf_cgroup_link_auto_detach(pl->link);
> > + }
> > kfree(pl);
> > static_branch_dec(&cgroup_bpf_enabled_key[atype]);
> > }
> > @@ -506,6 +614,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> > struct bpf_prog *old_prog = NULL;
> > struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> > struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> > + struct bpf_attach_target_info tgt_info = {};
> > enum cgroup_bpf_attach_type atype;
> > struct bpf_prog_list *pl;
> > struct hlist_head *progs;
> > @@ -522,9 +631,35 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> > /* replace_prog implies BPF_F_REPLACE, and vice versa */
> > return -EINVAL;
> >
> > - atype = to_cgroup_bpf_attach_type(type);
> > - if (atype < 0)
> > - return -EINVAL;
> > + if (type == BPF_LSM_CGROUP) {
> > + struct bpf_prog *p = prog ? : link->link.prog;
> > +
> > + if (replace_prog) {
> > + /* Reusing shim from the original program.
> > + */
> > + if (replace_prog->aux->attach_btf_id !=
> > + p->aux->attach_btf_id)
> > + return -EINVAL;
> > +
> > + atype = replace_prog->aux->cgroup_atype;
> > + } else {
> > + err = bpf_check_attach_target(NULL, p, NULL,
> > + p->aux->attach_btf_id,
> > + &tgt_info);
> > + if (err)
> > + return -EINVAL;
> > +
> > + atype = bpf_lsm_attach_type_get(p->aux->attach_btf_id);
> > + if (atype < 0)
> > + return atype;
> > + }
> > +
> > + p->aux->cgroup_atype = atype;
> > + } else {
> > + atype = to_cgroup_bpf_attach_type(type);
> > + if (atype < 0)
> > + return -EINVAL;
> > + }
> >
> > progs = &cgrp->bpf.progs[atype];
> >
> > @@ -580,13 +715,26 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> > if (err)
> > goto cleanup;
> >
> > + bpf_cgroup_storages_link(new_storage, cgrp, type);
> It looks everything is ready for the cgrp local_storage.
> How about also allowing bpf_get_local_storage() for BPF_LSM_CGROUP?
Definitely, we want local storage to work, let me actually exercise it
in the selftest and add the missing kernel bits to enable it.
> > +
> > + if (type == BPF_LSM_CGROUP && !old_prog) {
> > + struct bpf_prog *p = prog ? : link->link.prog;
> > +
> > + err = bpf_trampoline_link_cgroup_shim(p, &tgt_info);
> > + if (err)
> > + goto cleanup_trampoline;
> > + }
> > +
> > if (old_prog)
> > bpf_prog_put(old_prog);
> > else
> > static_branch_inc(&cgroup_bpf_enabled_key[atype]);
> > - bpf_cgroup_storages_link(new_storage, cgrp, type);
> > +
> > return 0;
> >
> > +cleanup_trampoline:
> > + bpf_cgroup_storages_unlink(new_storage);
> > +
> > cleanup:
> > if (old_prog) {
> > pl->prog = old_prog;
> > @@ -678,9 +826,13 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
> > struct hlist_head *progs;
> > bool found = false;
> >
> > - atype = to_cgroup_bpf_attach_type(link->type);
> > - if (atype < 0)
> > - return -EINVAL;
> > + if (link->type == BPF_LSM_CGROUP) {
> > + atype = link->link.prog->aux->cgroup_atype;
> > + } else {
> > + atype = to_cgroup_bpf_attach_type(link->type);
> > + if (atype < 0)
> > + return -EINVAL;
> > + }
> >
> > progs = &cgrp->bpf.progs[atype];
> >
> > @@ -696,6 +848,9 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
> > if (!found)
> > return -ENOENT;
> >
> > + if (link->type == BPF_LSM_CGROUP)
> > + new_prog->aux->cgroup_atype = atype;
> Does it also need to check attach_btf_id between the
> new_prog and the old prog?
Ah, forgot this one, good catch, thanks!
> > +
> > old_prog = xchg(&link->link.prog, new_prog);
> > replace_effective_prog(cgrp, atype, link);
> > bpf_prog_put(old_prog);
> > @@ -779,9 +934,15 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
> > u32 flags;
> > int err;
> >
> > - atype = to_cgroup_bpf_attach_type(type);
> > - if (atype < 0)
> > - return -EINVAL;
> > + if (type == BPF_LSM_CGROUP) {
> > + struct bpf_prog *p = prog ? : link->link.prog;
> > +
> > + atype = p->aux->cgroup_atype;
> > + } else {
> > + atype = to_cgroup_bpf_attach_type(type);
> > + if (atype < 0)
> > + return -EINVAL;
> > + }
> >
> > progs = &cgrp->bpf.progs[atype];
> > flags = cgrp->bpf.flags[atype];
> > @@ -803,6 +964,10 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
> > if (err)
> > goto cleanup;
> >
> > + if (type == BPF_LSM_CGROUP)
> > + bpf_cgroup_lsm_shim_release(prog ? : link->link.prog,
> > + atype);
> After looking at find_detach_entry(),
> the pl->prog may not be the same as the prog or link->link.prog here.
I'm assuming you're talking about "allow detaching with invalid FD
(prog==NULL) in legacy mode", right? I did miss that, so I will use
pl->prog instead, thanks!
> > +
> > /* now can actually delete it from this cgroup list */
> > hlist_del(&pl->node);
> >
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index e9621cfa09f2..d94f4951154e 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3139,6 +3139,11 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> > return prog->enforce_expected_attach_type &&
> > prog->expected_attach_type != attach_type ?
> > -EINVAL : 0;
> > + case BPF_PROG_TYPE_LSM:
> > + if (prog->expected_attach_type != BPF_LSM_CGROUP)
> > + return -EINVAL;
> > + return 0;
> > +
> > default:
> > return 0;
> > }
> > @@ -3194,6 +3199,8 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
> > return BPF_PROG_TYPE_SK_LOOKUP;
> > case BPF_XDP:
> > return BPF_PROG_TYPE_XDP;
> > + case BPF_LSM_CGROUP:
> > + return BPF_PROG_TYPE_LSM;
> > default:
> > return BPF_PROG_TYPE_UNSPEC;
> > }
> > @@ -3247,6 +3254,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> > case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> > case BPF_PROG_TYPE_CGROUP_SYSCTL:
> > case BPF_PROG_TYPE_SOCK_OPS:
> > + case BPF_PROG_TYPE_LSM:
> > ret = cgroup_bpf_prog_attach(attr, ptype, prog);
> > break;
> > default:
> > @@ -3284,6 +3292,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
> > case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> > case BPF_PROG_TYPE_CGROUP_SYSCTL:
> > case BPF_PROG_TYPE_SOCK_OPS:
> > + case BPF_PROG_TYPE_LSM:
> > return cgroup_bpf_prog_detach(attr, ptype);
> > default:
> > return -EINVAL;
> > @@ -4317,6 +4326,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> > case BPF_PROG_TYPE_CGROUP_DEVICE:
> > case BPF_PROG_TYPE_CGROUP_SYSCTL:
> > case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> > + case BPF_PROG_TYPE_LSM:
> > ret = cgroup_bpf_link_attach(attr, prog);
> > break;
> > case BPF_PROG_TYPE_TRACING:
> > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > index 0c4fd194e801..c76dfa4ea2d9 100644
> > --- a/kernel/bpf/trampoline.c
> > +++ b/kernel/bpf/trampoline.c
> > @@ -11,6 +11,8 @@
> > #include <linux/rcupdate_wait.h>
> > #include <linux/module.h>
> > #include <linux/static_call.h>
> > +#include <linux/bpf_verifier.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 = {
> > @@ -485,6 +487,149 @@ int bpf_trampoline_unlink_prog(struct bpf_prog *prog, struct bpf_trampoline *tr)
> > return err;
> > }
> >
> > +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_BPF_SYSCALL)
> > +static struct bpf_prog *cgroup_shim_alloc(const struct bpf_prog *prog,
> > + bpf_func_t bpf_func)
> > +{
> > + struct bpf_prog *p;
> > +
> > + p = bpf_prog_alloc(1, 0);
> > + if (!p)
> > + return NULL;
> > +
> > + p->jited = false;
> > + p->bpf_func = bpf_func;
> > +
> > + p->aux->cgroup_atype = prog->aux->cgroup_atype;
> > + p->aux->attach_func_proto = prog->aux->attach_func_proto;
> > + p->aux->attach_btf_id = prog->aux->attach_btf_id;
> > + p->aux->attach_btf = prog->aux->attach_btf;
> > + btf_get(p->aux->attach_btf);
> > + p->type = BPF_PROG_TYPE_LSM;
> > + p->expected_attach_type = BPF_LSM_MAC;
> > + bpf_prog_inc(p);
> > +
> > + return p;
> > +}
> > +
> > +static struct bpf_prog *cgroup_shim_find(struct bpf_trampoline *tr,
> > + bpf_func_t bpf_func)
> > +{
> > + const struct bpf_prog_aux *aux;
> > + int kind;
> > +
> > + for (kind = 0; kind < BPF_TRAMP_MAX; kind++) {
> > + hlist_for_each_entry(aux, &tr->progs_hlist[kind], tramp_hlist) {
> > + struct bpf_prog *p = aux->prog;
> > +
> > + if (p->bpf_func == bpf_func)
> > + return p;
> > + }
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +int bpf_trampoline_link_cgroup_shim(struct bpf_prog *prog,
> > + struct bpf_attach_target_info *tgt_info)
> > +{
> > + struct bpf_prog *shim_prog = NULL;
> > + struct bpf_trampoline *tr;
> > + bpf_func_t bpf_func;
> > + u64 key;
> > + int err;
> > +
> > + key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf,
> > + prog->aux->attach_btf_id);
> > +
> > + err = bpf_lsm_find_cgroup_shim(prog, &bpf_func);
> > + if (err)
> > + return err;
> > +
> > + tr = bpf_trampoline_get(key, tgt_info);
> > + if (!tr)
> > + return -ENOMEM;
> > +
> > + mutex_lock(&tr->mutex);
> > +
> > + shim_prog = cgroup_shim_find(tr, bpf_func);
> > + if (shim_prog) {
> > + /* Reusing existing shim attached by the other program.
> > + */
> nit. Avoid extra line in '*/' for one liner comment. Didn't
> see this convention in other bpf codes.
Ack, will do!
> Also, how about the earlier comment regarding to another __bpf_prog_enter
> and __bpf_prog_exit for BPF_LSM_CGROUP that don't do the active counts
> and stats collection ?
Ugh, I totally forgot that we've agreed to remove those active counts
for the lsm-cgroup case. Let me actually add that part..
> > + bpf_prog_inc(shim_prog);
> > + mutex_unlock(&tr->mutex);
> > + return 0;
> > + }
> > +
> > + /* Allocate and install new shim.
> > + */
> Same here.
Ack. Thank you for another round of review!
> > +
> > + shim_prog = cgroup_shim_alloc(prog, bpf_func);
> > + if (!shim_prog) {
> > + err = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + err = __bpf_trampoline_link_prog(shim_prog, tr);
> > + if (err)
> > + goto out;
> > +
> > + mutex_unlock(&tr->mutex);
> > +
> > + return 0;
> > +out:
> > + if (shim_prog)
> > + bpf_prog_put(shim_prog);
> > +
> > + mutex_unlock(&tr->mutex);
> > + return err;
> > +}
> > +
> > +void bpf_trampoline_unlink_cgroup_shim(struct bpf_prog *prog)
> > +{
> > + struct bpf_prog *shim_prog;
> > + struct bpf_trampoline *tr;
> > + bpf_func_t bpf_func;
> > + u64 key;
> > + int err;
> > +
> > + key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf,
> > + prog->aux->attach_btf_id);
> > +
> > + err = bpf_lsm_find_cgroup_shim(prog, &bpf_func);
> > + if (err)
> > + return;
> > +
> > + tr = bpf_trampoline_lookup(key);
> > + if (!tr)
> > + return;
> > +
> > + mutex_lock(&tr->mutex);
> > +
> > + shim_prog = cgroup_shim_find(tr, bpf_func);
> > + if (shim_prog) {
> > + /* We use shim_prog refcnt for tracking whether to
> > + * remove the shim program from the trampoline.
> > + * Trampoline's mutex is held while refcnt is
> > + * added/subtracted so we don't need to care about
> > + * potential races.
> > + */
> > +
> > + if (atomic64_read(&shim_prog->aux->refcnt) == 1)
> > + WARN_ON_ONCE(__bpf_trampoline_unlink_prog(shim_prog, tr));
> > +
> > + bpf_prog_put(shim_prog);
> > + }
> > +
> > + mutex_unlock(&tr->mutex);
> > +
> > + bpf_trampoline_put(tr); /* bpf_trampoline_lookup */
> > +
> > + if (shim_prog)
> > + bpf_trampoline_put(tr);
> > +}
> > +#endif
> > +
> > struct bpf_trampoline *bpf_trampoline_get(u64 key,
> > struct bpf_attach_target_info *tgt_info)
> > {
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 9c1a02b82ecd..cc84954846d7 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -14197,6 +14197,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > fallthrough;
> > case BPF_MODIFY_RETURN:
> > case BPF_LSM_MAC:
> > + case BPF_LSM_CGROUP:
> > case BPF_TRACE_FENTRY:
> > case BPF_TRACE_FEXIT:
> > if (!btf_type_is_func(t)) {
> > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> > index d14b10b85e51..bbe48a2dd852 100644
> > --- a/tools/include/uapi/linux/bpf.h
> > +++ b/tools/include/uapi/linux/bpf.h
> > @@ -998,6 +998,7 @@ enum bpf_attach_type {
> > BPF_SK_REUSEPORT_SELECT_OR_MIGRATE,
> > BPF_PERF_EVENT,
> > BPF_TRACE_KPROBE_MULTI,
> > + BPF_LSM_CGROUP,
> > __MAX_BPF_ATTACH_TYPE
> > };
> >
> > --
> > 2.36.0.rc0.470.gd361397f0d-goog
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor
2022-04-19 19:00 ` [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor Stanislav Fomichev
2022-04-26 6:27 ` Martin KaFai Lau
@ 2022-04-27 0:10 ` Martin KaFai Lau
2022-04-27 17:47 ` Stanislav Fomichev
1 sibling, 1 reply; 15+ messages in thread
From: Martin KaFai Lau @ 2022-04-27 0:10 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, bpf, ast, daniel, andrii
On Tue, Apr 19, 2022 at 12:00:48PM -0700, Stanislav Fomichev wrote:
> +static void bpf_cgroup_storages_unlink(struct bpf_cgroup_storage *storages[])
> +{
> + enum bpf_cgroup_storage_type stype;
> +
> + for_each_cgroup_storage_type(stype)
> + bpf_cgroup_storage_unlink(storages[stype]);
> +}
> +
> /* Called when bpf_cgroup_link is auto-detached from dying cgroup.
> * It drops cgroup and bpf_prog refcounts, and marks bpf_link as defunct. It
> * doesn't free link memory, which will eventually be done by bpf_link's
> @@ -166,6 +256,16 @@ static void bpf_cgroup_link_auto_detach(struct bpf_cgroup_link *link)
> link->cgroup = NULL;
> }
>
> +static void bpf_cgroup_lsm_shim_release(struct bpf_prog *prog,
> + enum cgroup_bpf_attach_type atype)
> +{
> + if (prog->aux->cgroup_atype < CGROUP_LSM_START ||
> + prog->aux->cgroup_atype > CGROUP_LSM_END)
> + return;
> +
> + bpf_trampoline_unlink_cgroup_shim(prog);
> +}
> +
> /**
> * cgroup_bpf_release() - put references of all bpf programs and
> * release all cgroup bpf data
> @@ -190,10 +290,18 @@ static void cgroup_bpf_release(struct work_struct *work)
>
> hlist_for_each_entry_safe(pl, pltmp, progs, node) {
> hlist_del(&pl->node);
> - if (pl->prog)
> + if (pl->prog) {
> + if (atype == BPF_LSM_CGROUP)
> + bpf_cgroup_lsm_shim_release(pl->prog,
> + atype);
> bpf_prog_put(pl->prog);
> - if (pl->link)
> + }
> + if (pl->link) {
> + if (atype == BPF_LSM_CGROUP)
> + bpf_cgroup_lsm_shim_release(pl->link->link.prog,
> + atype);
> bpf_cgroup_link_auto_detach(pl->link);
> + }
> kfree(pl);
> static_branch_dec(&cgroup_bpf_enabled_key[atype]);
> }
> @@ -506,6 +614,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> struct bpf_prog *old_prog = NULL;
> struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> + struct bpf_attach_target_info tgt_info = {};
> enum cgroup_bpf_attach_type atype;
> struct bpf_prog_list *pl;
> struct hlist_head *progs;
> @@ -522,9 +631,35 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> /* replace_prog implies BPF_F_REPLACE, and vice versa */
> return -EINVAL;
>
> - atype = to_cgroup_bpf_attach_type(type);
> - if (atype < 0)
> - return -EINVAL;
> + if (type == BPF_LSM_CGROUP) {
> + struct bpf_prog *p = prog ? : link->link.prog;
> +
> + if (replace_prog) {
> + /* Reusing shim from the original program.
> + */
> + if (replace_prog->aux->attach_btf_id !=
> + p->aux->attach_btf_id)
> + return -EINVAL;
> +
> + atype = replace_prog->aux->cgroup_atype;
> + } else {
> + err = bpf_check_attach_target(NULL, p, NULL,
> + p->aux->attach_btf_id,
> + &tgt_info);
> + if (err)
> + return -EINVAL;
> +
> + atype = bpf_lsm_attach_type_get(p->aux->attach_btf_id);
> + if (atype < 0)
> + return atype;
> + }
> +
> + p->aux->cgroup_atype = atype;
> + } else {
> + atype = to_cgroup_bpf_attach_type(type);
> + if (atype < 0)
> + return -EINVAL;
> + }
>
> progs = &cgrp->bpf.progs[atype];
>
> @@ -580,13 +715,26 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> if (err)
> goto cleanup;
>
> + bpf_cgroup_storages_link(new_storage, cgrp, type);
After looking between this patch 3 and the next patch 4, I can't
quite think this through quickly, so it may be faster to ask :)
I have questions on the ordering between update_effective_progs(),
bpf_cgroup_storages_link(), and bpf_trampoline_link_cgroup_shim().
Why bpf_cgroup_storages_link() has to be moved up and done before
bpf_trampoline_link_cgroup_shim() ?
> +
> + if (type == BPF_LSM_CGROUP && !old_prog) {
> + struct bpf_prog *p = prog ? : link->link.prog;
> +
> + err = bpf_trampoline_link_cgroup_shim(p, &tgt_info);
update_effective_progs() was done a few lines above, so the effective[atype]
array has the new_prog now.
If bpf_trampoline_link_cgroup_shim() did fail to add the
shim_prog to the trampoline, the new_prog will still be left in
effective[atype]. There is no shim_prog to execute effective[].
However, there may be places that access effective[]. e.g.
__cgroup_bpf_query() although I think to_cgroup_bpf_attach_type()
is not handling BPF_LSM_CGROUP now. More on __cgroup_bpf_query()
later.
Doing bpf_trampoline_link_cgroup_shim() just before activate_effective_progs() ?
Have you thought about what is needed to support __cgroup_bpf_query() ?
bpf_attach_type and cgroup_bpf_attach_type is no longer a 1:1 relationship.
Looping through cgroup_lsm_atype_usecnt[] and output them under BPF_LSM_CGROUP ?
Same goes for local_storage. All lsm-cgrp attaching to different
attach_btf_id sharing one local_storage because the key is only
cgroup-id and attach_type. Is it enough to start with that
first and the key could be extended later with a new map_flag?
This is related to the API.
> + if (err)
> + goto cleanup_trampoline;
> + }
> +
> if (old_prog)
> bpf_prog_put(old_prog);
> else
> static_branch_inc(&cgroup_bpf_enabled_key[atype]);
> - bpf_cgroup_storages_link(new_storage, cgrp, type);
> +
> return 0;
>
> +cleanup_trampoline:
> + bpf_cgroup_storages_unlink(new_storage);
> +
> cleanup:
> if (old_prog) {
> pl->prog = old_prog;
> @@ -678,9 +826,13 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
> struct hlist_head *progs;
> bool found = false;
>
> - atype = to_cgroup_bpf_attach_type(link->type);
> - if (atype < 0)
> - return -EINVAL;
> + if (link->type == BPF_LSM_CGROUP) {
> + atype = link->link.prog->aux->cgroup_atype;
> + } else {
> + atype = to_cgroup_bpf_attach_type(link->type);
> + if (atype < 0)
> + return -EINVAL;
> + }
>
> progs = &cgrp->bpf.progs[atype];
>
> @@ -696,6 +848,9 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
> if (!found)
> return -ENOENT;
>
> + if (link->type == BPF_LSM_CGROUP)
> + new_prog->aux->cgroup_atype = atype;
> +
> old_prog = xchg(&link->link.prog, new_prog);
> replace_effective_prog(cgrp, atype, link);
> bpf_prog_put(old_prog);
> @@ -779,9 +934,15 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
> u32 flags;
> int err;
>
> - atype = to_cgroup_bpf_attach_type(type);
> - if (atype < 0)
> - return -EINVAL;
> + if (type == BPF_LSM_CGROUP) {
> + struct bpf_prog *p = prog ? : link->link.prog;
> +
> + atype = p->aux->cgroup_atype;
> + } else {
> + atype = to_cgroup_bpf_attach_type(type);
> + if (atype < 0)
> + return -EINVAL;
> + }
>
> progs = &cgrp->bpf.progs[atype];
> flags = cgrp->bpf.flags[atype];
> @@ -803,6 +964,10 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
> if (err)
> goto cleanup;
>
> + if (type == BPF_LSM_CGROUP)
> + bpf_cgroup_lsm_shim_release(prog ? : link->link.prog,
> + atype);
> +
> /* now can actually delete it from this cgroup list */
> hlist_del(&pl->node);
>
[ ... ]
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 0c4fd194e801..c76dfa4ea2d9 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -11,6 +11,8 @@
> #include <linux/rcupdate_wait.h>
> #include <linux/module.h>
> #include <linux/static_call.h>
> +#include <linux/bpf_verifier.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 = {
> @@ -485,6 +487,149 @@ int bpf_trampoline_unlink_prog(struct bpf_prog *prog, struct bpf_trampoline *tr)
> return err;
> }
>
> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_BPF_SYSCALL)
> +static struct bpf_prog *cgroup_shim_alloc(const struct bpf_prog *prog,
> + bpf_func_t bpf_func)
> +{
> + struct bpf_prog *p;
> +
> + p = bpf_prog_alloc(1, 0);
> + if (!p)
> + return NULL;
> +
> + p->jited = false;
> + p->bpf_func = bpf_func;
> +
> + p->aux->cgroup_atype = prog->aux->cgroup_atype;
> + p->aux->attach_func_proto = prog->aux->attach_func_proto;
> + p->aux->attach_btf_id = prog->aux->attach_btf_id;
> + p->aux->attach_btf = prog->aux->attach_btf;
> + btf_get(p->aux->attach_btf);
> + p->type = BPF_PROG_TYPE_LSM;
> + p->expected_attach_type = BPF_LSM_MAC;
> + bpf_prog_inc(p);
> +
> + return p;
> +}
> +
> +static struct bpf_prog *cgroup_shim_find(struct bpf_trampoline *tr,
> + bpf_func_t bpf_func)
> +{
> + const struct bpf_prog_aux *aux;
> + int kind;
> +
> + for (kind = 0; kind < BPF_TRAMP_MAX; kind++) {
> + hlist_for_each_entry(aux, &tr->progs_hlist[kind], tramp_hlist) {
> + struct bpf_prog *p = aux->prog;
> +
> + if (p->bpf_func == bpf_func)
> + return p;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +int bpf_trampoline_link_cgroup_shim(struct bpf_prog *prog,
> + struct bpf_attach_target_info *tgt_info)
> +{
> + struct bpf_prog *shim_prog = NULL;
> + struct bpf_trampoline *tr;
> + bpf_func_t bpf_func;
> + u64 key;
> + int err;
> +
> + key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf,
> + prog->aux->attach_btf_id);
> +
> + err = bpf_lsm_find_cgroup_shim(prog, &bpf_func);
> + if (err)
> + return err;
> +
> + tr = bpf_trampoline_get(key, tgt_info);
> + if (!tr)
> + return -ENOMEM;
> +
> + mutex_lock(&tr->mutex);
> +
> + shim_prog = cgroup_shim_find(tr, bpf_func);
> + if (shim_prog) {
> + /* Reusing existing shim attached by the other program.
> + */
> + bpf_prog_inc(shim_prog);
> + mutex_unlock(&tr->mutex);
> + return 0;
> + }
> +
> + /* Allocate and install new shim.
> + */
> +
> + shim_prog = cgroup_shim_alloc(prog, bpf_func);
> + if (!shim_prog) {
> + err = -ENOMEM;
> + goto out;
> + }
> +
> + err = __bpf_trampoline_link_prog(shim_prog, tr);
> + if (err)
> + goto out;
> +
> + mutex_unlock(&tr->mutex);
> +
> + return 0;
> +out:
> + if (shim_prog)
> + bpf_prog_put(shim_prog);
> +
> + mutex_unlock(&tr->mutex);
> + return err;
> +}
> +
> +void bpf_trampoline_unlink_cgroup_shim(struct bpf_prog *prog)
> +{
> + struct bpf_prog *shim_prog;
> + struct bpf_trampoline *tr;
> + bpf_func_t bpf_func;
> + u64 key;
> + int err;
> +
> + key = bpf_trampoline_compute_key(NULL, prog->aux->attach_btf,
> + prog->aux->attach_btf_id);
> +
> + err = bpf_lsm_find_cgroup_shim(prog, &bpf_func);
> + if (err)
> + return;
> +
> + tr = bpf_trampoline_lookup(key);
> + if (!tr)
> + return;
> +
> + mutex_lock(&tr->mutex);
> +
> + shim_prog = cgroup_shim_find(tr, bpf_func);
> + if (shim_prog) {
> + /* We use shim_prog refcnt for tracking whether to
> + * remove the shim program from the trampoline.
> + * Trampoline's mutex is held while refcnt is
> + * added/subtracted so we don't need to care about
> + * potential races.
> + */
> +
> + if (atomic64_read(&shim_prog->aux->refcnt) == 1)
> + WARN_ON_ONCE(__bpf_trampoline_unlink_prog(shim_prog, tr));
> +
> + bpf_prog_put(shim_prog);
> + }
> +
> + mutex_unlock(&tr->mutex);
> +
> + bpf_trampoline_put(tr); /* bpf_trampoline_lookup */
> +
> + if (shim_prog)
> + bpf_trampoline_put(tr);
> +}
> +#endif
> +
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor
2022-04-27 0:10 ` Martin KaFai Lau
@ 2022-04-27 17:47 ` Stanislav Fomichev
2022-04-27 20:17 ` Martin KaFai Lau
0 siblings, 1 reply; 15+ messages in thread
From: Stanislav Fomichev @ 2022-04-27 17:47 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: netdev, bpf, ast, daniel, andrii
On Tue, Apr 26, 2022 at 5:10 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Tue, Apr 19, 2022 at 12:00:48PM -0700, Stanislav Fomichev wrote:
> > +static void bpf_cgroup_storages_unlink(struct bpf_cgroup_storage *storages[])
> > +{
> > + enum bpf_cgroup_storage_type stype;
> > +
> > + for_each_cgroup_storage_type(stype)
> > + bpf_cgroup_storage_unlink(storages[stype]);
> > +}
> > +
> > /* Called when bpf_cgroup_link is auto-detached from dying cgroup.
> > * It drops cgroup and bpf_prog refcounts, and marks bpf_link as defunct. It
> > * doesn't free link memory, which will eventually be done by bpf_link's
> > @@ -166,6 +256,16 @@ static void bpf_cgroup_link_auto_detach(struct bpf_cgroup_link *link)
> > link->cgroup = NULL;
> > }
> >
> > +static void bpf_cgroup_lsm_shim_release(struct bpf_prog *prog,
> > + enum cgroup_bpf_attach_type atype)
> > +{
> > + if (prog->aux->cgroup_atype < CGROUP_LSM_START ||
> > + prog->aux->cgroup_atype > CGROUP_LSM_END)
> > + return;
> > +
> > + bpf_trampoline_unlink_cgroup_shim(prog);
> > +}
> > +
> > /**
> > * cgroup_bpf_release() - put references of all bpf programs and
> > * release all cgroup bpf data
> > @@ -190,10 +290,18 @@ static void cgroup_bpf_release(struct work_struct *work)
> >
> > hlist_for_each_entry_safe(pl, pltmp, progs, node) {
> > hlist_del(&pl->node);
> > - if (pl->prog)
> > + if (pl->prog) {
> > + if (atype == BPF_LSM_CGROUP)
> > + bpf_cgroup_lsm_shim_release(pl->prog,
> > + atype);
> > bpf_prog_put(pl->prog);
> > - if (pl->link)
> > + }
> > + if (pl->link) {
> > + if (atype == BPF_LSM_CGROUP)
> > + bpf_cgroup_lsm_shim_release(pl->link->link.prog,
> > + atype);
> > bpf_cgroup_link_auto_detach(pl->link);
> > + }
> > kfree(pl);
> > static_branch_dec(&cgroup_bpf_enabled_key[atype]);
> > }
> > @@ -506,6 +614,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> > struct bpf_prog *old_prog = NULL;
> > struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> > struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
> > + struct bpf_attach_target_info tgt_info = {};
> > enum cgroup_bpf_attach_type atype;
> > struct bpf_prog_list *pl;
> > struct hlist_head *progs;
> > @@ -522,9 +631,35 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> > /* replace_prog implies BPF_F_REPLACE, and vice versa */
> > return -EINVAL;
> >
> > - atype = to_cgroup_bpf_attach_type(type);
> > - if (atype < 0)
> > - return -EINVAL;
> > + if (type == BPF_LSM_CGROUP) {
> > + struct bpf_prog *p = prog ? : link->link.prog;
> > +
> > + if (replace_prog) {
> > + /* Reusing shim from the original program.
> > + */
> > + if (replace_prog->aux->attach_btf_id !=
> > + p->aux->attach_btf_id)
> > + return -EINVAL;
> > +
> > + atype = replace_prog->aux->cgroup_atype;
> > + } else {
> > + err = bpf_check_attach_target(NULL, p, NULL,
> > + p->aux->attach_btf_id,
> > + &tgt_info);
> > + if (err)
> > + return -EINVAL;
> > +
> > + atype = bpf_lsm_attach_type_get(p->aux->attach_btf_id);
> > + if (atype < 0)
> > + return atype;
> > + }
> > +
> > + p->aux->cgroup_atype = atype;
> > + } else {
> > + atype = to_cgroup_bpf_attach_type(type);
> > + if (atype < 0)
> > + return -EINVAL;
> > + }
> >
> > progs = &cgrp->bpf.progs[atype];
> >
> > @@ -580,13 +715,26 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
> > if (err)
> > goto cleanup;
> >
> > + bpf_cgroup_storages_link(new_storage, cgrp, type);
> After looking between this patch 3 and the next patch 4, I can't
> quite think this through quickly, so it may be faster to ask :)
>
> I have questions on the ordering between update_effective_progs(),
> bpf_cgroup_storages_link(), and bpf_trampoline_link_cgroup_shim().
>
> Why bpf_cgroup_storages_link() has to be moved up and done before
> bpf_trampoline_link_cgroup_shim() ?
Looking at it again: I think I'm confusing bpf_cgroup_storages_assign
with bpf_cgroup_storages_link and we don't need to move the latter.
For context: my reasoning here was to make sure the prog has cgroup
storage before bpf_trampoline_link_cgroup_shim installs the actual
trampoline which might trigger the programs.
> > +
> > + if (type == BPF_LSM_CGROUP && !old_prog) {
> > + struct bpf_prog *p = prog ? : link->link.prog;
> > +
> > + err = bpf_trampoline_link_cgroup_shim(p, &tgt_info);
> update_effective_progs() was done a few lines above, so the effective[atype]
> array has the new_prog now.
>
> If bpf_trampoline_link_cgroup_shim() did fail to add the
> shim_prog to the trampoline, the new_prog will still be left in
> effective[atype]. There is no shim_prog to execute effective[].
> However, there may be places that access effective[]. e.g.
> __cgroup_bpf_query() although I think to_cgroup_bpf_attach_type()
> is not handling BPF_LSM_CGROUP now. More on __cgroup_bpf_query()
> later.
>
> Doing bpf_trampoline_link_cgroup_shim() just before activate_effective_progs() ?
Yeah, you're right, I thought that there was a cleanup path that
undoes update_effective_progs action :-( Moving link_cgroup_shim
before update_effective_progs/activate_effective_progs makes sense,
thank you!
> Have you thought about what is needed to support __cgroup_bpf_query() ?
> bpf_attach_type and cgroup_bpf_attach_type is no longer a 1:1 relationship.
> Looping through cgroup_lsm_atype_usecnt[] and output them under BPF_LSM_CGROUP ?
> Same goes for local_storage. All lsm-cgrp attaching to different
> attach_btf_id sharing one local_storage because the key is only
> cgroup-id and attach_type. Is it enough to start with that
> first and the key could be extended later with a new map_flag?
> This is related to the API.
Ugh, I think I was still under the impression that it would just work
out. But I haven't thought about __cgroup_bpf_query in the context of
the next change which breaks that 1:1 relationship. Good catch, I have
to look into that and will add a test to verify.
Regarding cgroup_id+attach_type key of local storage: maybe prohibit
that mode for BPF_LSM_CGROUP ? We have two modes: (1) keyed by
cgroup_id+attach_type and (2) keyed by cgroup_id only (and might be
shared across attach_types). The first one never made much sense to
me; the second one behaves exactly like the rest of local storages
(file/sk/etc). WDYT? (we can enable (1) if we ever decide that it's
needed)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor
2022-04-27 17:47 ` Stanislav Fomichev
@ 2022-04-27 20:17 ` Martin KaFai Lau
0 siblings, 0 replies; 15+ messages in thread
From: Martin KaFai Lau @ 2022-04-27 20:17 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, bpf, ast, daniel, andrii
On Wed, Apr 27, 2022 at 10:47:48AM -0700, Stanislav Fomichev wrote:
> Regarding cgroup_id+attach_type key of local storage: maybe prohibit
> that mode for BPF_LSM_CGROUP ? We have two modes: (1) keyed by
> cgroup_id+attach_type and (2) keyed by cgroup_id only (and might be
> shared across attach_types). The first one never made much sense to
> me; the second one behaves exactly like the rest of local storages
> (file/sk/etc). WDYT? (we can enable (1) if we ever decide that it's
> needed)
Ah right, cgroup local_storage has already allowed keys in different
granularity and is using key_size to decide which part of
the bpf_cgroup_storage_key should be used.
In that case, may be just allow (1) and (2) now to avoid a new
inconsistent usage surprise with other existing cgroup's prog type.
In the future, if a more granular cgroup local_storage is needed,
the attach_btf_id can be added to 'struct bpf_cgroup_storage_key'.
The existing non BPF_LSM_CGROUP prog will always have attach_btf_id 0.
Would that work?
One caveat though, sizeof(bpf_cgroup_storage_key) is 16 now with
4 bytes padding at the end, so the future __u32 attach_btf_id may
not be able to use the padding in order to get a different size.
I think it is minor and should be ok.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-04-27 20:18 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-19 19:00 [PATCH bpf-next v5 0/8] bpf: cgroup_sock lsm flavor Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 1/8] bpf: add bpf_func_t and trampoline helpers Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 2/8] bpf: convert cgroup_bpf.progs to hlist Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 3/8] bpf: per-cgroup lsm flavor Stanislav Fomichev
2022-04-26 6:27 ` Martin KaFai Lau
2022-04-26 6:30 ` Martin KaFai Lau
2022-04-26 22:44 ` Stanislav Fomichev
2022-04-27 0:10 ` Martin KaFai Lau
2022-04-27 17:47 ` Stanislav Fomichev
2022-04-27 20:17 ` Martin KaFai Lau
2022-04-19 19:00 ` [PATCH bpf-next v5 4/8] bpf: minimize number of allocated lsm slots per program Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 5/8] bpf: allow writing to a subset of sock fields from lsm progtype Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 6/8] libbpf: add lsm_cgoup_sock type Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 7/8] selftests/bpf: lsm_cgroup functional test Stanislav Fomichev
2022-04-19 19:00 ` [PATCH bpf-next v5 8/8] selftests/bpf: verify lsm_cgroup struct sock access Stanislav Fomichev
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.