Skip to content

Commit

Permalink
alloca: fix verifier failure checking the bounds of scalars
Browse files Browse the repository at this point in the history
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@oracle.com>
  • Loading branch information
nickalcock committed Mar 2, 2022
1 parent 6008d21 commit 8a0a3c6
Showing 1 changed file with 27 additions and 4 deletions.
31 changes: 27 additions & 4 deletions libdtrace/dt_cg.c
Original file line number Diff line number Diff line change
Expand Up @@ -2140,13 +2140,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));
Expand Down

0 comments on commit 8a0a3c6

Please sign in to comment.