[DTrace-devel] [PATCH v4 10/10] alloca: allow passing alloca pointers to actions and subrs
Kris Van Hees
kris.van.hees at oracle.com
Wed Apr 13 18:00:00 UTC 2022
This patch still contains the splitting out of the dt_strlen() call into
dt_cg_strlen() which is no longer needed because there is only one place
where it is used: dt_cg_subr_strlen(). Just stick with the original code
where that functionality was in dt_cg_subr_strlen() itself.
On Tue, Apr 12, 2022 at 11:59:09AM +0100, Nick Alcock wrote:
> A great many subrs and a good few actions allow the passing-in of
> pointers (mostly, but not entirely, char *) which may be in alloca()ed
> space. Since they then usually go on to dereference these pointers,
> the pointers must be bounds-checked. We already handled actions
> earlier (they don't do formal dereferences except insofar as it
> is already done via the deref operator, so we only needed to check
> them for nullity); but subrs remain unhandled.
>
> We for non-alloca pointers we can just keep on doing null checking
> (we split out the guts of strlen into a new dt_cg_strlen against
> possible future need for dynamic checks); for alloca pointers,
> we need to do the usual access_check/ptr dance, with a length derived
> from the parser's idea of the length (which is passed through
> identifiers accurately).
>
> In addition to subrs, we also need to handle alloca'ed pointers in
> codegen for stringof(), since the arg to stringof() might be in
> alloca()ed space too.
>
> A new test that tests more or less all of this is added: it has no
> expected results because all we actually care about is that there are no
> verifier failures.
>
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
> libdtrace/dt_cg.c | 95 ++++++----
> test/unittest/funcs/alloca/tst.alloca-funcs.d | 164 ++++++++++++++++++
> test/unittest/funcs/alloca/tst.alloca-funcs.r | 2 +
> .../unittest/funcs/alloca/tst.string-alloca.d | 29 ++++
> .../unittest/funcs/alloca/tst.string-alloca.r | 1 +
> 5 files changed, 260 insertions(+), 31 deletions(-)
> create mode 100644 test/unittest/funcs/alloca/tst.alloca-funcs.d
> create mode 100644 test/unittest/funcs/alloca/tst.alloca-funcs.r
> create mode 100644 test/unittest/funcs/alloca/tst.string-alloca.d
> create mode 100644 test/unittest/funcs/alloca/tst.string-alloca.r
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 847860072c7a..5960214ff711 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -28,6 +28,7 @@
> #define DT_ISREG 1
>
> static void dt_cg_node(dt_node_t *, dt_irlist_t *, dt_regset_t *);
> +static void dt_cg_strlen(dt_irlist_t *dlp, dt_regset_t *drp, int str);
>
> /*
> * Generate the generic prologue of the trampoline BPF program.
> @@ -957,6 +958,46 @@ dt_cg_alloca_ptr(dt_irlist_t *dlp, dt_regset_t *drp, int dreg, int sreg)
> }
> }
>
> +/*
> + * Convert an alloca pointer value into an actual scratchmem pointer.
> + * Generate code to check a pointer argument (specifically for subroutine
> + * arguments) to ensure that it is not NULL. If the arguent pointer is an
> + * alloca pointer, we also need to perform an access check and convert the
> + * alloca pointer value into a real pointer.
> + */
> +static void
> +dt_cg_check_ptr_arg(dt_irlist_t *dlp, dt_regset_t *drp, dt_node_t *dnp)
> +{
> + if (dnp->dn_flags & DT_NF_ALLOCA) {
> + dtrace_diftype_t vtype;
> +
> + dt_node_diftype(yypcb->pcb_hdl, dnp, &vtype);
> + dt_cg_alloca_access_check(dlp, drp, dnp->dn_reg,
> + DT_ISIMM, vtype.dtdt_size);
> + dt_cg_alloca_ptr(dlp, drp, dnp->dn_reg, dnp->dn_reg);
> + } else
> + dt_cg_check_notnull(dlp, drp, dnp->dn_reg);
> +}
> +
> +/*
> + * Generate code to take the length of the string in reg STR: return it in
> + * BPF_REG_0 (which must be xalloced before the call).
> + */
> +static void
> +dt_cg_strlen(dt_irlist_t *dlp, dt_regset_t *drp, int str)
> +{
> + dt_ident_t *idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_strlen");
> +
> + assert(idp != NULL);
> +
> + if (dt_regset_xalloc_args(drp) == -1)
> + longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> + emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_FP, DT_STK_DCTX));
> + emit(dlp, BPF_MOV_REG(BPF_REG_2, str));
> + emite(dlp, BPF_CALL_FUNC(idp->di_id), idp);
> + dt_regset_free_args(drp);
> +}
> +
> static const uint_t ldstw[] = {
> 0,
> BPF_B, BPF_H, 0, BPF_W,
> @@ -3684,10 +3725,10 @@ dt_cg_subr_path_helper(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>
> TRACE_REGSET(" subr-path_helper:Begin");
> dt_cg_node(str, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, str->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, str);
>
> /*
> - * The result needs be be a temporary string, so we request one.
> + * The result needs to be a temporary string, so we request one.
> */
> dnp->dn_reg = dt_regset_alloc(drp);
> if (dnp->dn_reg == -1)
> @@ -3743,9 +3784,9 @@ dt_cg_subr_index(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> TRACE_REGSET(" subr-index:Begin");
>
> dt_cg_node(s, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, s->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, s);
> dt_cg_node(t, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, t->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, t);
> if (start != NULL)
> dt_cg_node(start, dlp, drp);
>
> @@ -3859,9 +3900,9 @@ dt_cg_subr_rindex(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>
> /* evaluate arguments to D subroutine rindex() */
> dt_cg_node(s, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, s->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, s);
> dt_cg_node(t, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, t->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, t);
> if (start != NULL)
> dt_cg_node(start, dlp, drp);
>
> @@ -4129,7 +4170,7 @@ dt_cg_subr_strchr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>
> TRACE_REGSET(" subr-strchr:Begin");
> dt_cg_node(str, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, str->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, str);
> dt_cg_node(chr, dlp, drp);
>
> if (dt_regset_xalloc_args(drp) == -1)
> @@ -4142,7 +4183,7 @@ dt_cg_subr_strchr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> dt_regset_free(drp, chr->dn_reg);
>
> /*
> - * The result needs be be a temporary string, so we request one.
> + * The result needs to be a temporary string, so we request one.
> */
> dnp->dn_reg = dt_regset_alloc(drp);
> if (dnp->dn_reg == -1)
> @@ -4185,7 +4226,7 @@ dt_cg_subr_strrchr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>
> TRACE_REGSET(" subr-strrchr:Begin");
> dt_cg_node(str, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, str->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, str);
> dt_cg_node(chr, dlp, drp);
>
> if (dt_regset_xalloc_args(drp) == -1)
> @@ -4198,7 +4239,7 @@ dt_cg_subr_strrchr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> dt_regset_free(drp, chr->dn_reg);
>
> /*
> - * The result needs be be a temporary string, so we request one.
> + * The result needs to be a temporary string, so we request one.
> */
> dnp->dn_reg = dt_regset_alloc(drp);
> if (dnp->dn_reg == -1)
> @@ -4228,28 +4269,19 @@ dt_cg_subr_strrchr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> static void
> dt_cg_subr_strlen(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> {
> - dt_ident_t *idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_strlen");
> dt_node_t *str = dnp->dn_args;
>
> - assert(idp != NULL);
> -
> TRACE_REGSET(" subr-strlen:Begin");
>
> dt_cg_node(str, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, str->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, str);
> dnp->dn_reg = str->dn_reg; /* re-use register */
>
> - if (dt_regset_xalloc_args(drp) == -1)
> - longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> -
> - emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_FP, DT_STK_DCTX));
> - emit(dlp, BPF_MOV_REG(BPF_REG_2, str->dn_reg));
> - dt_regset_free(drp, str->dn_reg);
> - dt_cg_tstring_free(yypcb, str);
> dt_regset_xalloc(drp, BPF_REG_0);
> - emite(dlp,BPF_CALL_FUNC(idp->di_id), idp);
> + dt_cg_strlen(dlp, drp, str->dn_reg);
> + dt_cg_tstring_free(yypcb, str);
>
> - dt_regset_free_args(drp);
> + dt_regset_free (drp, str->dn_reg);
> dnp->dn_reg = dt_regset_alloc(drp);
> if (dnp->dn_reg == -1)
> longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> @@ -4271,12 +4303,12 @@ dt_cg_subr_strjoin(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> TRACE_REGSET(" subr-strjoin:Begin");
>
> dt_cg_node(s1, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, s1->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, s1);
> dt_cg_node(s2, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, s2->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, s2);
>
> /*
> - * The result needs be be a temporary string, so we request one.
> + * The result needs to be a temporary string, so we request one.
> */
> dnp->dn_reg = dt_regset_alloc(drp);
> if (dnp->dn_reg == -1)
> @@ -4318,9 +4350,9 @@ dt_cg_subr_strstr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>
> /* get args */
> dt_cg_node(s, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, s->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, s);
> dt_cg_node(t, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, t->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, t);
>
> /* call dt_index() call */
> if (dt_regset_xalloc_args(drp) == -1)
> @@ -4421,7 +4453,7 @@ dt_cg_subr_strtok(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> if (str->dn_op != DT_TOK_INT || str->dn_value != 0) {
> /* string is present: copy it to internal state */
> dt_cg_node(str, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, str->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, str);
>
> if (dt_regset_xalloc_args(drp) == -1)
> longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> @@ -4450,7 +4482,7 @@ dt_cg_subr_strtok(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>
> /* get delimiters */
> dt_cg_node(del, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, del->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, del);
>
> /* allocate temporary string for result */
> dnp->dn_reg = dt_regset_alloc(drp);
> @@ -4501,7 +4533,7 @@ dt_cg_subr_substr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> TRACE_REGSET(" subr-substr:Begin");
>
> dt_cg_node(str, dlp, drp);
> - dt_cg_check_notnull(dlp, drp, str->dn_reg);
> + dt_cg_check_ptr_arg(dlp, drp, str);
> dt_cg_node(idx, dlp, drp);
> if (cnt != NULL)
> dt_cg_node(cnt, dlp, drp);
> @@ -4956,6 +4988,7 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>
> case DT_TOK_STRINGOF:
> dt_cg_node(dnp->dn_child, dlp, drp);
> + dt_cg_check_ptr_arg(dlp, drp, dnp->dn_child);
> dnp->dn_reg = dnp->dn_child->dn_reg;
> break;
>
> diff --git a/test/unittest/funcs/alloca/tst.alloca-funcs.d b/test/unittest/funcs/alloca/tst.alloca-funcs.d
> new file mode 100644
> index 000000000000..abeaa0463d36
> --- /dev/null
> +++ b/test/unittest/funcs/alloca/tst.alloca-funcs.d
> @@ -0,0 +1,164 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/*
> + * ASSERTION: Function calls to alloca()ed space work.
> + *
> + * SECTION: Actions and Subroutines/alloca()
> + */
> +
> +#pragma D option quiet
> +
> +/*
> + * Everything is in an independent clause with its own alloca(),
> + * to try to make sure that the verifier's bound proofs don't leak
> + * from one test to the next.
> + */
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + x[0] = 'a';
> + x[1] = '/';
> + x[2] = 'b';
> + x[3] = 0;
> + printf("%s\n", stringof(x));
> + trace(x);
> +}
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + x[0] = 'a';
> + x[1] = '/';
> + x[2] = 'b';
> + x[3] = 0;
> + trace(basename(x));
> +}
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + y = (char *) alloca(8);
> + x[0] = 'a';
> + x[1] = '/';
> + x[2] = 'b';
> + x[3] = 0;
> + y[0] = '/';
> + y[1] = 0;
> + trace(index(x, y));
> +}
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + y = (char *) alloca(8);
> + x[0] = 'a';
> + x[1] = '/';
> + x[2] = 'b';
> + x[3] = 0;
> + y[0] = '/';
> + y[1] = 0;
> + trace(rindex(x, y));
> +}
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + x[0] = 'a';
> + x[1] = '/';
> + x[2] = 'b';
> + x[3] = 0;
> + trace(strchr(x, '/'));
> +}
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + x[0] = 'a';
> + x[1] = '/';
> + x[2] = 'b';
> + x[3] = 0;
> + trace(strrchr(x, '/'));
> +}
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + x[0] = 'a';
> + x[1] = '/';
> + x[2] = 'b';
> + x[3] = 0;
> + trace(strlen(x));
> +}
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + y = (char *) alloca(8);
> + x[0] = 'a';
> + x[1] = '/';
> + x[2] = 'b';
> + x[3] = 0;
> + y[0] = '/';
> + y[1] = 0;
> + trace(strjoin(x, y));
> +}
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + y = (char *) alloca(8);
> + x[0] = 'a';
> + x[1] = '/';
> + x[2] = 'b';
> + x[3] = 0;
> + y[0] = '/';
> + y[1] = 0;
> + trace(strstr(x, y));
> +}
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + y = (char *) alloca(8);
> + x[0] = 'a';
> + x[1] = '/';
> + x[2] = 'b';
> + x[3] = 0;
> + y[0] = '/';
> + y[1] = 0;
> + trace(strtok(x, y));
> +}
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + x[0] = '/';
> + x[1] = 0;
> + trace(strtok(NULL, x));
> +}
> +
> +BEGIN
> +{
> + x = (char *) alloca(8);
> + x[0] = 'a';
> + x[1] = '/';
> + x[2] = 'b';
> + x[3] = 0;
> + trace(substr(x, 0, 1));
> +}
> +
> +BEGIN
> +{
> + exit(0);
> +}
> +
> +ERROR
> +{
> + exit(0);
> +}
> diff --git a/test/unittest/funcs/alloca/tst.alloca-funcs.r b/test/unittest/funcs/alloca/tst.alloca-funcs.r
> new file mode 100644
> index 000000000000..7a618a62aa0a
> --- /dev/null
> +++ b/test/unittest/funcs/alloca/tst.alloca-funcs.r
> @@ -0,0 +1,2 @@
> +a/b
> +8b11/b/b3a/b//baba
> diff --git a/test/unittest/funcs/alloca/tst.string-alloca.d b/test/unittest/funcs/alloca/tst.string-alloca.d
> new file mode 100644
> index 000000000000..96138f4ccb60
> --- /dev/null
> +++ b/test/unittest/funcs/alloca/tst.string-alloca.d
> @@ -0,0 +1,29 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/*
> + * ASSERTION: You can copy a string into an alloca'ed region and read
> + * it out again.
> + *
> + * SECTION: Actions and Subroutines/alloca()
> + */
> +
> +#pragma D option quiet
> +#pragma D option scratchsize=512
> +
> +BEGIN
> +{
> + x = (string *)alloca(sizeof(string) + 1);
> + *x = "abc";
> + trace(*x);
> + exit(0);
> +}
> +
> +ERROR
> +{
> + exit(1);
> +}
> diff --git a/test/unittest/funcs/alloca/tst.string-alloca.r b/test/unittest/funcs/alloca/tst.string-alloca.r
> new file mode 100644
> index 000000000000..8baef1b4abc4
> --- /dev/null
> +++ b/test/unittest/funcs/alloca/tst.string-alloca.r
> @@ -0,0 +1 @@
> +abc
> --
> 2.35.1
More information about the DTrace-devel
mailing list