On Mon, May 09, 2022 at 04:38:36PM -0700, Stanislav Fomichev wrote:

quoted

quoted

+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;

From lsm_hook_defs.h, there are some default return values that are not 0.
Is it ok to always return 0 in cases like the cgroup array is empty ?

That's a good point, I haven't thought about it. You're right, it
seems like attaching to this hook for some LSMs will change the
default from some error to zero.
Let's start by prohibiting those hooks for now? I guess in theory,
when we generate a trampoline, we can put this default value as an
input arg to these new __cgroup_bpf_run_lsm_xxx helpers (in the
future)?

After looking at arch_prepare_bpf_trampoline, return 0 here should be fine.
If I read it correctly, when the shim_prog returns 0, the trampoline
will call the original kernel function which is the bpf_lsm_##NAME()
defined in bpf_lsm.c and it will then return the zero/-ve DEFAULT.

Another thing that seems to be related: there are a bunch of hooks
that return void, so returning EPERM from the cgroup programs won't
work as expected.
I can probably record, at verification time, whether lsm_cgroup
programs return any "non-success" return codes and prohibit attaching
these progs to the void hooks?

hmm...yeah, BPF_LSM_CGROUP can be enforced to return either 0 or 1 as
most other cgroup-progs do.

Do you have a use case that needs to return something other than -EPERM ?

quoted

quoted

+
+     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, NULL);
+     rcu_read_unlock();
+     return ret;
+}