[DTrace-devel] [PATCH v3 04/19] memcpy: bounds-check

[Kris Van Hees]

On Thu, Mar 31, 2022 at 12:13:21AM -0400, Kris Van Hees via DTrace-devel wrote:
> First of, the subject of this patch is misleading.  The patch itself has
> nothing to do with bounds checking.  It simply adds a check to see whether
> the bpf_probe_read() helper call returned an error, and if it does, it
> raises a BADADDR fault.

Forgot to add that this patch is also not needed for the alloca patch series,
so it would be best to keep it separate.

> On Thu, Mar 24, 2022 at 06:24:30PM +0000, Nick Alcock via DTrace-devel wrote:
> > This FIXME is pretty easy to implement.
> > 
> > Signed-off-by: Nick Alcock <nick.alcock 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 ffbe5e95670f..2c82ee9eb67b 100644
> > --- a/libdtrace/dt_cg.c
> > +++ b/libdtrace/dt_cg.c
> > @@ -821,6 +821,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);
> >  
> > @@ -829,8 +831,13 @@ dt_cg_memcpy(dt_irlist_t *dlp, dt_regset_t *drp, int dst, int src, size_t size)
> >  	emit(dlp, BPF_MOV_REG(BPF_REG_3, src));
> >  	dt_regset_xalloc(drp, BPF_REG_0);
> >  	emit(dlp, BPF_CALL_HELPER(BPF_FUNC_probe_read));
> > +
> > +	emit(dlp,  BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_ok));
> 
> s/,  /, / (remove extra space)
> 
> > +	dt_cg_probe_error(yypcb, DT_LBL_NONE, DTRACEFLT_BADADDR, 0);
> 
> Use -1 as the illval argument instead of 0.  We certainly do not want to report
> the bpf_probe_read() failure as a NULL pointer dereference.  I just posted a
> patch to not display an address for a BADADDR fault if -1 is passed.
> 
> > +	emitl(dlp, lbl_ok,
> > +	      BPF_NOP());
> 
> Align the BPF_NOP() with lbl_ok, please.
> 
> > +
> >  	dt_regset_free_args(drp);
> > -	/* FIXME: check BPF_REG_0 for error? */
> >  	dt_regset_free(drp, BPF_REG_0);
> 
> Move the freeing of registers right before the dt_cg_probe_error() call.
> 
> >  }
> >  
> > -- 
> > 2.35.1.261.g8402f930ba.dirty
> > 
> > 
> > _______________________________________________
> > DTrace-devel mailing list
> > DTrace-devel at oss.oracle.com
> > https://oss.oracle.com/mailman/listinfo/dtrace-devel
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel