Re: [PATCH v2] error: ERROR probe firing should not corrupt probe arguments

Kris Van Hees
2024-09-10T18:38:05Z

On Tue, Sep 10, 2024 at 11:20:28AM -0400, Eugene Loh wrote:
> On 9/10/24 02:20, Kris Van Hees wrote:
>
> > On Tue, Sep 10, 2024 at 01:51:41AM -0400, Eugene Loh wrote:
> > > On 9/10/24 00:33, Kris Van Hees wrote:
> > > > diff --git a/libdtrace/dt_dctx.h b/libdtrace/dt_dctx.h
> > > > @@ -31,6 +31,7 @@ typedef struct dt_mstate {
> > > > dt_pt_regs regs; /* CPU registers */
> > > > uint64_t argv[10]; /* Probe arguments */
> > > > uint64_t saved_argv[10]; /* Saved probe arguments */
> > > > + uint64_t error_argv[6]; /* ERROR probe saved arguments */
> > > I don't think such a thing is needed in mstate.  It suffices to use local
> > > storage in dt_probe_error(), which is probably what you really meant to do.
> > > Plus, calling it error_argv[] is misleading since these are the args that
> > > you are *NOT* using for the ERROR probe.
> > No, because that makes the stack too large and then the BPF verifier rejects
> > the program.
>
> Just to be clear, do you mean the test program will fail?  Or the test
> program passes, but you simply do not want to add to stack pressure?

No, the test program will not fail with argv[] in dt_probe_error(), but other
tests *do* fail if we store the arguments on the stack.