Re: [PATCH bpf-next 2/2] selftests/bpf: Add linked_externs test for externs in allocated sections

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bpf: TOCTOU and missing error check for mkstemp in test_relink()
- [Medium] bpf: Missing NULL check for bpf_object__btf() in test_relink()
--

commit 2e7da3dcbd552604f0f6c052dc8cda7a3628ffb8
Author: Andrii Nakryiko <andrii@kernel.org>

selftests/bpf: Add linked_externs test for externs in allocated sections

This commit adds two objects that define an arena global each and declare the
other one's as extern, linking them into a single skeleton. It also introduces
a test to drive the libbpf linker directly to verify the relinking process.

> diff --git a/tools/testing/selftests/bpf/prog_tests/linked_externs.c b/tools/testing/selftests/bpf/prog_tests/linked_externs.c
> new file mode 100644
> index 0000000000000..29358c82dc356
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/linked_externs.c
[ ... ]
> +/* link in1 on its own first, then link the result with in2 */
> +static void test_relink(const char *in1, const char *in2, const char *sec_name)
> +{
> +	char out1[] = "/tmp/linked_externs.XXXXXX", out2[] = "/tmp/linked_externs.XXXXXX";
> +	const struct btf_var_secinfo *vi;
> +	struct bpf_object *obj = NULL;
> +	const struct btf_type *t;
> +	struct btf *btf;
> +	int i, id, n;
> +
> +	close(mkstemp(out1));
> +	close(mkstemp(out2));

[Severity: Medium]
Could this result in a Time-Of-Check to Time-Of-Use (TOCTOU) race condition
and silently ignore failures?

mkstemp() can fail, returning -1 and leaving the template unchanged. Because
the return value is discarded, could concurrent test runs end up using the
identical fallback filename "/tmp/linked_externs.XXXXXX" and corrupt each
other's output?

Additionally, even on success, does immediately closing the temporary file and
reopening it by name in a world-writable directory (/tmp) introduce a classic
TOCTOU race condition?

> +
> +	if (!ASSERT_OK(link_objs(out1, in1, NULL), "link_stage1") ||
> +	    !ASSERT_OK(link_objs(out2, out1, in2), "link_stage2"))
> +		goto cleanup;
> +
> +	obj = bpf_object__open_file(out2, NULL);
> +	if (!ASSERT_OK_PTR(obj, "obj_open"))
> +		goto cleanup;
> +
> +	btf = bpf_object__btf(obj);
> +	id = btf__find_by_name_kind(btf, sec_name, BTF_KIND_DATASEC);

[Severity: Medium]
Does this code dereference a potentially NULL pointer?

bpf_object__btf(obj) can return NULL if the loaded BPF object lacks BTF
information. The returned btf pointer is then passed to
btf__find_by_name_kind(), which dereferences it internally (btf->start_id).

Could this lead to a segmentation fault on malformed objects instead of a
graceful test failure (e.g., via ASSERT_OK_PTR())?

> +	if (!ASSERT_GT(id, 0, "find_datasec"))
> +		goto cleanup;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925224631.1490488-1-andrii@kernel.org?part=2