[DTrace-devel] [PATCH 05/20] memcpy: error-check

[Kris Van Hees]

On Wed, May 11, 2022 at 10:12:47PM +0100, Nick Alcock via DTrace-devel wrote:
> This FIXME is pretty easy to implement.
> 
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_cg.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index d35d71394a5f..ac5e41553188 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -800,6 +800,8 @@ dt_cg_trace(dt_irlist_t *dlp _dt_unused_, dt_regset_t *drp _dt_unused_,
>  static void
>  dt_cg_memcpy(dt_irlist_t *dlp, dt_regset_t *drp, int dst, int src, size_t size)
>  {
> +	uint_t		lbl_ok = dt_irlist_label(dlp);
> +
>  	if (dt_regset_xalloc_args(drp) == -1)
>  		longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  
> @@ -809,7 +811,12 @@ dt_cg_memcpy(dt_irlist_t *dlp, dt_regset_t *drp, int dst, int src, size_t size)
>  	dt_regset_xalloc(drp, BPF_REG_0);
>  	emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read));
>  	dt_regset_free_args(drp);
> -	/* FIXME: check BPF_REG_0 for error? */
> +
> +	emit(dlp,  BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_ok));
> +	dt_cg_probe_error(yypcb, DTRACEFLT_BADADDR, 0, 0);
> +	emitl(dlp, lbl_ok,
> +	      BPF_NOP());

So, the issue here is: which address is the bad one?  Are we copying from a
bad address or to a bad address?  Since the probe_read() helper is likely to
be checked by the verifier in terms of its destination (prohibiting writing to
non-BPF memory), it would have to be the source address.

Since we have the source address in a register, we should report it in the
BADADDR fault by passing DT_ISREG, src rather than 0, 0.

Sadly we do not really have a way to test this because dt_cg_memcpy() is only
used (right now) for cases where we copy between BPF memory locations so it
won't ever fail in those cases (unless something else is already very wrong).

I'll make the obvious change locally and put on dev.  So with that change,

Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>


> +
>  	dt_regset_free(drp, BPF_REG_0);
>  }
>  
> -- 
> 2.36.1.263.g194b774378.dirty
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel