Re: [RFC PATCH bpf-next seccomp 12/12] seccomp-ebpf: support task storage from BPF-LSM, defaulting to group leader

On Tue, May 11, 2021 at 12:44:46AM -0500, YiFei Zhu wrote:

On Mon, May 10, 2021 at 8:58 PM Alexei Starovoitov
[off-list ref] wrote:

quoted

On Mon, May 10, 2021 at 12:22:49PM -0500, YiFei Zhu wrote:

quoted

+
+BPF_CALL_4(bpf_task_storage_get_default_leader, struct bpf_map *, map,
+        struct task_struct *, task, void *, value, u64, flags)
+{
+     if (!task)
+             task = current->group_leader;

Did you actually need it to be group_leader or current is enough?
If so loading BTF is not necessary.

I think if task_storage were to be used to apply a policy onto a set
of tasks, there are probably more use cases to perform the state
transitions across an entire process than state transitions across a
single thread. Though, since seccomp only applies to the process tree
a lot of use cases of a per-process storage would be covered by a
global datasec too.

quoted

You could have exposed it bpf_get_current_task_btf() and passed its
return value into bpf_task_storage_get.

On the other side loading BTF can be relaxed to unpriv,
but doing current->group_leader deref will make it priv only anyway.

Yeah, that deref is what I was concerned about. It seems that if I
expose BTF structs to a prog type it gains the ability to deref it,
and I definitely don't want unpriv reading task_structs. Though yeah
we could potentially change the verifier to prohibit PTR_TO_BTF_ID
deref and any pointer arithmetic on it...

How about, we expose bpf_get_current_task_btf to unpriv, prohibit
unpriv deref and pointer arithmetic, and have NULL be
current->group_leader? This way, unpriv has access to both per-thread
and per-process.

NULL to mean current->group_leader is too specific and not extensible.
I'd rather add another bpf_get_current_task_btf-like helper that
returns parent or group_leader depending on flags.
For now I wouldn't even do that. As you said hashmap with key=pid will
work fine. task_local_storage is a performance optimization.
I'd focus on landing the key bits before optimizing.