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: <263cd78b-0392-45d4-a13c-b2b0463c4c8c@bootlin.com>
Date: Thu, 1 Aug 2024 12:00:34 +0200
From: Alexis Lothoré <alexis.lothore@...tlin.com>
To: Alan Maguire <alan.maguire@...cle.com>,
 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@...ichev.me>, Hao Luo <haoluo@...gle.com>,
 Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com>,
 Shuah Khan <shuah@...nel.org>
Cc: ebpf@...uxfoundation.org, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>, linux-kernel@...r.kernel.org,
 bpf@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf-next 3/4] selftests/bpf: add proper section name to
 bpf prog and rename it

On 8/1/24 10:35, Alan Maguire wrote:
> On 31/07/2024 11:38, Alexis Lothoré (eBPF Foundation) wrote:
>> test_skb_cgroup_id_kern.c is currently involved in a manual test. In its
>> current form, it can not be used with the auto-generated skeleton APIs,
>> because the section name is not valid to allow libbpf to deduce the program
>> type.
>>
>> Update section name to allow skeleton APIs usage. Also rename the program
>> name to make it shorter and more straighforward regarding the API it is
>> testing. While doing so, make sure that test_skb_cgroup_id.sh passes to get
>> a working reference before converting it to test_progs
>> - update the obj name
>> - fix loading issue (verifier rejecting the program when loaded through tc,
>>   because of map not found), by preloading the whole obj with bpftool
>>
>> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@...tlin.com>
> 
> Reviewed-by: Alan Maguire <alan.maguire@...cle.com>

[...]

>>  	tc qdisc add dev ${TEST_IF} clsact
>> -	tc filter add dev ${TEST_IF} egress bpf obj ${BPF_PROG_OBJ} \
>> -		sec ${BPF_PROG_SECTION} da
>> +	mkdir -p /sys/fs/bpf/${BPF_PROG_PIN}
>> +	bpftool prog loadall ${BPF_PROG_OBJ} /sys/fs/bpf/${BPF_PROG_PIN} type tc
>> +	tc filter add dev ${TEST_IF} egress bpf da object-pinned \
>> +		/sys/fs/bpf/${BPF_PROG_PIN}/${BPF_PROG_NAME}
>>
> 
> Just out of curiosity; did you find that the pin is needed? I would have
> thought tc attach would be enough to keep the program aroud.

That's more because that's the only way I found to make the filter addition
work. With the current command, the tc command fails because of the verifier
rejecting the program:

Verifier analysis:

func#0 @0
0: R1=ctx() R10=fp0
0: (bf) r6 = r1                       ; R1=ctx() R6_w=ctx()
1: (b4) w1 = 0                        ; R1_w=0
2: (63) *(u32 *)(r10 -4) = r1
mark_precise: frame0: last_idx 2 first_idx 0 subseq_idx -1
mark_precise: frame0: regs=r1 stack= before 1: (b4) w1 = 0
3: R1_w=0 R10=fp0 fp-8=0000????
3: (bf) r1 = r6                       ; R1_w=ctx() R6_w=ctx()
4: (b4) w2 = 0                        ; R2_w=0
5: (85) call bpf_skb_ancestor_cgroup_id#83    ; R0_w=scalar()
6: (7b) *(u64 *)(r10 -16) = r0        ; R0_w=scalar(id=1) R10=fp0
fp-16_w=scalar(id=1)
7: (bf) r2 = r10                      ; R2_w=fp0 R10=fp0
8: (07) r2 += -4                      ; R2_w=fp-4
9: (bf) r3 = r10                      ; R3_w=fp0 R10=fp0
10: (07) r3 += -16                    ; R3_w=fp-16
11: (18) r1 = 0x0                     ; R1_w=0
13: (b7) r4 = 0                       ; R4_w=0
14: (85) call bpf_map_update_elem#2
R1 type=scalar expected=map_ptr
processed 14 insns (limit 1000000) max_states_per_insn 0 total_states 0
peak_states 0 mark_read 0

(I also tried to remove the sec argument from the tc commandline, in case it
could allow tc to load everything, but the issue remains the same)

IIUC the verifier output, there's an issue with the variable representing the map.
When stracing the tc filter add command, I indeed see the BPF_PROG_LOAD syscall
but no BPF_MAP_CREATE at all, so I assumed tc did not read/create the map
correctly. That's why I used bpftool to make sure everything is loaded, but as a
consequence I need to provide a pin path when using load/loadall. But maybe I am
missing something ?

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