Received-SPF: pass (google.com: domain of linux-kernel+bounces-19763-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Subject: Re: [PATCH bpf-next v2] bpf: Return -ENOTSUPP if calls are not
 allowed in non-JITed programs
To: Jiri Olsa <olsajiri@gmail.com>, Tiezhu Yang <yangtiezhu@loongson.cn>
Cc: Alexei Starovoitov <ast@kernel.org>, Andrii Nakryiko <andrii@kernel.org>,
 Eduard Zingerman <eddyz87@gmail.com>, bpf@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20240104130817.1221-1-yangtiezhu@loongson.cn>
 <ZZvI6g2gGRoebPiO@krava>
From: Daniel Borkmann <daniel@iogearbox.net>
Message-ID: <306f4ceb-0d25-d0a3-fc70-1141b6db06c8@iogearbox.net>
Date: Mon, 8 Jan 2024 16:27:20 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <ZZvI6g2gGRoebPiO@krava>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit

On 1/8/24 11:05 AM, Jiri Olsa wrote:
> On Thu, Jan 04, 2024 at 09:08:17PM +0800, Tiezhu Yang wrote:
>> If CONFIG_BPF_JIT_ALWAYS_ON is not set and bpf_jit_enable is 0, there
>> exist 6 failed tests.
>>
>>    [root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable
>>    [root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled
>>    [root@linux bpf]# ./test_verifier | grep FAIL
>>    #106/p inline simple bpf_loop call FAIL
>>    #107/p don't inline bpf_loop call, flags non-zero FAIL
>>    #108/p don't inline bpf_loop call, callback non-constant FAIL
>>    #109/p bpf_loop_inline and a dead func FAIL
>>    #110/p bpf_loop_inline stack locations for loop vars FAIL
>>    #111/p inline bpf_loop call in a big program FAIL
>>    Summary: 768 PASSED, 15 SKIPPED, 6 FAILED
>>
>> The test log shows that callbacks are not allowed in non-JITed programs,
>> interpreter doesn't support them yet, thus these tests should be skipped
>> if jit is disabled, just return -ENOTSUPP instead of -EINVAL for pseudo
>> calls in fixup_call_args().
>>
>> With this patch:
>>
>>    [root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable
>>    [root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled
>>    [root@linux bpf]# ./test_verifier | grep FAIL
>>    Summary: 768 PASSED, 21 SKIPPED, 0 FAILED
>>
>> Additionally, as Eduard suggested, return -ENOTSUPP instead of -EINVAL
>> for the other three places where "non-JITed" is used in error messages
>> to keep consistent.
>>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>>
>> v2:
>>    -- rebase on the latest bpf-next tree.
>>    -- return -ENOTSUPP instead of -EINVAL for the other three places
>>       where "non-JITed" is used in error messages to keep consistent.
>>    -- update the patch subject and commit message.
>>
>>   kernel/bpf/verifier.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index d5f4ff1eb235..99558a5186b2 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -8908,7 +8908,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
>>   			goto error;
>>   		if (env->subprog_cnt > 1 && !allow_tail_call_in_subprogs(env)) {
>>   			verbose(env, "tail_calls are not allowed in non-JITed programs with bpf-to-bpf calls\n");
>> -			return -EINVAL;
>> +			return -ENOTSUPP;
> 
> FWIW I agree with John review earlier [1], also there's chance (however small)
> we could mess up with some app already checking on that

+1, the ship on this has sailed unfortunately. Tiezhu, it would be good if you could
update the selftest handling instead.

> jirka
> 
> [1] https://lore.kernel.org/bpf/6594a4c15a677_11e86208cd@john.notmuch/