Re: [PATCH] bpf: reject blacklisted symbols in kprobe_multi to avoid recursive trap

[Yonghong Song]

On 5/12/23 9:17 PM, Steven Rostedt wrote:
> On Fri, 12 May 2023 07:29:02 -0700
> Yonghong Song <yhs@xxxxxxxx> wrote:
>
> > A fprobe_blacklist might make sense indeed as fprobe and kprobe are
> > quite different... Thanks for working on this.
>
> Hmm, I think I see the problem:
>
> fprobe_kprobe_handler() {
>     kprobe_busy_begin() {
>        preempt_disable() {
>           preempt_count_add() {  <-- trace
>              fprobe_kprobe_handler() {
> 		[ wash, rinse, repeat, CRASH!!! ]
>
> Either the kprobe_busy_begin() needs to use preempt_disable_notrace()
> versions, or fprobe_kprobe_handle() needs a
> ftrace_test_recursion_trylock() call.

Currently, in verifier we have:

BTF_SET_START(btf_id_deny)
BTF_ID_UNUSED
#ifdef CONFIG_SMP
BTF_ID(func, migrate_disable)
BTF_ID(func, migrate_enable)
#endif
#if !defined CONFIG_PREEMPT_RCU && !defined CONFIG_TINY_RCU
BTF_ID(func, rcu_read_unlock_strict)
#endif
#if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_TRACE_PREEMPT_TOGGLE)
BTF_ID(func, preempt_count_add)
BTF_ID(func, preempt_count_sub)
#endif
#ifdef CONFIG_PREEMPT_RCU
BTF_ID(func, __rcu_read_lock)
BTF_ID(func, __rcu_read_unlock)
#endif
BTF_SET_END(btf_id_deny)

        ...
        } else if (prog->type == BPF_PROG_TYPE_TRACING &&
                   btf_id_set_contains(&btf_id_deny, btf_id)) {
                return -EINVAL;
        }

Since we do not have a explicit deny list available to user space,
the above checking will prevent to trace a few functions for
tracing prog (fentry, fexit, fmod_ret).

For fprobe_kprobe case, if we can construct a user visible deny
list which will be the best. Otherwise, we can add a
btf_id_deny_fprobe btf set which should work too.

> -- Steve