linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Roberto Sassu <roberto.sassu@huaweicloud.com>
To: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	martin.lau@linux.dev, song@kernel.org, yhs@fb.com,
	john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
	haoluo@google.com, jolsa@kernel.org, revest@chromium.org,
	jackmanb@chromium.org, mykolal@fb.com, paul@paul-moore.com,
	jmorris@namei.org, serge@hallyn.com, shuah@kernel.org
Cc: bpf@vger.kernel.org, linux-security-module@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	Roberto Sassu <roberto.sassu@huawei.com>
Subject: [RFC][PATCH v2 2/7] bpf: Mark ALU32 operations in bpf_reg_state structure
Date: Wed,  7 Dec 2022 18:24:29 +0100	[thread overview]
Message-ID: <20221207172434.435893-3-roberto.sassu@huaweicloud.com> (raw)
In-Reply-To: <20221207172434.435893-1-roberto.sassu@huaweicloud.com>

From: Roberto Sassu <roberto.sassu@huawei.com>

BPF LSM needs a reliable source of information to determine if the return
value given by eBPF programs is acceptable or not. At the moment, choosing
either the 64 bit or the 32 bit one does not seem to be an option
(selftests fail).

If we choose the 64 bit one, the following happens.

      14:	61 10 00 00 00 00 00 00	r0 = *(u32 *)(r1 + 0)
      15:	74 00 00 00 15 00 00 00	w0 >>= 21
      16:	54 00 00 00 01 00 00 00	w0 &= 1
      17:	04 00 00 00 ff ff ff ff	w0 += -1

This is the last part of test_deny_namespace. After #16, the register
values are:

smin_value = 0x0, smax_value = 0x1,
s32_min_value = 0x0, s32_max_value = 0x1,

After #17, they become:

smin_value = 0x0, smax_value = 0xffffffff,
s32_min_value = 0xffffffff, s32_max_value = 0x0

where only the 32 bit values are correct.

If we choose the 32 bit ones, the following happens.

0000000000000000 <check_access>:
       0:	79 12 00 00 00 00 00 00	r2 = *(u64 *)(r1 + 0)
       1:	79 10 08 00 00 00 00 00	r0 = *(u64 *)(r1 + 8)
       2:	67 00 00 00 3e 00 00 00	r0 <<= 62
       3:	c7 00 00 00 3f 00 00 00	r0 s>>= 63

This is part of test_libbpf_get_fd_by_id_opts (no_alu32 version). In this
case, 64 bit register values should be used (for the 32 bit ones, there is
no precise information from the verifier).

As the examples above suggest that which register values to use depends on
the specific case, mark ALU32 operations in bpf_reg_state structure, so
that BPF LSM can choose the proper ones.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/bpf_verifier.h |  1 +
 kernel/bpf/verifier.c        | 10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 70d06a99f0b8..29c9cf6b0d01 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -181,6 +181,7 @@ struct bpf_reg_state {
 	enum bpf_reg_liveness live;
 	/* if (!precise && SCALAR_VALUE) min/max/tnum don't affect safety */
 	bool precise;
+	bool alu32;
 };
 
 enum bpf_stack_slot_type {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8c5f0adbbde3..edce85c425a2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10524,9 +10524,13 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 		break;
 	}
 
+	dst_reg->alu32 = false;
+
 	/* ALU32 ops are zero extended into 64bit register */
-	if (alu32)
+	if (alu32) {
 		zext_32_to_64(dst_reg);
+		dst_reg->alu32 = true;
+	}
 	reg_bounds_sync(dst_reg);
 	return 0;
 }
@@ -10700,6 +10704,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
 				*dst_reg = *src_reg;
 				dst_reg->live |= REG_LIVE_WRITTEN;
 				dst_reg->subreg_def = DEF_NOT_SUBREG;
+				dst_reg->alu32 = false;
 			} else {
 				/* R1 = (u32) R2 */
 				if (is_pointer_value(env, insn->src_reg)) {
@@ -10716,6 +10721,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
 					dst_reg->id = 0;
 					dst_reg->live |= REG_LIVE_WRITTEN;
 					dst_reg->subreg_def = env->insn_idx + 1;
+					dst_reg->alu32 = true;
 				} else {
 					mark_reg_unknown(env, regs,
 							 insn->dst_reg);
@@ -10733,9 +10739,11 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
 			if (BPF_CLASS(insn->code) == BPF_ALU64) {
 				__mark_reg_known(regs + insn->dst_reg,
 						 insn->imm);
+				regs[insn->dst_reg].alu32 = false;
 			} else {
 				__mark_reg_known(regs + insn->dst_reg,
 						 (u32)insn->imm);
+				regs[insn->dst_reg].alu32 = true;
 			}
 		}
 
-- 
2.25.1


  parent reply	other threads:[~2022-12-07 17:25 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-07 17:24 [RFC][PATCH v2 0/7] bpf-lsm: Check return values of security modules Roberto Sassu
2022-12-07 17:24 ` [RFC][PATCH v2 1/7] bpf: Remove superfluous btf_id_set_contains() declaration Roberto Sassu
2022-12-07 17:24 ` Roberto Sassu [this message]
2022-12-11  2:28   ` [RFC][PATCH v2 2/7] bpf: Mark ALU32 operations in bpf_reg_state structure Alexei Starovoitov
2022-12-12 12:44     ` Roberto Sassu
2022-12-12 17:04       ` Alexei Starovoitov
2022-12-12 18:10         ` Roberto Sassu
2022-12-07 17:24 ` [RFC][PATCH v2 3/7] lsm: Redefine LSM_HOOK() macro to add return value flags as argument Roberto Sassu
2022-12-07 17:24 ` [RFC][PATCH v2 4/7] bpf-lsm: Enforce return value limitations on security modules Roberto Sassu
2022-12-07 17:24 ` [RFC][PATCH v2 5/7] selftests/bpf: Check if return values of LSM programs are allowed Roberto Sassu
2022-12-07 17:24 ` [RFC][PATCH v2 6/7] selftests/bpf: Prevent positive ret values in test_lsm and verify_pkcs7_sig Roberto Sassu
2022-12-07 17:24 ` [RFC][PATCH v2 7/7] selftests/bpf: Change return value in test_libbpf_get_fd_by_id_opts.c Roberto Sassu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221207172434.435893-3-roberto.sassu@huaweicloud.com \
    --to=roberto.sassu@huaweicloud.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=jackmanb@chromium.org \
    --cc=jmorris@namei.org \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=mykolal@fb.com \
    --cc=paul@paul-moore.com \
    --cc=revest@chromium.org \
    --cc=roberto.sassu@huawei.com \
    --cc=sdf@google.com \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.org \
    --cc=song@kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).