linux-kernel - Re: [PATCH bpf-next v3 2/3] selftests/bpf: convert test_dev_cgroup to test

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <06f7a546-aec8-4804-8f80-1b7000229120@linux.dev>
Date: Tue, 30 Jul 2024 17:34:10 -0700
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Alexis Lothoré (eBPF Foundation)
 <alexis.lothore@...tlin.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>,
 Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
 Yonghong Song <yonghong.song@...ux.dev>,
 John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>,
 Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>,
 Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com>,
 Shuah Khan <shuah@...nel.org>, ebpf@...uxfoundation.org,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 Alan Maguire <alan.maguire@...cle.com>, bpf@...r.kernel.org,
 linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v3 2/3] selftests/bpf: convert test_dev_cgroup to
 test_progs

On 7/30/24 4:59 AM, Alexis Lothoré (eBPF Foundation) wrote:
> +static void test_read(const char *path, char *buf, int buf_size,
> +		      int expected_ret)
> +{
> +	int ret, fd;
> +
> +	fd = open(path, O_RDONLY);
> +
> +	/* A bare open on unauthorized device should fail */
> +	if (expected_ret < 0) {
> +		ASSERT_EQ(fd, expected_ret, "open file for read");

One nit. expected_ret is actually expected_errno. It just happens -EPERM is -1, 
so testing fd against expected_errno works here but is confusing to read. How 
about separating the fd and errno test in the access rejected case. First test 
for fd == -1 and then test for errno == expected_errno.

Please also carry Stanislav's Ack in patch 1 and 3 in the next respin.

Thanks for helping to move this test to test_progs.

pw-bot: cr

> +		if (fd >= 0)
> +			close(fd);
> +		return;
> +	}
> +
> +	if (!ASSERT_OK_FD(fd, "open file for read"))
> +		return;
> +
> +	ret = read(fd, buf, buf_size);
> +	ASSERT_EQ(ret, expected_ret, "read");
> +
> +	close(fd);
> +}
> +
> +static void test_write(const char *path, char *buf, int buf_size,
> +		       int expected_ret)
> +{
> +	int ret, fd;
> +
> +	fd = open(path, O_WRONLY);
> +
> +	/* A bare open on unauthorized device should fail */
> +	if (expected_ret < 0) {
> +		ASSERT_EQ(fd, expected_ret, "open file for write");
> +		if (fd >= 0)
> +			close(fd);
> +		return;
> +	}
> +
> +	if (!ASSERT_OK_FD(fd, "open file for write"))
> +		return;
> +
> +	ret = write(fd, buf, buf_size);
> +	ASSERT_EQ(ret, expected_ret, "write");
> +
> +	close(fd);
> +}
> +
> +void test_cgroup_dev(void)
> +{
> +	char buf[TEST_BUFFER_SIZE] = "some random test data";
> +	struct dev_cgroup *skel;
> +	int cgroup_fd;
> +
> +	cgroup_fd = cgroup_setup_and_join(TEST_CGROUP);
> +	if (!ASSERT_OK_FD(cgroup_fd, "cgroup switch"))
> +		return;
> +
> +	skel = dev_cgroup__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "load program"))
> +		goto cleanup_cgroup;
> +
> +	skel->links.bpf_prog1 =
> +		bpf_program__attach_cgroup(skel->progs.bpf_prog1, cgroup_fd);
> +	if (!ASSERT_OK_PTR(skel->links.bpf_prog1, "attach_program"))
> +		goto cleanup_progs;
> +
> +	if (test__start_subtest("allow-mknod"))
> +		test_mknod("/dev/test_dev_cgroup_null", S_IFCHR, 1, 3, 0);
> +
> +	if (test__start_subtest("allow-read"))
> +		test_read("/dev/urandom", buf, TEST_BUFFER_SIZE,
> +			  TEST_BUFFER_SIZE);
> +
> +	if (test__start_subtest("allow-write"))
> +		test_write("/dev/null", buf, TEST_BUFFER_SIZE,
> +			   TEST_BUFFER_SIZE);
> +
> +	if (test__start_subtest("deny-mknod"))
> +		test_mknod("/dev/test_dev_cgroup_zero", S_IFCHR, 1, 5, -EPERM);
> +
> +	if (test__start_subtest("deny-read"))
> +		test_read("/dev/random", buf, TEST_BUFFER_SIZE, -EPERM);
> +
> +	if (test__start_subtest("deny-write"))
> +		test_write("/dev/zero", buf, TEST_BUFFER_SIZE, -EPERM);
> +
> +cleanup_progs:
> +	dev_cgroup__destroy(skel);
> +cleanup_cgroup:
> +	cleanup_cgroup_environment();
> +}