[DTrace-devel] [PATCH 09/12] Add support for the ustack() action

[Kris Van Hees]

On Thu, Jun 10, 2021 at 05:53:02PM -0400, Eugene Loh wrote:
> I posted a v2 of the patch, but...
> 
> On 6/9/21 5:50 PM, Kris Van Hees wrote:
> 
> > On Fri, May 28, 2021 at 02:35:13PM -0400, eugene.loh at oracle.com wrote:
> >> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> >> @@ -1503,37 +1503,82 @@ dt_cg_act_trunc(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
> >>   static void
> >>   dt_cg_act_ustack(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)
> >>   {
> >> 	[...]
> >> +
> >> +	/* Now call bpf_get_stack(ctx, buf, size, flags). */
> >> +	emit(dlp,  BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_FP, DT_STK_DCTX));
> >> +	emit(dlp,  BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_1, DCTX_CTX));
> >> +	emit(dlp,  BPF_MOV_REG(BPF_REG_2, BPF_REG_9));
> >> +	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, off + 8));
> >> +	emit(dlp,  BPF_MOV_IMM(BPF_REG_3, 8 * nframes));
> >> +	emit(dlp,  BPF_MOV_IMM(BPF_REG_4, (skip & BPF_F_SKIP_FIELD_MASK) | BPF_F_USER_STACK));
> >> +	emit(dlp,  BPF_CALL_HELPER(BPF_FUNC_get_stack));
> >> +	dt_regset_free_args(drp);
> >> +	emit(dlp,  BPF_BRANCH_IMM(BPF_JSGE, BPF_REG_0, 0, lbl_valid));
> >> +	/* FIXME: what should the illval be? */
> > I believe illval is undefined in the case of a BADSTACK fault, so I would go
> > with 0 indeed.
> 
> I sure am confused about this one.
> 
> I looked at the docs;  e.g.:
> https://docs.oracle.com/en/operating-systems/oracle-linux/dtrace-guide/dt_prov.html#dt_error_prov
> 
> It says that DTRACEFLT_BADSTACK means "Invalid user process stack" and 
> arg5/illval is "Address of invalid stack pointer".  Does this mean that 
> illval=0 is fine for kernel stack() errors, but for ustack() errors we 
> have to report the SP?

Somehow DTrace seems to have had the notion (from the early beginning) that
getting a kernel stack will never fail.  That's probably true because it would
be indicative of a much bigger problem.  For userspace stacks we certainly can
encounter corrupted stacks, but we won't have access to whatever the (presumed)
bad SP might be as far as I can see.  Putting a non-zero value in the illval
argument would be deceiving, so 0 seems to be most prudent.

If you can somehow determine the correct (bad SP) value, feel free to add that
of course because that would be the ideal.
> 
> >> +	dt_cg_probe_error(pcb, -1, DTRACEFLT_BADSTACK, 0);
> >> +	emitl(dlp, lbl_valid,
> >> +		   BPF_NOP());
> >> +	dt_regset_free(drp, BPF_REG_0);
> >>   }
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel