[DTrace-devel] [PATCH] Need -w for destructive actions, even if clause is not used
Kris Van Hees
kris.van.hees at oracle.com
Fri Aug 1 17:49:23 UTC 2025
Another question...
On Fri, Aug 01, 2025 at 11:36:17AM -0400, Kris Van Hees wrote:
> In addition to Nick's comments...
>
> On Fri, Jul 11, 2025 at 12:40:24AM -0400, eugene.loh at oracle.com wrote:
> > From: Eugene Loh <eugene.loh at oracle.com>
> >
> > If a clause includes a destructive action but -w is not used, dtrace
> > should not start up, even if the clause is ignored (due to -Z).
> > Solaris treated this as a runtime error. We should do the same.
> >
> > The test err.Z_no-w.sh was misguided and is replaced by a more
> > direct test.
> >
> > Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> > ---
> > libdtrace/dt_bpf.c | 14 ++++
> > libdtrace/dt_cg.c | 4 +-
> > libdtrace/dt_impl.h | 1 +
> > .../options/err.no-w-or-destructive2.d | 25 ++++++
> > .../options/err.no-w-or-destructive2.r | 3 +
> > test/unittest/usdt/err.Z_no-w.r | 4 -
> > test/unittest/usdt/err.Z_no-w.sh | 76 -------------------
> > 7 files changed, 46 insertions(+), 81 deletions(-)
> > create mode 100644 test/unittest/options/err.no-w-or-destructive2.d
> > create mode 100644 test/unittest/options/err.no-w-or-destructive2.r
> > delete mode 100644 test/unittest/usdt/err.Z_no-w.r
> > delete mode 100755 test/unittest/usdt/err.Z_no-w.sh
Why did you delete this test? Isn't it still a valid test anyway, and since
it tests this in a more practical sense, it seems to have value.
> >
> > diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> > index 4e7618e05..e2c3bfebc 100644
> > --- a/libdtrace/dt_bpf.c
> > +++ b/libdtrace/dt_bpf.c
> > @@ -1286,6 +1286,15 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> > */
> > dtrace_getopt(dtp, "destructive", &dest_ok);
> >
> > + /*
> > + * If we have any destructive actions at all and -w is not set,
> > + * error out. Solaris would reject this as a runtime error. So,
> > + * although we could have detected this problem at compilation,
> > + * we mimic Solaris and wait until now to report.
> > + */
> > + if (dtp->dt_havedest && dest_ok == DTRACEOPT_UNSET)
> > + return dt_set_errno(dtp, EDT_DESTRUCTIVE);
>
> How about using dt_destructive as name?
>
> > +
> > for (prp = dt_list_next(&dtp->dt_enablings); prp != NULL;
> > prp = dt_list_next(prp)) {
> > int fd;
> > @@ -1304,6 +1313,11 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> >
> > DT_DISASM_PROG_LINKED(dtp, cflags, dp, stderr, NULL, prp->desc);
> >
> > + /*
> > + * This check should never fail since, if any action is
> > + * destructive and -w is not set, we should already have
> > + * failed.
> > + */
> > if (dp->dtdo_flags & DIFOFLG_DESTRUCTIVE &&
> > dest_ok == DTRACEOPT_UNSET)
> > return dt_set_errno(dtp, EDT_DESTRUCTIVE);
>
> If this check should never fail, then it should be removed -or- turned into
> an assert as a safeguard. But rmoving it ought to be OK.
>
> > diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> > index e1bb59289..0e0648969 100644
> > --- a/libdtrace/dt_cg.c
> > +++ b/libdtrace/dt_cg.c
> > @@ -1833,8 +1833,10 @@ dt_cg_clsflags(dt_pcb_t *pcb, dtrace_actkind_t kind, const dt_node_t *dnp)
> > {
> > int *cfp = &pcb->pcb_stmt->dtsd_clauseflags;
> >
> > - if (DTRACEACT_ISDESTRUCTIVE(kind))
> > + if (DTRACEACT_ISDESTRUCTIVE(kind)) {
> > *cfp |= DT_CLSFLAG_DESTRUCT;
> > + pcb->pcb_hdl->dt_havedest |= 1;
>
> dt_destructive
>
> > + }
> >
> > if (kind == DTRACEACT_COMMIT) {
> > if (*cfp & (DT_CLSFLAG_DATAREC | DT_CLSFLAG_AGGREGATION))
> > diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> > index 1e105b6e9..cdb222b3f 100644
> > --- a/libdtrace/dt_impl.h
> > +++ b/libdtrace/dt_impl.h
> > @@ -431,6 +431,7 @@ struct dtrace_hdl {
> > dt_list_t dt_lib_dep_sorted; /* dependency sorted library list */
> > dt_global_pcap_t dt_pcap; /* global tshark/pcap state */
> > char *dt_freopen_filename; /* filename for freopen() action */
> > + int dt_havedest; /* have any destructive actions */
>
> dt_destructive, and add it after dt_droptags to keep the booleans together.
> In a later patch, we should combine those uint_t booleans into a single
> uint_t dt_flags member.
>
> > };
> >
> > /*
> > diff --git a/test/unittest/options/err.no-w-or-destructive2.d b/test/unittest/options/err.no-w-or-destructive2.d
> > new file mode 100644
> > index 000000000..eb9365fea
> > --- /dev/null
> > +++ b/test/unittest/options/err.no-w-or-destructive2.d
> > @@ -0,0 +1,25 @@
> > +/*
> > + * Oracle Linux DTrace.
> > + * Copyright (c) 2025, 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: Without -w or -xdestructive, destructive operations are not ok,
> > + * even if a clause will be ignored since it does not exist and
> > + * -Z was specified.
>
> This could do with some rewriting. The issue is that DTrace used to always
> load all tracing programs, and the kernel would activate them as needed. We
> now only load programs for enabled probes (and with -Z active, we may load
> some programs later).
>
> What we need to be testing here is that if any clauses of a tracing script
> contain destructive actions, and we are using the script for probing, then
> an error should be reported unless we are allowing destructive actions (-w
> or -xdestructive).
>
> > + *
> > + * SECTION: Options and Tunables/Consumer Options
> > + */
> > +/* @@runtest-opts: -Z */
> > +
> > +BEGIN
> > +{
> > + exit(0);
> > +}
> > +
> > +bogus:bogus:bogus:bogus
> > +{
> > + system("echo ok");
> > +}
> > diff --git a/test/unittest/options/err.no-w-or-destructive2.r b/test/unittest/options/err.no-w-or-destructive2.r
> > new file mode 100644
> > index 000000000..1a6959475
> > --- /dev/null
> > +++ b/test/unittest/options/err.no-w-or-destructive2.r
> > @@ -0,0 +1,3 @@
> > +-- @@stderr --
> > +dtrace: script 'test/unittest/options/err.no-w-or-destructive2.d' matched 1 probe
> > +dtrace: could not enable tracing: Destructive actions not allowed
> > diff --git a/test/unittest/usdt/err.Z_no-w.r b/test/unittest/usdt/err.Z_no-w.r
> > deleted file mode 100644
> > index b81622481..000000000
> > --- a/test/unittest/usdt/err.Z_no-w.r
> > +++ /dev/null
> > @@ -1,4 +0,0 @@
> > -dtrace is running so start the trigger
> > -dtrace died as expected after trigger started
> > --- @@stderr --
> > -dtrace: processing aborted: Destructive actions not allowed
> > diff --git a/test/unittest/usdt/err.Z_no-w.sh b/test/unittest/usdt/err.Z_no-w.sh
> > deleted file mode 100755
> > index 4f129341d..000000000
> > --- a/test/unittest/usdt/err.Z_no-w.sh
> > +++ /dev/null
> > @@ -1,76 +0,0 @@
> > -#!/bin/bash
> > -#
> > -# Oracle Linux DTrace.
> > -# Copyright (c) 2024, 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.
> > -#
> > -# This test verifies that dtrace will not run a destructive script
> > -# for USDT probes if -w is not specified.
> > -#
> > -# Specifically, the script is launched with -Z and no USDT processes are
> > -# initially present. Only once a USDT process is detected does dtrace
> > -# fail due to the destructive action.
> > -
> > -dtrace=$1
> > -trigger=`pwd`/test/triggers/usdt-tst-defer
> > -
> > -# Set up test directory.
> > -
> > -DIRNAME=$tmpdir/Z_no-w.$$.$RANDOM
> > -mkdir -p $DIRNAME
> > -cd $DIRNAME
> > -
> > -# Make a private copy of the trigger executable so that we get our
> > -# own DOF stash.
> > -
> > -cp $trigger main
> > -
> > -# Run dtrace.
> > -
> > -$dtrace $dt_flags -Zq -o dtrace.out -n '
> > -testprov*:::foo
> > -{
> > - raise(SIGUSR1);
> > -}' &
> > -dtpid=$!
> > -
> > -# Wait up to half of the timeout period for dtrace to start up.
> > -
> > -iter=$((timeout / 2))
> > -while [ $iter -gt 0 ]; do
> > - sleep 1
> > - if [ -e dtrace.out ]; then
> > - break
> > - fi
> > - iter=$((iter - 1))
> > -done
> > -if [[ $iter -eq 0 ]]; then
> > - echo ERROR starting DTrace job
> > - cat dtrace.out
> > - exit 1
> > -fi
> > -
> > -# Start a trigger process.
> > -
> > -echo dtrace is running so start the trigger
> > -./main > main.out &
> > -pid=$!
> > -
> > -# Check again if dtrace is still running.
> > -
> > -sleep 2
> > -if [[ ! -d /proc/$dtpid ]]; then
> > - echo dtrace died as expected after trigger started
> > -else
> > - echo dtrace is unexpectedly still running
> > - kill -9 $dtpid
> > - wait $dtpid
> > -fi
> > -
> > -# Tell the trigger to proceed to completion.
> > -
> > -kill -USR1 $pid
> > -wait $pid
> > -
> > -exit 1
> > --
> > 2.43.5
> >
More information about the DTrace-devel
mailing list