From 06351f7f372c670a630e7595fcd77913ed05742d Mon Sep 17 00:00:00 2001

From: Serhei Makarov <smakarov@redhat.com>

Date: Thu, 8 Nov 2018 16:40:40 -0500

Subject: [PATCH 25/32] PR23860: additional stack protection for strings

Fixes for verifier rejection of some cases requiring string copy,

since the verifier would reject string copy code extending beyond the

end of the string even if it was not reachable.

* bpf-opt.cxx (alloc_literal_str): make sure the offset for a short

  string is at least BPF_MAXSTRINGLEN.

(zero_stack): New function.

(program::generate): Use zero_stack() to zero temporary area and

  prevent verifier complaints.

* testsuite/systemtap.bpf/asm_tests/pr23860.stp: New testcase.

---

 bpf-opt.cxx                                   | 32 ++++++++++++++++++++++++++-

 testsuite/systemtap.bpf/asm_tests/pr23860.stp | 24 ++++++++++++++++++++

 2 files changed, 55 insertions(+), 1 deletion(-)

 create mode 100644 testsuite/systemtap.bpf/asm_tests/pr23860.stp

diff --git a/bpf-opt.cxx b/bpf-opt.cxx

index 8b9a6ea60..089dcde55 100644

--- a/bpf-opt.cxx

+++ b/bpf-opt.cxx

@@ -36,7 +36,19 @@ alloc_literal_str(program &p, insn_inserter &ins, std::string &str)

   if (tmp_space + str_bytes > MAX_BPF_STACK)

     throw std::runtime_error("string allocation failed due to lack of room on stack");

-  tmp_space += str_bytes; // TODO: round up for safety?

+  tmp_space += str_bytes;

+

+#if 1

+  // XXX PR23860: Passing a short (non-padded) string constant can fail

+  // the verifier, which is not smart enough to determine that accesses

+  // past the end of the string will never occur. To fix this, make sure

+  // the string offset is at least -BPF_MAXSTRINGLEN.

+  //

+  // Not ideal because an unlucky ordering of allocations may waste space.

+  if (tmp_space < BPF_MAXSTRINGLEN)

+    tmp_space = BPF_MAXSTRINGLEN;

+#endif

+

   p.use_tmp_space(tmp_space);

   int ofs = -tmp_space;

@@ -932,19 +944,37 @@ post_alloc_cleanup (program &p)

     }

 }

+// XXX PR23860: Passing a short (non-padded) string constant can fail

+// the verifier, which is not smart enough to determine that accesses

+// past the end of the string will never occur. To fix this, start the

+// program with some code to zero out the temporary stack space.

+void

+zero_stack(program &p)

+{

+  block *entry_block = p.blocks[0];

+  insn_before_inserter ins(entry_block, entry_block->first, "zero_stack");

+  value *frame = p.lookup_reg(BPF_REG_10);

+  for (int32_t ofs = -(int32_t)p.max_tmp_space; ofs < 0; ofs += 4)

+    p.mk_st(ins, BPF_W, frame, (int32_t)ofs, p.new_imm(0));

+}

+

 void

 program::generate()

 {

 #ifdef DEBUG_CODEGEN

   std::cerr << "DEBUG BEFORE OPT " << *this << std::endl;

 #endif

+

   lower_str_values(*this);

+  zero_stack(*this);

+

   fixup_operands(*this);

   thread_jumps(*this);

   fold_jumps(*this);

   reorder_blocks(*this);

   reg_alloc(*this);

   post_alloc_cleanup(*this);

+

 #ifdef DEBUG_CODEGEN

   std::cerr << "DEBUG AFTER OPT " << *this << std::endl;

 #endif

diff --git a/testsuite/systemtap.bpf/asm_tests/pr23860.stp b/testsuite/systemtap.bpf/asm_tests/pr23860.stp

new file mode 100644

index 000000000..bd3e05f66

--- /dev/null

+++ b/testsuite/systemtap.bpf/asm_tests/pr23860.stp

@@ -0,0 +1,24 @@

+function foo:string ()

+%{ /* bpf */ /* pure */ /* unprivileged */ /* stable */

+  0xbf, $$, "key", -, -; /* mov $$, "key" */

+%}

+

+function bar:string ()

+{

+  return "kez"

+}

+

+global t

+

+probe begin {

+  t[foo()] = 4

+  t[bar()] = 6

+  printf("U t[key]=%d, t[kez]=%d should be 4,6\n", t["key"], t["kez"])

+}

+

+probe kernel.function("vfs_read") {

+  t[foo()] = 5

+  t[bar()] = 7

+  printf("K t[key]=%d, t[kez]=%d should be 5,7\n", t["key"], t["kez"])

+  exit()

+}

-- 

2.14.5