bpf: Support aggregate return values up to 16 bytes

Documentation/bpf/kfuncs.rst

@@ -575,6 +575,68 @@ is also covered by this recovery. A kfunc handed an arena pointer may
 therefore access up to ``GUARD_SZ / 2`` past it without bounds-checking
 against the arena. Larger accesses must verify the range explicitly.
 
+2.9 kfunc Return Values
+-----------------------
+
+A kfunc may return a scalar, a pointer, or a small struct or union by
+value. A scalar or pointer of up to 8 bytes is returned in R0, as usual.
+
+A struct or union returned by value must be composed only of scalars
+(recursively), where a scalar is an integer or an enum; arrays of scalars are
+allowed as members. Its bytes are handed back to the program as the raw
+contents of R0 (and R2), so a pointer field would be laundered into a scalar
+and escape the verifier's pointer provenance and reference tracking. A struct
+or union with a pointer member is therefore rejected at load time, and so is
+one with a floating-point member, which the ABI may not return in R0:R2 at
+all.
+
+A kfunc may also return a value larger than 8 bytes and up to 16 bytes -- a
+scalar-only struct or union, or an ``__int128``. Such a value is returned
+in the register pair R0:R2, matching the convention LLVM uses for the BPF
+target: the first 8 bytes in R0 and the second 8 bytes in R2. A struct or
+union of 8 bytes or less is returned in R0 alone.
+
+::
+
+        struct bpf_pair { __u64 a, b; };   /* 16 bytes */
+
+        __bpf_kfunc struct bpf_pair bpf_kfunc_get_pair(void)
+        {
+                struct bpf_pair p = { .a = 1, .b = 2 };
+
+                return p;      /* p.a in R0, p.b in R2 */
+        }
+
+Returning a value in the R0:R2 pair requires the JIT to place the second
+half of the return value into R2, which not every architecture supports
+right now. A kfunc with a return value larger than 8 bytes is therefore
+rejected at load time on a JIT that does not advertise this capability (see
+``bpf_jit_supports_kfunc_ret_reg_pair()``), and such a program is never run
+by the interpreter. A return value larger than 16 bytes is not supported.
+
+The same R0:R2 convention applies to a BPF subprogram, global or static,
+that returns an ``__int128`` or a struct or union larger than 8 bytes. Such a
+program also requires the JIT, since the interpreter propagates only R0 out
+of a subprogram. A global subprogram is verified in isolation, so its
+by-value struct or union return is restricted to scalars just like a kfunc's;
+a static subprogram is verified inline and has no such restriction. The main
+program cannot return more than 8 bytes, as its return value is the program's
+exit code.
+
+A global subprogram must leave a scalar in *every* register of the pair, so
+both halves of the returned value have to be assigned. Leaving the upper half
+uninitialized is not merely untidy: the compiler is then free to leave R2
+holding whatever it happened to hold, which for a subprogram taking a pointer
+argument is typically that pointer. Handing the caller an unknown scalar built
+from a pointer is a leak, so the verifier rejects it with::
+
+        At subprogram exit the register R2 is not a scalar value (...)
+
+Initialize the whole return value, for example ``struct pair p = {};``, to
+avoid this. A static subprogram is exempt: it is verified inline, so an
+unassigned R2 is simply passed back to the caller as uninitialized and only a
+caller that reads it fails.
+
 .. _BPF_kfunc_lifecycle_expectations:
 
 3. kfunc lifecycle expectations

arch/arm64/net/bpf_jit_comp.c

@@ -2330,6 +2330,11 @@ bool bpf_jit_supports_kfunc_call(void)
 	return true;
 }
 
+bool bpf_jit_supports_kfunc_ret_reg_pair(void)
+{
+	return true;
+}
+
 bool bpf_jit_supports_stack_args(void)
 {
 	return true;

arch/riscv/net/bpf_jit_comp64.c

@@ -2111,6 +2111,11 @@ bool bpf_jit_supports_kfunc_call(void)
 	return true;
 }
 
