Re: [PATCH] selftests/kvm: check fopen() return in get_run_delay()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/kvm: infinite loop when fopen() fails in get_run_delay()
--

commit fed2657e5d999eb78e5e508f23ea617f2787e14a
Author: zhangchujun <zhangchujun@cmss.chinamobile.com>

selftests/kvm: check fopen() return in get_run_delay()

This commit adds a NULL check on the FILE pointer returned by fopen() in
get_run_delay() so that it returns MIN_RUN_DELAY_NS instead of passing NULL to
fscanf() and fclose() when /proc/<tid>/schedstat cannot be opened.

> diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> index f5f7189f96a84..2827f7d7d905e 100644
> --- a/tools/testing/selftests/kvm/lib/test_util.c
> +++ b/tools/testing/selftests/kvm/lib/test_util.c
> @@ -421,6 +421,8 @@ long get_run_delay(void)
>  	sprintf(path, "/proc/%ld/schedstat", (long)kvm_gettid());
>  	fp = fopen(path, "r");
>  	/* Return MIN_RUN_DELAY_NS upon failure just to be safe */
> +	if (!fp)
> +		return MIN_RUN_DELAY_NS;

[Severity: Medium]
If fopen() fails to open /proc/<tid>/schedstat (for instance, when
CONFIG_SCHED_INFO is disabled in the host kernel), will returning the constant
MIN_RUN_DELAY_NS here cause the callers of get_run_delay() to hang in an
infinite loop?

get_run_delay() reads the calling thread's cumulative scheduler run_delay
counter, and both callers in the tree wait for subsequent readings to advance
past an initial baseline.

In tools/testing/selftests/kvm/steal_time.c:main():

		run_delay = get_run_delay();
		kvm_pthread_create(&thread, NULL, do_steal_time, NULL);
		kvm_pthread_getaffinity(thread, &cpuset);
		TEST_ASSERT(CPU_COUNT(&cpuset) == 1 && CPU_ISSET(cpu, &cpuset),
			    "Worker failed to inherit parent's CPU affinity");

		do
			sched_yield();
		while (get_run_delay() - run_delay < MIN_RUN_DELAY_NS);

When get_run_delay() returns MIN_RUN_DELAY_NS on every call,
get_run_delay() - run_delay evaluates to 0, which is always less than
MIN_RUN_DELAY_NS, causing this loop to spin forever.

Similarly, in tools/testing/selftests/kvm/x86/xen_shinfo_test.c:main():

				rundelay = get_run_delay() + MIN_STEAL_TIME;
				do {
					sched_yield();
				} while (get_run_delay() < rundelay);

Here, rundelay becomes MIN_RUN_DELAY_NS + MIN_STEAL_TIME while get_run_delay()
continues returning MIN_RUN_DELAY_NS, which also spins forever.

Should get_run_delay() instead fail or skip via TEST_ASSERT() or
TEST_REQUIRE() when fopen() fails, similar to other file-opening helpers in
tools/testing/selftests/kvm/lib/test_util.c such as get_sysfs_val() and
get_def_hugetlb_pagesz()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104080721.2169-1-zhangchujun@cmss.chinamobile.com?part=1