libbpf: fix UAF in strset__add_str()

[kernel-patches-daemon-bpf]

Pull request for series with
subject: libbpf: fix UAF in strset__add_str()
version: 4
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1099851

[kernel-patches-daemon-bpf]

Upstream branch: b1fcdf9
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1099851
version: 4

[kernel-patches-review-bot]

commit b9305f9740897b69fc5f555e2bfe5c73064ead81
Author: Carlos Llamas <cmllamas@google.com>

libbpf: fix UAF in strset__add_str()

This commit fixes a use-after-free in strset__add_str() where
strset_add_str_mem() might reallocate the strset data buffer, causing
a dangling pointer if the input string 's' points to data already
present in the buffer.

> 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@kernel.org on lore but was not
addressed in subsequent patch versions.

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

> +
>  /* 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

In-Reply-To-Subject: libbpf: fix UAF in strset__add_str()
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26338132147

[kernel-patches-daemon-bpf]

Forwarding comment 4526008988 via email
In-Reply-To: 20260523162722.2718940-1-cmllamas@google.com
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260523162722.2718940-1-cmllamas@google.com/