Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: bgpd: fix chash ch_table_resize error handling
To:
Theo Buehler <tb@theobuehler.org>
Cc:
tech@openbsd.org
Date:
Tue, 12 May 2026 17:54:28 +0200

Download raw body.

Thread
On Tue, May 12, 2026 at 05:02:51PM +0200, Theo Buehler wrote:
> On Tue, May 12, 2026 at 04:56:30PM +0200, Theo Buehler wrote:
> > On Tue, May 12, 2026 at 11:46:03AM +0200, Claudio Jeker wrote:
> > > Doing a double reallocarray call is tricky.
> > > If the first reallocarray succeeds and the 2nd one fails then the pointer
> > > for the first reallocarray needs to be updated and not freed.
> > > 
> > > Simply update the t->ch_tables pointer to the new location and return an
> > > error here. That way the ch_table remains consisten and can still be used.
> > 
> > I knew something smelt off here...
> > 
> > ok tb
> 
> Wait. Doesn't that cause the t->ch_level++ to get out of sync?

I want to keep the old level, since we did not resize in the end.
Now there is indeed an issue if the allocation failed on an empty table.
This comes from that fact that ch_table_resize is abused to create a new
table.

Can I put this in now and the rethink the extendible hash bits and
ch_table_resize()?
 
> > 
> > > 
> > > -- 
> > > :wq Claudio
> > > 
> > > Index: chash.c
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/bgpd/chash.c,v
> > > diff -u -p -r1.10 chash.c
> > > --- chash.c	7 May 2026 09:22:10 -0000	1.10
> > > +++ chash.c	12 May 2026 09:32:07 -0000
> > > @@ -548,7 +548,13 @@ ch_table_resize(const struct ch_type *ty
> > >  		return -1;
> > >  	metas = reallocarray(t->ch_metas, newsize, sizeof(*metas));
> > >  	if (metas == NULL) {
> > > -		free(tables);
> > > +		/*
> > > +		 * tables was correctly reallocated, so update that
> > > +		 * pointer before failing hard. If the caller recovers
> > > +		 * somehow the next reallocarray of ch_tables will simply
> > > +		 * do nothing.
> > > +		 */
> > > +		t->ch_tables = tables;
> > >  		return -1;
> > >  	}
> > >  
> > > 
> > 
> 

-- 
:wq Claudio