[DTrace-devel] [PATCH 13/14] alloca: support null pointers
Nick Alcock
nick.alcock at oracle.com
Wed Mar 2 13:45:06 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 one-byte BPF
array, and set null pointers to point at 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 doesn't need to do anything to detect that because null pointers
are outside scratch space anyway. This means null pointer detection
adds very little code except to bcopy.)
Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
libdtrace/dt_bpf.c | 10 +++
libdtrace/dt_cg.c | 69 ++++++++++++++++++-
libdtrace/dt_dctx.h | 2 +
libdtrace/dt_dlibs.c | 1 +
libdtrace/dt_parser.c | 14 +++-
test/unittest/codegen/tst.stack_layout.r | 22 +++---
.../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} | 15 ++--
.../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 +
12 files changed, 127 insertions(+), 31 deletions(-)
copy test/unittest/funcs/alloca/{err.D_ALLOCA_INCOMPAT.var-clash.d => err.alloca-null-deref-lvalue.d} (60%)
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..ad58bb6ad7ac 100644
--- a/libdtrace/dt_bpf.c
+++ b/libdtrace/dt_bpf.c
@@ -330,6 +330,16 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
2, 1) == -1)
return -1; /* dt_errno is set for us */
+ /*
+ * This weird hack is a pointer for null pointers to point to. This
+ * lets us do arithmetic on them without verifier failures, while
+ * still allowing us to detect null pointers unambiguously.
+ */
+ if (create_gmap(dtp, "null", BPF_MAP_TYPE_ARRAY,
+ sizeof(uint32_t),
+ 1, 1) == -1)
+ return -1; /* dt_errno is set for us */
+
/*
* We need to create the global (consolidated) string table. We store
* the actual length (for in-code BPF validation purposes) but augment
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index b9dfb2bb6c66..6c8be2c92c8a 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.
*
@@ -199,6 +205,7 @@ dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act)
DT_CG_STORE_MAP_PTR("strtab", DCTX_STRTAB);
DT_CG_STORE_MAP_PTR("scratchmem", DCTX_SCRATCHMEM);
+ DT_CG_STORE_MAP_PTR("null", DCTX_NULL);
if (dt_idhash_datasize(dtp->dt_aggs) > 0)
DT_CG_STORE_MAP_PTR("aggs", DCTX_AGG);
if (dt_idhash_datasize(dtp->dt_globals) > 0)
@@ -2262,17 +2269,23 @@ 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_nonzero = dt_irlist_label(dlp);
+ uint_t lbl_nonnull = dt_irlist_label(dlp);
+ uint_t lbl_null = 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);
uint_t lbl_ok = dt_irlist_label(dlp);
- int scratchlen, scratchbot, mst, origreg;
+ int scratchlen, scratchbot, mst, origreg, null, tmp;
/*
* Size checks first. If SIZEMAX is set, make sure SIZE is no bigger
@@ -2325,6 +2338,43 @@ 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. Null pointers can have two
+ * representations: a straight 0-as-a-pointer (for things not stored in
+ * variables) or a pointer to the "null pointer space" (for things read
+ * from variables). We must check for both.
+ */
+
+ if ((null = dt_regset_alloc(drp)) == -1 ||
+ (tmp = dt_regset_alloc(drp)) == -1)
+ longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
+
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSGT, reg, clp2(opt_scratchsize + 1) - 1,
+ lbl_nonzero));
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSLT, reg, -clp2(opt_scratchsize + 1) - 1,
+ lbl_nonzero));
+
+ emit(dlp, BPF_JUMP(lbl_null));
+
+ emitl(dlp, lbl_nonzero,
+ BPF_LOAD(BPF_DW, null, BPF_REG_FP, DT_STK_DCTX));
+ emit(dlp, BPF_LOAD(BPF_DW, null, null, DCTX_NULL));
+ emit(dlp, BPF_STORE(BPF_DW, mst, DMST_SCALARIZER, null));
+ emit(dlp, BPF_LOAD(BPF_DW, null, mst, DMST_SCALARIZER));
+
+ emit(dlp, BPF_MOV_REG(tmp, reg));
+ emit(dlp, BPF_ALU64_REG(BPF_SUB, tmp, null));
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSGT, tmp, clp2(opt_scratchsize + 1) - 1,
+ lbl_nonnull));
+ emit(dlp, BPF_BRANCH_IMM(BPF_JSLT, tmp, -clp2(opt_scratchsize + 1) - 1,
+ lbl_nonnull));
+ dt_cg_probe_error(yypcb, lbl_null, -1, DTRACEFLT_BADADDR, 0);
+
+ emitl(dlp, lbl_nonnull,
+ BPF_NOP());
+ dt_regset_free(drp, null);
+ dt_regset_free(drp, tmp);
dt_regset_free(drp, mst);
/*
@@ -2400,6 +2450,21 @@ 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_nonnull = dt_irlist_label(dlp);
+ uint_t lbl_done = dt_irlist_label(dlp);
+
+ /*
+ * First, check for a null pointer and turn it into a pointer to
+ * the null pointer space, which the verifier at least considers
+ * to be a pointer.
+ */
+ emit(dlp, BPF_BRANCH_IMM(BPF_JNE, dst->dn_reg,
+ DT_CG_ALLOCA_NULLPTR, lbl_nonnull));
+ emit(dlp, BPF_LOAD(BPF_DW, dst->dn_reg, BPF_REG_FP, DT_STK_DCTX));
+ emit(dlp, BPF_LOAD(BPF_DW, dst->dn_reg, dst->dn_reg, DCTX_NULL));
+ emit(dlp, BPF_JUMP(lbl_done));
+
+ emitl(dlp, lbl_nonnull, BPF_NOP());
/*
* Get the scratch length and check it.
@@ -2434,6 +2499,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());
}
}
diff --git a/libdtrace/dt_dctx.h b/libdtrace/dt_dctx.h
index bc2ead6172b5..5a6440c984b8 100644
--- a/libdtrace/dt_dctx.h
+++ b/libdtrace/dt_dctx.h
@@ -42,6 +42,7 @@ typedef struct dt_dctx {
char *buf; /* Output buffer scratch memory */
char *mem; /* General scratch memory */
char *scratchmem; /* Scratch space for alloca, etc */
+ char *null; /* "Null pointer space". */
char *strtab; /* String constant table */
char *agg; /* Aggregation data */
char *gvars; /* Global variables */
@@ -54,6 +55,7 @@ typedef struct dt_dctx {
#define DCTX_BUF offsetof(dt_dctx_t, buf)
#define DCTX_MEM offsetof(dt_dctx_t, mem)
#define DCTX_SCRATCHMEM offsetof(dt_dctx_t, scratchmem)
+#define DCTX_NULL offsetof(dt_dctx_t, null)
#define DCTX_STRTAB offsetof(dt_dctx_t, strtab)
#define DCTX_AGG offsetof(dt_dctx_t, agg)
#define DCTX_GVARS offsetof(dt_dctx_t, gvars)
diff --git a/libdtrace/dt_dlibs.c b/libdtrace/dt_dlibs.c
index 2079e526e9bc..cc5894d2b1c3 100644
--- a/libdtrace/dt_dlibs.c
+++ b/libdtrace/dt_dlibs.c
@@ -83,6 +83,7 @@ static const dt_ident_t dt_bpf_symbols[] = {
DT_BPF_SYMBOL(gvars, DT_IDENT_PTR),
DT_BPF_SYMBOL(lvars, DT_IDENT_PTR),
DT_BPF_SYMBOL(mem, DT_IDENT_PTR),
+ DT_BPF_SYMBOL(null, DT_IDENT_PTR),
DT_BPF_SYMBOL(probes, DT_IDENT_PTR),
DT_BPF_SYMBOL(scratchmem, DT_IDENT_PTR),
DT_BPF_SYMBOL(specs, DT_IDENT_PTR),
diff --git a/libdtrace/dt_parser.c b/libdtrace/dt_parser.c
index 945223083844..a4ded3354cbe 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/codegen/tst.stack_layout.r b/test/unittest/codegen/tst.stack_layout.r
index 40cf0d5c1a2f..ebf439c2bb1b 100644
--- a/test/unittest/codegen/tst.stack_layout.r
+++ b/test/unittest/codegen/tst.stack_layout.r
@@ -1,12 +1,12 @@
Base: 0
-dctx: -80
-%r0: -88
-%r1: -96
-%r2: -104
-%r3: -112
-%r4: -120
-%r5: -128
-%r6: -136
-%r7: -144
-%r8: -152
-scratch: -153 .. -512
+dctx: -88
+%r0: -96
+%r1: -104
+%r2: -112
+%r3: -120
+%r4: -128
+%r5: -136
+%r6: -144
+%r7: -152
+%r8: -160
+scratch: -161 .. -512
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 60%
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..e03e5d6373ec 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,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 via an lvalue.
*
* SECTION: Actions and Subroutines/alloca()
*/
@@ -16,13 +15,13 @@
BEGIN
{
- a = (char *) alloca(1);
+ s = (char *) alloca(10);
+ s = NULL;
+ s[0] = 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
--
2.35.0.260.gb82b153193.dirty
More information about the DTrace-devel
mailing list