[DTrace-devel] [PATCH] Fix size of string data in the trace output buffer

[Kris Van Hees]

On Wed, Sep 08, 2021 at 11:51:03AM -0400, Kris Van Hees wrote:
> On Wed, Sep 08, 2021 at 10:52:53AM -0400, Eugene Loh wrote:
> > On 9/8/21 2:30 AM, Kris Van Hees wrote:
> > 
> > > On Wed, Sep 08, 2021 at 12:49:12AM -0400, Eugene Loh wrote:
> > >> Also, does dt_cg_store_var() have the same issues?
> > > No, this is only really needed for store_val because of how the consumer
> > > handles strings.
> > 
> > Let's put it this way:  dt_cg_store_var() has issues.  Perhaps they are 
> > not the "same" issues since they have nothing to do with the consumer, 
> > but the fix (replacing a straight copy with a custom prefix+data+NULL 
> > construction) would be the same.  E.g., you have a test case:
> >      #pragma D option strsize=5
> >      BEGIN
> >      {
> >          trace(probeprov);
> >      }
> > If we modified that to be:
> >      #pragma D option strsize=5
> >      BEGIN
> >      {
> >          p = "dtrace";
> >          trace(p);
> >      }
> > the BPF verifier would erupt.  I think that's because in 
> > dt_cg_store_var(), we assume we'll have enough space to copy over the 
> > string.  We probably need, instead, well, basically the same fix you 
> > give in this patch.  If you like, I can propose a patch.  Or, we can 
> > wait... especially if the length prefix is going away anyhow.
> 
> Ah, correct.  Fixing now.  I overlooked that string constants not getting
> capped at compile time would still be able to cause an overrun here when
> storing to variables.

I am correcting my correction - it is a very different fix because here we
were incorrectly copying based on the size of the source rather than taking
into account that the destination may be smaller.

So, fix needed but different.

e _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel