linux-kernel - Re: [RFC PATCH bpf-next RESEND 03/16] bpf: Improve bpf kfuncs pointer arguments chain of trust

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP01T74pq7pozpMi_LJUA8wehjpATMR3oM4vj7HHxohBPb0LbA@mail.gmail.com>
Date: Tue, 23 Jul 2024 02:20:56 +0200
From: Kumar Kartikeya Dwivedi <memxor@...il.com>
To: Juntong Deng <juntong.deng@...look.com>
Cc: ast@...nel.org, daniel@...earbox.net, john.fastabend@...il.com, 
	martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org, 
	yonghong.song@...ux.dev, kpsingh@...nel.org, sdf@...ichev.me, 
	haoluo@...gle.com, jolsa@...nel.org, andrii@...nel.org, avagin@...il.com, 
	snorcht@...il.com, bpf@...r.kernel.org, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH bpf-next RESEND 03/16] bpf: Improve bpf kfuncs pointer
 arguments chain of trust

On Thu, 11 Jul 2024 at 13:26, Juntong Deng <juntong.deng@...look.com> 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@google.com/
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@...look.com>
> ---

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.