[DTrace-devel] [PATCH 03/22] cg, test: fix copyinstr() implementation to use a tstring
Eugene Loh
eugene.loh at oracle.com
Sat Jan 14 04:53:50 UTC 2023
I guess I'm basically fine with this patch, except for one thing. The
new test copyinstr/tst.copyinstr-high-maxsize.d copies existing
copyinstr() tests, but I think those tests should be cleaned up before
they are propagated further. All that is the subject of another patch I
posted; I'll try to integrate your comments there. Ideally, we can
converge on that other patch first, and then this one can go on top of them.
Also, files need 2023 copyrights.
On 1/12/23 20:12, Kris Van Hees via DTrace-devel wrote:
> Since copyinstr() returns a string, it should be using a temporary
> string like other string functions. Also, semantics needed fixing to
> apply strsize as an upper limit to the optional size rather than
> triggering a fault if the size is greater than strsize.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> libdtrace/dt_cg.c | 23 +++++++------
> libdtrace/dt_open.c | 2 +-
> .../funcs/copyinstr/err.D_ALLOCA_SIZE.d | 26 --------------
> .../copyinstr/tst.copyinstr-high-maxsize.d | 34 +++++++++++++++++++
> .../copyinstr/tst.copyinstr-high-maxsize.r | 3 ++
> 5 files changed, 50 insertions(+), 38 deletions(-)
> delete mode 100644 test/unittest/funcs/copyinstr/err.D_ALLOCA_SIZE.d
> create mode 100644 test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.d
> create mode 100644 test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.r
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index a23731d5..fbdc6bce 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -4518,7 +4518,7 @@ dt_cg_subr_copyinstr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> dt_node_t *size = src->dn_list;
> int maxsize = dtp->dt_options[DTRACEOPT_STRSIZE];
> uint_t lbl_ok = dt_irlist_label(dlp);
> - uint_t lbl_badsize = dt_irlist_label(dlp);
> + uint_t lbl_oksize = dt_irlist_label(dlp);
>
> TRACE_REGSET(" subr-copyinstr:Begin");
>
> @@ -4530,15 +4530,19 @@ dt_cg_subr_copyinstr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>
> dt_cg_node(size, dlp, drp);
>
> - emit(dlp, BPF_BRANCH_IMM(BPF_JSLT, size->dn_reg, 0, lbl_badsize));
> - emit(dlp, BPF_BRANCH_IMM(BPF_JGT, size->dn_reg, maxsize, lbl_badsize));
> + emit(dlp, BPF_BRANCH_IMM(BPF_JLE, size->dn_reg, maxsize, lbl_oksize));
> + emit(dlp, BPF_MOV_IMM(size->dn_reg, maxsize));
> + emitl(dlp, lbl_oksize,
> + BPF_NOP());
>
> - if ((dnp->dn_reg = dt_regset_alloc(drp)) == -1)
> + /* Allocate a temporary string for the destination (return value). */
> + dnp->dn_reg = dt_regset_alloc(drp);
> + if (dnp->dn_reg == -1)
> longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> -
> - /* Allocate scratch space. */
> - dt_cg_subr_alloca_impl(dnp, size, dlp, drp);
> - dt_cg_alloca_ptr(dlp, drp, dnp->dn_reg, dnp->dn_reg);
> + dt_cg_tstring_alloc(yypcb, dnp);
> + emit(dlp, BPF_LOAD(BPF_DW, dnp->dn_reg, BPF_REG_FP, DT_STK_DCTX));
> + emit(dlp, BPF_LOAD(BPF_DW, dnp->dn_reg, dnp->dn_reg, DCTX_MEM));
> + emit(dlp, BPF_ALU64_IMM(BPF_ADD, dnp->dn_reg, dnp->dn_tstring->dn_value));
>
> dt_cg_node(src, dlp, drp);
>
> @@ -4553,9 +4557,6 @@ dt_cg_subr_copyinstr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> dt_regset_free_args(drp);
> emit(dlp, BPF_BRANCH_IMM(BPF_JSGT, BPF_REG_0, 0, lbl_ok));
> dt_cg_probe_error(yypcb, DTRACEFLT_BADADDR, DT_ISREG, src->dn_reg);
> - emitl(dlp, lbl_badsize,
> - BPF_NOP());
> - dt_cg_probe_error(yypcb, DTRACEFLT_BADSIZE, DT_ISREG, size->dn_reg);
> emitl(dlp, lbl_ok,
> BPF_NOP());
> dt_regset_free(drp, BPF_REG_0);
> diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
> index a3754a32..8cd861e9 100644
> --- a/libdtrace/dt_open.c
> +++ b/libdtrace/dt_open.c
> @@ -131,7 +131,7 @@ static const dt_ident_t _dtrace_globals[] = {
> &dt_idops_func, "void(int)" },
> { "copyin", DT_IDENT_FUNC, 0, DIF_SUBR_COPYIN, DT_ATTR_STABCMN, DT_VERS_1_0,
> &dt_idops_func, "void *(uintptr_t, size_t)" },
> -{ "copyinstr", DT_IDENT_FUNC, 0, DIF_SUBR_COPYINSTR,
> +{ "copyinstr", DT_IDENT_FUNC, DT_IDFLG_DPTR, DIF_SUBR_COPYINSTR,
> DT_ATTR_STABCMN, DT_VERS_1_0,
> &dt_idops_func, "string(uintptr_t, [size_t])" },
> { "copyinto", DT_IDENT_FUNC, 0, DIF_SUBR_COPYINTO, DT_ATTR_STABCMN,
> diff --git a/test/unittest/funcs/copyinstr/err.D_ALLOCA_SIZE.d b/test/unittest/funcs/copyinstr/err.D_ALLOCA_SIZE.d
> deleted file mode 100644
> index c9f25721..00000000
> --- a/test/unittest/funcs/copyinstr/err.D_ALLOCA_SIZE.d
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -/*
> - * 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: The copyinstr() size must not exceed the (adjusted) scratchsize.
> - *
> - * SECTION: Actions and Subroutines/copyinstr()
> - * User Process Tracing/copyin() and copyinstr()
> - */
> -
> -#pragma D option scratchsize=64
> -
> -BEGIN
> -{
> - copyinstr(0, 65);
> - exit(0);
> -}
> -
> -ERROR
> -{
> - exit(1);
> -}
> diff --git a/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.d b/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.d
> new file mode 100644
> index 00000000..c55aeecd
> --- /dev/null
> +++ b/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.d
> @@ -0,0 +1,34 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2006, 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: It is possible to read a string from userspace addresses.
> + *
> + * SECTION: Actions and Subroutines/copyinstr()
> + * User Process Tracing/copyin() and copyinstr() Subroutines
> + */
> +
> +#pragma D option quiet
> +#pragma D option destructive
> +#pragma D option strsize=64
> +
> +BEGIN
> +{
> + system("echo dtrace-copyinstr-test");
> +}
> +
> +syscall::write:entry
> +/(s = copyinstr(arg1, 96))[6] == '-'/
> +{
> + printf("'%s'", s);
> + exit(0);
> +}
> +
> +ERROR
> +{
> + exit(1);
> +}
> diff --git a/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.r b/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.r
> new file mode 100644
> index 00000000..2be678ab
> --- /dev/null
> +++ b/test/unittest/funcs/copyinstr/tst.copyinstr-high-maxsize.r
> @@ -0,0 +1,3 @@
> +dtrace-copyinstr-test
> +'dtrace-copyinstr-test
> +'
More information about the DTrace-devel
mailing list