The goal of this patch is to protect the JIT against an attacker with a
write-in-memory primitive. The JIT allocates a buffer which will eventually
be marked +x, so we need to make sure that what was written to this buffer
is what was intended.
We acheive this by building a hash of the instruction buffer as
instructions are emittted and then comparing that to a hash at the end of
the JIT compile after the buffer has been marked read-only.
Signed-off-by: Tycho Andersen <[email protected]>
CC: Daniel Borkmann <[email protected]>
CC: Alexei Starovoitov <[email protected]>
CC: Kees Cook <[email protected]>
CC: Mickaël Salaün <[email protected]>
---
arch/x86/Kconfig | 11 ++++
arch/x86/net/bpf_jit_comp.c | 147 ++++++++++++++++++++++++++++++++++++++++----
2 files changed, 147 insertions(+), 11 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc98d5a..7b2db2c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2789,6 +2789,17 @@ config X86_DMA_REMAP
source "net/Kconfig"
+config EBPF_JIT_HASH_OUTPUT
+ def_bool y
+ depends on HAVE_EBPF_JIT
+ depends on BPF_JIT
+ select CRYPTO_SHA256
+ ---help---
+ Enables a double check of the JIT's output after it is marked read-only to
+ ensure that it matches what the JIT generated.
+
+ Note, only applies when /proc/sys/net/core/bpf_jit_harden > 0.
+
source "drivers/Kconfig"
source "drivers/firmware/Kconfig"
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 32322ce..be1271e 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -13,9 +13,15 @@
#include <linux/if_vlan.h>
#include <asm/cacheflush.h>
#include <linux/bpf.h>
+#include <linux/crypto.h>
+#include <crypto/hash.h>
int bpf_jit_enable __read_mostly;
+#ifdef CONFIG_EBPF_JIT_HASH_OUTPUT
+struct crypto_shash *tfm __read_mostly;
+#endif
+
/*
* assembly code in arch/x86/net/bpf_jit.S
*/
@@ -25,7 +31,8 @@ extern u8 sk_load_byte_positive_offset[];
extern u8 sk_load_word_negative_offset[], sk_load_half_negative_offset[];
extern u8 sk_load_byte_negative_offset[];
-static u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
+static u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len,
+ struct shash_desc *hash)
{
if (len == 1)
*ptr = bytes;
@@ -35,11 +42,15 @@ static u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
*(u32 *)ptr = bytes;
barrier();
}
+
+ if (IS_ENABLED(CONFIG_EBPF_JIT_HASH_OUTPUT) && hash)
+ crypto_shash_update(hash, (u8 *) &bytes, len);
+
return ptr + len;
}
#define EMIT(bytes, len) \
- do { prog = emit_code(prog, bytes, len); cnt += len; } while (0)
+ do { prog = emit_code(prog, bytes, len, hash); cnt += len; } while (0)
#define EMIT1(b1) EMIT(b1, 1)
#define EMIT2(b1, b2) EMIT((b1) + ((b2) << 8), 2)
@@ -206,7 +217,7 @@ struct jit_context {
/* emit x64 prologue code for BPF program and check it's size.
* bpf_tail_call helper will skip it while jumping into another program
*/
-static void emit_prologue(u8 **pprog)
+static void emit_prologue(u8 **pprog, struct shash_desc *hash)
{
u8 *prog = *pprog;
int cnt = 0;
@@ -264,7 +275,7 @@ static void emit_prologue(u8 **pprog)
* goto *(prog->bpf_func + prologue_size);
* out:
*/
-static void emit_bpf_tail_call(u8 **pprog)
+static void emit_bpf_tail_call(u8 **pprog, struct shash_desc *hash)
{
u8 *prog = *pprog;
int label1, label2, label3;
@@ -328,7 +339,7 @@ static void emit_bpf_tail_call(u8 **pprog)
}
-static void emit_load_skb_data_hlen(u8 **pprog)
+static void emit_load_skb_data_hlen(u8 **pprog, struct shash_desc *hash)
{
u8 *prog = *pprog;
int cnt = 0;
@@ -348,7 +359,8 @@ static void emit_load_skb_data_hlen(u8 **pprog)
}
static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
- int oldproglen, struct jit_context *ctx)
+ int oldproglen, struct jit_context *ctx,
+ struct shash_desc *hash)
{
struct bpf_insn *insn = bpf_prog->insnsi;
int insn_cnt = bpf_prog->len;
@@ -360,10 +372,10 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
int proglen = 0;
u8 *prog = temp;
- emit_prologue(&prog);
+ emit_prologue(&prog, hash);
if (seen_ld_abs)
- emit_load_skb_data_hlen(&prog);
+ emit_load_skb_data_hlen(&prog, hash);
for (i = 0; i < insn_cnt; i++, insn++) {
const s32 imm32 = insn->imm;
@@ -875,7 +887,7 @@ xadd: if (is_imm8(insn->off))
if (seen_ld_abs) {
if (reload_skb_data) {
EMIT1(0x5F); /* pop %rdi */
- emit_load_skb_data_hlen(&prog);
+ emit_load_skb_data_hlen(&prog, hash);
} else {
EMIT2(0x41, 0x59); /* pop %r9 */
EMIT2(0x41, 0x5A); /* pop %r10 */
@@ -884,7 +896,7 @@ xadd: if (is_imm8(insn->off))
break;
case BPF_JMP | BPF_CALL | BPF_X:
- emit_bpf_tail_call(&prog);
+ emit_bpf_tail_call(&prog, hash);
break;
/* cond jump */
@@ -1085,6 +1097,106 @@ xadd: if (is_imm8(insn->off))
return proglen;
}
+#ifdef CONFIG_EBPF_JIT_HASH_OUTPUT
+static struct shash_desc *bpf_alloc_hash_desc(void)
+{
+ struct shash_desc *hash;
+ int sz = sizeof(struct shash_desc) + crypto_shash_descsize(tfm);
+
+ hash = kzalloc(sz, GFP_KERNEL);
+ if (hash)
+ hash->tfm = tfm;
+ return hash;
+}
+
+static int init_hash(struct shash_desc **hash, u32 *nonce)
+{
+ if (!bpf_jit_harden)
+ return 0;
+
+ *nonce = get_random_int();
+
+ if (!tfm) {
+ tfm = crypto_alloc_shash("sha256", 0, 0);
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);
+ }
+
+ if (!*hash) {
+ *hash = bpf_alloc_hash_desc();
+ if (!*hash)
+ return -ENOMEM;
+ }
+
+ if (crypto_shash_init(*hash) < 0)
+ return -1;
+
+ return crypto_shash_update(*hash, (u8 *) nonce, sizeof(*nonce));
+}
+
+static bool check_jit_hash(u8 *buf, u32 len, struct shash_desc *out_d,
+ u32 nonce)
+{
+ struct shash_desc *check_d;
+ void *out, *check;
+ unsigned int sz;
+ bool match = false;
+
+ if (!out_d)
+ return 0;
+
+ BUG_ON(out_d->tfm != tfm);
+
+ sz = crypto_shash_digestsize(out_d->tfm);
+ out = kzalloc(2 * sz, GFP_KERNEL);
+ if (!out)
+ return false;
+
+ if (crypto_shash_final(out_d, out) < 0) {
+ kfree(out);
+ return false;
+ }
+
+ check_d = bpf_alloc_hash_desc();
+ if (!check_d) {
+ kfree(out);
+ return false;
+ }
+
+ if (crypto_shash_init(check_d) < 0)
+ goto out;
+
+ if (crypto_shash_update(check_d, (u8 *) &nonce, sizeof(nonce)) < 0)
+ goto out;
+
+ if (crypto_shash_update(check_d, buf, len) < 0)
+ goto out;
+
+ check = out + sz;
+ if (crypto_shash_final(check_d, check) < 0)
+ goto out;
+
+ if (!memcmp(out, check, sz))
+ match = true;
+
+out:
+ kfree(out);
+ kfree(check_d);
+ return match;
+}
+#else
+static int init_hash(struct shash_desc **hash, u32 *nonce)
+{
+ return 0;
+}
+
+static bool check_jit_hash(u8 *buf, u32 len, struct shash_desc *out_d,
+ u32 nonce)
+{
+ return true;
+}
+#endif
+
struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
{
struct bpf_binary_header *header = NULL;
@@ -1096,6 +1208,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
int *addrs;
int pass;
int i;
+ struct shash_desc *hash = NULL;
+ u32 nonce;
if (!bpf_jit_enable)
return orig_prog;
@@ -1132,7 +1246,15 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
* pass to emit the final image
*/
for (pass = 0; pass < 10 || image; pass++) {
- proglen = do_jit(prog, addrs, image, oldproglen, &ctx);
+ if (init_hash(&hash, &nonce) < 0) {
+ image = NULL;
+ if (header)
+ bpf_jit_binary_free(header);
+ prog = orig_prog;
+ goto out_addrs;
+ }
+
+ proglen = do_jit(prog, addrs, image, oldproglen, &ctx, hash);
if (proglen <= 0) {
image = NULL;
if (header)
@@ -1166,6 +1288,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
if (image) {
bpf_flush_icache(header, image + proglen);
bpf_jit_binary_lock_ro(header);
+ if (!check_jit_hash(image, proglen, hash, nonce))
+ BUG();
prog->bpf_func = (void *)image;
prog->jited = 1;
} else {
@@ -1174,6 +1298,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
out_addrs:
kfree(addrs);
+ kfree(hash);
out:
if (tmp_blinded)
bpf_jit_prog_release_other(prog, prog == orig_prog ?
--
2.9.3
On Tue, Apr 4, 2017 at 3:08 PM, Tycho Andersen <[email protected]> wrote:
> The goal of this patch is to protect the JIT against an attacker with a
> write-in-memory primitive. The JIT allocates a buffer which will eventually
> be marked +x, so we need to make sure that what was written to this buffer
> is what was intended.
>
> We acheive this by building a hash of the instruction buffer as
> instructions are emittted and then comparing that to a hash at the end of
> the JIT compile after the buffer has been marked read-only.
>
> Signed-off-by: Tycho Andersen <[email protected]>
> CC: Daniel Borkmann <[email protected]>
> CC: Alexei Starovoitov <[email protected]>
> CC: Kees Cook <[email protected]>
> CC: Mickaël Salaün <[email protected]>
Cool! This closes the race condition on producing the JIT vs going
read-only. I wonder if it might be possible to make this a more
generic interface to the BPF which would be allocate the hash, provide
the update callback during emit, and then do the hash check itself at
the end of bpf_jit_binary_lock_ro()?
-Kees
> ---
> arch/x86/Kconfig | 11 ++++
> arch/x86/net/bpf_jit_comp.c | 147 ++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 147 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index cc98d5a..7b2db2c 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2789,6 +2789,17 @@ config X86_DMA_REMAP
>
> source "net/Kconfig"
>
> +config EBPF_JIT_HASH_OUTPUT
> + def_bool y
> + depends on HAVE_EBPF_JIT
> + depends on BPF_JIT
> + select CRYPTO_SHA256
> + ---help---
> + Enables a double check of the JIT's output after it is marked read-only to
> + ensure that it matches what the JIT generated.
> +
> + Note, only applies when /proc/sys/net/core/bpf_jit_harden > 0.
> +
> source "drivers/Kconfig"
>
> source "drivers/firmware/Kconfig"
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 32322ce..be1271e 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -13,9 +13,15 @@
> #include <linux/if_vlan.h>
> #include <asm/cacheflush.h>
> #include <linux/bpf.h>
> +#include <linux/crypto.h>
> +#include <crypto/hash.h>
>
> int bpf_jit_enable __read_mostly;
>
> +#ifdef CONFIG_EBPF_JIT_HASH_OUTPUT
> +struct crypto_shash *tfm __read_mostly;
> +#endif
> +
> /*
> * assembly code in arch/x86/net/bpf_jit.S
> */
> @@ -25,7 +31,8 @@ extern u8 sk_load_byte_positive_offset[];
> extern u8 sk_load_word_negative_offset[], sk_load_half_negative_offset[];
> extern u8 sk_load_byte_negative_offset[];
>
> -static u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
> +static u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len,
> + struct shash_desc *hash)
> {
> if (len == 1)
> *ptr = bytes;
> @@ -35,11 +42,15 @@ static u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
> *(u32 *)ptr = bytes;
> barrier();
> }
> +
> + if (IS_ENABLED(CONFIG_EBPF_JIT_HASH_OUTPUT) && hash)
> + crypto_shash_update(hash, (u8 *) &bytes, len);
> +
> return ptr + len;
> }
>
> #define EMIT(bytes, len) \
> - do { prog = emit_code(prog, bytes, len); cnt += len; } while (0)
> + do { prog = emit_code(prog, bytes, len, hash); cnt += len; } while (0)
>
> #define EMIT1(b1) EMIT(b1, 1)
> #define EMIT2(b1, b2) EMIT((b1) + ((b2) << 8), 2)
> @@ -206,7 +217,7 @@ struct jit_context {
> /* emit x64 prologue code for BPF program and check it's size.
> * bpf_tail_call helper will skip it while jumping into another program
> */
> -static void emit_prologue(u8 **pprog)
> +static void emit_prologue(u8 **pprog, struct shash_desc *hash)
> {
> u8 *prog = *pprog;
> int cnt = 0;
> @@ -264,7 +275,7 @@ static void emit_prologue(u8 **pprog)
> * goto *(prog->bpf_func + prologue_size);
> * out:
> */
> -static void emit_bpf_tail_call(u8 **pprog)
> +static void emit_bpf_tail_call(u8 **pprog, struct shash_desc *hash)
> {
> u8 *prog = *pprog;
> int label1, label2, label3;
> @@ -328,7 +339,7 @@ static void emit_bpf_tail_call(u8 **pprog)
> }
>
>
> -static void emit_load_skb_data_hlen(u8 **pprog)
> +static void emit_load_skb_data_hlen(u8 **pprog, struct shash_desc *hash)
> {
> u8 *prog = *pprog;
> int cnt = 0;
> @@ -348,7 +359,8 @@ static void emit_load_skb_data_hlen(u8 **pprog)
> }
>
> static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
> - int oldproglen, struct jit_context *ctx)
> + int oldproglen, struct jit_context *ctx,
> + struct shash_desc *hash)
> {
> struct bpf_insn *insn = bpf_prog->insnsi;
> int insn_cnt = bpf_prog->len;
> @@ -360,10 +372,10 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
> int proglen = 0;
> u8 *prog = temp;
>
> - emit_prologue(&prog);
> + emit_prologue(&prog, hash);
>
> if (seen_ld_abs)
> - emit_load_skb_data_hlen(&prog);
> + emit_load_skb_data_hlen(&prog, hash);
>
> for (i = 0; i < insn_cnt; i++, insn++) {
> const s32 imm32 = insn->imm;
> @@ -875,7 +887,7 @@ xadd: if (is_imm8(insn->off))
> if (seen_ld_abs) {
> if (reload_skb_data) {
> EMIT1(0x5F); /* pop %rdi */
> - emit_load_skb_data_hlen(&prog);
> + emit_load_skb_data_hlen(&prog, hash);
> } else {
> EMIT2(0x41, 0x59); /* pop %r9 */
> EMIT2(0x41, 0x5A); /* pop %r10 */
> @@ -884,7 +896,7 @@ xadd: if (is_imm8(insn->off))
> break;
>
> case BPF_JMP | BPF_CALL | BPF_X:
> - emit_bpf_tail_call(&prog);
> + emit_bpf_tail_call(&prog, hash);
> break;
>
> /* cond jump */
> @@ -1085,6 +1097,106 @@ xadd: if (is_imm8(insn->off))
> return proglen;
> }
>
> +#ifdef CONFIG_EBPF_JIT_HASH_OUTPUT
> +static struct shash_desc *bpf_alloc_hash_desc(void)
> +{
> + struct shash_desc *hash;
> + int sz = sizeof(struct shash_desc) + crypto_shash_descsize(tfm);
> +
> + hash = kzalloc(sz, GFP_KERNEL);
> + if (hash)
> + hash->tfm = tfm;
> + return hash;
> +}
> +
> +static int init_hash(struct shash_desc **hash, u32 *nonce)
> +{
> + if (!bpf_jit_harden)
> + return 0;
> +
> + *nonce = get_random_int();
> +
> + if (!tfm) {
> + tfm = crypto_alloc_shash("sha256", 0, 0);
> + if (IS_ERR(tfm))
> + return PTR_ERR(tfm);
> + }
> +
> + if (!*hash) {
> + *hash = bpf_alloc_hash_desc();
> + if (!*hash)
> + return -ENOMEM;
> + }
> +
> + if (crypto_shash_init(*hash) < 0)
> + return -1;
> +
> + return crypto_shash_update(*hash, (u8 *) nonce, sizeof(*nonce));
> +}
> +
> +static bool check_jit_hash(u8 *buf, u32 len, struct shash_desc *out_d,
> + u32 nonce)
> +{
> + struct shash_desc *check_d;
> + void *out, *check;
> + unsigned int sz;
> + bool match = false;
> +
> + if (!out_d)
> + return 0;
> +
> + BUG_ON(out_d->tfm != tfm);
> +
> + sz = crypto_shash_digestsize(out_d->tfm);
> + out = kzalloc(2 * sz, GFP_KERNEL);
> + if (!out)
> + return false;
> +
> + if (crypto_shash_final(out_d, out) < 0) {
> + kfree(out);
> + return false;
> + }
> +
> + check_d = bpf_alloc_hash_desc();
> + if (!check_d) {
> + kfree(out);
> + return false;
> + }
> +
> + if (crypto_shash_init(check_d) < 0)
> + goto out;
> +
> + if (crypto_shash_update(check_d, (u8 *) &nonce, sizeof(nonce)) < 0)
> + goto out;
> +
> + if (crypto_shash_update(check_d, buf, len) < 0)
> + goto out;
> +
> + check = out + sz;
> + if (crypto_shash_final(check_d, check) < 0)
> + goto out;
> +
> + if (!memcmp(out, check, sz))
> + match = true;
> +
> +out:
> + kfree(out);
> + kfree(check_d);
> + return match;
> +}
> +#else
> +static int init_hash(struct shash_desc **hash, u32 *nonce)
> +{
> + return 0;
> +}
> +
> +static bool check_jit_hash(u8 *buf, u32 len, struct shash_desc *out_d,
> + u32 nonce)
> +{
> + return true;
> +}
> +#endif
> +
> struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> {
> struct bpf_binary_header *header = NULL;
> @@ -1096,6 +1208,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> int *addrs;
> int pass;
> int i;
> + struct shash_desc *hash = NULL;
> + u32 nonce;
>
> if (!bpf_jit_enable)
> return orig_prog;
> @@ -1132,7 +1246,15 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> * pass to emit the final image
> */
> for (pass = 0; pass < 10 || image; pass++) {
> - proglen = do_jit(prog, addrs, image, oldproglen, &ctx);
> + if (init_hash(&hash, &nonce) < 0) {
> + image = NULL;
> + if (header)
> + bpf_jit_binary_free(header);
> + prog = orig_prog;
> + goto out_addrs;
> + }
> +
> + proglen = do_jit(prog, addrs, image, oldproglen, &ctx, hash);
> if (proglen <= 0) {
> image = NULL;
> if (header)
> @@ -1166,6 +1288,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> if (image) {
> bpf_flush_icache(header, image + proglen);
> bpf_jit_binary_lock_ro(header);
> + if (!check_jit_hash(image, proglen, hash, nonce))
> + BUG();
> prog->bpf_func = (void *)image;
> prog->jited = 1;
> } else {
> @@ -1174,6 +1298,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>
> out_addrs:
> kfree(addrs);
> + kfree(hash);
> out:
> if (tmp_blinded)
> bpf_jit_prog_release_other(prog, prog == orig_prog ?
> --
> 2.9.3
>
--
Kees Cook
Pixel Security
Hi Kees,
On Tue, Apr 04, 2017 at 03:17:57PM -0700, Kees Cook wrote:
> On Tue, Apr 4, 2017 at 3:08 PM, Tycho Andersen <[email protected]> wrote:
> > The goal of this patch is to protect the JIT against an attacker with a
> > write-in-memory primitive. The JIT allocates a buffer which will eventually
> > be marked +x, so we need to make sure that what was written to this buffer
> > is what was intended.
> >
> > We acheive this by building a hash of the instruction buffer as
> > instructions are emittted and then comparing that to a hash at the end of
> > the JIT compile after the buffer has been marked read-only.
> >
> > Signed-off-by: Tycho Andersen <[email protected]>
> > CC: Daniel Borkmann <[email protected]>
> > CC: Alexei Starovoitov <[email protected]>
> > CC: Kees Cook <[email protected]>
> > CC: Micka?l Sala?n <[email protected]>
>
> Cool! This closes the race condition on producing the JIT vs going
> read-only. I wonder if it might be possible to make this a more
> generic interface to the BPF which would be allocate the hash, provide
> the update callback during emit, and then do the hash check itself at
> the end of bpf_jit_binary_lock_ro()?
Yes, probably so. I can look into that for the next version.
Tycho
Hi Tycho,
[auto build test WARNING on net/master]
[also build test WARNING on v4.11-rc5 next-20170405]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Tycho-Andersen/ebpf-verify-the-output-of-the-JIT/20170406-004746
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
arch/x86/net/bpf_jit_comp.c: In function 'do_jit':
>> arch/x86/net/bpf_jit_comp.c:1098:1: warning: the frame size of 13024 bytes is larger than 8192 bytes [-Wframe-larger-than=]
}
^
vim +1098 arch/x86/net/bpf_jit_comp.c
9383191d Daniel Borkmann 2017-02-16 1082 pr_err("bpf_jit: fatal insn size error\n");
e0ee9c12 Alexei Starovoitov 2014-10-10 1083 return -EFAULT;
e0ee9c12 Alexei Starovoitov 2014-10-10 1084 }
e0ee9c12 Alexei Starovoitov 2014-10-10 1085
0a14842f Eric Dumazet 2011-04-20 1086 if (image) {
0a14842f Eric Dumazet 2011-04-20 1087 if (unlikely(proglen + ilen > oldproglen)) {
9383191d Daniel Borkmann 2017-02-16 1088 pr_err("bpf_jit: fatal error\n");
f3c2af7b Alexei Starovoitov 2014-05-13 1089 return -EFAULT;
0a14842f Eric Dumazet 2011-04-20 1090 }
0a14842f Eric Dumazet 2011-04-20 1091 memcpy(image + proglen, temp, ilen);
0a14842f Eric Dumazet 2011-04-20 1092 }
0a14842f Eric Dumazet 2011-04-20 1093 proglen += ilen;
0a14842f Eric Dumazet 2011-04-20 1094 addrs[i] = proglen;
0a14842f Eric Dumazet 2011-04-20 1095 prog = temp;
0a14842f Eric Dumazet 2011-04-20 1096 }
f3c2af7b Alexei Starovoitov 2014-05-13 1097 return proglen;
f3c2af7b Alexei Starovoitov 2014-05-13 @1098 }
f3c2af7b Alexei Starovoitov 2014-05-13 1099
19d23b2d Tycho Andersen 2017-04-04 1100 #ifdef CONFIG_EBPF_JIT_HASH_OUTPUT
19d23b2d Tycho Andersen 2017-04-04 1101 static struct shash_desc *bpf_alloc_hash_desc(void)
19d23b2d Tycho Andersen 2017-04-04 1102 {
19d23b2d Tycho Andersen 2017-04-04 1103 struct shash_desc *hash;
19d23b2d Tycho Andersen 2017-04-04 1104 int sz = sizeof(struct shash_desc) + crypto_shash_descsize(tfm);
19d23b2d Tycho Andersen 2017-04-04 1105
19d23b2d Tycho Andersen 2017-04-04 1106 hash = kzalloc(sz, GFP_KERNEL);
:::::: The code at line 1098 was first introduced by commit
:::::: f3c2af7ba17a83809806880062c9ad541744fb95 net: filter: x86: split bpf_jit_compile()
:::::: TO: Alexei Starovoitov <[email protected]>
:::::: CC: David S. Miller <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, Apr 04, 2017 at 09:45:36PM -0600, Tycho Andersen wrote:
> Hi Kees,
>
> On Tue, Apr 04, 2017 at 03:17:57PM -0700, Kees Cook wrote:
> > On Tue, Apr 4, 2017 at 3:08 PM, Tycho Andersen <[email protected]> wrote:
> > > The goal of this patch is to protect the JIT against an attacker with a
> > > write-in-memory primitive. The JIT allocates a buffer which will eventually
> > > be marked +x, so we need to make sure that what was written to this buffer
> > > is what was intended.
> > >
> > > We acheive this by building a hash of the instruction buffer as
> > > instructions are emittted and then comparing that to a hash at the end of
> > > the JIT compile after the buffer has been marked read-only.
> > >
> > > Signed-off-by: Tycho Andersen <[email protected]>
> > > CC: Daniel Borkmann <[email protected]>
> > > CC: Alexei Starovoitov <[email protected]>
> > > CC: Kees Cook <[email protected]>
> > > CC: Micka?l Sala?n <[email protected]>
> >
> > Cool! This closes the race condition on producing the JIT vs going
> > read-only. I wonder if it might be possible to make this a more
> > generic interface to the BPF which would be allocate the hash, provide
> > the update callback during emit, and then do the hash check itself at
> > the end of bpf_jit_binary_lock_ro()?
>
> Yes, probably so. I can look into that for the next version.
Nack.
Please stop wasting yours and our time with buggy code that pretends
to fix a problem that doesn't exist.
This security paranoia around JIT must stop.
Make sure that CONFIG_BPF_JIT is off in your system.