Re: [RFC bpf-next v3] bpf, selftests/bpf: Support PTR_MAYBE_NULL for struct_ops arguments.

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

On 1/22/24 1:22 PM, thinker.li@gmail.com wrote:
> From: Kui-Feng Lee <thinker.li@gmail.com>
> 
> Allow passing a null pointer to the operators provided by a struct_ops
> object. This is an RFC to collect feedbacks/opinions.
> 
> The previous discussions against v1 came to the conclusion that the
> developer should did it in ".is_valid_access". However, recently, kCFI for
> struct_ops has been landed. We found it is possible to provide a generic
> way to annotate arguments by adding a suffix after argument names of stub
> functions. So, this RFC is resent to present the new idea.
> 
> The function pointers that are passed to struct_ops operators (the function
> pointers) are always considered reliable until now. They cannot be
> null. However, in certain scenarios, it should be possible to pass null
> pointers to these operators. For instance, sched_ext may pass a null
> pointer in the struct task type to an operator that is provided by its
> struct_ops objects.
> 
> The proposed solution here is to add PTR_MAYBE_NULL annotations to
> arguments and create instances of struct bpf_ctx_arg_aux (arg_info) for
> these arguments. These arg_infos will be installed at
> prog->aux->ctx_arg_info and will be checked by the BPF verifier when
> loading the programs. When a struct_ops program accesses arguments in the
> ctx, the verifier will call btf_ctx_access() (through
> bpf_verifier_ops->is_valid_access) to verify the access. btf_ctx_access()
> will check arg_info and use the information of the matched arg_info to
> properly set reg_type.



> 
> For nullable arguments, this patch sets an arg_info to label them with
> PTR_TO_BTF_ID | PTR_TRUSTED | PTR_MAYBE_NULL. This enforces the verifier to
> check programs and ensure that they properly check the pointer. The
> programs should check if the pointer is null before reading/writing the
> pointed memory.
> 
> The implementer of a struct_ops should annotate the arguments that can be
> null. The implementer should define a stub function (empty) as a
> placeholder for each defined operator. The name of a stub function should
> be in the pattern "<st_op_type>_stub_<operator name>". For example, for
> test_maybe_null of struct bpf_testmod_ops, it's stub function name should
> be "bpf_testmod_ops_stub_test_maybe_null". You mark an argument nullable by
> suffixing the argument name with "__nullable" at the stub function.  Here
> is the example in bpf_testmod.c.

Neat idea to reuse the cfi stub. Some high level comments.

bpf_struct_ops_desc_init is also collecting the details of each func_proto 
member. Check if this "__nullable" collection can be done in the same loop.

Simplify the implementation of the member_arg_info allocations. There is no need 
to compact everything in one continuous memory.