Re: [RFC bpf-next 2/5] bpf: Handle BPF_KPTR_USER in verifier.

[Kui-Feng Lee <sinquersw@gmail.com>]

On 8/12/24 09:48, Alexei Starovoitov wrote:
> On Wed, Aug 7, 2024 at 4:58 PM Kui-Feng Lee <thinker.li@gmail.com> wrote:
>>
>> Give PTR_MAYBE_NULL | PTR_UNTRUSTED | MEM_ALLOC | NON_OWN_REF to kptr_user
>> to the memory pointed by it readable and writable.
>>
>> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
>> ---
>>   kernel/bpf/verifier.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index df3be12096cf..84647e599595 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -5340,6 +5340,10 @@ static int map_kptr_match_type(struct bpf_verifier_env *env,
>>          int perm_flags;
>>          const char *reg_name = "";
>>
>> +       if (kptr_field->type == BPF_KPTR_USER)
>> +               /* BPF programs should not change any user kptr */
>> +               return -EACCES;
>> +
>>          if (btf_is_kernel(reg->btf)) {
>>                  perm_flags = PTR_MAYBE_NULL | PTR_TRUSTED | MEM_RCU;
>>
>> @@ -5483,6 +5487,12 @@ static u32 btf_ld_kptr_type(struct bpf_verifier_env *env, struct btf_field *kptr
>>                          ret |= NON_OWN_REF;
>>          } else {
>>                  ret |= PTR_UNTRUSTED;
>> +               if (kptr_field->type == BPF_KPTR_USER)
>> +                       /* In oder to access directly from bpf
>> +                        * programs. NON_OWN_REF make the memory
>> +                        * writable. Check check_ptr_to_btf_access().
>> +                        */
>> +                       ret |= MEM_ALLOC | NON_OWN_REF;
> 
> UNTRUSTED | MEM_ALLOC | NON_OWN_REF ?!
> 
> That doesn't fit into any of the existing verifier schemes.
> I cannot make sense of this part.
> 
> UNTRUSTED | MEM_ALLOC is read only through exceptions logic.
> The uptr has to be read/write through normal load/store.

I will remove UNTRUSTED and leave MEM_ALLOC and NON_OWN_REF.
Does it make sense to you?