[DTrace-devel] [PATCH] Do not allocate a 0-size strtab
Kris Van Hees
kris.van.hees at oracle.com
Wed Apr 8 13:40:16 PDT 2020
On Wed, Apr 08, 2020 at 01:35:55PM -0700, Eugene Loh wrote:
> On 04/08/2020 08:00 AM, Kris Van Hees wrote:
>
> > If there is no stirng data to be stored in a DIFO, we were allocating
> stirng -> string
>
> FWIW, if I look at the code, I typically see dtdo_strtab dereferenced
> without any guards against the scenario dtdo_strtab==NULL. I don't know
> the code well enough to know whether to worry. Should I?
If the memory allocation for dtdo_strtab failed, we bail by way of a fatal
error, which means nothing ever gets to use this. So yes, it is safe.
> > a 0-size memory block. There is no need for that - we can just assign
> > NULL to the dtdo_strtab member.
> >
> > In the case of the dt_link_stmt() code, we also happened to take the
> > dtdo_strlen value from the wrong DIFO (fdp instead of dp) causing a
> > 0-size strtab to be allocated into which we then try to write actual
> > data. That caused memory corruption.
> >
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> > libdtrace/dt_cc.c | 14 +++++++++-----
> > libdtrace/dt_dlibs.c | 23 +++++++++++++----------
> > 2 files changed, 22 insertions(+), 15 deletions(-)
> >
> > diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
> > index db009461..fc678692 100644
> > --- a/libdtrace/dt_cc.c
> > +++ b/libdtrace/dt_cc.c
> > @@ -2381,11 +2381,15 @@ dt_link_stmt(dtrace_hdl_t *dtp, dtrace_prog_t *pgp, dtrace_stmtdesc_t *sdp,
> > * Write out the new string table.
> > */
> > dp->dtdo_strlen = dt_strtab_size(stab);
> > - dp->dtdo_strtab = dt_zalloc(dtp, fdp->dtdo_strlen);
> > - if (dp->dtdo_strtab == NULL)
> > - goto fail;
> > - dt_strtab_write(stab, (dt_strtab_write_f *)dt_strtab_copystr,
> > - dp->dtdo_strtab);
> > + if (dp->dtdo_strlen > 0) {
> > + dp->dtdo_strtab = dt_zalloc(dtp, dp->dtdo_strlen);
> > + if (dp->dtdo_strtab == NULL)
> > + goto fail;
> > + dt_strtab_write(stab, (dt_strtab_write_f *)dt_strtab_copystr,
> > + dp->dtdo_strtab);
> > + } else
> > + dp->dtdo_strtab = NULL;
> > +
> > dt_strtab_destroy(stab);
> >
> > /*
> > diff --git a/libdtrace/dt_dlibs.c b/libdtrace/dt_dlibs.c
> > index 58e01221..2b8fe21d 100644
> > --- a/libdtrace/dt_dlibs.c
> > +++ b/libdtrace/dt_dlibs.c
> > @@ -554,7 +554,6 @@ done:
> > for (fid = fun0; fid < symc; fid++) {
> > dtrace_difo_t *dp;
> > dof_relodesc_t *brp;
> > - size_t slen;
> >
> > fp = funcs[fid];
> > dp = fp->difo;
> > @@ -583,16 +582,20 @@ done:
> > rp++;
> > }
> >
> > - slen = dt_strtab_size(stab);
> > - dp->dtdo_strtab = dt_alloc(dtp, slen);
> > - dp->dtdo_strlen = slen;
> > - if (dp->dtdo_strtab == NULL) {
> > - fprintf(stderr, "Failed to alloc strtab\n");
> > - goto out;
> > - }
> > + dp->dtdo_strlen = dt_strtab_size(stab);
> > + if (dp->dtdo_strlen > 0) {
> > + dp->dtdo_strtab = dt_alloc(dtp, dp->dtdo_strlen);
> > + if (dp->dtdo_strtab == NULL) {
> > + fprintf(stderr, "Failed to alloc strtab\n");
> > + goto out;
> > + }
> > +
> > + dt_strtab_write(stab,
> > + (dt_strtab_write_f *)dt_strtab_copystr,
> > + dp->dtdo_strtab);
> > + } else
> > + dp->dtdo_strtab = NULL;
> >
> > - dt_strtab_write(stab, (dt_strtab_write_f *)dt_strtab_copystr,
> > - dp->dtdo_strtab);
> > dt_strtab_destroy(stab);
> > }
> >
>
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list