Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add tests for string kfuncs

[Eduard Zingerman <eddyz87@gmail.com>]

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@redhat.com>
> ---

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.

[...]