Re: [DTrace-devel] [PATCH] error: ERROR probe firing should not corrupt probe arguments

[Kris Van Hees <kris.van.hees@oracle.com>]

Working on a v2 - a couple of tests are failing with a too-large-stack failure
from the BPF verifier due to using 48 extra bytes in the error handling.

On Fri, Sep 06, 2024 at 10:26:18PM -0400, Kris Van Hees via DTrace-devel wrote:
> When an ERROR probe fires due to a fault happening while processing a
> probe firing, it was corrupting the first 6 probe arguments of the
> probe causing the fault because they were being overwritten.  But since
> a fault only aborts the execution of the clause it occurs in, those
> original probe arguments might still be needed for other clauses that
> are executed for the original probe.
> 
> Save arg0 through arg5 prior to calling the ERROR probe, and restore
> them after the ERROR probe finishes.
> 
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> ---
>  bpf/probe_error.c                             |  3 ++
>  .../error/tst.argv-corruption-by-error.d      | 48 +++++++++++++++++++
>  2 files changed, 51 insertions(+)
>  create mode 100644 test/unittest/error/tst.argv-corruption-by-error.d
> 
> diff --git a/bpf/probe_error.c b/bpf/probe_error.c
> index c8ddcdfa..49822172 100644
> --- a/bpf/probe_error.c
> +++ b/bpf/probe_error.c
> @@ -26,7 +26,9 @@ noinline void dt_probe_error(const dt_dctx_t *dctx, uint64_t pc, uint64_t fault,
>  			     uint64_t illval)
>  {
>  	dt_mstate_t	*mst = dctx->mst;
> +	uint64_t	argv[6];
>  
> +	__builtin_memcpy(argv, mst->argv, sizeof(argv));
>  	mst->argv[0] = 0;
>  	mst->argv[1] = mst->epid;
>  	mst->argv[2] = mst->clid;
> @@ -36,5 +38,6 @@ noinline void dt_probe_error(const dt_dctx_t *dctx, uint64_t pc, uint64_t fault,
>  
>  	dt_error(dctx);
>  
> +	__builtin_memcpy(mst->argv, argv, sizeof(argv));
>  	mst->fault = fault;
>  }
> diff --git a/test/unittest/error/tst.argv-corruption-by-error.d b/test/unittest/error/tst.argv-corruption-by-error.d
> new file mode 100644
> index 00000000..6fd1834e
> --- /dev/null
> +++ b/test/unittest/error/tst.argv-corruption-by-error.d
> @@ -0,0 +1,48 @@
> +/*
> + * 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: A fault that triggers the ERROR probe terminates the execution of
> + *            the current clause, while other clauses for the same probe should
> + *            still be executed.  This tests that the ERROR probe invocation
> + *            does not corrupt the arguments of the original probe.
> + *
> + * SECTION: dtrace Provider
> + */
> +
> +#pragma D option quiet
> +
> +syscall::write*:entry
> +{
> +	self->arg0 = arg0;
> +	self->arg1 = arg1;
> +	self->arg2 = arg2;
> +	self->arg3 = arg3;
> +	self->arg4 = arg4;
> +	self->arg5 = arg5;
> +
> +	printf("%d / %d / %d / %d / %d / %d\n",
> +	       arg0, arg1, arg2, arg3, arg4, arg5);
> +}
> +
> +syscall::write*:entry
> +{
> +	trace(*(int *)0);
> +}
> +
> +syscall::write*:entry,
> +ERROR {
> +	printf("%d / %d / %d / %d / %d / %d\n",
> +	       arg0, arg1, arg2, arg3, arg4, arg5);
> +}
> +
> +syscall::write*:entry
> +{
> +	exit(self->arg0 != arg0 || self->arg1 != arg1 || self->arg2 != arg2 ||
> +	     self->arg3 != arg3 || self->arg4 != arg4 || self->arg5 != arg5
> +		? 1 : 0);
> +}
> -- 
> 2.45.2
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel