[DTrace-devel] [PATCH v3 12/19] alloca: fix verifier failure checking the bounds of scalars

[Nick Alcock]

dt_check_scratch_bounds works for all genuine pointer inputs... but the
nature of the beast that the caller might pass it *anything*.  What if
the caller passes it a random integer, or a random kernel address?
The first thing we do is turn it into an offset by subtracting the
baseptr from it, and oops that is now maths between a scalar and a
map_value and the verifier fails.

So don't rely on the offset reduction to turn the reg into a scalar,
since it can fail if it already *is* one; instead, scalarize it
explicitly like we do for out-of-bounds checking.  That means we
have to scalarize the baseptr too, since if we already scalarized the
reg, subtracting the baseptr from it is now maths between a map_value
and a scalar again!

(We work on a copy of the baseptr, since we need a real map_value
baseptr at the end to turn the scalar reg offset back into a map_value.)

Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
 libdtrace/dt_cg.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 210aee4248b2..0f2fc02a316b 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -2143,13 +2143,36 @@ dt_cg_check_bounds(dt_irlist_t *dlp, dt_regset_t *drp, int regptr, int basereg,
 
 	/*
 	 * Check for below/above scratch space by reduction to offsets and
-	 * comparision, to allow the verifier to bounds-check.  Satisfy
-	 * the verifier after reduction via masking.
+	 * comparision, to allow the verifier to bounds-check.  Satisfy the
+	 * verifier after reduction via masking.  (This also catches null
+	 * pointers of both sorts, which are outside scratch space by
+	 * definition).  Annoyingly we need to take a copy of the basereg and
+	 * scalarize it first, or we might find an out-of-bounds address
+	 * yielding a verifier failure: this means we need to scalarize the reg
+	 * too, even though the subtraction would scalarize it in any case!
+	 * Round and round we go...
 	 */
 
 	if (regptr && basereg > -1) {
-		emit(dlp,  BPF_BRANCH_REG(BPF_JLT, reg, basereg, lbl_err));
-		emit(dlp, BPF_ALU64_REG(BPF_SUB, reg, basereg));
+		int mst, scalarbase;
+
+		if ((scalarbase = dt_regset_alloc(drp)) == -1 ||
+		    (mst = dt_regset_alloc(drp)) == 1)
+			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
+
+		emit(dlp,  BPF_LOAD(BPF_DW, mst, BPF_REG_FP, DT_STK_DCTX));
+		emit(dlp,  BPF_LOAD(BPF_DW, mst, mst, DCTX_MST));
+
+		emit(dlp,  BPF_STORE(BPF_DW, mst, DMST_SCALARIZER, basereg));
+		emit(dlp,  BPF_LOAD(BPF_DW, scalarbase, mst, DMST_SCALARIZER));
+		emit(dlp,  BPF_STORE(BPF_DW, mst, DMST_SCALARIZER, reg));
+		emit(dlp,  BPF_LOAD(BPF_DW, reg, mst, DMST_SCALARIZER));
+
+		emit(dlp, BPF_BRANCH_REG(BPF_JLT, reg, scalarbase, lbl_err));
+		emit(dlp, BPF_ALU64_REG(BPF_SUB, reg, scalarbase));
+
+		dt_regset_free(drp, scalarbase);
+		dt_regset_free(drp, mst);
 	}
 
 	emit(dlp,  BPF_BRANCH_IMM(BPF_JSLT, reg, 0, lbl_err));
-- 
2.35.1.261.g8402f930ba.dirty