> diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
> index b7e388f99d0b..f934452e52f0 100644
> --- a/tools/lib/bpf/features.c
> +++ b/tools/lib/bpf/features.c
> @@ -16,7 +16,14 @@ int probe_fd(int fd)
> {
> if (fd >= 0)
> close(fd);
> - return fd >= 0;
> + /* Return 1 on success, negative error on failure, so
> + * feat_supported() can distinguish probe errors from
> + * genuine feature absence. When a BPF token is present,
> + * a negative return triggers the rescue path that marks
> + * the feature as SUPPORTED (token creation itself proves
> + * the kernel BPF subsystem works).
> + */
> + return fd >= 0 ? 1 : fd;
> }
Does this return a literal -1 instead of the actual error code? Several
feature probes use internal direct syscall wrappers like sys_bpf_prog_load()
which return -1 on failure and leave the real error in errno. Since probe_fd()
returns this -1 unmodified instead of -errno, feat_supported() will pass -1 to
errstr():
tools/lib/bpf/features.c:feat_supported() {
...
pr_warn("Detection of kernel %s support failed: %s\n",
feat->desc, errstr(ret));
...
}
Because -1 equals -EPERM, does this cause the error string to unconditionally
resolve to "Operation not permitted" and mask the true reason for the probe
failure (like -EINVAL)?
https://lore.kernel.org/bpf/[email protected]/
>
> static int probe_kern_prog_name(int token_fd)
[ ... ]
> @@ -725,9 +732,21 @@ bool feat_supported(struct kern_feature_cache *cache,
> enum kern_feature_id feat_
> } else if (ret == 0) {
> WRITE_ONCE(cache->res[feat_id], FEAT_MISSING);
> } else {
> + /*
> + * A BPF token may restrict which program/map types
> + * are permitted, causing the probe to fail even
> + * though the kernel supports the feature. When a
> + * token is present the probe is best-effort: BPF
> + * token creation itself proves the kernel has a
> + * working BPF subsystem. Real BPF issues will be
> + * caught during actual program/map loading.
> + */
> + if (cache->token_fd)
> + WRITE_ONCE(cache->res[feat_id], FEAT_SUPPORTED);
> + else
> + WRITE_ONCE(cache->res[feat_id], FEAT_MISSING);
Will this forcefully override genuine missing features to FEAT_SUPPORTED if a
token is present?
By returning < 0 from probe_fd(), genuine feature absences now fall into this
error-handling path. If a newer BPF feature is missing on an older kernel that
supports tokens, libbpf might incorrectly mark it as supported and skip
essential sanitizations (like stripping unsupported BTF tags).
Does this bypass the fallback logic and cause the final program load to fatally
fail?
https://lore.kernel.org/bpf/[email protected]/
> pr_warn("Detection of kernel %s support failed: %s\n",
> feat->desc, errstr(ret));
> - WRITE_ONCE(cache->res[feat_id], FEAT_MISSING);
> }
> }
Are there existing probes that still return 0 on failure and bypass this rescue
mechanism?
For instance, probe_prog_bind_map() returns 0 if bpf_prog_load() fails:
tools/lib/bpf/features.c:probe_prog_bind_map() {
...
if (prog < 0) {
close(map);
return 0;
}
...
}
Since these functions were not updated to propagate negative error codes, they
evaluate via the else if (ret == 0) branch and are immediately marked as
FEAT_MISSING.
Does this leave unprivileged BPF token support broken for these specific
features?
https://lore.kernel.org/bpf/[email protected]/
Without curly braces around the else block, the pr_warn() call executes
unconditionally for any negative return value. Combined with probe_fd() now
returning < 0 for all missing features, will this loudly spam warnings to the
console for normal feature absence checks?
https://lore.kernel.org/bpf/[email protected]/
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26928747574