[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<AM6PR03MB5848BEDEF27DB0203C094AF299952@AM6PR03MB5848.eurprd03.prod.outlook.com>
Date: Wed, 28 Aug 2024 21:12:34 +0100
From: Juntong Deng <juntong.deng@...look.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
John Fastabend <john.fastabend@...il.com>,
Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
<martin.lau@...ux.dev>, Eddy Z <eddyz87@...il.com>,
Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH bpf-next 1/2] bpf: Relax KF_ACQUIRE kfuncs strict type
matching constraint on non-zero offset pointers
On 8/21/24 19:02, Alexei Starovoitov wrote:
> On Fri, Aug 16, 2024 at 6:24 AM Juntong Deng <juntong.deng@...look.com> wrote:
>>
>> Currently the non-zero offset pointer constraint for KF_TRUSTED_ARGS
>> kfuncs has been relaxed in commit 605c96997d89 ("bpf: relax zero fixed
>> offset constraint on KF_TRUSTED_ARGS/KF_RCU"), which means that non-zero
>> offset does not affect whether a pointer is valid.
>>
>> But currently we still cannot pass non-zero offset pointers to
>> KF_ACQUIRE kfuncs. This is because KF_ACQUIRE kfuncs requires strict
>> type matching, but non-zero offset does not change the type of pointer,
>> which causes the ebpf program to be rejected by the verifier.
>>
>> This can cause some problems, one example is that bpf_skb_peek_tail
>> kfunc [0] cannot be implemented by just passing in non-zero offset
>> pointers.
>>
>> This patch makes KF_ACQUIRE kfuncs not require strict type matching
>> on non-zero offset pointers.
>>
>> [0]: https://lore.kernel.org/bpf/AM6PR03MB5848CA39CB4B7A4397D380B099B12@AM6PR03MB5848.eurprd03.prod.outlook.com/
>>
>> Signed-off-by: Juntong Deng <juntong.deng@...look.com>
>> ---
>> kernel/bpf/verifier.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index ebec74c28ae3..3a14002d24a0 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -11484,7 +11484,7 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env,
>> * btf_struct_ids_match() to walk the struct at the 0th offset, and
>> * resolve types.
>> */
>> - if (is_kfunc_acquire(meta) ||
>> + if ((is_kfunc_acquire(meta) && !reg->off) ||
>
> Agree that relaxing is fine and calling acquire kfunc like:
> bpf_kfunc_nested_acquire_test(&sk->sk_write_queue);
>
> should be allowed,
> but above check is strange, since
> if offsetof(&sk_write_queue) == 0
> it will disallow calling a kfunc.
> I mean if the field is the first in the outer struct this
> condition will force strict type match which will fail, right?
>
> So should we remove the above is_kfunc_acquire() check instead?
>
> pw-bot: cr
I agree that if strict type matching is not required for both zero
offset and non-zero pointers, then we can remove the strict type
matching for KF_ACQUIRE kfuncs.
I sent the version 2 patch set:
https://lore.kernel.org/bpf/AM6PR03MB5848FD2BD89BF0B6B5AA3B4C99952@AM6PR03MB5848.eurprd03.prod.outlook.com/T/#t
Powered by blists - more mailing lists