All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Piotr Krysiuk <piotras@gmail.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	John Fastabend <john.fastabend@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Frank van der Linden <fllinden@amazon.com>
Subject: [PATCH 4.14 40/47] bpf: Fix leakage of uninitialized bpf stack under speculation
Date: Tue,  8 Jun 2021 20:27:23 +0200	[thread overview]
Message-ID: <20210608175931.793308942@linuxfoundation.org> (raw)
In-Reply-To: <20210608175930.477274100@linuxfoundation.org>

From: Daniel Borkmann <daniel@iogearbox.net>

commit 801c6058d14a82179a7ee17a4b532cac6fad067f upstream.

The current implemented mechanisms to mitigate data disclosure under
speculation mainly address stack and map value oob access from the
speculative domain. However, Piotr discovered that uninitialized BPF
stack is not protected yet, and thus old data from the kernel stack,
potentially including addresses of kernel structures, could still be
extracted from that 512 bytes large window. The BPF stack is special
compared to map values since it's not zero initialized for every
program invocation, whereas map values /are/ zero initialized upon
their initial allocation and thus cannot leak any prior data in either
domain. In the non-speculative domain, the verifier ensures that every
stack slot read must have a prior stack slot write by the BPF program
to avoid such data leaking issue.

However, this is not enough: for example, when the pointer arithmetic
operation moves the stack pointer from the last valid stack offset to
the first valid offset, the sanitation logic allows for any intermediate
offsets during speculative execution, which could then be used to
extract any restricted stack content via side-channel.

Given for unprivileged stack pointer arithmetic the use of unknown
but bounded scalars is generally forbidden, we can simply turn the
register-based arithmetic operation into an immediate-based arithmetic
operation without the need for masking. This also gives the benefit
of reducing the needed instructions for the operation. Given after
the work in 7fedb63a8307 ("bpf: Tighten speculative pointer arithmetic
mask"), the aux->alu_limit already holds the final immediate value for
the offset register with the known scalar. Thus, a simple mov of the
immediate to AX register with using AX as the source for the original
instruction is sufficient and possible now in this case.

Reported-by: Piotr Krysiuk <piotras@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Tested-by: Piotr Krysiuk <piotras@gmail.com>
Reviewed-by: Piotr Krysiuk <piotras@gmail.com>
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
[fllinden@amazon.com: fixed minor 4.14 conflict because of renamed function]
Signed-off-by: Frank van der Linden <fllinden@amazon.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/bpf_verifier.h |    5 +++--
 kernel/bpf/verifier.c        |   27 +++++++++++++++++----------
 2 files changed, 20 insertions(+), 12 deletions(-)

--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -114,10 +114,11 @@ struct bpf_verifier_state_list {
 };
 
 /* Possible states for alu_state member. */
-#define BPF_ALU_SANITIZE_SRC		1U
-#define BPF_ALU_SANITIZE_DST		2U
+#define BPF_ALU_SANITIZE_SRC		(1U << 0)
+#define BPF_ALU_SANITIZE_DST		(1U << 1)
 #define BPF_ALU_NEG_VALUE		(1U << 2)
 #define BPF_ALU_NON_POINTER		(1U << 3)
+#define BPF_ALU_IMMEDIATE		(1U << 4)
 #define BPF_ALU_SANITIZE		(BPF_ALU_SANITIZE_SRC | \
 					 BPF_ALU_SANITIZE_DST)
 
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2120,6 +2120,7 @@ static int sanitize_ptr_alu(struct bpf_v
 {
 	struct bpf_insn_aux_data *aux = commit_window ? cur_aux(env) : tmp_aux;
 	struct bpf_verifier_state *vstate = env->cur_state;
+	bool off_is_imm = tnum_is_const(off_reg->var_off);
 	bool off_is_neg = off_reg->smin_value < 0;
 	bool ptr_is_dst_reg = ptr_reg == dst_reg;
 	u8 opcode = BPF_OP(insn->code);
@@ -2150,6 +2151,7 @@ static int sanitize_ptr_alu(struct bpf_v
 		alu_limit = abs(tmp_aux->alu_limit - alu_limit);
 	} else {
 		alu_state  = off_is_neg ? BPF_ALU_NEG_VALUE : 0;
+		alu_state |= off_is_imm ? BPF_ALU_IMMEDIATE : 0;
 		alu_state |= ptr_is_dst_reg ?
 			     BPF_ALU_SANITIZE_SRC : BPF_ALU_SANITIZE_DST;
 	}
@@ -4850,7 +4852,7 @@ static int fixup_bpf_calls(struct bpf_ve
 			const u8 code_sub = BPF_ALU64 | BPF_SUB | BPF_X;
 			struct bpf_insn insn_buf[16];
 			struct bpf_insn *patch = &insn_buf[0];
-			bool issrc, isneg;
+			bool issrc, isneg, isimm;
 			u32 off_reg;
 
 			aux = &env->insn_aux_data[i + delta];
@@ -4861,16 +4863,21 @@ static int fixup_bpf_calls(struct bpf_ve
 			isneg = aux->alu_state & BPF_ALU_NEG_VALUE;
 			issrc = (aux->alu_state & BPF_ALU_SANITIZE) ==
 				BPF_ALU_SANITIZE_SRC;
+			isimm = aux->alu_state & BPF_ALU_IMMEDIATE;
 
 			off_reg = issrc ? insn->src_reg : insn->dst_reg;
-			if (isneg)
-				*patch++ = BPF_ALU64_IMM(BPF_MUL, off_reg, -1);
-			*patch++ = BPF_MOV32_IMM(BPF_REG_AX, aux->alu_limit);
-			*patch++ = BPF_ALU64_REG(BPF_SUB, BPF_REG_AX, off_reg);
-			*patch++ = BPF_ALU64_REG(BPF_OR, BPF_REG_AX, off_reg);
-			*patch++ = BPF_ALU64_IMM(BPF_NEG, BPF_REG_AX, 0);
-			*patch++ = BPF_ALU64_IMM(BPF_ARSH, BPF_REG_AX, 63);
-			*patch++ = BPF_ALU64_REG(BPF_AND, BPF_REG_AX, off_reg);
+			if (isimm) {
+				*patch++ = BPF_MOV32_IMM(BPF_REG_AX, aux->alu_limit);
+			} else {
+				if (isneg)
+					*patch++ = BPF_ALU64_IMM(BPF_MUL, off_reg, -1);
+				*patch++ = BPF_MOV32_IMM(BPF_REG_AX, aux->alu_limit);
+				*patch++ = BPF_ALU64_REG(BPF_SUB, BPF_REG_AX, off_reg);
+				*patch++ = BPF_ALU64_REG(BPF_OR, BPF_REG_AX, off_reg);
+				*patch++ = BPF_ALU64_IMM(BPF_NEG, BPF_REG_AX, 0);
+				*patch++ = BPF_ALU64_IMM(BPF_ARSH, BPF_REG_AX, 63);
+				*patch++ = BPF_ALU64_REG(BPF_AND, BPF_REG_AX, off_reg);
+			}
 			if (!issrc)
 				*patch++ = BPF_MOV64_REG(insn->dst_reg, insn->src_reg);
 			insn->src_reg = BPF_REG_AX;
@@ -4878,7 +4885,7 @@ static int fixup_bpf_calls(struct bpf_ve
 				insn->code = insn->code == code_add ?
 					     code_sub : code_add;
 			*patch++ = *insn;
-			if (issrc && isneg)
+			if (issrc && isneg && !isimm)
 				*patch++ = BPF_ALU64_IMM(BPF_MUL, off_reg, -1);
 			cnt = patch - insn_buf;
 



  parent reply	other threads:[~2021-06-08 18:35 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-08 18:26 [PATCH 4.14 00/47] 4.14.236-rc1 review Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 01/47] net: usb: cdc_ncm: dont spew notifications Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 02/47] efi: Allow EFI_MEMORY_XP and EFI_MEMORY_RO both to be cleared Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 03/47] efi: cper: fix snprintf() use in cper_dimm_err_location() Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 04/47] vfio/pci: Fix error return code in vfio_ecap_init() Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 05/47] vfio/pci: zap_vma_ptes() needs MMU Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 06/47] vfio/platform: fix module_put call in error flow Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 07/47] ipvs: ignore IP_VS_SVC_F_HASHED flag when adding service Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 08/47] HID: pidff: fix error return code in hid_pidff_init() Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 09/47] HID: i2c-hid: fix format string mismatch Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 10/47] netfilter: nfnetlink_cthelper: hit EBUSY on updates if size mismatches Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 11/47] ieee802154: fix error return code in ieee802154_add_iface() Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 12/47] ieee802154: fix error return code in ieee802154_llsec_getparams() Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 13/47] Bluetooth: fix the erroneous flush_work() order Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 14/47] Bluetooth: use correct lock to prevent UAF of hdev object Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 15/47] net: caif: added cfserl_release function Greg Kroah-Hartman
