Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
Re: rpki-client: add support for draft-ietf-sidrops-rpki-prefixlist
To:
Job Snijders <job@openbsd.org>, tech@openbsd.org
Date:
Wed, 21 Feb 2024 15:42:47 +0100

Download raw body.

Thread
> > Now there is one problem with this:
> > 
> > > +       /* merge content of multiple SPLs */
> > > +       vsp->prefixes = recallocarray(vsp->prefixes, vsp->prefixesz,
> > > +           vsp->prefixesz + spl->pfxsz, sizeof(struct spl_pfx));
> > > +       if (vsp->prefixes == NULL)
> > > +               err(1, NULL);
> > 
> > On first round this is fine. vsp->prefixes is NULL and this is a calloc.
> > On round two this is fine. vsp->prefixesz covers the full allocation but
> > on round three this becomes a problem. vsp->prefixesz can be smaller than
> > the allocation done before because of duplicates.
> > 
> > The manpage has the following about recallocarray():
> >      If ptr is not NULL, oldnmemb must be a value such that
> >      oldnmemb * size is the size of the earlier allocation
> >      that returned ptr, otherwise the behavior is undefined.
> > 
> > So right now that code above hits the undefined behavior case.
> > Since vsp->prefixesz can be smaller than the earlier allocation.
> > 
> > Maybe it is enough to call reallocarray(vsp->prefixes, vsp->prefixesz,
> > sizeof(struct spl_pfx)) after the merge to adjust the allocation size.
> 
> I'd say we should do this.

No, actually I think we should use reallocarray(), not recallocarray()
for vsp->prefixes, just like for v->providers in aspa.c.

If we do stick with recallocarray() then we should track the proper
size, but also adjust the aspa code to do that.