]> git.ipfire.org Git - thirdparty/kernel/stable-queue.git/commitdiff
4.17-stable patches
authorGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 12 Jul 2018 09:08:11 +0000 (11:08 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 12 Jul 2018 09:08:11 +0000 (11:08 +0200)
added patches:
bpf-reject-passing-modified-ctx-to-helper-functions.patch

queue-4.17/bpf-reject-passing-modified-ctx-to-helper-functions.patch [new file with mode: 0644]
queue-4.17/series
queue-4.9/series [new file with mode: 0644]

diff --git a/queue-4.17/bpf-reject-passing-modified-ctx-to-helper-functions.patch b/queue-4.17/bpf-reject-passing-modified-ctx-to-helper-functions.patch
new file mode 100644 (file)
index 0000000..b311692
--- /dev/null
@@ -0,0 +1,205 @@
+From 58990d1ff3f7896ee341030e9a7c2e4002570683 Mon Sep 17 00:00:00 2001
+From: Daniel Borkmann <daniel@iogearbox.net>
+Date: Thu, 7 Jun 2018 17:40:03 +0200
+Subject: bpf: reject passing modified ctx to helper functions
+
+From: Daniel Borkmann <daniel@iogearbox.net>
+
+commit 58990d1ff3f7896ee341030e9a7c2e4002570683 upstream.
+
+As commit 28e33f9d78ee ("bpf: disallow arithmetic operations on
+context pointer") already describes, f1174f77b50c ("bpf/verifier:
+rework value tracking") removed the specific white-listed cases
+we had previously where we would allow for pointer arithmetic in
+order to further generalize it, and allow e.g. context access via
+modified registers. While the dereferencing of modified context
+pointers had been forbidden through 28e33f9d78ee, syzkaller did
+recently manage to trigger several KASAN splats for slab out of
+bounds access and use after frees by simply passing a modified
+context pointer to a helper function which would then do the bad
+access since verifier allowed it in adjust_ptr_min_max_vals().
+
+Rejecting arithmetic on ctx pointer in adjust_ptr_min_max_vals()
+generally could break existing programs as there's a valid use
+case in tracing in combination with passing the ctx to helpers as
+bpf_probe_read(), where the register then becomes unknown at
+verification time due to adding a non-constant offset to it. An
+access sequence may look like the following:
+
+  offset = args->filename;  /* field __data_loc filename */
+  bpf_probe_read(&dst, len, (char *)args + offset); // args is ctx
+
+There are two options: i) we could special case the ctx and as
+soon as we add a constant or bounded offset to it (hence ctx type
+wouldn't change) we could turn the ctx into an unknown scalar, or
+ii) we generalize the sanity test for ctx member access into a
+small helper and assert it on the ctx register that was passed
+as a function argument. Fwiw, latter is more obvious and less
+complex at the same time, and one case that may potentially be
+legitimate in future for ctx member access at least would be for
+ctx to carry a const offset. Therefore, fix follows approach
+from ii) and adds test cases to BPF kselftests.
+
+Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
+Reported-by: syzbot+3d0b2441dbb71751615e@syzkaller.appspotmail.com
+Reported-by: syzbot+c8504affd4fdd0c1b626@syzkaller.appspotmail.com
+Reported-by: syzbot+e5190cb881d8660fb1a3@syzkaller.appspotmail.com
+Reported-by: syzbot+efae31b384d5badbd620@syzkaller.appspotmail.com
+Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
+Acked-by: Alexei Starovoitov <ast@kernel.org>
+Acked-by: Yonghong Song <yhs@fb.com>
+Acked-by: Edward Cree <ecree@solarflare.com>
+Signed-off-by: Alexei Starovoitov <ast@kernel.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ kernel/bpf/verifier.c                       |   48 ++++++++++++++---------
+ tools/testing/selftests/bpf/test_verifier.c |   58 +++++++++++++++++++++++++++-
+ 2 files changed, 88 insertions(+), 18 deletions(-)
+
+--- a/kernel/bpf/verifier.c
++++ b/kernel/bpf/verifier.c
+@@ -1610,6 +1610,30 @@ static int get_callee_stack_depth(struct
+ }
+ #endif
++static int check_ctx_reg(struct bpf_verifier_env *env,
++                       const struct bpf_reg_state *reg, int regno)
++{
++      /* Access to ctx or passing it to a helper is only allowed in
++       * its original, unmodified form.
++       */
++
++      if (reg->off) {
++              verbose(env, "dereference of modified ctx ptr R%d off=%d disallowed\n",
++                      regno, reg->off);
++              return -EACCES;
++      }
++
++      if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
++              char tn_buf[48];
++
++              tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
++              verbose(env, "variable ctx access var_off=%s disallowed\n", tn_buf);
++              return -EACCES;
++      }
++
++      return 0;
++}
++
+ /* truncate register to smaller size (in bytes)
+  * must be called with size < BPF_REG_SIZE
+  */
+@@ -1679,24 +1703,11 @@ static int check_mem_access(struct bpf_v
+                       verbose(env, "R%d leaks addr into ctx\n", value_regno);
+                       return -EACCES;
+               }
+-              /* ctx accesses must be at a fixed offset, so that we can
+-               * determine what type of data were returned.
+-               */
+-              if (reg->off) {
+-                      verbose(env,
+-                              "dereference of modified ctx ptr R%d off=%d+%d, ctx+const is allowed, ctx+const+const is not\n",
+-                              regno, reg->off, off - reg->off);
+-                      return -EACCES;
+-              }
+-              if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
+-                      char tn_buf[48];
+-                      tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
+-                      verbose(env,
+-                              "variable ctx access var_off=%s off=%d size=%d",
+-                              tn_buf, off, size);
+-                      return -EACCES;
+-              }
++              err = check_ctx_reg(env, reg, regno);
++              if (err < 0)
++                      return err;
++
+               err = check_ctx_access(env, insn_idx, off, size, t, &reg_type);
+               if (!err && t == BPF_READ && value_regno >= 0) {
+                       /* ctx access returns either a scalar, or a
+@@ -1977,6 +1988,9 @@ static int check_func_arg(struct bpf_ver
+               expected_type = PTR_TO_CTX;
+               if (type != expected_type)
+                       goto err_type;
++              err = check_ctx_reg(env, reg, regno);
++              if (err < 0)
++                      return err;
+       } else if (arg_type_is_mem_ptr(arg_type)) {
+               expected_type = PTR_TO_STACK;
+               /* One exception here. In case function allows for NULL to be
+--- a/tools/testing/selftests/bpf/test_verifier.c
++++ b/tools/testing/selftests/bpf/test_verifier.c
+@@ -8190,7 +8190,7 @@ static struct bpf_test tests[] = {
+                                   offsetof(struct __sk_buff, mark)),
+                       BPF_EXIT_INSN(),
+               },
+-              .errstr = "dereference of modified ctx ptr R1 off=68+8, ctx+const is allowed, ctx+const+const is not",
++              .errstr = "dereference of modified ctx ptr",
+               .result = REJECT,
+               .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+       },
+@@ -11423,6 +11423,62 @@ static struct bpf_test tests[] = {
+               .errstr = "BPF_XADD stores into R2 packet",
+               .prog_type = BPF_PROG_TYPE_XDP,
+       },
++      {
++              "pass unmodified ctx pointer to helper",
++              .insns = {
++                      BPF_MOV64_IMM(BPF_REG_2, 0),
++                      BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
++                                   BPF_FUNC_csum_update),
++                      BPF_MOV64_IMM(BPF_REG_0, 0),
++                      BPF_EXIT_INSN(),
++              },
++              .prog_type = BPF_PROG_TYPE_SCHED_CLS,
++              .result = ACCEPT,
++      },
++      {
++              "pass modified ctx pointer to helper, 1",
++              .insns = {
++                      BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -612),
++                      BPF_MOV64_IMM(BPF_REG_2, 0),
++                      BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
++                                   BPF_FUNC_csum_update),
++                      BPF_MOV64_IMM(BPF_REG_0, 0),
++                      BPF_EXIT_INSN(),
++              },
++              .prog_type = BPF_PROG_TYPE_SCHED_CLS,
++              .result = REJECT,
++              .errstr = "dereference of modified ctx ptr",
++      },
++      {
++              "pass modified ctx pointer to helper, 2",
++              .insns = {
++                      BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -612),
++                      BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
++                                   BPF_FUNC_get_socket_cookie),
++                      BPF_MOV64_IMM(BPF_REG_0, 0),
++                      BPF_EXIT_INSN(),
++              },
++              .result_unpriv = REJECT,
++              .result = REJECT,
++              .errstr_unpriv = "dereference of modified ctx ptr",
++              .errstr = "dereference of modified ctx ptr",
++      },
++      {
++              "pass modified ctx pointer to helper, 3",
++              .insns = {
++                      BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1, 0),
++                      BPF_ALU64_IMM(BPF_AND, BPF_REG_3, 4),
++                      BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
++                      BPF_MOV64_IMM(BPF_REG_2, 0),
++                      BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
++                                   BPF_FUNC_csum_update),
++                      BPF_MOV64_IMM(BPF_REG_0, 0),
++                      BPF_EXIT_INSN(),
++              },
++              .prog_type = BPF_PROG_TYPE_SCHED_CLS,
++              .result = REJECT,
++              .errstr = "variable ctx access var_off=(0x0; 0x4)",
++      },
+ };
+ static int probe_filter_length(const struct bpf_insn *fp)
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..55fa86d56bb71c21089dc894d8c3356085a04f04 100644 (file)
@@ -0,0 +1 @@
+bpf-reject-passing-modified-ctx-to-helper-functions.patch
diff --git a/queue-4.9/series b/queue-4.9/series
new file mode 100644 (file)
index 0000000..e69de29