[DTrace-devel] [PATCH 3/3] Add support for copyoutstr() subroutine
Kris Van Hees
kris.van.hees at oracle.com
Sat Feb 4 05:08:52 UTC 2023
On Fri, Feb 03, 2023 at 10:31:31PM -0500, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> While the semantics of copyoutstr() are generally obvious, it is
> not stated how NUL terminating bytes are handled. We are told that
> the size is limited to strsize. Meanwhile, if a string has strsize
> chars, we typically expect a NUL byte in addition to them. So,
> let us say that the size specifies the number of bytes, and that
> a NUL byte follows them. This is a slight departure from Solaris,
> which in cases does not write a terminating byte.
No... The maxlen passed (with upper bound strsize) is the maximum total
number of bytes written.
The existing implementations of copyoutstr() (Solaris and the legacy Linux
version) both do *NOT* guarantee that a terminating NUL byte is present.
Copying takes place until a NUL byte is encountered in the source string (and
that NUL Byte is copied) unless the maxlen limit is hit first. I.e. if the
NUL Byte occurs in the source string beyond maxlen bytes, the data that is
written with this subroutine will not terminate in a NUL byte.
> Move tests to their own copyoutstr/ subdirectory to mimic other copy*
> tests.
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> libdtrace/dt_cg.c | 103 +++++++++++++++++-
> .../err.D_PROTO_LEN.copyoutstrbadarg.d | 0
> .../err.D_PROTO_LEN.copyoutstrbadarg.r | 2 +
> .../err.D_PROTO_LEN.copyoutstrtoofew.d | 0
> .../err.D_PROTO_LEN.copyoutstrtoofew.r | 2 +
> .../err.copyoutstrbadaddr.aarch64.x | 0
> .../{ => copyoutstr}/err.copyoutstrbadaddr.sh | 0
> .../funcs/copyoutstr/tst.copyoutstr.r | 21 ++++
> .../funcs/copyoutstr/tst.copyoutstr.sh | 79 ++++++++++++++
> .../funcs/err.D_PROTO_LEN.copyoutstrbadarg.r | 2 -
> .../funcs/err.D_PROTO_LEN.copyoutstrtoofew.r | 2 -
> .../err.D_ACT_SPEC.SpeculateWithCopyOutStr.d | 4 +-
> 12 files changed, 208 insertions(+), 7 deletions(-)
> rename test/unittest/funcs/{ => copyoutstr}/err.D_PROTO_LEN.copyoutstrbadarg.d (100%)
> create mode 100644 test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrbadarg.r
> rename test/unittest/funcs/{ => copyoutstr}/err.D_PROTO_LEN.copyoutstrtoofew.d (100%)
> create mode 100644 test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrtoofew.r
> rename test/unittest/funcs/{ => copyoutstr}/err.copyoutstrbadaddr.aarch64.x (100%)
> rename test/unittest/funcs/{ => copyoutstr}/err.copyoutstrbadaddr.sh (100%)
> create mode 100644 test/unittest/funcs/copyoutstr/tst.copyoutstr.r
> create mode 100755 test/unittest/funcs/copyoutstr/tst.copyoutstr.sh
> delete mode 100644 test/unittest/funcs/err.D_PROTO_LEN.copyoutstrbadarg.r
> delete mode 100644 test/unittest/funcs/err.D_PROTO_LEN.copyoutstrtoofew.r
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 44209a05..48aaf179 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -4755,6 +4755,107 @@ dt_cg_subr_copyout(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> TRACE_REGSET(" subr-copyout:End ");
> }
>
> +static void
> +dt_cg_subr_copyoutstr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> +{
> + dt_node_t *src = dnp->dn_args;
> + dt_node_t *dst = src->dn_list;
> + dt_node_t *size = dst->dn_list;
> + size_t strsize = yypcb->pcb_hdl->dt_options[DTRACEOPT_STRSIZE];
> + uint64_t off;
> + uint_t L1 = dt_irlist_label(dlp);
> + uint_t L2 = dt_irlist_label(dlp);
> + uint_t L3 = dt_irlist_label(dlp);
> + uint_t L4 = dt_irlist_label(dlp);
> + uint_t L5 = dt_irlist_label(dlp);
> +
> + dt_cg_clsflags(yypcb, DTRACEACT_PROC_DESTRUCTIVE, dnp);
> +
> + TRACE_REGSET(" subr-copyoutstr:Begin");
> +
> + dt_cg_node(src, dlp, drp);
> + dt_cg_node(dst, dlp, drp);
> + dt_cg_node(size, dlp, drp);
> +
> + /* if size<=0 || size>strsize; size = strsize */
> + emit(dlp, BPF_BRANCH_IMM(BPF_JSLE, size->dn_reg, 0, L1));
> + emit(dlp, BPF_BRANCH_IMM(BPF_JLE, size->dn_reg, strsize, L2));
> + emitl(dlp, L1,
> + BPF_NOP());
> + emit(dlp, BPF_MOV_IMM(size->dn_reg, strsize));
> + emitl(dlp, L2,
> + BPF_NOP());
> +
> + /* Validate the pointers. */
> + dt_cg_check_ptr_arg(dlp, drp, src, size);
> + dt_cg_check_notnull(dlp, drp, dst->dn_reg);
> +
> + /* make room for a NUL byte */
> + emit(dlp, BPF_ALU64_IMM(BPF_ADD, size->dn_reg, 1));
> +
> + /* Determine the string length, copying into a temporary string. */
> + if (dt_regset_xalloc_args(drp) == -1)
> + longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> + emit(dlp, BPF_MOV_REG(BPF_REG_2, size->dn_reg));
> + emit(dlp, BPF_MOV_REG(BPF_REG_3, src->dn_reg));
> + off = dt_cg_tstring_xalloc(yypcb);
> + emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_FP, DT_STK_DCTX));
> + emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_1, DCTX_MEM));
> + emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, off));
> + dt_regset_xalloc(drp, BPF_REG_0);
> + emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read_str));
> + dt_regset_free_args(drp);
> +
> + /* Use the return value to set the size. */
> + emit(dlp, BPF_BRANCH_IMM(BPF_JSGT, BPF_REG_0, 0, L3));
> + dt_cg_probe_error(yypcb, DTRACEFLT_BADADDR, DT_ISREG, src->dn_reg); // FIXME: is this right?
> + emitl(dlp, L3,
> + BPF_NOP());
> + emit(dlp, BPF_BRANCH_REG(BPF_JGT, BPF_REG_0, size->dn_reg, L4));
> + emit(dlp, BPF_MOV_REG(size->dn_reg, BPF_REG_0));
> + emitl(dlp, L4,
> + BPF_NOP());
> + dt_regset_free(drp, BPF_REG_0);
> +
> + /* Now do the copy with the adjusted size. */
> + if (dt_regset_xalloc_args(drp) == -1)
> + longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> + emit(dlp, BPF_MOV_REG(BPF_REG_1, dst->dn_reg));
> +#if 0
> + /*
> + * The problem with this is that we might be reading a gvar string,
> + * which might not have a terminating NUL. Instead, let us use the
> + * copy in the tstring, which we just made and has a terminating NUL.
> + */
> + emit(dlp, BPF_MOV_REG(BPF_REG_2, src->dn_reg));
> +#else
> + emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_2, BPF_REG_FP, DT_STK_DCTX));
> + emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_2, BPF_REG_2, DCTX_MEM));
> + emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, off));
> +#endif
> + emit(dlp, BPF_MOV_REG(BPF_REG_3, size->dn_reg));
> + dt_regset_xalloc(drp, BPF_REG_0);
> + emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_write_user));
> + dt_regset_free_args(drp);
> +
> + /*
> + * At this point the src is validated, so any problem must be with
> + * the dst address.
> + */
> + emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, L5));
> + dt_regset_free(drp, BPF_REG_0);
> + dt_cg_probe_error(yypcb, DTRACEFLT_BADADDR, DT_ISREG, dst->dn_reg);
> + emitl(dlp, L5,
> + BPF_NOP());
> +
> + dt_regset_free(drp, src->dn_reg);
> + dt_regset_free(drp, dst->dn_reg);
> + dt_regset_free(drp, size->dn_reg);
> + dt_cg_tstring_xfree(yypcb, off);
> +
> + TRACE_REGSET(" subr-copyoutstr:End ");
> +}
> +
> static void
> dt_cg_subr_strchr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> {
> @@ -5232,7 +5333,7 @@ static dt_cg_subr_f *_dt_cg_subr[DIF_SUBR_MAX + 1] = {
> [DIF_SUBR_PROGENYOF] = &dt_cg_subr_progenyof,
> [DIF_SUBR_STRLEN] = &dt_cg_subr_strlen,
> [DIF_SUBR_COPYOUT] = &dt_cg_subr_copyout,
> - [DIF_SUBR_COPYOUTSTR] = NULL,
> + [DIF_SUBR_COPYOUTSTR] = &dt_cg_subr_copyoutstr,
> [DIF_SUBR_ALLOCA] = &dt_cg_subr_alloca,
> [DIF_SUBR_BCOPY] = &dt_cg_subr_bcopy,
> [DIF_SUBR_COPYINTO] = &dt_cg_subr_copyinto,
> diff --git a/test/unittest/funcs/err.D_PROTO_LEN.copyoutstrbadarg.d b/test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrbadarg.d
> similarity index 100%
> rename from test/unittest/funcs/err.D_PROTO_LEN.copyoutstrbadarg.d
> rename to test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrbadarg.d
> diff --git a/test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrbadarg.r b/test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrbadarg.r
> new file mode 100644
> index 00000000..6b36f9bf
> --- /dev/null
> +++ b/test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrbadarg.r
> @@ -0,0 +1,2 @@
> +-- @@stderr --
> +dtrace: failed to compile script test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrbadarg.d: [D_PROTO_LEN] line 19: copyoutstr( ) prototype mismatch: 2 args passed, 3 expected
> diff --git a/test/unittest/funcs/err.D_PROTO_LEN.copyoutstrtoofew.d b/test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrtoofew.d
> similarity index 100%
> rename from test/unittest/funcs/err.D_PROTO_LEN.copyoutstrtoofew.d
> rename to test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrtoofew.d
> diff --git a/test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrtoofew.r b/test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrtoofew.r
> new file mode 100644
> index 00000000..7b205988
> --- /dev/null
> +++ b/test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrtoofew.r
> @@ -0,0 +1,2 @@
> +-- @@stderr --
> +dtrace: failed to compile script test/unittest/funcs/copyoutstr/err.D_PROTO_LEN.copyoutstrtoofew.d: [D_PROTO_LEN] line 19: copyoutstr( ) prototype mismatch: 1 arg passed, 3 expected
> diff --git a/test/unittest/funcs/err.copyoutstrbadaddr.aarch64.x b/test/unittest/funcs/copyoutstr/err.copyoutstrbadaddr.aarch64.x
> similarity index 100%
> rename from test/unittest/funcs/err.copyoutstrbadaddr.aarch64.x
> rename to test/unittest/funcs/copyoutstr/err.copyoutstrbadaddr.aarch64.x
> diff --git a/test/unittest/funcs/err.copyoutstrbadaddr.sh b/test/unittest/funcs/copyoutstr/err.copyoutstrbadaddr.sh
> similarity index 100%
> rename from test/unittest/funcs/err.copyoutstrbadaddr.sh
> rename to test/unittest/funcs/copyoutstr/err.copyoutstrbadaddr.sh
> diff --git a/test/unittest/funcs/copyoutstr/tst.copyoutstr.r b/test/unittest/funcs/copyoutstr/tst.copyoutstr.r
> new file mode 100644
> index 00000000..1c756b47
> --- /dev/null
> +++ b/test/unittest/funcs/copyoutstr/tst.copyoutstr.r
> @@ -0,0 +1,21 @@
> +size is longer than the string; expect full string
> +HELLO WORLD; YOU HAVE A LONG MESSAGE TO DELIVER|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> +
> +size is shorter than the string; expect 32 chars
> +HELLO WORLD; YOU HAVE A LONG MES|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> +
> +size is negative; expect 16 chars
> +HELLO WORLD; YOU|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> +
> +size is zero; expect 16 chars
> +HELLO WORLD; YOU|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> +
> +size is small; expect 12 chars
> +HELLO WORLD;|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> +
> +size is strsize; expect 16 chars
> +HELLO WORLD; YOU|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> +
> +size is large; expect 16 chars
> +HELLO WORLD; YOU|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> +
> diff --git a/test/unittest/funcs/copyoutstr/tst.copyoutstr.sh b/test/unittest/funcs/copyoutstr/tst.copyoutstr.sh
> new file mode 100755
> index 00000000..d7a00834
> --- /dev/null
> +++ b/test/unittest/funcs/copyoutstr/tst.copyoutstr.sh
> @@ -0,0 +1,79 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2023, 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.
> +
> +dtrace=$1
> +DIRNAME="$tmpdir/copyoutstr.$$.$RANDOM"
> +mkdir -p $DIRNAME
> +cd $DIRNAME
> +
> +# Construct a C program that:
> +# - initializes a user buffer
> +# - passes that buffer to a syscall (where DTrace will overwrite the buffer)
> +# - checks the buffer
> +cat << EOF > main.c
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <string.h>
> +int main(int c, char **v) {
> + int i, fd = open("/dev/null", O_WRONLY);
> + char s[256];
> +
> + /* initialize buffer */
> + memset(s, '-', sizeof(s));
> + s[sizeof(s) - 1] = '\0';
> +
> + /* write buffer to /dev/null (the D script will overwrite s) */
> + write(fd, s, strlen(s));
> + close(fd);
> +
> + /* turn NUL chars (except last one) into printable '|' */
> + for (i = 0; i < sizeof(s) - 1; i++)
> + if (s[i] == '\0')
> + s[i] = '|';
> +
> + /* print buffer contents */
> + printf("%s\n", s);
> + return 0;
> +}
> +EOF
> +
> +gcc main.c
> +if [ $? -ne 0 ]; then
> + echo "compilation error"
> + exit 1
> +fi
> +
> +# $1 is copyoutstr() size
> +# $2 is DTrace options
> +# $3 is description of test
> +function mytest() {
> + echo $3
> + $dtrace $dt_flags $2 -qwn '
> + syscall::write:entry
> + /pid == $target/
> + {
> + s = "HELLO WORLD; YOU HAVE A LONG MESSAGE TO DELIVER";
> + copyoutstr(s, arg1, '$1');
> + exit(0);
> + }' -c ./a.out
> + if [ $? -ne 0 ]; then
> + echo "DTrace error"
> + exit 1
> + fi
> +}
> +
> +mytest 64 " " "size is longer than the string; expect full string"
> +mytest 32 " " "size is shorter than the string; expect 32 chars"
> +
> +mytest -1 "-xstrsize=16" "size is negative; expect 16 chars"
> +mytest 0 "-xstrsize=16" "size is zero; expect 16 chars"
> +mytest 12 "-xstrsize=16" "size is small; expect 12 chars"
> +mytest 16 "-xstrsize=16" "size is strsize; expect 16 chars "
> +mytest 512 "-xstrsize=16" "size is large; expect 16 chars"
> +
> +exit 0
> diff --git a/test/unittest/funcs/err.D_PROTO_LEN.copyoutstrbadarg.r b/test/unittest/funcs/err.D_PROTO_LEN.copyoutstrbadarg.r
> deleted file mode 100644
> index f13ac2ed..00000000
> --- a/test/unittest/funcs/err.D_PROTO_LEN.copyoutstrbadarg.r
> +++ /dev/null
> @@ -1,2 +0,0 @@
> --- @@stderr --
> -dtrace: failed to compile script test/unittest/funcs/err.D_PROTO_LEN.copyoutstrbadarg.d: [D_PROTO_LEN] line 19: copyoutstr( ) prototype mismatch: 2 args passed, 3 expected
> diff --git a/test/unittest/funcs/err.D_PROTO_LEN.copyoutstrtoofew.r b/test/unittest/funcs/err.D_PROTO_LEN.copyoutstrtoofew.r
> deleted file mode 100644
> index fce3d846..00000000
> --- a/test/unittest/funcs/err.D_PROTO_LEN.copyoutstrtoofew.r
> +++ /dev/null
> @@ -1,2 +0,0 @@
> --- @@stderr --
> -dtrace: failed to compile script test/unittest/funcs/err.D_PROTO_LEN.copyoutstrtoofew.d: [D_PROTO_LEN] line 19: copyoutstr( ) prototype mismatch: 1 arg passed, 3 expected
> diff --git a/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOutStr.d b/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOutStr.d
> index 1b23ec3b..43a41440 100644
> --- a/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOutStr.d
> +++ b/test/unittest/speculation/err.D_ACT_SPEC.SpeculateWithCopyOutStr.d
> @@ -1,11 +1,11 @@
> #!/usr/sbin/dtrace -ws
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2006, 2021, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2006, 2023, 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.
> */
> -/* @@skip: dtv2, no copyoutstr yet */
> +
> /*
> * ASSERTION: Destructive actions may never be speculative.
> *
> --
> 2.18.4
>
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list