Re: [PATCH v2 bpf-next 1/2] libbpf: Allow use of feature cache for non-token cases

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `bpf_object_prepare_token` unconditionally overwrites `obj->feat_cache` if BPF tokens are supported, leaking any custom cache set by `bpf_object_set_feat_cache` and causing test mocking to be environment-dependent.
--

--- Patch [1]: [PATCH v2 bpf-next 1/2] libbpf: Allow use of feature cache for non-token cases ---
commit 118614ae13eed03565933d30357d27d00e3145b4
Author: Alan Maguire <[email protected]>

libbpf: Allow use of feature cache for non-token cases

This patch exposes a new internal API, bpf_object_set_feat_cache(),
allowing BPF selftests to assign a custom feature cache to a bpf_object.
This enables simulating missing kernel features to test BTF sanitization
fallback paths.

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 42bdba4efd0c6..b7b636786d387 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
[ ... ]
> @@ -5203,12 +5203,20 @@ bool kernel_supports(const struct bpf_object *obj, enum kern_feature_id feat_id)
>  		 */
>  		return true;
>  
> -	if (obj->token_fd)
> +	if (obj->feat_cache)
>  		return feat_supported(obj->feat_cache, feat_id);
>  
>  	return feat_supported(NULL, feat_id);
>  }
>  
> +/* Used in testing to simulate missing features. */
> +void bpf_object_set_feat_cache(struct bpf_object *obj, struct kern_feature_cache *cache)
> +{
> +	if (obj->feat_cache)
> +		zfree(&obj->feat_cache);
> +	obj->feat_cache = cache;
> +}
> +

[Severity: Medium]
If a test sets a custom cache using this function and then loads the object,
could bpf_object_prepare_token() overwrite and leak this custom cache if BPF
token creation succeeds?

Looking at bpf_object_prepare_token(), it unconditionally allocates a new
cache when token creation is successful:

tools/lib/bpf/libbpf.c:bpf_object_prepare_token() {
    ...
	obj->feat_cache = calloc(1, sizeof(*obj->feat_cache));
	if (!obj->feat_cache) {
		close(token_fd);
		return -ENOMEM;
	}
    ...
}

Would this cause testing environments to behave non-deterministically?
It seems tests might successfully preserve the mock cache on systems without
token delegation (where bpf_token_create fails and returns early), but lose
the mocked state and leak the initially injected cache on systems with token
support.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1