From: <gregkh@linuxfoundation.org>
To: jannh@google.com, ast@kernel.org, daniel@iogearbox.net,
davem@davemloft.net, gregkh@linuxfoundation.org
Cc: <stable@vger.kernel.org>, <stable-commits@vger.kernel.org>
Subject: Patch "bpf: fix incorrect sign extension in check_alu_op()" has been added to the 4.4-stable tree
Date: Mon, 19 Mar 2018 18:13:02 +0100 [thread overview]
Message-ID: <15214795823916@kroah.com> (raw)
In-Reply-To: <20180319165552.146891-1-jannh@google.com>
This is a note to let you know that I've just added the patch titled
bpf: fix incorrect sign extension in check_alu_op()
to the 4.4-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-incorrect-sign-extension-in-check_alu_op.patch
and it can be found in the queue-4.4 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 jannh@google.com Mon Mar 19 18:11:54 2018
From: Jann Horn <jannh@google.com>
Date: Mon, 19 Mar 2018 17:55:52 +0100
Subject: bpf: fix incorrect sign extension in check_alu_op()
To: stable@vger.kernel.org
Cc: security@kernel.org, "David S. Miller" <davem@davemloft.net>, Daniel Borkmann <daniel@iogearbox.net>, Alexei Starovoitov <ast@kernel.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Message-ID: <20180319165552.146891-1-jannh@google.com>
From: Jann Horn <jannh@google.com>
commit 95a762e2c8c942780948091f8f2a4f32fce1ac6f upstream.
Distinguish between
BPF_ALU64|BPF_MOV|BPF_K (load 32-bit immediate, sign-extended to 64-bit)
and BPF_ALU|BPF_MOV|BPF_K (load 32-bit immediate, zero-padded to 64-bit);
only perform sign extension in the first case.
This patch differs from the mainline one because the verifier's internals
have changed in the meantime. Mainline tracks register values as 64-bit
values; however, 4.4 still stores tracked register values as 32-bit
values with sign extension. Therefore, in the case of a 32-bit op with
negative immediate, the value can't be tracked; leave the register as
UNKNOWN_VALUE (set by the preceding check_reg_arg() call).
I have manually tested this patch on top of 4.4.122. For the following BPF
bytecode:
BPF_MOV64_IMM(BPF_REG_1, 1),
BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 1),
BPF_EXIT_INSN(),
BPF_MOV32_IMM(BPF_REG_1, 1),
BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 1),
BPF_EXIT_INSN(),
BPF_MOV64_IMM(BPF_REG_1, -1),
BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, -1, 1),
BPF_EXIT_INSN(),
BPF_MOV32_IMM(BPF_REG_1, -1),
BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, -1, 2),
BPF_MOV32_IMM(BPF_REG_0, 42),
BPF_EXIT_INSN(),
BPF_MOV32_IMM(BPF_REG_0, 43),
BPF_EXIT_INSN()
Verifier output on 4.4.122 without this patch:
0: (b7) r1 = 1
1: (15) if r1 == 0x1 goto pc+1
3: (b4) (u32) r1 = (u32) 1
4: (15) if r1 == 0x1 goto pc+1
6: (b7) r1 = -1
7: (15) if r1 == 0xffffffff goto pc+1
9: (b4) (u32) r1 = (u32) -1
10: (15) if r1 == 0xffffffff goto pc+2
13: (b4) (u32) r0 = (u32) 43
14: (95) exit
Verifier output on 4.4.122+ with this patch:
0: (b7) r1 = 1
1: (15) if r1 == 0x1 goto pc+1
3: (b4) (u32) r1 = (u32) 1
4: (15) if r1 == 0x1 goto pc+1
6: (b7) r1 = -1
7: (15) if r1 == 0xffffffff goto pc+1
9: (b4) (u32) r1 = (u32) -1
10: (15) if r1 == 0xffffffff goto pc+2
R1=inv R10=fp
11: (b4) (u32) r0 = (u32) 42
12: (95) exit
from 10 to 13: R1=imm-1 R10=fp
13: (b4) (u32) r0 = (u32) 43
14: (95) exit
Signed-off-by: Jann Horn <jannh@google.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
kernel/bpf/verifier.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1135,7 +1135,8 @@ static int check_alu_op(struct verifier_
regs[insn->dst_reg].type = UNKNOWN_VALUE;
regs[insn->dst_reg].map_ptr = NULL;
}
- } else {
+ } else if (BPF_CLASS(insn->code) == BPF_ALU64 ||
+ insn->imm >= 0) {
/* case: R = imm
* remember the value we stored into this reg
*/
Patches currently in stable-queue which might be from jannh@google.com are
queue-4.4/fs-aio-add-explicit-rcu-grace-period-when-freeing-kioctx.patch
queue-4.4/fs-aio-use-rcu-accessors-for-kioctx_table-table.patch
queue-4.4/bpf-fix-incorrect-sign-extension-in-check_alu_op.patch
next prev parent reply other threads:[~2018-03-19 17:13 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-18 2:17 requesting stable backport for 4.1 and 4.4: 95a762e2c8c9 ("bpf: fix incorrect sign extension in check_alu_op()") Jann Horn
2018-03-18 12:57 ` Greg Kroah-Hartman
2018-03-19 16:55 ` [PATCH for 4.1,4.4] bpf: fix incorrect sign extension in check_alu_op() Jann Horn
2018-03-19 16:58 ` Daniel Borkmann
2018-03-19 17:13 ` gregkh [this message]
2018-03-19 17:13 ` Greg Kroah-Hartman
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=15214795823916@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=ast@kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=jannh@google.com \
--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