Re: [PATCH] tracepoint: Add tracepoint_probe_register_may_exist() for BPF tracing

On Tue, Jun 29, 2021 at 6:55 AM Steven Rostedt [off-list ref] wrote:

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

All internal use cases for tracepoint_probe_register() is set to not ever
be called with the same function and data. If it is, it is considered a
bug, as that means the accounting of handling tracepoints is corrupted.
If the function and data for a tracepoint is already registered when
tracepoint_probe_register() is called, it will call WARN_ON_ONCE() and
return with EEXISTS.

The BPF system call can end up calling tracepoint_probe_register() with
the same data, which now means that this can trigger the warning because
of a user space process. As WARN_ON_ONCE() should not be called because
user space called a system call with bad data, there needs to be a way to
register a tracepoint without triggering a warning.

Enter tracepoint_probe_register_may_exist(), which can be called, but will
not cause a WARN_ON() if the probe already exists. It will still error out
with EEXIST, which will then be sent to the user space that performed the
BPF system call.

This keeps the previous testing for issues with other users of the
tracepoint code, while letting BPF call it with duplicated data and not
warn about it.

There doesn't seem to be anything conceptually wrong with attaching
the same BPF program twice to the same tracepoint. Is it a hard
requirement to have a unique tp+callback combination, or was it done
mostly to detect an API misuse? How hard is it to support such use
cases?

I was surprised to discover this is not supported (though I never had
a use for this, had to construct a test to see the warning).

Link: https://lore.kernel.org/lkml/20210626135845.4080-1-penguin-kernel@I-love.SAKURA.ne.jp/ (local)
Link: https://syzkaller.appspot.com/bug?id=41f4318cf01762389f4d1c1c459da4f542fe5153 [1]`

Cc: stable@vger.kernel.org
Fixes: c4f6699dfcb85 ("bpf: introduce BPF_RAW_TRACEPOINT")
Reported-by: syzbot <redacted>
Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

 include/linux/tracepoint.h | 10 ++++++++++
 kernel/trace/bpf_trace.c   |  3 ++-
 kernel/tracepoint.c        | 33 ++++++++++++++++++++++++++++++---
 3 files changed, 42 insertions(+), 4 deletions(-)

[...]