[DTrace-devel] [PATCH v2 13/17] alloca: support null pointers
Nick Alcock
nick.alcock at oracle.com
Mon Mar 14 21:30:21 UTC 2022
Variables used to store pointers obtained from alloca() cannot be reused
for any other purpose -- but there is one purpose people are almost
certain to want, which is assigning NULL to them. You don't want that
to elicit an error message!
Making this work is quite tricky. We can't just stuff a 0 into
variables and load it, because that would lead to us subtracting
scratchbot from it and then adding scratchbot back afterwards, giving
null pointers a huge random value in the variables. So we detect
the specific case of var = NULL at codegen time by literal matching the
parse tree and turn it into a store of a sentinel value,
DT_CG_ALLOCA_NULLPTR (~(1<<30)). We can then detect that at load time
and turn it back into a NULL.
But that's not enough. We might say "it's up to the user to not
dereference it", but the verifier forces us to ensure that this can't
happen. We can't leave a 0 in there either because it's not a pointer
value and the verifier's tracking of what happens if a 0 is dereferenced
hits map_value-versus-literal problems: since every *other* value we
can hold in an alloca parse tree node is a pointer, we should arrange
for this to be one too.
But a pointer to what? It can't be a pointer to scratch space, and it
certainly can't be a pointer to NULL -- that's not a map_value! So we
introduce a new "null pointer space", in the shape of a BPF array twice
the size of scratch space, and set null pointers to point halfway
through that at load time. Then it's just a matter of having the
bounds-checkers check for both 0 and the null pointer space value and
raise suitable out-of-bounds errors in both cases. (As usual,
in-scratch and out-of-scratch bounds checkers diverge: *both* consider
null pointers out of bounds, but the in-scratch checker only checks for
the null-pointer-space version so it can raise a suitable error, since
null pointers that are still 0 raise the right error anyway. This means
null pointer detection adds relatively little code except to bcopy.)
Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
libdtrace/dt_bpf.c | 4 +-
libdtrace/dt_cg.c | 131 ++++++++++++++----
libdtrace/dt_parser.c | 14 +-
.../alloca/err.D_ALLOCA_INCOMPAT.var-clash.d | 2 +
.../alloca/err.D_ALLOCA_INCOMPAT.var-clash.r | 2 +-
...clash.d => err.alloca-null-deref-lvalue.d} | 17 +--
.../alloca/err.alloca-null-deref-lvalue.r | 3 +
...AT.var-clash.d => err.alloca-null-deref.d} | 15 +-
.../funcs/alloca/err.alloca-null-deref.r | 3 +
test/unittest/inline/tst.InlineKinds.d | 7 +
10 files changed, 149 insertions(+), 49 deletions(-)
copy test/unittest/funcs/alloca/{err.D_ALLOCA_INCOMPAT.var-clash.d => err.alloca-null-deref-lvalue.d} (56%)
create mode 100644 test/unittest/funcs/alloca/err.alloca-null-deref-lvalue.r
copy test/unittest/funcs/alloca/{err.D_ALLOCA_INCOMPAT.var-clash.d => err.alloca-null-deref.d} (60%)
create mode 100644 test/unittest/funcs/alloca/err.alloca-null-deref.r
diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
index da294eb1f13b..159737a139f5 100644
--- a/libdtrace/dt_bpf.c
+++ b/libdtrace/dt_bpf.c
@@ -326,8 +326,8 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
*/
if (create_gmap(dtp, "scratchmem", BPF_MAP_TYPE_PERCPU_ARRAY,
sizeof(uint32_t),
- dtp->dt_options[DTRACEOPT_SCRATCHSIZE] *
- 2, 1) == -1)
+ dtp->dt_options[DTRACEOPT_SCRATCHSIZE] * 2,
+ 1) == -1)
return -1; /* dt_errno is set for us */
/*
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 912a2c10e987..1f622d365464 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -29,6 +29,12 @@ static void _dt_unused_
dt_cg_trace(dt_irlist_t *dlp _dt_unused_, dt_regset_t *drp _dt_unused_,
int isreg _dt_unused_, int counter _dt_unused_, uint64_t val _dt_unused_);
+/*
+ * The value stored in an alloca'ed variable if what was stored is literal
+ * NULL.
+ */
+#define DT_CG_ALLOCA_NULLPTR ~(1<<30)
+
/*
* Generate the generic prologue of the trampoline BPF program.
*
@@ -741,10 +747,16 @@ dt_cg_probe_error_regval(dt_pcb_t *pcb, int lbl, uint32_t off,
static void
dt_cg_check_notnull(dt_irlist_t *dlp, dt_regset_t *drp, int reg)
{
+ int opt_scratchsize = yypcb->pcb_hdl->dt_options[DTRACEOPT_SCRATCHSIZE];
uint_t lbl_notnull = dt_irlist_label(dlp);
-
- emit(dlp, BPF_BRANCH_IMM(BPF_JNE, reg, 0, lbl_notnull));
- dt_cg_probe_error(yypcb, DT_LBL_NONE, -1, DTRACEFLT_BADADDR, 0);
+ uint_t lbl_null = dt_irlist_label(dlp);
+
+ emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, reg, 0, lbl_null));
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSGT, reg, opt_scratchsize,
+ lbl_notnull));
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSLT, reg, -opt_scratchsize,
+ lbl_notnull));
+ dt_cg_probe_error(yypcb, lbl_null, -1, DTRACEFLT_BADADDR, 0);
emitl(dlp, lbl_notnull,
BPF_NOP());
}
@@ -2139,7 +2151,7 @@ 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
+ * comparison, 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
@@ -2150,6 +2162,7 @@ dt_cg_check_bounds(dt_irlist_t *dlp, dt_regset_t *drp, int regptr, int basereg,
*/
if (regptr && basereg > -1) {
+ uint_t lbl_notnull = dt_irlist_label(dlp);
int mst, scalarbase;
if ((scalarbase = dt_regset_alloc(drp)) == -1 ||
@@ -2164,8 +2177,18 @@ dt_cg_check_bounds(dt_irlist_t *dlp, dt_regset_t *drp, int regptr, int basereg,
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));
+ /*
+ * Null pointers are always out of bounds. So is everything
+ * within LENMAX of a null pointer.
+ */
+
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSGT, reg, lenmax, lbl_notnull));
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSLT, reg, -lenmax, lbl_notnull));
+ dt_cg_probe_error(yypcb, DT_LBL_NONE, -1, DTRACEFLT_BADADDR, 0);
+
+ emitl(dlp, lbl_notnull,
+ 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);
@@ -2264,12 +2287,16 @@ dt_cg_check_scratch_bounds(dt_irlist_t *dlp, dt_regset_t *drp, int reg, int size
* outside scratch space. The REG is scalarized in the course of processing, so
* will quite possibly end up with much more extreme bounds than it started
* with. If this is a problem, work from a temp copy.
+ *
+ * Null pointers are considered to be invalid even though they are technically
+ * outside the bounds.
*/
static void
dt_cg_check_outscratch_bounds(dt_irlist_t *dlp, dt_regset_t *drp, int reg,
ssize_t size, int sizemax)
{
int opt_scratchsize = yypcb->pcb_hdl->dt_options[DTRACEOPT_SCRATCHSIZE];
+ uint_t lbl_notnull = dt_irlist_label(dlp);
uint_t lbl_err = dt_irlist_label(dlp);
uint_t lbl_size_err = dt_irlist_label(dlp);
uint_t lbl_above = dt_irlist_label(dlp);
@@ -2327,6 +2354,20 @@ dt_cg_check_outscratch_bounds(dt_irlist_t *dlp, dt_regset_t *drp, int reg,
emit(dlp, BPF_LOAD(BPF_DW, reg, mst, DMST_SCALARIZER));
emit(dlp, BPF_MOV_REG(origreg, reg));
+ /*
+ * Null pointers are always out of bounds. So is everything within
+ * scratchlen of a null pointer.
+ */
+
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSGT, reg, opt_scratchsize,
+ lbl_notnull));
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSLT, reg, -opt_scratchsize,
+ lbl_notnull));
+
+ dt_cg_probe_error(yypcb, DT_LBL_NONE, -1, DTRACEFLT_BADADDR, 0);
+
+ emitl(dlp, lbl_notnull,
+ BPF_NOP());
dt_regset_free(drp, mst);
/*
@@ -2402,6 +2443,23 @@ dt_cg_load_alloca(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp,
if ((idp->di_flags & DT_IDFLG_ALLOCA) && (dst->dn_flags & DT_NF_ALLOCA)) {
int opt_scratchsize = yypcb->pcb_hdl->dt_options[DTRACEOPT_SCRATCHSIZE];
int scratchbot, scratchlen;
+ uint_t lbl_notnull = dt_irlist_label(dlp);
+ uint_t lbl_done = dt_irlist_label(dlp);
+
+ /*
+ * First, check for a null pointer, by reduction to zero (or
+ * near-zero, for the case of a pointer near NULL) and
+ * comparison.
+ */
+ emit(dlp, BPF_ALU64_IMM(BPF_SUB, dst->dn_reg, DT_CG_ALLOCA_NULLPTR));
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSGT, dst->dn_reg,
+ opt_scratchsize * 2, lbl_notnull));
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSLT, dst->dn_reg,
+ -(opt_scratchsize * 2), lbl_notnull));
+ emit(dlp, BPF_JUMP(lbl_done));
+
+ emitl(dlp, lbl_notnull,
+ BPF_ALU64_IMM(BPF_ADD, dst->dn_reg, DT_CG_ALLOCA_NULLPTR));
/*
* Get the scratch length and check it.
@@ -2436,6 +2494,8 @@ dt_cg_load_alloca(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp,
dt_regset_free(drp, scratchbot);
dt_regset_free(drp, scratchlen);
+
+ emitl(dlp, lbl_done, BPF_NOP());
}
}
@@ -2799,36 +2859,51 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
/*
* Stores of DT_NF_ALLOCA nodes to identifiers with DT_IDFLG_ALLOCA set
* reduce the value stored by the value of DCTX_SCRATCHMEM first,
- * turning it into a scratchmem-relative offset. Literal nulls load in
- * an otherwise-invalid value, statically distinguishable from all valid
- * ones.
+ * turning it into a scratchmem-relative offset. Nulls, and values near
+ * nulls, load in an otherwise-invalid value, statically distinguishable
+ * from all valid ones.
*
* This is all done in a temporary register, to avoid disturbing the
* return value of =.
*/
if (dnp->dn_flags & DT_NF_ALLOCA && idp->di_flags & DT_IDFLG_ALLOCA) {
- int scratchbot, isnull = 0;
-
- if ((store_reg = dt_regset_alloc(drp)) == -1)
+ int opt_scratchsize = yypcb->pcb_hdl->dt_options[DTRACEOPT_SCRATCHSIZE];
+ int lbl_notnull = dt_irlist_label(dlp);
+ int lbl_loaded = dt_irlist_label(dlp);
+ int scratchbot, tmp;
+
+ if ((store_reg = dt_regset_alloc(drp)) == -1 ||
+ (scratchbot = dt_regset_alloc(drp)) == -1 ||
+ (tmp = dt_regset_alloc(drp)) == -1)
longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
- if (dnp->dn_kind == DT_NODE_OP2 && dnp->dn_op == DT_TOK_ASGN &&
- dnp->dn_right && dnp->dn_right->dn_kind == DT_NODE_INT &&
- dnp->dn_right->dn_value == 0)
- isnull = 1;
-
- if (!isnull) {
- if ((scratchbot = dt_regset_alloc(drp)) == -1)
- longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
-
- emit(dlp, BPF_LOAD(BPF_DW, scratchbot, BPF_REG_FP, DT_STK_DCTX));
- emit(dlp, BPF_LOAD(BPF_DW, scratchbot, scratchbot, DCTX_SCRATCHMEM));
- emit(dlp, BPF_MOV_REG(store_reg, dnp->dn_reg));
- emit(dlp, BPF_ALU64_REG(BPF_SUB, store_reg, scratchbot));
+ emit(dlp, BPF_LOAD(BPF_DW, scratchbot, BPF_REG_FP, DT_STK_DCTX));
+ emit(dlp, BPF_LOAD(BPF_DW, scratchbot, scratchbot, DCTX_SCRATCHMEM));
+
+ emit(dlp, BPF_LOAD(BPF_DW, tmp, BPF_REG_FP, DT_STK_DCTX));
+ emit(dlp, BPF_LOAD(BPF_DW, tmp, tmp, DCTX_MST));
+ emit(dlp, BPF_STORE(BPF_DW, tmp, DMST_SCALARIZER, scratchbot));
+ emit(dlp, BPF_LOAD(BPF_DW, scratchbot, tmp, DMST_SCALARIZER));
+ emit(dlp, BPF_STORE(BPF_DW, tmp, DMST_SCALARIZER, dnp->dn_reg));
+ emit(dlp, BPF_LOAD(BPF_DW, tmp, tmp, DMST_SCALARIZER));
+
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSGT, dnp->dn_reg,
+ opt_scratchsize * 2, lbl_notnull));
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSLT, dnp->dn_reg,
+ -(opt_scratchsize * 2), lbl_notnull));
+
+ emit(dlp, BPF_MOV_IMM(store_reg, DT_CG_ALLOCA_NULLPTR));
+ emit(dlp, BPF_ALU64_REG(BPF_ADD, store_reg, tmp));
+ emit(dlp, BPF_JUMP(lbl_loaded));
+
+ emitl(dlp, lbl_notnull,
+ BPF_MOV_REG(store_reg, tmp));
+ emit(dlp, BPF_ALU64_REG(BPF_SUB, store_reg, scratchbot));
+ emitl(dlp, lbl_loaded,
+ BPF_NOP());
- dt_regset_free(drp, scratchbot);
- } else
- emit(dlp, BPF_MOV_IMM(store_reg, DT_CG_ALLOCA_NULLPTR));
+ dt_regset_free(drp, scratchbot);
+ dt_regset_free(drp, tmp);
}
/* global and local variables (that is, not thread-local) */
diff --git a/libdtrace/dt_parser.c b/libdtrace/dt_parser.c
index f8017d3976c0..0be4c66ca441 100644
--- a/libdtrace/dt_parser.c
+++ b/libdtrace/dt_parser.c
@@ -3820,9 +3820,14 @@ asgn_common:
if (lp->dn_kind == DT_NODE_VAR)
lp_idp = lp->dn_ident;
+ /*
+ * Transfer alloca taint. Stores of non-alloca, non-literal-0
+ * values turn on DT_IDFLG_NONALLOCA to prevent this identifier
+ * from being used for alloca storage anywhere in the program.
+ */
if (rp->dn_flags & DT_NF_ALLOCA)
dt_cook_taint_alloca(lp, lp_idp, rp);
- else if (lp_idp)
+ else if (lp_idp && !(rp->dn_kind == DT_NODE_INT && rp->dn_value == 0))
lp_idp->di_flags |= DT_IDFLG_NONALLOCA;
dt_node_type_propagate(lp, dnp); /* see K&R[A7.17] */
@@ -4064,9 +4069,14 @@ asgn_common:
dnp->dn_args = rp;
dnp->dn_list = NULL;
+ /*
+ * Transfer alloca taint. Stores of non-alloca, non-literal-0
+ * values turn on DT_IDFLG_NONALLOCA to prevent this identifier
+ * from being used for alloca storage anywhere in the program.
+ */
if (dnp->dn_args->dn_flags & DT_NF_ALLOCA)
dt_cook_taint_alloca(dnp, idp, dnp->dn_args);
- else
+ else if (dnp->dn_kind != DT_NODE_INT || dnp->dn_value != 0)
idp->di_flags |= DT_IDFLG_NONALLOCA;
dt_node_free(lp);
diff --git a/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d b/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d
index d6474d5f18ba..fe9e274985d1 100644
--- a/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d
+++ b/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d
@@ -8,6 +8,7 @@
/*
* ASSERTION: the same variable cannot be reused for alloca and
* non-alloca pointers, even in different clauses.
+ * You can't fake it out by assigning NULL in between.
*
* SECTION: Actions and Subroutines/alloca()
*/
@@ -22,6 +23,7 @@ BEGIN
BEGIN
{
b = (char *) &`max_pfn;
+ a = NULL;
a = b + 5;
trace(a);
exit(0);
diff --git a/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.r b/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.r
index a2fdf1035407..ed642dc71f27 100644
--- a/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.r
+++ b/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.r
@@ -1,2 +1,2 @@
-- @@stderr --
-dtrace: failed to compile script test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d: [D_ALLOCA_INCOMPAT] line 22: a: cannot reuse the same identifier for both alloca and non-alloca allocations
+dtrace: failed to compile script test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d: [D_ALLOCA_INCOMPAT] line 23: a: cannot reuse the same identifier for both alloca and non-alloca allocations
diff --git a/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d b/test/unittest/funcs/alloca/err.alloca-null-deref-lvalue.d
similarity index 56%
copy from test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d
copy to test/unittest/funcs/alloca/err.alloca-null-deref-lvalue.d
index d6474d5f18ba..97d13f12a482 100644
--- a/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d
+++ b/test/unittest/funcs/alloca/err.alloca-null-deref-lvalue.d
@@ -6,23 +6,24 @@
*/
/*
- * ASSERTION: the same variable cannot be reused for alloca and
- * non-alloca pointers, even in different clauses.
+ * ASSERTION: You can't dereference a nullified alloca pointer via an lvalue,
+ * even if the resulting address is merely almost null.
*
* SECTION: Actions and Subroutines/alloca()
*/
#pragma D option quiet
+#pragma D option scratchsize=51
BEGIN
{
- a = (char *) alloca(1);
+ s = (char *) alloca(10);
+ s = NULL;
+ s[50] = 4;
+ exit(0);
}
-BEGIN
+ERROR
{
- b = (char *) &`max_pfn;
- a = b + 5;
- trace(a);
- exit(0);
+ exit(1);
}
diff --git a/test/unittest/funcs/alloca/err.alloca-null-deref-lvalue.r b/test/unittest/funcs/alloca/err.alloca-null-deref-lvalue.r
new file mode 100644
index 000000000000..ef424aedd5a8
--- /dev/null
+++ b/test/unittest/funcs/alloca/err.alloca-null-deref-lvalue.r
@@ -0,0 +1,3 @@
+
+-- @@stderr --
+dtrace: error on enabled probe ID 3 (ID 1: dtrace:::BEGIN): invalid address ({ptr}) in action #1
diff --git a/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d b/test/unittest/funcs/alloca/err.alloca-null-deref.d
similarity index 60%
copy from test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d
copy to test/unittest/funcs/alloca/err.alloca-null-deref.d
index d6474d5f18ba..77ee730651cd 100644
--- a/test/unittest/funcs/alloca/err.D_ALLOCA_INCOMPAT.var-clash.d
+++ b/test/unittest/funcs/alloca/err.alloca-null-deref.d
@@ -6,8 +6,7 @@
*/
/*
- * ASSERTION: the same variable cannot be reused for alloca and
- * non-alloca pointers, even in different clauses.
+ * ASSERTION: You can't dereference a nullified alloca pointer.
*
* SECTION: Actions and Subroutines/alloca()
*/
@@ -16,13 +15,13 @@
BEGIN
{
- a = (char *) alloca(1);
+ s = (char *) alloca(10);
+ s = NULL;
+ *s = 4;
+ exit(0);
}
-BEGIN
+ERROR
{
- b = (char *) &`max_pfn;
- a = b + 5;
- trace(a);
- exit(0);
+ exit(1);
}
diff --git a/test/unittest/funcs/alloca/err.alloca-null-deref.r b/test/unittest/funcs/alloca/err.alloca-null-deref.r
new file mode 100644
index 000000000000..ef424aedd5a8
--- /dev/null
+++ b/test/unittest/funcs/alloca/err.alloca-null-deref.r
@@ -0,0 +1,3 @@
+
+-- @@stderr --
+dtrace: error on enabled probe ID 3 (ID 1: dtrace:::BEGIN): invalid address ({ptr}) in action #1
diff --git a/test/unittest/inline/tst.InlineKinds.d b/test/unittest/inline/tst.InlineKinds.d
index 5f2ec1b89bf4..6d743fc73b79 100644
--- a/test/unittest/inline/tst.InlineKinds.d
+++ b/test/unittest/inline/tst.InlineKinds.d
@@ -11,6 +11,8 @@
* associative array inlines, and inlines using translators.
*/
+/* @@xfail: dtv2, xlators from int and null checking */
+
#pragma D option quiet
inline int i0 = 100 + 23; /* constant-folded integer constant */
@@ -41,3 +43,8 @@ BEGIN
exit(0);
}
+
+ERROR
+{
+ exit(1);
+}
--
2.35.1.261.g8402f930ba.dirty
More information about the DTrace-devel
mailing list