Re: [PATCH bpf-next v7 21/26] bpf: Add 'release on unlock' logic for bpf_list_push_{front,back}

[Dave Marchevsky <davemarchevsky@meta.com>]

On 11/14/22 2:15 PM, Kumar Kartikeya Dwivedi wrote:
> This commit implements the delayed release logic for bpf_list_push_front
> and bpf_list_push_back.
> 
> Once a node has been added to the list, it's pointer changes to
> PTR_UNTRUSTED. However, it is only released once the lock protecting the
> list is unlocked. For such PTR_TO_BTF_ID | MEM_ALLOC with PTR_UNTRUSTED
> set but an active ref_obj_id, it is still permitted to read them as long
> as the lock is held. Writing to them is not allowed.
> 
> This allows having read access to push items we no longer own until we
> release the lock guarding the list, allowing a little more flexibility
> when working with these APIs.
> 
> Note that enabling write support has fairly tricky interactions with
> what happens inside the critical section. Just as an example, currently,
> bpf_obj_drop is not permitted, but if it were, being able to write to
> the PTR_UNTRUSTED pointer while the object gets released back to the
> memory allocator would violate safety properties we wish to guarantee
> (i.e. not crashing the kernel). The memory could be reused for a
> different type in the BPF program or even in the kernel as it gets
> eventually kfree'd.
> 
> Not enabling bpf_obj_drop inside the critical section would appear to
> prevent all of the above, but that is more of an artifical limitation
> right now. Since the write support is tangled with how we handle
> potential aliasing of nodes inside the critical section that may or may
> not be part of the list anymore, it has been deferred to a future patch.
> 
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---

Can the two WARN_ON_ONCE in this patch be converted to
verifier-log-and-EFAULT? Looks like they're both in
functions with access to 'env' and are checking for
scenarios that should be considered bugs in the verifier.

Aside from that style nit, logic and patch summary updates
here LGTM.

Acked-by: Dave Marchevsky <davemarchevsky@fb.com>