On 9/27/24 03:57, Eduard Zingerman wrote:
> On Thu, 2024-09-26 at 09:29 +0200, Viktor Malik wrote:
>> The tests attach to `raw_tp/bpf_testmod_test_write_bare` triggerred by
>> `trigger_module_test_write` which writes the string "aaa..." of the
>> given size.
>>
>> Signed-off-by: Viktor Malik <vmalik@xxxxxxxxxx>
>> ---
>
> I thought about making these tests more terse as follows:
>
> --- 8< ----------------------------------
>
> // SPDX-License-Identifier: GPL-2.0
>
> #include <linux/bpf.h>
> #include <bpf/bpf_helpers.h>
> #include "bpf_misc.h"
>
> int bpf_strcmp(const char *cs, const char *ct) __ksym;
> char *bpf_strchr(const char *s, int c) __ksym;
>
> static char *abc = "abc";
>
> #define __test(retval) SEC("raw_tp") __success __retval(retval)
>
> __test(2) int test_strcmp(void *ctx) { return bpf_strcmp(abc, "abd"); }
> __test(1) int test_strchr(void *ctx) { return bpf_strchr(abc, 'b') - abc; }
>
> char _license[] SEC("license") = "GPL";
>
> ---------------------------------- >8 ---
>
> (plus registration in tools/testing/selftests/bpf/prog_tests/verifier.c)
>
> However, this does not pass verification with the following error:
>
> VERIFIER LOG:
> =============
> arg#0 reference type('UNKNOWN ') size cannot be determined: -22
> 0: R1=ctx() R10=fp0
> ; __test(2) int test_strcmp(void *ctx) { return bpf_strcmp(abc, "abd"); } @ verifier_str.c:15
> 0: (18) r1 = 0xffff8881019533dc ; R1_w=map_value(map=.rodata.str1.1,ks=4,vs=8,off=4)
> 2: (18) r2 = 0xffff8881019533d8 ; R2_w=map_value(map=.rodata.str1.1,ks=4,vs=8)
> 4: (85) call bpf_strcmp#64714
> write into map forbidden, value_size=8 off=4 size=1
> processed 3 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
> =============
> #503/1 verifier_str/test_strcmp:FAIL
>
> Note that each string literal in the BPF program is in fact a pointer
> to a read-only map. Hence in current form these new functions are not
> very ergonomic. I think verifier should be extended to check 'const'
> qualifiers for the kfuncs and allowing access in such cases.
Yeah, I noticed the same problem when I was trying to come with shorter
tests. Teaching verifier to allow passing pointers to read-only maps to
these kfuncs is certainly a good thing, I'll have a look into it.
On the other hand, bpftrace stores string literals on stack so these
kfuncs would be useful to us straight away.
Viktor
>
> [...]
>
