[DTrace-devel] [PATCH] cg: use indirect load instructions for scalar DPTR and ALLOCA pointers

[Nick Alcock]

On 20 Feb 2023, Kris Van Hees via DTrace-devel uttered the following:

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

I guess this is an efficiency improvement, not a correctness fix,
because there's no need to mess around doing probe_reads on something we
know is perfectly accessible (and the verifier should know it too).

> ---
>  libdtrace/dt_cg.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index cc6ca05c..38c43df3 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
(DT_TOK_DEREF)
> @@ -5395,7 +5395,7 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  						 dnp->dn_reg);
>  			}
>  
> -			if (dnp->dn_child->dn_flags & DT_NF_DPTR)
> +			if (dnp->dn_child->dn_flags & (DT_NF_ALLOCA | DT_NF_DPTR))
>  				emit(dlp, BPF_LOAD(op, dnp->dn_reg, dnp->dn_reg, 0));
>  			else
>  				dt_cg_load_scalar(dnp, op, size, dlp, drp);

ode right above here is a dt_cg_alloca_access_check and a
dt_cg_alloca_ptr to turn the alloca pointer into a real dereferenceable
pointer...

(DT_TOK_DOT)
> @@ -5546,7 +5546,7 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  
>  			op = dt_cg_ldsize(dnp, ctfp, m.ctm_type, &size);
>  
> -			if (dnp->dn_left->dn_flags & DT_NF_DPTR)
> +			if (dnp->dn_left->dn_flags & (DT_NF_ALLOCA | DT_NF_DPTR))
>  				emit(dlp, BPF_LOAD(op, dnp->dn_left->dn_reg, dnp->dn_left->dn_reg, 0));
>  			else
>  				dt_cg_load_scalar(dnp->dn_left, op, size, dlp, drp);

... but this doesn't have either of those, not even indirectly, because
it must already have been dereffed. I'd think no alloca-marked pointer
should be able to propagate this far: the whole point of an alloca
marking is that it needs an access check and ptr before it can be used,
which happens at dereference time. (And indeed we do not transfer alloca
taint past DT_TOK_DOT.)

Why do you think this hunk is necessary? (What am I missing? Is this for
structures in alloca space? If so, we need to spot that case and do a
dt_cg_alloca_access_check and dt_cg_alloca_ptr here too. We still don't
need to propagate taint past DT_TOK_DOT because checking the taintedness
of the LHS of the dot operator will suffice: by the time we process the
RHS, a few lines lower, we will have done the necessary access_check/ptr
dance, so the RHS and points beyond it should not be alloca-tainted,
just as DT_TOK_DEREF doesn't propagate alloca taint.)

-- 
NULL && (void)