From: <gregkh@linuxfoundation.org>
To: daniel@iogearbox.net, ast@fb.com, ast@kernel.org,
gregkh@linuxfoundation.org
Cc: <stable@vger.kernel.org>, <stable-commits@vger.kernel.org>
Subject: Patch "bpf: fix branch pruning logic" has been added to the 4.9-stable tree
Date: Fri, 22 Dec 2017 16:58:02 +0100 [thread overview]
Message-ID: <1513958282183103@kroah.com> (raw)
In-Reply-To: <20171222152905.3455-3-daniel@iogearbox.net>
This is a note to let you know that I've just added the patch titled
bpf: fix branch pruning logic
to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
bpf-fix-branch-pruning-logic.patch
and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
>From foo@baz Fri Dec 22 16:57:35 CET 2017
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Fri, 22 Dec 2017 16:29:03 +0100
Subject: bpf: fix branch pruning logic
To: gregkh@linuxfoundation.org
Cc: ast@kernel.org, daniel@iogearbox.net, jannh@google.com, stable@vger.kernel.org, Alexei Starovoitov <ast@fb.com>
Message-ID: <20171222152905.3455-3-daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
From: Alexei Starovoitov <ast@fb.com>
[ Upstream commit c131187db2d3fa2f8bf32fdf4e9a4ef805168467 ]
when the verifier detects that register contains a runtime constant
and it's compared with another constant it will prune exploration
of the branch that is guaranteed not to be taken at runtime.
This is all correct, but malicious program may be constructed
in such a way that it always has a constant comparison and
the other branch is never taken under any conditions.
In this case such path through the program will not be explored
by the verifier. It won't be taken at run-time either, but since
all instructions are JITed the malicious program may cause JITs
to complain about using reserved fields, etc.
To fix the issue we have to track the instructions explored by
the verifier and sanitize instructions that are dead at run time
with NOPs. We cannot reject such dead code, since llvm generates
it for valid C code, since it doesn't do as much data flow
analysis as the verifier does.
Fixes: 17a5267067f3 ("bpf: verifier (add verifier core)")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
include/linux/bpf_verifier.h | 1 +
kernel/bpf/verifier.c | 27 +++++++++++++++++++++++++++
2 files changed, 28 insertions(+)
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -68,6 +68,7 @@ struct bpf_verifier_state_list {
struct bpf_insn_aux_data {
enum bpf_reg_type ptr_type; /* pointer type for load/store insns */
+ bool seen; /* this insn was processed by the verifier */
};
#define MAX_USED_MAPS 64 /* max number of maps accessed by one eBPF program */
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2862,6 +2862,7 @@ static int do_check(struct bpf_verifier_
if (err)
return err;
+ env->insn_aux_data[insn_idx].seen = true;
if (class == BPF_ALU || class == BPF_ALU64) {
err = check_alu_op(env, insn);
if (err)
@@ -3059,6 +3060,7 @@ process_bpf_exit:
return err;
insn_idx++;
+ env->insn_aux_data[insn_idx].seen = true;
} else {
verbose("invalid BPF_LD mode\n");
return -EINVAL;
@@ -3218,6 +3220,7 @@ static int adjust_insn_aux_data(struct b
u32 off, u32 cnt)
{
struct bpf_insn_aux_data *new_data, *old_data = env->insn_aux_data;
+ int i;
if (cnt == 1)
return 0;
@@ -3227,6 +3230,8 @@ static int adjust_insn_aux_data(struct b
memcpy(new_data, old_data, sizeof(struct bpf_insn_aux_data) * off);
memcpy(new_data + off + cnt - 1, old_data + off,
sizeof(struct bpf_insn_aux_data) * (prog_len - off - cnt + 1));
+ for (i = off; i < off + cnt - 1; i++)
+ new_data[i].seen = true;
env->insn_aux_data = new_data;
vfree(old_data);
return 0;
@@ -3245,6 +3250,25 @@ static struct bpf_prog *bpf_patch_insn_d
return new_prog;
}
+/* The verifier does more data flow analysis than llvm and will not explore
+ * branches that are dead at run time. Malicious programs can have dead code
+ * too. Therefore replace all dead at-run-time code with nops.
+ */
+static void sanitize_dead_code(struct bpf_verifier_env *env)
+{
+ struct bpf_insn_aux_data *aux_data = env->insn_aux_data;
+ struct bpf_insn nop = BPF_MOV64_REG(BPF_REG_0, BPF_REG_0);
+ struct bpf_insn *insn = env->prog->insnsi;
+ const int insn_cnt = env->prog->len;
+ int i;
+
+ for (i = 0; i < insn_cnt; i++) {
+ if (aux_data[i].seen)
+ continue;
+ memcpy(insn + i, &nop, sizeof(nop));
+ }
+}
+
/* convert load instructions that access fields of 'struct __sk_buff'
* into sequence of instructions that access fields of 'struct sk_buff'
*/
@@ -3407,6 +3431,9 @@ skip_full_check:
free_states(env);
if (ret == 0)
+ sanitize_dead_code(env);
+
+ if (ret == 0)
/* program is valid, convert *(u32*)(ctx + off) accesses */
ret = convert_ctx_accesses(env);
Patches currently in stable-queue which might be from daniel@iogearbox.net are
queue-4.9/bpf-fix-branch-pruning-logic.patch
queue-4.9/bpf-adjust-insn_aux_data-when-patching-insns.patch
queue-4.9/bpf-fix-incorrect-sign-extension-in-check_alu_op.patch
queue-4.9/bpf-reject-out-of-bounds-stack-pointer-calculation.patch
next prev parent reply other threads:[~2017-12-22 15:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-22 15:29 [PATCH stable/4.9 0/4] BPF stable patches for 4.9 Daniel Borkmann
2017-12-22 15:29 ` [PATCH stable/4.9 1/4] bpf: adjust insn_aux_data when patching insns Daniel Borkmann
2017-12-22 15:58 ` Patch "bpf: adjust insn_aux_data when patching insns" has been added to the 4.9-stable tree gregkh
2017-12-22 15:29 ` [PATCH stable/4.9 2/4] bpf: fix branch pruning logic Daniel Borkmann
2017-12-22 15:58 ` gregkh [this message]
2017-12-22 15:29 ` [PATCH stable/4.9 3/4] bpf: reject out-of-bounds stack pointer calculation Daniel Borkmann
2017-12-22 15:58 ` Patch "bpf: reject out-of-bounds stack pointer calculation" has been added to the 4.9-stable tree gregkh
2017-12-22 15:29 ` [PATCH stable/4.9 4/4] bpf: fix incorrect sign extension in check_alu_op() Daniel Borkmann
2017-12-22 15:58 ` Patch "bpf: fix incorrect sign extension in check_alu_op()" has been added to the 4.9-stable tree gregkh
2017-12-22 16:04 ` [PATCH stable/4.9 0/4] BPF stable patches for 4.9 Greg KH
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=1513958282183103@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=ast@fb.com \
--cc=ast@kernel.org \
--cc=daniel@iogearbox.net \
--cc=stable-commits@vger.kernel.org \
--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 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).