[DTrace-devel] [PATCH 2/2] usdt: introduce DOF_VERSION_3 and use it when is-enabled probes are present
Kris Van Hees
kris.van.hees at oracle.com
Sat May 13 01:59:23 UTC 2023
On Wed, Apr 19, 2023 at 04:44:13PM +0100, Nick Alcock via DTrace-devel wrote:
> This guardrail prevents DTrace v2 from picking up v1-style is-enabled
> probes, by suppressing is-enabled probes when DOF_VERSION <= 3, setting
> the DOF_VERSION to 3 for programs containing is-enabled probes, and
> refusing to -G-link object files containing DOF_VERSION_2 and is-enabled
> probes. This is all necessary because v1-style probes have no function
> arguments, so it would be dangerous to try to pick up the first
> argument, treat it as a pointer, and write a 1 down it.
>
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
... but this should be coming before the USDT implementation patch. Also,
some questions below.
> ---
> include/dtrace/dof_defines.h | 5 +++--
> libcommon/dof_parser.c | 15 +++++++++++----
> libdtrace/dt_link.c | 21 +++++++++++++++++----
> 3 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/include/dtrace/dof_defines.h b/include/dtrace/dof_defines.h
> index 4a7c4185fa4b5..20ef88436b683 100644
> --- a/include/dtrace/dof_defines.h
> +++ b/include/dtrace/dof_defines.h
> @@ -2,7 +2,7 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> *
> - * Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2009, 2023, Oracle and/or its affiliates. All rights reserved.
> */
>
> /*
> @@ -111,7 +111,8 @@
>
> #define DOF_VERSION_1 1
> #define DOF_VERSION_2 2
> -#define DOF_VERSION DOF_VERSION_2
> +#define DOF_VERSION_3 3
> +#define DOF_VERSION DOF_VERSION_3
>
> #define DOF_FL_VALID 0 /* mask of all valid dofh_flags bits */
>
> diff --git a/libcommon/dof_parser.c b/libcommon/dof_parser.c
> index 52ad0263c4b1f..d6631a1afdcb9 100644
> --- a/libcommon/dof_parser.c
> +++ b/libcommon/dof_parser.c
> @@ -1,6 +1,6 @@
> /*
> * Oracle Linux DTrace; DOF parser.
> - * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2010, 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.
> */
> @@ -403,7 +403,8 @@ dof_slurp(int out, dof_hdr_t *dof, uint64_t ubase)
> }
>
> if (dof->dofh_ident[DOF_ID_VERSION] != DOF_VERSION_1 &&
> - dof->dofh_ident[DOF_ID_VERSION] != DOF_VERSION_2) {
> + dof->dofh_ident[DOF_ID_VERSION] != DOF_VERSION_2 &&
> + dof->dofh_ident[DOF_ID_VERSION] != DOF_VERSION_3) {
> dof_error(out, EINVAL, "DOF version mismatch: %i",
> dof->dofh_ident[DOF_ID_VERSION]);
> return -1;
> @@ -990,9 +991,15 @@ emit_provider(int out, dof_helper_t *dhp,
> enoff = NULL;
>
> /*
> - * See validate_probes().
> + * See validate_probe(). Ignore is-enabled probes for DOF version 2:
> + * these probes rely on modifying the return value of the is-enabled
> + * probe, and using the modern approach of writing a 1 to the first
> + * argument will write to a nonexistent argument and cause, at best, a
> + * crash, at worst, memory corruption. Avoiding parsing them ultimately
> + * avoids creating the system-level probes at all, avoiding the
> + * corruption at the cost of causing the probes to always return 0.
> */
> - if (dof->dofh_ident[DOF_ID_VERSION] != DOF_VERSION_1 &&
> + if (dof->dofh_ident[DOF_ID_VERSION] >= DOF_VERSION_3 &&
> prov->dofpv_prenoffs != DOF_SECT_NONE) {
> enoff_sec = (dof_sec_t *)(uintptr_t)
> (daddr + dof->dofh_secoff +
> diff --git a/libdtrace/dt_link.c b/libdtrace/dt_link.c
> index e6059e9840cde..3346720fcf62e 100644
> --- a/libdtrace/dt_link.c
> +++ b/libdtrace/dt_link.c
> @@ -1539,16 +1539,29 @@ dtrace_program_link(dtrace_hdl_t *dtp, dtrace_prog_t *pgp, uint_t dflags,
> return 0;
> }
>
> - for (i = 0; i < objc; i++)
> + for (i = 0; i < objc; i++) {
> if (process_obj(dtp, objv[i], &eprobes) != 0)
> return -1; /* errno is set for us */
>
> + /*
> + * If there are is-enabled probes then we need to error out if
> + * DOF version 2 was used, preventing linkage of pre-existing
> + * objects that contain return-value-driven is-enabled probes
> + * with a DTrace that implements argument-driven is-enabled
> + * probes. (New objects will at this stage have DOF version 1.)
> + */
How do new object end up with DOF version 1? Shouldn't new objects be using
DOF version 3 after this patch?
> + if (eprobes && pgp->dp_dofversion == DOF_VERSION_2)
Shouldn't this be pgp->dp_dofversion <` DOF_VERSION_3 ?
> + return dt_link_error(dtp, NULL, -1, NULL,
> + "DOF in %s is too old: regenerate with dtrace -G",
> + objv[i]);
> + }
> +
> /*
> * If there are is-enabled probes then we need to force use of DOF
> - * version 2.
> + * version 3.
> */
> - if (eprobes && pgp->dp_dofversion < DOF_VERSION_2)
> - pgp->dp_dofversion = DOF_VERSION_2;
> + if (eprobes && pgp->dp_dofversion < DOF_VERSION_3)
> + pgp->dp_dofversion = DOF_VERSION_3;
>
> if ((dof = dtrace_dof_create(dtp, pgp, dflags)) == NULL)
> return -1; /* errno is set for us */
> --
> 2.39.1.268.g9de2f9a303
>
>
> _______________________________________________
> 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