Re: [PATCH bpf-next 1/3] bpf: Introduce sleepable BPF programs

On 27-May 22:33, Alexei Starovoitov wrote:

From: Alexei Starovoitov <ast@kernel.org>

Introduce sleepable BPF programs that can request such property for themselves
via BPF_F_SLEEPABLE flag at program load time. In such case they will be able
to use helpers like bpf_copy_from_user() that might sleep. At present only
fentry/fexit/fmod_ret and lsm programs can request to be sleepable and only
when they are attached to kernel functions that are known to allow sleeping.

The non-sleepable programs are relying on implicit rcu_read_lock() and
migrate_disable() to protect life time of programs, maps that they use and
per-cpu kernel structures used to pass info between bpf programs and the
kernel. The sleepable programs cannot be enclosed into rcu_read_lock().
migrate_disable() maps to preempt_disable() in non-RT kernels, so the progs
should not be enclosed in migrate_disable() as well. Therefore bpf_srcu is used
to protect the life time of sleepable progs.

There are many networking and tracing program types. In many cases the
'struct bpf_prog *' pointer itself is rcu protected within some other kernel
data structure and the kernel code is using rcu_dereference() to load that
program pointer and call BPF_PROG_RUN() on it. All these cases are not touched.
Instead sleepable bpf programs are allowed with bpf trampoline only. The
program pointers are hard-coded into generated assembly of bpf trampoline and
synchronize_srcu(&bpf_srcu) is used to protect the life time of the program.
The same trampoline can hold both sleepable and non-sleepable progs.

When bpf_srcu lock is held it means that some sleepable bpf program is running
from bpf trampoline. Those programs can use bpf arrays and preallocated hash/lru
maps. These map types are waiting on programs to complete via
synchronize_srcu(&bpf_srcu);

Updates to trampoline now has to do synchronize_srcu + synchronize_rcu_tasks
to wait for sleepable progs to finish and for trampoline assembly to finish.

In the future srcu will be replaced with upcoming rcu_trace.
That will complete the first step of introducing sleepable progs.

After that dynamically allocated hash maps can be allowed. All map elements
would have to be srcu protected instead of normal rcu.
per-cpu maps will be allowed. Either via the following pattern:
void *elem = bpf_map_lookup_elem(map, key);
if (elem) {
   // access elem
   bpf_map_release_elem(map, elem);
}
where modified lookup() helper will do migrate_disable() and
new bpf_map_release_elem() will do corresponding migrate_enable().
Or explicit bpf_migrate_disable/enable() helpers will be introduced.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Thanks! This will be really helpful for LSM programs.

Acked-by: KP Singh <redacted>

---
 arch/x86/net/bpf_jit_comp.c    | 36 +++++++++++++++-------
 include/linux/bpf.h            |  4 +++
 include/uapi/linux/bpf.h       |  8 +++++
 kernel/bpf/arraymap.c          |  5 +++
 kernel/bpf/hashtab.c           | 19 ++++++++----
 kernel/bpf/syscall.c           | 12 ++++++--
 kernel/bpf/trampoline.c        | 33 +++++++++++++++++++-
 kernel/bpf/verifier.c          | 56 ++++++++++++++++++++++++++--------
 tools/include/uapi/linux/bpf.h |  8 +++++
 9 files changed, 147 insertions(+), 34 deletions(-)

[...]

+			if (ret)
+				verbose(env, "%s() is not modifiable\n",
+					prog->aux->attach_func_name);
+		} else if (prog->aux->sleepable && prog->type == BPF_PROG_TYPE_TRACING) {
+			/* fentry/fexit progs can be sleepable only if they are
+			 * attached to ALLOW_ERROR_INJECTION or security_*() funcs.
+			 * LSM progs check that they are attached to bpf_lsm_*() funcs
+			 * which are sleepable too.

I know of one LSM hook which is not sleepable and is executed in an
RCU callback i.e. task_free. I don't think t's a problem to run under
SRCU for that (I tried it and it does not cause any issues).

We can add a blacklisting mechanism later for the sleepable flags or
just the sleeping helpers (based on some of the work going on to
whitelist functions for helper usage).

- KP

+			 */
+			ret = check_attach_modify_return(prog, addr);
+			if (ret)
+				verbose(env, "%s is not sleepable\n",

[...]

  * two extensions:
  *
-- 
2.23.0