2021-06-08 18:26 ` [PATCH 4.14 16/47] net: caif: add proper error handling Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 17/47] net: caif: fix memory leak in caif_device_notify Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 18/47] net: caif: fix memory leak in cfusbl_device_notify Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 19/47] ALSA: timer: Fix master timer notification Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 20/47] ext4: fix bug on in ext4_es_cache_extent as ext4_split_extent_at failed Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 21/47] pid: take a reference when initializing `cad_pid` Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 22/47] ocfs2: fix data corruption by fallocate Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 23/47] nfc: fix NULL ptr dereference in llcp_sock_getname() after failed connect Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 24/47] btrfs: fix error handling in btrfs_del_csums Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 25/47] btrfs: fixup error handling in fixup_inode_link_counts Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 26/47] mm, hugetlb: fix simple resv_huge_pages underflow on UFFDIO_COPY Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 27/47] bpf, selftests: Fix up some test_verifier cases for unprivileged Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 28/47] bpf: Move off_reg into sanitize_ptr_alu Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 29/47] bpf: Ensure off_reg has no mixed signed bounds for all types Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 30/47] bpf: Rework ptr_limit into alu_limit and add common error path Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 31/47] bpf: Improve verifier error messages for users Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 32/47] bpf: Refactor and streamline bounds check into helper Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 33/47] bpf: Move sanitize_val_alu out of op switch Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 34/47] bpf: Tighten speculative pointer arithmetic mask Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 35/47] bpf: Update selftests to reflect new error states Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 36/47] bpf: do not allow root to mangle valid pointers Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 37/47] bpf/verifier: disallow pointer subtraction Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 38/47] selftests/bpf: fix test_align Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 39/47] selftests/bpf: make dubious pointer arithmetic test useful Greg Kroah-Hartman
2021-06-08 18:27 ` Greg Kroah-Hartman [this message]
2021-06-08 18:27 ` [PATCH 4.14 41/47] bpf: Wrap aux data inside bpf_sanitize_info container Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 42/47] bpf: Fix mask direction swap upon off reg sign change Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 43/47] bpf: No need to simulate speculative domain for immediates Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 44/47] bnxt_en: Remove the setting of dev_port Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 45/47] KVM: SVM: Truncate GPR value for DR and CR accesses in !64-bit mode Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 46/47] sched/fair: Optimize select_idle_cpu Greg Kroah-Hartman
2021-06-08 18:27 ` [PATCH 4.14 47/47] xen-pciback: redo VF placement in the virtual topology Greg Kroah-Hartman
2021-06-09  9:33 ` [PATCH 4.14 00/47] 4.14.236-rc1 review Jon Hunter
2021-06-09 11:25 ` Naresh Kamboju
2021-06-09 18:48 ` Guenter Roeck

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=20210608175931.793308942@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=fllinden@amazon.com \
    --cc=john.fastabend@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=piotras@gmail.com \
    --cc=stable@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.