linux-kernel - Re: [PATCH bpf-next v2] selftests/bpf: Move test_dev_cgroup to prog

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]
Date: Wed, 28 Feb 2024 18:10:32 +0500
From: Muhammad Usama Anjum <usama.anjum@...labora.com>
To: Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>,
 Martin KaFai Lau <martin.lau@...ux.dev>, 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@...gle.com>, Hao Luo <haoluo@...gle.com>,
 Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com>,
 Shuah Khan <shuah@...nel.org>
Cc: Muhammad Usama Anjum <usama.anjum@...labora.com>, kernel@...labora.com,
 linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
 linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf-next v2] selftests/bpf: Move test_dev_cgroup to
 prog_tests

On 2/21/24 7:06 PM, Muhammad Usama Anjum wrote:
> On 2/21/24 2:22 PM, Muhammad Usama Anjum wrote:
>> Move test_dev_cgroup.c to prog_tests/dev_cgroup.c to be able to run it
>> with test_progs. Replace dev_cgroup.bpf.o with skel header file,
>> dev_cgroup.skel.h and load program from it accourdingly.
>>
>>   ./test_progs -t test_dev_cgroup
>>   mknod: /tmp/test_dev_cgroup_null: Operation not permitted
>>   64+0 records in
>>   64+0 records out
>>   32768 bytes (33 kB, 32 KiB) copied, 0.000856684 s, 38.2 MB/s
>>   dd: failed to open '/dev/full': Operation not permitted
>>   dd: failed to open '/dev/random': Operation not permitted
>>   #365     test_dev_cgroup:OK
>>   Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
>> ---
>> I've tested the patch with vmtest.sh on bpf-next/for-next and linux
>> next. It is passing on both. Not sure why it was failed on BPFCI.
>> Test run with vmtest.h:
>> sudo LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh ./test_progs -t dev_cgroup
>> ./test_progs -t dev_cgroup
>> mknod: /tmp/test_dev_cgroup_null: Operation not permitted
>> 64+0 records in
>> 64+0 records out
>> 32768 bytes (33 kB, 32 KiB) copied, 0.000403432 s, 81.2 MB/s
>> dd: failed to open '/dev/full': Operation not permitted
>> dd: failed to open '/dev/random': Operation not permitted
>>  #69      dev_cgroup:OK
>> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
> Locally this test passes, but fails on BPFCI:
> https://github.com/kernel-patches/bpf/actions/runs/7986809998/job/21808178301#step:5:9744
The test run results with vmtest.sh and BPFCI are conflicting. What should
we do to debug the problem now? Any ideas are welcome.

I've tried to debug on my end. Not sure why it fails on the BPF CI.

> 
>>
>> Changes since v1:
>> - Rename file from test_dev_cgroup.c to dev_cgroup.c
>> - Use ASSERT_* in-place of CHECK
>> ---
>>  .../selftests/bpf/prog_tests/dev_cgroup.c     | 58 +++++++++++++
>>  tools/testing/selftests/bpf/test_dev_cgroup.c | 85 -------------------
>>  2 files changed, 58 insertions(+), 85 deletions(-)
>>  create mode 100644 tools/testing/selftests/bpf/prog_tests/dev_cgroup.c
>>  delete mode 100644 tools/testing/selftests/bpf/test_dev_cgroup.c
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/dev_cgroup.c b/tools/testing/selftests/bpf/prog_tests/dev_cgroup.c
>> new file mode 100644
>> index 0000000000000..980b015a116ff
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/dev_cgroup.c
>> @@ -0,0 +1,58 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/* Copyright (c) 2017 Facebook
>> + */
>> +
>> +#include <test_progs.h>
>> +#include <time.h>
>> +#include "cgroup_helpers.h"
>> +#include "dev_cgroup.skel.h"
>> +
>> +#define TEST_CGROUP "/test-bpf-based-device-cgroup/"
>> +
>> +void test_dev_cgroup(void)
>> +{
>> +	struct dev_cgroup *skel;
>> +	int cgroup_fd, err;
>> +	__u32 prog_cnt;
>> +
>> +	skel = dev_cgroup__open_and_load();
>> +	if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
>> +		goto cleanup;
>> +
>> +	cgroup_fd = cgroup_setup_and_join(TEST_CGROUP);
>> +	if (!ASSERT_GT(cgroup_fd, 0, "cgroup_setup_and_join"))
>> +		goto cleanup;
>> +
>> +	err = bpf_prog_attach(bpf_program__fd(skel->progs.bpf_prog1), cgroup_fd,
>> +			      BPF_CGROUP_DEVICE, 0);
>> +	if (!ASSERT_EQ(err, 0, "bpf_attach"))
>> +		goto cleanup;
>> +
>> +	err = bpf_prog_query(cgroup_fd, BPF_CGROUP_DEVICE, 0, NULL, NULL, &prog_cnt);
>> +	if (!ASSERT_EQ(err, 0, "bpf_query") || (!ASSERT_EQ(prog_cnt, 1, "bpf_query")))
>> +		goto cleanup;
>> +
>> +	/* All operations with /dev/zero and /dev/urandom are allowed,
>> +	 * everything else is forbidden.
>> +	 */
>> +	ASSERT_EQ(system("rm -f /tmp/test_dev_cgroup_null"), 0, "rm");
>> +	ASSERT_NEQ(system("mknod /tmp/test_dev_cgroup_null c 1 3"), 0, "mknod");
>> +	ASSERT_EQ(system("rm -f /tmp/test_dev_cgroup_null"), 0, "rm");
>> +
>> +	/* /dev/zero is whitelisted */
>> +	ASSERT_EQ(system("rm -f /tmp/test_dev_cgroup_zero"), 0, "rm");
>> +	ASSERT_EQ(system("mknod /tmp/test_dev_cgroup_zero c 1 5"), 0, "mknod");
> Access to major number 1 and minor number 5 is allowed. The return code of
> 0 is expected, but on CI we are getting 256 which indicates error. mknod
> help page mentions the same:
> 
>> An exit status of zero indicates success, and a nonzero value indicates
> failure.
> 

-- 
BR,
Muhammad Usama Anjum