Re: [PATCH v1 bpf-next 3/4] bpf: Support bpf_kptr_xchg into local kptr

[Martin KaFai Lau <martin.lau@linux.dev>]

On 7/27/24 8:01 PM, Amery Hung wrote:
> @@ -8399,7 +8408,12 @@ static const struct bpf_reg_types func_ptr_types = { .types = { PTR_TO_FUNC } };
>   static const struct bpf_reg_types stack_ptr_types = { .types = { PTR_TO_STACK } };
>   static const struct bpf_reg_types const_str_ptr_types = { .types = { PTR_TO_MAP_VALUE } };
>   static const struct bpf_reg_types timer_types = { .types = { PTR_TO_MAP_VALUE } };
> -static const struct bpf_reg_types kptr_xchg_dest_types = { .types = { PTR_TO_MAP_VALUE } };
> +static const struct bpf_reg_types kptr_xchg_dest_types = {
> +	.types = {
> +		PTR_TO_MAP_VALUE,
> +		PTR_TO_BTF_ID | MEM_ALLOC
> +	}
> +};
>   static const struct bpf_reg_types dynptr_types = {
>   	.types = {
>   		PTR_TO_STACK,
> @@ -8470,7 +8484,8 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
>   	if (base_type(arg_type) == ARG_PTR_TO_MEM)
>   		type &= ~DYNPTR_TYPE_FLAG_MASK;
>   
> -	if (meta->func_id == BPF_FUNC_kptr_xchg && type_is_alloc(type)) {
> +	/* local kptr types are allowed as the source argument of bpf_kptr_xchg */
> +	if (meta->func_id == BPF_FUNC_kptr_xchg && type_is_alloc(type) && regno == BPF_REG_2) {
>   		type &= ~MEM_ALLOC;
>   		type &= ~MEM_PERCPU;
>   	}
> @@ -8563,7 +8578,7 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
>   			verbose(env, "verifier internal error: unimplemented handling of MEM_ALLOC\n");
>   			return -EFAULT;
>   		}
> -		if (meta->func_id == BPF_FUNC_kptr_xchg) {
> +		if (meta->func_id == BPF_FUNC_kptr_xchg && regno == BPF_REG_2) {

I think this BPF_REG_2 check is because the dst (BPF_REG_1) can be MEM_ALLOC 
now. Just want to ensure I understand it correctly.

One nit. Please also update the document for bpf_kptr_xchg in uapi/linux/bpf.h:

==== >8888 ====
  * void *bpf_kptr_xchg(void *map_value, void *ptr)
  *      Description
  *              Exchange kptr at pointer *map_value* with *ptr*, and return the
==== 8888< ====