Re: [PATCH v4] libbpf: fix UAF in strset__add_str()

[bot+bpf-ci]

> diff --git a/tools/lib/bpf/strset.c b/tools/lib/bpf/strset.c
> index 2464bcbd04e0..b9faca828f09 100644
> --- a/tools/lib/bpf/strset.c
> +++ b/tools/lib/bpf/strset.c
> @@ -107,6 +107,38 @@ static void *strset_add_str_mem(struct strset *set, size_t add_sz)
>  			      set->strs_data_len, set->strs_data_max_len, add_sz);
>  }
>
> +static long strset_str_append(struct strset *set, const char *s)
> +{
> +	uintptr_t old_data = (uintptr_t)set->strs_data;
> +	uintptr_t old_s = (uintptr_t)s;
> +	long len = strlen(s) + 1;
> +	void *p;
> +
> +	/* Hashmap keys are always offsets within set->strs_data, so to even
> +	 * look up some string from the "outside", we need to first append it
> +	 * at the end, so that it can be addressed with an offset. Luckily,
> +	 * until set->strs_data_len is incremented, that string is just a piece
> +	 * of garbage for the rest of the code, so no harm, no foul. On the
> +	 * other hand, if the string is unique, it's already appended and
> +	 * ready to be used, only a simple set->strs_data_len increment away.
> +	 */
> +	p = strset_add_str_mem(set, len);
> +	if (!p)
> +		return -ENOMEM;
> +
> +	/* The set->strs_data might have reallocated and if 's' pointed
> +	 * to an internal string within the old buffer, then it became
> +	 * dangling and needs to be reconstructed before the copy.
> +	 */
> +	if (old_data && old_data != (uintptr_t)set->strs_data &&
> +	    old_s >= old_data && old_s < old_data + set->strs_data_len)
> +		s = set->strs_data + (old_s - old_data);
> +
> +	memcpy(p, s, len);
> +
> +	return len;
> +}

Does a similar use-after-free still exist in the btf__add_* APIs (e.g.
btf__add_str) when called with strings from btf__name_by_offset() on an
unmodifiable BTF?

If a user passes a string retrieved via btf__name_by_offset(), the
string pointer points directly into btf->raw_data. Inside the btf__add_*
APIs, btf_ensure_modifiable(btf) is called first, which calls
btf_invalidate_raw_data() and frees btf->raw_data.

Could this leave the s pointer dangling before it even reaches this
check? The check added here uses old_data which corresponds to
btf->strs_set, not the freed btf->raw_data, so it wouldn't rescue the
pointer.

This concern was raised by sashiko-bot@xxxxxxxxxx on lore but was not
addressed in subsequent patch versions.

Reference: https://lore.kernel.org/all/20260518053124.2EF28C2BCB7@xxxxxxxxxxxxxxx/

> +
>  /* Find string offset that corresponds to a given string *s*.
>   * Returns:
>   *   - >0 offset into string data, if string is found;

[ ... ]


---
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/26338132147