On Thu, 11 Jul 2024 at 13:26, Juntong Deng <juntong.deng@xxxxxxxxxxx> wrote: > > Currently we have only three ways to get valid pointers: > > 1. Pointers which are passed as tracepoint or struct_ops > callback arguments. > > 2. Pointers which were returned from a KF_ACQUIRE kfunc. > > 3. Guaranteed valid nested pointers (e.g. using the > BTF_TYPE_SAFE_TRUSTED macro) > > But this does not cover all cases and we cannot get valid > pointers to some objects, causing the chain of trust to be > broken (we cannot get a valid object pointer from another > valid object pointer). > > The following are some examples of cases that are not covered: > > 1. struct socket > There is no reference counting in a struct socket, the reference > counting is actually in the struct file, so it does not make sense > to use a combination of KF_ACQUIRE and KF_RELEASE to trick the > verifier to make the pointer to struct socket valid. Yes, but the KF_OBTAIN like flag also needs to ensure that lifetime relationships are reflected in the verifier state. If we return a trusted pointer A using bpf_sock_from_file, but argument B it takes is later released, the verifier needs to ensure that the pointer A whose lifetime belongs to that pointer B also gets scrubbed. > > 2. sk_write_queue in struct sock > sk_write_queue is a struct member in struct sock, not a pointer > member, so we cannot use the guaranteed valid nested pointer method > to get a valid pointer to sk_write_queue. I think Matt recently had a patch addressing this issue: https://lore.kernel.org/bpf/20240709210939.1544011-1-mattbobrowski@xxxxxxxxxx/ I believe that should resolve this one (as far as passing them into KF_TRUSTED_ARGS kfuncs is concerned atleast). > > 3. The pointer returned by iterator next method > Currently we cannot pass the pointer returned by the iterator next > method as argument to the KF_TRUSTED_ARGS kfuncs, because the pointer > returned by the iterator next method is not "valid". This does sound ok though. > > This patch adds the KF_OBTAIN flag to solve examples 1 and 2, for cases > where a valid pointer can be obtained without manipulating the reference > count. For KF_OBTAIN kfuncs, the arguments must be valid pointers. > KF_OBTAIN kfuncs guarantees that if the passed pointer argument is valid, > then the pointer returned by KF_OBTAIN kfuncs is also valid. > > For example, bpf_socket_from_file() is KF_OBTAIN, and if the struct file > pointer passed in is valid (KF_ACQUIRE), then the struct socket pointer > returned is also valid. Another example, bpf_receive_queue_from_sock() is > KF_OBTAIN, and if the struct sock pointer passed in is valid, then the > sk_receive_queue pointer returned is also valid. > > In addition, this patch sets the pointer returned by the iterator next > method to be valid. This is based on the fact that if the iterator is > implemented correctly, then the pointer returned from the iterator next > method should be valid. This does not make the NULL pointer valid. > If the iterator next method has the KF_RET_NULL flag, then the verifier > will ask the ebpf program to check the NULL pointer. > > Signed-off-by: Juntong Deng <juntong.deng@xxxxxxxxxxx> > --- I think you should look at bpf_tcp_sock helper (and others), which converts struct bpf_sock to bpf_tcp_sock. It also transfers the ref_obj_id into the return value to ensure ownership is reflected correctly regardless of the type. That pattern has a specific name (is_ptr_cast_function), but idk what to call this.
