[DTrace-devel] [PATCH] Relocation processing for the ERROR program is done too early
Kris Van Hees
kris.van.hees at oracle.com
Wed Sep 18 09:02:10 UTC 2024
On Mon, Sep 16, 2024 at 06:58:01PM -0400, Eugene Loh wrote:
> Thanks. I see your R-b. So, no further review is needed? If this patch
> has broader scope than the initial patch, should further checks be added to
> the test -- e.g., to demonstrate that other issues are fixed as well?
Yes and no. You found the first known instance of this issue, and while I
indicated that there are other cases, testing them all would be tricky (and
may change over time). I think that the test you have is sufficient for all
cases because the fix is not specific to the test.
> On 9/13/24 22:58, Kris Van Hees wrote:
> > When we call dtrace_go(), we do something like this:
> >
> > dt_bpf_make_progs()
> > dt_program_construct() // just for ERROR
> > dt_link()
> > dt_link_construct()
> > dt_bpf_gmap_create()
> > dt_bpf_load_progs() // other
> > dt_link()
> > dt_link_construct()
> >
> > In dt_link_construct() we dive down and find dt_get_bvar(). One of the
> > relocations is to supply the value of STBSZ. The first dt_link() is for
> > ERROR, while the subsequent calls in dt_bpf_load_progs() are for other
> > clauses -- that is, two separate versions of dt_get_bvar() are used.
> > Meanwhile, the value of STBSZ is not set until dt_bpf_gmap_create().
> > This means that the ERROR copy of dt_get_bvar() does not have STBSZ set
> > properly. This means that if ERROR accesses probeprov or probename,
> > dt_get_bvar() returns the beginning of the string table, which is a NUL
> > terminator. Some other relocation values have similar issues.
> >
> > Move the ERROR program construction to dt_bpf_load_progs().
> >
> > Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> > libdtrace/dt_bpf.c | 40 +++++++++++----------
> > test/unittest/builtinvar/tst.probe_dtrace.d | 34 ++++++++++++++++++
> > test/unittest/builtinvar/tst.probe_dtrace.r | 6 ++++
> > 3 files changed, 61 insertions(+), 19 deletions(-)
> > create mode 100644 test/unittest/builtinvar/tst.probe_dtrace.d
> > create mode 100644 test/unittest/builtinvar/tst.probe_dtrace.r
> >
> > diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> > index 70597d65..df427c86 100644
> > --- a/libdtrace/dt_bpf.c
> > +++ b/libdtrace/dt_bpf.c
> > @@ -1212,30 +1212,14 @@ int
> > dt_bpf_make_progs(dtrace_hdl_t *dtp, uint_t cflags)
> > {
> > dt_probe_t *prp;
> > - dtrace_difo_t *dp;
> > - dt_ident_t *idp = dt_dlib_get_func(dtp, "dt_error");
> > -
> > - assert(idp != NULL);
> > -
> > - /*
> > - * First construct the ERROR probe program (to be included in probe
> > - * programs that may trigger a fault).
> > - *
> > - * After constructing the program, we need to patch up any calls to
> > - * dt_error because DTrace cannot handle faults in ERROR itself.
> > - */
> > - dp = dt_program_construct(dtp, dtp->dt_error, cflags, idp);
> > - if (dp == NULL)
> > - return -1;
> > -
> > - idp->di_flags |= DT_IDFLG_CGREG; /* mark it as inline-ready */
> > - dt_bpf_reloc_error_prog(dtp, dp);
> > /*
> > * Now construct all the other programs.
> > */
> > for (prp = dt_list_next(&dtp->dt_enablings); prp != NULL;
> > prp = dt_list_next(prp)) {
> > + dtrace_difo_t *dp;
> > +
> > /* Already done. */
> > if (prp == dtp->dt_error)
> > continue;
> > @@ -1263,7 +1247,25 @@ int
> > dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> > {
> > dt_probe_t *prp;
> > + dtrace_difo_t *dp;
> > dtrace_optval_t dest_ok = DTRACEOPT_UNSET;
> > + dt_ident_t *idp = dt_dlib_get_func(dtp, "dt_error");
> > +
> > + assert(idp != NULL);
> > +
> > + /*
> > + * First construct the ERROR probe program (to be linked in probe
> > + * programs that may trigger a fault).
> > + *
> > + * After constructing the program, we need to patch up any calls to
> > + * dt_error because DTrace cannot handle faults in ERROR itself (yet).
> > + */
> > + dp = dt_program_construct(dtp, dtp->dt_error, cflags, idp);
> > + if (dp == NULL)
> > + return -1;
> > +
> > + idp->di_flags |= DT_IDFLG_CGREG; /* mark it as inline-ready */
> > + dt_bpf_reloc_error_prog(dtp, dp);
> > /*
> > * Determine whether we can allow destructive actions.
> > @@ -1272,10 +1274,10 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
> > for (prp = dt_list_next(&dtp->dt_enablings); prp != NULL;
> > prp = dt_list_next(prp)) {
> > - dtrace_difo_t *dp = prp->difo;
> > int fd;
> > int rc = -1;
> > + dp = prp->difo;
> > if (dp == NULL)
> > continue;
> > diff --git a/test/unittest/builtinvar/tst.probe_dtrace.d b/test/unittest/builtinvar/tst.probe_dtrace.d
> > new file mode 100644
> > index 00000000..d08067ad
> > --- /dev/null
> > +++ b/test/unittest/builtinvar/tst.probe_dtrace.d
> > @@ -0,0 +1,34 @@
> > +/*
> > + * 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.
> > + */
> > +
> > +/*
> > + * ASSERTION:
> > + * Built-in variables listing provider, module, function, and name
> > + * are correct from dtrace-provider probes.
> > + *
> > + * SECTION: Variables/Built-in Variables
> > + */
> > +
> > +#pragma D option quiet
> > +
> > +/* Check build-in variables from the dtrace-provider probes. */
> > +BEGIN,ERROR,END
> > +{
> > + printf("%s:%s:%s:%s\n", probeprov, probemod, probefunc, probename);
> > +}
> > +
> > +/* Cause the ERROR probe to fire. */
> > +BEGIN
> > +{
> > + *((int*)0);
> > +}
> > +
> > +/* Cause the END probe to fire. */
> > +BEGIN,ERROR
> > +{
> > + exit(0);
> > +}
> > diff --git a/test/unittest/builtinvar/tst.probe_dtrace.r b/test/unittest/builtinvar/tst.probe_dtrace.r
> > new file mode 100644
> > index 00000000..e6cccc88
> > --- /dev/null
> > +++ b/test/unittest/builtinvar/tst.probe_dtrace.r
> > @@ -0,0 +1,6 @@
> > +dtrace:::BEGIN
> > +dtrace:::ERROR
> > +dtrace:::END
> > +
> > +-- @@stderr --
> > +dtrace: error in dt_clause_3 for probe ID 1 (dtrace:::BEGIN): invalid address ({ptr}) at BPF pc NNN
More information about the DTrace-devel
mailing list