linux-kernel - [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:
 <AM6PR03MB58488FA2AC1D67328C26167399A52@AM6PR03MB5848.eurprd03.prod.outlook.com>
Date: Thu, 11 Jul 2024 12:19:25 +0100
From: Juntong Deng <juntong.deng@...look.com>
To: 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
Cc: bpf@...r.kernel.org,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [RFC PATCH bpf-next RESEND 03/16] bpf: Improve bpf kfuncs pointer arguments chain of trust

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.

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.

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 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>
---
 include/linux/btf.h   |  1 +
 kernel/bpf/verifier.c | 12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/linux/btf.h b/include/linux/btf.h
index 323a74489562..624f1e3d6287 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -77,6 +77,7 @@
 #define KF_RCU_PROTECTED (1 << 11) /* kfunc should be protected by rcu cs when they are invoked */
 #define KF_ITER_GETTER   (1 << 12) /* kfunc implements BPF iter getter */
 #define KF_ITER_SETTER   (1 << 13) /* kfunc implements BPF iter setter */
+#define KF_OBTAIN        (1 << 14) /* kfunc is an obtain function */

 /*
  * Tag marking a kernel function as a kfunc. This is meant to minimize the
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 51302a256c30..177c98448b05 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10819,9 +10819,15 @@ static bool is_kfunc_release(struct bpf_kfunc_call_arg_meta *meta)
 	return meta->kfunc_flags & KF_RELEASE;
 }

+static bool is_kfunc_obtain(struct bpf_kfunc_call_arg_meta *meta)
+{
+	return meta->kfunc_flags & KF_OBTAIN;
+}
+
 static bool is_kfunc_trusted_args(struct bpf_kfunc_call_arg_meta *meta)
 {
-	return (meta->kfunc_flags & KF_TRUSTED_ARGS) || is_kfunc_release(meta);
+	return (meta->kfunc_flags & KF_TRUSTED_ARGS) || is_kfunc_release(meta) ||
+		is_kfunc_obtain(meta);
 }

 static bool is_kfunc_sleepable(struct bpf_kfunc_call_arg_meta *meta)
@@ -12682,6 +12688,10 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 			/* For mark_ptr_or_null_reg, see 93c230e3f5bd6 */
 			regs[BPF_REG_0].id = ++env->id_gen;
 		}
+
+		if (is_kfunc_obtain(&meta) || is_iter_next_kfunc(&meta))
+			regs[BPF_REG_0].type |= PTR_TRUSTED;
+
 		mark_btf_func_reg_size(env, BPF_REG_0, sizeof(void *));
 		if (is_kfunc_acquire(&meta)) {
 			int id = acquire_reference_state(env, insn_idx);
-- 
2.39.2