Re: [PATCH bpf-next v3 7/9] selftests/proc: Add KALLSYMS_QUERY_* ioctl tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] selftests/proc: main() bypasses kselftest reporting and harness
  helpers
--

commit cea94aa72f17dbd75f99a6a582f1fe997f0b838e
Author: Samuel Wu <wusamuel@google.com>

selftests/proc: Add KALLSYMS_QUERY_* ioctl tests

This commit adds a selftest for the KALLSYMS_QUERY_BY_NAME and
KALLSYMS_QUERY_BY_ADDR ioctls on /proc/kallsyms.

> diff --git a/tools/testing/selftests/proc/proc-kallsyms-query.c b/tools/testing/selftests/proc/proc-kallsyms-query.c
> new file mode 100644
> index 0000000000000..7788cb0f30f83
> --- /dev/null
> +++ b/tools/testing/selftests/proc/proc-kallsyms-query.c

[ ... ]

> +int main(void)
> +{
> +	struct {
> +		struct kallsyms_query_by_name q;
> +		__u64 extra;
> +	} qx = {};
> +	struct {
> +		struct kallsyms_query_sym s;
> +		__u64 extra;
> +	} sx = {};
> +	struct kallsyms_query_by_addr qa;
> +	struct kallsyms_query_by_name q;
> +	struct kallsyms_query_sym sym;
> +	unsigned int cnt = 0;
> +	__u64 first_pos = 0;
> +	char name[512];
> +	__u64 size;
> +	__u64 len;
> +	int err;
> +	int fd;
> +
> +	fd = open("/proc/kallsyms", O_RDONLY | O_CLOEXEC);
> +	if (fd < 0)
> +		ksft_exit_skip("open(/proc/kallsyms): %s\n", strerror(errno));
> +	scan_text();
> +	if (!ref.vfs_read_found)
> +		ksft_exit_skip("vfs_read is not in /proc/kallsyms\n");
> +
> +	/* Exact name in vmlinux, with name and module buffers */
> +	sym = bufs(sizeof(name_buf), sizeof(mod_buf));
> +	q = name_q("vfs_read", "vmlinux", &sym);
> +	err = by_name(fd, &q);
> +	if (err == -ENOTTY)
> +		ksft_exit_skip("KALLSYMS_QUERY_BY_NAME is not supported\n");
> +	if (!ref.vfs_read_addr) {
> +		qa = addr_q(0, &sym);
> +		assert(err == -EPERM && by_addr(fd, &qa) == -EPERM);
> +		ksft_exit_skip("kallsyms addresses are hidden\n");
> +	}
> +	assert(!err);
> +	assert(sym.sym_addr == ref.vfs_read_addr && sym.sym_size > 0);

[ ... ]

> +	close(fd);
> +	return 0;
> +}

[Severity: Low]
Could this test use kselftest_harness.h (FIXTURE(), TEST_F(), and the
ASSERT_*/EXPECT_* macros) or the kselftest.h result helpers
(ksft_print_header(), ksft_set_plan(), ksft_test_result_*(), and
ksft_finished()) instead of running sequential raw assert() checks in main()
and exiting with a bare return 0?  As written, a normal run emits no KTAP
plan or result lines, and any failed check aborts the process with SIGABRT
rather than reporting which subtest failed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113221248.3535369-11-wusamuel@google.com?part=7