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

[Nick Alcock]

On 20 Feb 2023, Kris Van Hees said:

> On Mon, Feb 20, 2023 at 04:45:58PM +0000, Nick Alcock wrote:
>> On 20 Feb 2023, Kris Van Hees via DTrace-devel uttered the following:
>> > -			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.)
>
> You are correct that we currently cannot have an ALLOCA pointer here *but*
> that is because there seems to be a propagation that is missed when we use
> something like:
>
> struct outer {
>         struct inner {
>                 int     in;
>         }       out;
> } *p;
>
> BEGIN {
>         p = alloca(sizeof(struct outer));
> 	trace(p->out.in);
>         exit(0);
> }
>
> In this case, we lose the ALLOCA marker once p->out is handled by the code
> generator, and .in is then applied to an arbitrary pointer value rather than
> a known entity (ALLOCA pointer).  So for now, the code always defaults to
> using the dt_cg_load_scalar() mechanism.

Ugh! OK it seems that we can fix this by either propagating taint
through DT_TOK_DOT (but that might do too much...) or by having the cg
for DT_TOK_DOT check both sides as here, or possibly both.

> I'll add to the todo list that we need to sort out this ALLOCA propagation in
> this case, which will then ensure that an indirect load is generated for this
> expression (p->out.in).
>
> So, I am going to leave this chunk in because it should be needed :)

Agreed! It certainly does no harm in any case and will be necessary
later, go for it.

Reviewed-by: Nick Alcock <nick.alcock at oracle.com>

-- 
NULL && (void)