+bool bpf_jit_supports_kfunc_ret_reg_pair(void)
+{
+	return true;
+}
+
 bool bpf_jit_supports_ptr_xchg(void)
 {
 	return true;

arch/x86/net/bpf_jit_comp.c

@@ -2647,6 +2647,22 @@ st:			insn_off = insn->off;
 				return -EINVAL;
 			if (priv_frame_ptr)
 				pop_r9(&prog);
+			if (src_reg == BPF_PSEUDO_KFUNC_CALL) {
+				const struct btf_func_model *fm;
+
+				/*
+				 * A kfunc returning a >8 byte aggregate hands the
+				 * second half back in RDX (the native ABI's second
+				 * return reg), but BPF expects it in R0:R2. BPF R0
+				 * is RAX (no move needed), while BPF R2 is RSI, so
+				 * copy RDX into RSI.
+				 */
+				fm = bpf_jit_find_kfunc_model(bpf_prog, insn);
+				if (!fm)
+					return -EFAULT;
+				if (fm->ret_size > 8)
+					emit_mov_reg(&prog, true, BPF_REG_2, BPF_REG_3);
+			}
 			break;
 		}
 
@@ -4137,6 +4153,11 @@ bool bpf_jit_supports_kfunc_call(void)
 	return true;
 }
 
+bool bpf_jit_supports_kfunc_ret_reg_pair(void)
+{
+	return true;
+}
+
 bool bpf_jit_supports_stack_args(void)
 {
 	return true;

include/linux/bpf_verifier.h

@@ -811,6 +811,8 @@ struct bpf_subprog_info {
 	bool is_async_cb: 1;
 	bool is_exception_cb: 1;
 	bool args_cached: 1;
+	/* true if the return value is passed in the R0:R2 register pair */
+	bool ret_reg_pair: 1;
 	/* true if bpf_fastcall stack region is used by functions that can't be inlined */
 	bool keep_fastcall_stack: 1;
 	bool changes_pkt_data: 1;
@@ -1044,6 +1046,16 @@ static inline struct bpf_subprog_info *subprog_info(struct bpf_verifier_env *env
 	return &env->subprog_info[subprog];
 }
 
+/*
+ * True if @subprog returns its value in the R0:R2 register pair. Cached by
+ * bpf_compute_subprog_ret_regs(), since this is queried on hot paths: at
+ * every subprogram call and at every subprogram exit.
+ */
+static inline bool bpf_ret_reg_pair(struct bpf_verifier_env *env, int subprog)
+{
+	return subprog_info(env, subprog)->ret_reg_pair;
+}
+
 struct bpf_call_summary {
 	u8 num_params;
 	bool is_void;
@@ -1435,6 +1447,10 @@ int bpf_jmp_offset(struct bpf_insn *insn);
 struct bpf_iarray *bpf_insn_successors(struct bpf_verifier_env *env, u32 idx);
 void bpf_fmt_stack_mask(char *buf, ssize_t buf_sz, u64 stack_mask);
 bool bpf_subprog_is_global(const struct bpf_verifier_env *env, int subprog);
+int bpf_get_kfunc_ret_size(const struct bpf_prog *prog, u32 func_id,
+			   u16 btf_fd_idx, u8 *ret_size);
+bool __btf_type_is_scalar_struct(struct bpf_verifier_env *env, const struct btf *btf,
+				 const struct btf_type *t, int rec);
 
 int bpf_find_subprog(struct bpf_verifier_env *env, int off);
 bool bpf_is_throw_kfunc(struct bpf_insn *insn);

include/linux/filter.h

@@ -1213,6 +1213,7 @@ bool bpf_jit_inlines_helper_call(s32 imm);
 bool bpf_jit_supports_subprog_tailcalls(void);
 bool bpf_jit_supports_percpu_insn(void);
 bool bpf_jit_supports_kfunc_call(void);
+bool bpf_jit_supports_kfunc_ret_reg_pair(void);
 bool bpf_jit_supports_stack_args(void);
 bool bpf_jit_supports_arena_args(void);
 bool bpf_jit_supports_far_kfunc_call(void);

kernel/bpf/backtrack.c

@@ -425,6 +425,15 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
 				 */
 				verifier_bug_if(idx + 1 != subseq_idx, env,
 						"extra insn from subprog");
+				/*
+				 * a global subprog returning more than 8 bytes
+				 * sets R2 as well. R2 is part of the args mask
+				 * checked just below, so it has to be cleared
+				 * here rather than next to R0.
+				 */
+				if (bt_is_reg_set(bt, BPF_REG_2) &&
+				    bpf_ret_reg_pair(env, subprog))
+					bt_clear_reg(bt, BPF_REG_2);
 				/* r1-r5 are invalidated after subprog call,
 				 * so for global func call it shouldn't be set
 				 * anymore
@@ -508,6 +517,19 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
 				return -ENOTSUPP;
 			/* regular helper call sets R0 */
 			bt_clear_reg(bt, BPF_REG_0);
+			/* a kfunc returning more than 8 bytes also sets R2 */
+			if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
+			    bt_is_reg_set(bt, BPF_REG_2)) {
+				u8 ret_size;
+				int err;
+
+				err = bpf_get_kfunc_ret_size(env->prog, insn->imm, insn->off,
+							     &ret_size);
+				if (verifier_bug_if(err, env, "no kfunc desc for insn %d", idx))
+					return -EFAULT;
+				if (ret_size > 8)
+					bt_clear_reg(bt, BPF_REG_2);
+			}
 			if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
 				/* if backtracking was looking for registers R1-R5
 				 * they should have been found already.
@@ -522,7 +544,30 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
 					return -EFAULT;
 			}
 		} else if (opcode == BPF_EXIT) {
-			bool r0_precise;
+			bool from_subprog_call, r0_precise, r2_precise = false;
+
+			/*
+			 * BPF_EXIT in subprog or callback always returns
+			 * right after the call instruction, so by checking
+			 * whether the instruction at subseq_idx-1 is subprog
+			 * call or not we can distinguish actual exit from
+			 * *subprog* from exit from *callback*. In the former
+			 * case, we need to propagate the precision of the
+			 * return registers, if necessary. In the latter we
+			 * never do that.
+			 */
+			from_subprog_call = subseq_idx - 1 >= 0 &&
+					    bpf_pseudo_call(&env->prog->insnsi[subseq_idx - 1]);
+			if (from_subprog_call && bt_is_reg_set(bt, BPF_REG_2)) {
+				struct bpf_subprog_info *callee;
+
+				/* 'idx' is the exit insn, so it is in the callee */
+				callee = bpf_find_containing_subprog(env, idx);
+				if (verifier_bug_if(!callee, env,
+						    "no subprog contains exit insn %d", idx))
+					return -EFAULT;
+				r2_precise = bpf_ret_reg_pair(env, callee - env->subprog_info);
+			}
 
 			/* Backtracking to a nested function call, 'idx' is a part of
 			 * the inner frame 'subseq_idx' is a part of the outer frame.
@@ -535,30 +580,24 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
 			if (subseq_idx >= 0 && bpf_calls_callback(env, subseq_idx))
 				for (i = BPF_REG_1; i <= BPF_REG_5; i++)
 					bt_clear_reg(bt, i);
+			if (r2_precise)
+				bt_clear_reg(bt, BPF_REG_2);
 			if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
 				verifier_bug(env, "backtracking exit unexpected regs %x",
 					     bt_reg_mask(bt));
 				return -EFAULT;
 			}
 
-			/* BPF_EXIT in subprog or callback always returns
-			 * right after the call instruction, so by checking
-			 * whether the instruction at subseq_idx-1 is subprog
-			 * call or not we can distinguish actual exit from
-			 * *subprog* from exit from *callback*. In the former
-			 * case, we need to propagate r0 precision, if
-			 * necessary. In the former we never do that.
-			 */
-			r0_precise = subseq_idx - 1 >= 0 &&
-				     bpf_pseudo_call(&env->prog->insnsi[subseq_idx - 1]) &&
-				     bt_is_reg_set(bt, BPF_REG_0);
+			r0_precise = from_subprog_call && bt_is_reg_set(bt, BPF_REG_0);
 
 			bt_clear_reg(bt, BPF_REG_0);
 			if (bt_subprog_enter(bt))
 				return -EFAULT;
 
 			if (r0_precise)
 				bt_set_reg(bt, BPF_REG_0);
+			if (r2_precise)
+				bt_set_reg(bt, BPF_REG_2);
 			/* r6-r9 and stack slots will stay set in caller frame
 			 * bitmasks until we return back from callee(s)
 			 */

kernel/bpf/btf.c

@@ -7592,7 +7592,12 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
 		return -EINVAL;
 	}
 	ret = __get_type_size(btf, func->type, &t);
-	if (ret < 0 || btf_type_is_struct(t)) {
+	/*
+	 * __get_type_size() already restricts a non-negative ret to void, a
+	 * pointer, an int, an enum or a struct/union, so only the size is checked
+	 * here.
+	 */
+	if (ret < 0 || ret > 16) {
 		bpf_log(log,
 			"The function %s return type %s is unsupported.\n",
 			tname, btf_type_str(t));
@@ -7965,7 +7970,7 @@ static int btf_scan_type_tags(struct bpf_verifier_env *env,
 
 /* Check whether the type is a valid return type. */
 static int btf_validate_return_type(struct bpf_verifier_env *env, struct btf *btf,
-		const struct btf_type *t, int subprog)
+		const struct btf_type *t, int subprog, bool is_global)
 {
 	u32 tags = 0;
 	int err;
@@ -7988,6 +7993,35 @@ static int btf_validate_return_type(struct bpf_verifier_env *env, struct btf *bt
 	if (btf_type_is_void(t) || btf_type_is_int(t) || btf_is_any_enum(t))
 		return 0;
 
+	if (btf_type_is_struct(t) && t->size <= 16) {
+		/*
+		 * A >8 byte struct/union is returned in the R0:R2 register pair.
+		 * A global function is verified in isolation, so its caller models
+		 * the return as an opaque R0:R2 scalar pair; it must therefore
+		 * contain only scalars, otherwise a pointer field would be
+		 * laundered into a scalar and escape provenance and reference
+		 * tracking. That requirement is enforced here: do_check_common()
+		 * propagates the error for global functions and for the main
+		 * program.
+		 *
+		 * A local (static) function is verified inline and its R0:R2 are
+		 * copied as precise register state (with the JIT forced on when
+		 * the pair is consumed), so a pointer field stays tracked and needs
+		 * no such restriction. Accepting it here is not by itself what
+		 * makes it legal: btf_check_subprog_call() drops any error other
+		 * than -EFAULT. What it avoids is needlessly marking the
+		 * subprogram's BTF unreliable.
+		 *
+		 * The main program (subprog 0) takes the scalar-only path as well,
+		 * but its return value is the program's exit code, so a >8 byte
+		 * return is rejected separately at BPF_EXIT.
+		 */
+		bool local_func = subprog && !is_global;
+
+		if (local_func || __btf_type_is_scalar_struct(env, btf, t, 0))
+			return 0;
+	}
+
 	return -EOPNOTSUPP;
 }
 
@@ -8075,12 +8109,12 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 		return -EINVAL;
 	}
 
-	err = btf_validate_return_type(env, btf, t, subprog);
+	err = btf_validate_return_type(env, btf, t, subprog, is_global);
 	if (err) {
 		if (is_global) {
 			bpf_log(log,
-				"Global function %s() return value not void or scalar. "
-				"Only those are supported.\n",
+				"Global function %s() has unsupported return type. "
+				"Only void, scalar, or a scalar-only struct/union up to 16 bytes is supported.\n",
 				tname);
 		}
 		return err;

kernel/bpf/core.c

@@ -3303,6 +3303,11 @@ bool __weak bpf_jit_supports_kfunc_call(void)
 	return false;
 }
 
+bool __weak bpf_jit_supports_kfunc_ret_reg_pair(void)
+{
+	return false;
+}
+
 bool __weak bpf_jit_supports_stack_args(void)
 {
 	return false;