diff mbox series

[RFC,v2,4/7] bpf-lsm: Enforce return value limitations on security modules

Message ID 20221207172434.435893-5-roberto.sassu@huaweicloud.com (mailing list archive)
State RFC
Delegated to: BPF
Headers show
Series bpf-lsm: Check return values of security modules | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply, async
bpf/vmtest-bpf-PR success PR summary
bpf/vmtest-bpf-VM_Test-1 success Logs for ${{ matrix.test }} on ${{ matrix.arch }} with ${{ matrix.toolchain }}
bpf/vmtest-bpf-VM_Test-2 success Logs for ShellCheck
bpf/vmtest-bpf-VM_Test-3 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-VM_Test-4 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-5 fail Logs for build for s390x with gcc
bpf/vmtest-bpf-VM_Test-6 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-7 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-8 success Logs for llvm-toolchain
bpf/vmtest-bpf-VM_Test-9 success Logs for set-matrix

Commit Message

Roberto Sassu Dec. 7, 2022, 5:24 p.m. UTC 
From: Roberto Sassu <roberto.sassu@huawei.com>

With the patch for the LSM infrastructure to redefine the LSM_HOOK() macro
and to introduce the return value flags, it becomes straightforward for
eBPF to leverage this information to enforce return values limitations on
eBPF programs implementing security hooks.

Update the bpf_lsm_hooks BTF ID set, by including the return value flags.
Then, introduce bpf_lsm_is_ret_value_allowed(), which determines in which
intervals the R0 register (which contains the return value) falls, and
checks if the corresponding return value flag is set for those intervals.

In addition, for the interval including zero, ensure that the hook is not
inode_init_security, otherwise report that zero is not allowed. By LSM
conventions, LSMs should return zero only if they set an xattr, which
currently eBPF programs cannot do.

Finally, expose the new function and add a call to it in the verifier.

Cc: stable@vger.kernel.org # 5.7.x
Fixes: 9d3fdea789c8 ("bpf: lsm: Provide attachment points for BPF LSM programs")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/bpf_lsm.h |  9 +++++
 kernel/bpf/bpf_lsm.c    | 78 ++++++++++++++++++++++++++++++++++++++---
 kernel/bpf/verifier.c   |  7 ++--
 3 files changed, 87 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
index 2f5757085dfd..0ce5948f3662 100644
--- a/include/linux/bpf_lsm.h
+++ b/include/linux/bpf_lsm.h
@@ -29,6 +29,8 @@  int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
 
 bool bpf_lsm_is_sleepable_hook(u32 btf_id);
 bool bpf_lsm_is_trusted(const struct bpf_prog *prog);
+bool bpf_lsm_is_ret_value_allowed(struct bpf_verifier_log *vlog,
+				  struct bpf_reg_state *reg, u32 btf_id);
 
 static inline struct bpf_storage_blob *bpf_inode(
 	const struct inode *inode)
@@ -57,6 +59,13 @@  static inline bool bpf_lsm_is_trusted(const struct bpf_prog *prog)
 	return false;
 }
 
