On 8/27/25 2:28 PM, Andrii Nakryiko wrote:
On Wed, Aug 27, 2025 at 2:04 PM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote:
On Wed, 2025-08-27 at 22:54 +0200, Ilya Leoshkevich wrote:
On Wed, 2025-08-27 at 13:05 -0700, Andrii Nakryiko wrote:
On Wed, Aug 27, 2025 at 11:34 AM Ilya Leoshkevich
<iii@xxxxxxxxxxxxx>
wrote:
On Wed, 2025-08-27 at 10:32 -0700, Andrii Nakryiko wrote:
On Wed, Aug 27, 2025 at 6:05 AM Ilya Leoshkevich
<iii@xxxxxxxxxxxxx>
wrote:
The verifier requires that pointers returned by
bpf_obj_new_impl()
are
either dropped or stored in a map. Therefore programs that do
not
use
its return values will fail to load. Make the compiler point
out
these
issues. Adjust selftests that check that the verifier does
indeed
spot
these bugs.
Link:
https://lore.kernel.org/bpf/CAADnVQL6Q+QRv3_JwEd26biwGpFYcwD_=BjBJWLAtpgOP9CKRw@xxxxxxxxxxxxxx/
Suggested-by: Alexei Starovoitov <ast@xxxxxxxxxx>
Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
---
tools/lib/bpf/bpf_helpers.h | 4
++++
tools/testing/selftests/bpf/bpf_experimental.h | 2 +-
tools/testing/selftests/bpf/progs/linked_list_fail.c | 8
++++-
---
3 files changed, 9 insertions(+), 5 deletions(-)
[...]
/* When utilizing vmlinux.h with BPF CO-RE, user BPF
programs
can't include
* any system-level headers (such as stddef.h,
linux/version.h,
etc), and
* commonly-used macros like NULL and KERNEL_VERSION aren't
available through
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h
b/tools/testing/selftests/bpf/bpf_experimental.h
index da7e230f2781..e5ef4792da42 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -20,7 +20,7 @@
* A pointer to an object of the type corresponding to
the
passed in
* 'local_type_id', or NULL on failure.
*/
-extern void *bpf_obj_new_impl(__u64 local_type_id, void
*meta)
__ksym;
+extern __must_check void *bpf_obj_new_impl(__u64
local_type_id,
void *meta) __ksym;
bpf_obj_new_impl will generally come from vmlinux.h nowadays,
and
that
one won't have __must_check annotation, is that a problem?
It should be fine according to [1]:
Compatible attribute specifications on distinct declarations of
the
same function are merged.
I will add this to the commit message in v3.
Sure, for BPF selftests it will work. My question was broader, for
anyone using bpf_obj_new in the wild, they won't have __must_check
annotation from vmlinux.h (and I doubt they will manually add it
like
we do here for BPF selftests), so if that's important, I guess we
need
to think how to wire that up so that it happens automatically
through
vmlinux.h.
"It's not that important to bother" is a fine answer as well :)
I see. Seems like it's tough:
- The attribute is not available in DWARF
- But we could introduce KF_MUST_CHECK flag
- Which pahole would extract from .BTF_ids and convert to
a btf_decl_tag
- This will make pahole depend on .BTF_ids format though, which
might
be undesirable
Hm, I should have checked that before hitting "send": apparently pahole
already parses both .BTF_ids and __BTF_ID__set8__*.
Correct, this isn't any new dependency.
Still, DW_TAG_GNU_annotation looks like a better long-term solution.
Ihor a bit earlier added BTF-specific way to attach any random
attribute to BTF type (it's a special form of
BTF_TYPE_TAG/BTF_DECL_TAG), I think it's still on his plate to wire up
the use of that for a few long-standing issues with vmlinux.h, so this
might be yet another reason and use case for that.
Acked. Thanks for the reminder...
Although in this particular case (__must_check) there is still an
issue of passing through the attribute from kernel source to pahole,
and AFAIU the only feasible way to do that right now is via KF_ flags.
So this DW_TAG_GNU_annotation looks promising, because with that
pahole could read annotations from DWARF directly, without special
handling of .BTF_ids data.
But then we probably don't want pahole to put _all_ the attributes in
BTF, there might be too many of attributes we don't care about. Still,
a filter on a generic DWARF tag is much simpler then what we have now.
Ilya, thanks for sharing. I wasn't aware of DW_TAG_GNU_annotation
development.
But agreed, for now I'd go with BPF selftests-specific mitigation.
- Then bpftool would convert this btf_decl_tag to __must_check
Seems like they are attempting to upstream the new
DW_TAG_GNU_annotation right now [1], if that lands and is available
for non-BPF targets, we could put
__attribute((btf_decl_tag("must_check"))) on kernel's
bpf_obj_new_impl() and directly access it from bpftool.
So for now I would propose to limit the solution to selftests.
[1] https://gcc.gnu.org/pipermail/gcc-patches/2025-August/692445.html
[1]
https://gcc.gnu.org/onlinedocs/gcc-12.4.0/gcc/Function-Attributes.html
[...]
