Re: [PATCH bpf-next v4 2/2] selftests/bpf: add memcg async reclaim test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Insecure temporary file creation pattern leading to a TOCTOU race condition.
--

--- Patch [2]: [PATCH bpf-next v4 2/2] selftests/bpf: add memcg async reclaim test ---
commit 690f8d9075834023b364e130547a2026753c92e6
Author: Hui Zhu <zhuhui@kylinos.cn>

selftests/bpf: add memcg async reclaim test

Add memcg_async_reclaim selftest that verifies BPF-driven async
proactive reclaim can mitigate refault-induced slowdown under memory
pressure.

> diff --git a/tools/testing/selftests/bpf/prog_tests/memcg_async_reclaim.c b/tools/testing/selftests/bpf/prog_tests/memcg_async_reclaim.c
> new file mode 100644
> index 0000000000000..ece7bceaab3c4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/memcg_async_reclaim.c

[ ... ]

> +static int
> +run_high_low_workload(double *high_elapsed, double *low_elapsed, int read_times)
> +{
> +	char high_data_file[PATH_MAX];
> +	char low_data_file[PATH_MAX];
> +	char high_time_file[PATH_MAX];
> +	char low_time_file[PATH_MAX];
> +	const char *dir = workload_files_dir();
> +	pid_t high_pid = -1, low_pid = -1;
> +	pid_t wait_ret;
> +	int fd, status;
> +	int ret = -1;
> +
> +	snprintf(high_data_file, sizeof(high_data_file),
> +		 "%s/memcg_async_high_data_XXXXXX", dir);

[ ... ]

> +	fd = mkstemp(high_data_file);
> +	if (!ASSERT_GE(fd, 0, "mkstemp"))
> +		goto cleanup;
> +	close(fd);

[Severity: Medium]
Is this temporary file creation vulnerable to a time-of-check to time-of-use
race condition?

By immediately closing the file descriptor securely created by mkstemp()
and reopening it by name later in write_file():

write_file() {
	fp = fopen(filename, "wb");
}

Could this allow a symlink attack in the window between the close and the
open if the file is replaced?

While selftests run in a controlled environment, this pattern is frequently
flagged as a security anti-pattern by static analysis tools.

Would it be possible to pass the file descriptor directly to the child
process instead of closing it, or use exclusive creation flags when
reopening?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787205002.git.zhuhui@kylinos.cn?part=2