| Summary: | Syscall param bpf(attr->raw_tracepoint.name) points to unaddressable byte(s) in libbpf-tools | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Andreas Gerstmayr <andreas> |
| Component: | general | Assignee: | Mark Wielaard <mark> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | mark |
| Priority: | NOR | ||
| Version First Reported In: | 3.18.1 | ||
| Target Milestone: | --- | ||
| Platform: | Fedora RPMs | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
|
Description
Andreas Gerstmayr
2022-03-17 14:55:44 UTC
So there are two issues here: - WARNING: unhandled eBPF command 35 This seems to be BPF_LINK_DETACH, which valgrind indeed doesn't handle. - Syscall param bpf(attr->raw_tracepoint.name) points to unaddressable byte(s) Address 0x0 is not stack'd, malloc'd or (recently) free'd This check is only done for BPF_RAW_TRACEPOINT_OPEN It isn't immediately clear to me that is is allowed to be NULL. Do you happen to have documentation which explains what BPF_RAW_TRACEPOINT_OPEN does when the name is NULL? (In reply to Mark Wielaard from comment #1) > So there are two issues here: > > - WARNING: unhandled eBPF command 35 > This seems to be BPF_LINK_DETACH, which valgrind indeed doesn't handle. Yep, that's fine so far. I'm more concerned about the error below. > - Syscall param bpf(attr->raw_tracepoint.name) points to unaddressable > byte(s) > Address 0x0 is not stack'd, malloc'd or (recently) free'd > This check is only done for BPF_RAW_TRACEPOINT_OPEN > It isn't immediately clear to me that is is allowed to be NULL. > > Do you happen to have documentation which explains what > BPF_RAW_TRACEPOINT_OPEN does when the name is NULL? Good question :) I looked in the caller, and it's quite explicit about setting the name to NULL: https://github.com/libbpf/libbpf/blob/d6783c28b40e8355d2e3bd4a8141b88da7704f6d/src/libbpf.c#L10491 Then I looked into the syscall on the kernel side (https://github.com/torvalds/linux/blob/56e337f2cf1326323844927a04e9dbce9a244835/kernel/bpf/syscall.c#L3042-L3056) and it goes as far as erroring out if the name is set to anything other than NULL with the following comment: "The attach point for this category of programs should be specified via btf_id during program load." A few lines later tp_name is set to prog->aux->attach_func_name. prog->type is BPF_PROG_TYPE_TRACING in our case, I've verified that in the bpf_program__attach_btf_id function on the libbpf side. So afaics, when using BTF, only the raw_tracepoint.prog_fd should be set. Thanks for doing the research. The fix is simple in that case:
diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index b9d531de3..38edccc98 100644
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -12920,8 +12920,9 @@ PRE(sys_bpf)
break;
}
/* Name is limited to 128 characters in kernel/bpf/syscall.c. */
- pre_asciiz_str(tid, attr->raw_tracepoint.name, 128,
- "bpf(attr->raw_tracepoint.name)");
+ if (attr->raw_tracepoint.name != NULL)
+ pre_asciiz_str(tid, attr->raw_tracepoint.name, 128,
+ "bpf(attr->raw_tracepoint.name)");
}
break;
case VKI_BPF_BTF_LOAD:
https://code.wildebeest.org/git/user/mjw/valgrind/commit/?h=bpf-raw_tracepoint-name
(In reply to Mark Wielaard from comment #3) > Thanks for doing the research. The fix is simple in that case: > > diff --git a/coregrind/m_syswrap/syswrap-linux.c > b/coregrind/m_syswrap/syswrap-linux.c > index b9d531de3..38edccc98 100644 > --- a/coregrind/m_syswrap/syswrap-linux.c > +++ b/coregrind/m_syswrap/syswrap-linux.c > @@ -12920,8 +12920,9 @@ PRE(sys_bpf) > break; > } > /* Name is limited to 128 characters in kernel/bpf/syscall.c. */ > - pre_asciiz_str(tid, attr->raw_tracepoint.name, 128, > - "bpf(attr->raw_tracepoint.name)"); > + if (attr->raw_tracepoint.name != NULL) > + pre_asciiz_str(tid, attr->raw_tracepoint.name, 128, > + "bpf(attr->raw_tracepoint.name)"); > } > break; > case VKI_BPF_BTF_LOAD: > > https://code.wildebeest.org/git/user/mjw/valgrind/commit/?h=bpf- > raw_tracepoint-name Thanks! I've verified that the above fix resolves this issue. Thanks for testing. Pushed as: commit 957339db27f7d1603a7217a0f891d91d204d64aa Author: Mark Wielaard <mark@klomp.org> Date: Sat Mar 19 01:06:40 2022 +0100 bpf attr->raw_tracepoint.name may be NULL for BPF_RAW_TRACEPOINT_OPEN. For BPF_RAW_TRACEPOINT_OPEN attr->raw_tracepoint.name may be NULL. Otherwise it should point to a valid (max 128 char) string. Only raw_tracepoint.prog_fd needs to be set. https://bugs.kde.org/show_bug.cgi?id=451626 |