+static inline bool bpf_lsm_is_ret_value_allowed(struct bpf_verifier_log *vlog,
+						struct bpf_reg_state *reg,
+						u32 btf_id)
+{
+	return false;
+}
+
 static inline int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
 				      const struct bpf_prog *prog)
 {
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index 98f810f661a6..39ddafc06021 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -31,11 +31,11 @@  noinline RET bpf_lsm_##NAME(__VA_ARGS__)	\
 #undef LSM_HOOK
 
 #define LSM_HOOK(RET, DEFAULT, RET_FLAGS, NAME, ...) \
-	BTF_ID(func, bpf_lsm_##NAME)
-BTF_SET_START(bpf_lsm_hooks)
+	BTF_ID_FLAGS(func, bpf_lsm_##NAME, RET_FLAGS)
+BTF_SET8_START(bpf_lsm_hooks)
 #include <linux/lsm_hook_defs.h>
 #undef LSM_HOOK
-BTF_SET_END(bpf_lsm_hooks)
+BTF_SET8_END(bpf_lsm_hooks)
 
 /* List of LSM hooks that should operate on 'current' cgroup regardless
  * of function signature.
@@ -105,7 +105,7 @@  int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
 		return -EINVAL;
 	}
 
-	if (!btf_id_set_contains(&bpf_lsm_hooks, prog->aux->attach_btf_id)) {
+	if (!btf_id_set8_contains(&bpf_lsm_hooks, prog->aux->attach_btf_id)) {
 		bpf_log(vlog, "attach_btf_id %u points to wrong type name %s\n",
 			prog->aux->attach_btf_id, prog->aux->attach_func_name);
 		return -EINVAL;
@@ -367,6 +367,76 @@  bool bpf_lsm_is_trusted(const struct bpf_prog *prog)
 	return !btf_id_set_contains(&untrusted_lsm_hooks, prog->aux->attach_btf_id);
 }
 
+BTF_SET_START(zero_forbidden_lsm_hooks)
+BTF_ID(func, bpf_lsm_inode_init_security)
+BTF_SET_END(zero_forbidden_lsm_hooks)
+
+bool bpf_lsm_is_ret_value_allowed(struct bpf_verifier_log *vlog,
+				  struct bpf_reg_state *reg, u32 btf_id)
+{
+	u32 *id = btf_id_set8_contains(&bpf_lsm_hooks, btf_id);
+	s64 smin_value = reg->smin_value;
+	s64 smax_value = reg->smax_value;
+	u32 *ret_flags = id + 1;
+
+	/* See no_alu32/test_bpf_cookie.bpf.o for how return -EPERM is compiled:
+	 *
+	 * 11:	18 06 00 00 ff ff ff ff 00 00 00 00 00 00 00 00	r6 = 4294967295 ll
+	 * 13:	67 00 00 00 20 00 00 00	r0 <<= 32
+	 * 14:	77 00 00 00 20 00 00 00	r0 >>= 32
+	 * 15:	5d 08 07 00 00 00 00 00	if r8 != r0 goto +7 <LBB11_3>
+	 *
+	 * This causes predicted values to be:
+	 * smin_value = 0xffffffff, smax_value = 0xffffffff,
+	 * s32_min_value = 0xffffffff, s32_max_value = 0xffffffff,
+	 *
+	 * despite it is an ALU64 operation. So, checking reg->alu32 is not
+	 * enough. Then, if after casting the 64 bit values they are equal to
+	 * the 32 bit ones, use the latter ones (the LSM infrastructure takes
+	 * an int).
+	 */
+	if ((reg->s32_min_value == (u32)smin_value &&
+	    reg->s32_max_value == (u32)smax_value) || reg->alu32) {
+		smin_value = reg->s32_min_value;
+		smax_value = reg->s32_max_value;
+	}
+
+	/* Interval includes < 0 values. */
+	if (smin_value < 0) {
+		if (!(*ret_flags & LSM_RET_NEG)) {
+			bpf_log(vlog, "Invalid R0, cannot return < 0\n");
+			return false;
+		}
+	}
+
+	/* Interval includes 0. */
+	if (smin_value <= 0 && smax_value >= 0) {
+		if (!(*ret_flags & LSM_RET_ZERO) ||
+		    btf_id_set_contains(&zero_forbidden_lsm_hooks, btf_id)) {
+			bpf_log(vlog, "Invalid R0, cannot return 0\n");
+			return false;
+		}
+	}
+
+	/* Interval includes 1. */
+	if (smin_value <= 1 && smax_value >= 1) {
+		if (!(*ret_flags & LSM_RET_ONE)) {
+			bpf_log(vlog, "Invalid R0, cannot return 1\n");
+			return false;
+		}
+	}
+
+	/* Interval includes > 1 values. */
+	if (smax_value > 1) {
+		if (!(*ret_flags & LSM_RET_GT_ONE)) {
+			bpf_log(vlog, "Invalid R0, cannot return > 1\n");
+			return false;
+		}
+	}
+
+	return true;
+}
+
 const struct bpf_prog_ops lsm_prog_ops = {
 };
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index edce85c425a2..5d13b7f42238 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12064,9 +12064,10 @@  static int check_return_code(struct bpf_verifier_env *env)
 
 	case BPF_PROG_TYPE_LSM:
 		if (env->prog->expected_attach_type != BPF_LSM_CGROUP) {
-			/* Regular BPF_PROG_TYPE_LSM programs can return
-			 * any value.
-			 */
+			if (!bpf_lsm_is_ret_value_allowed(&env->log, reg,
+						env->prog->aux->attach_btf_id))
+				return -EINVAL;
+
 			return 0;
 		}
 		if (!env->prog->aux->attach_func_proto->type) {