From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: fix use after free bug in rde_reload_done()
To: tech@openbsd.org
Date: Tue, 4 Nov 2025 13:40:28 +0100

On Tue, Nov 04, 2025 at 01:29:55PM +0100, Claudio Jeker wrote:
> On Tue, Nov 04, 2025 at 01:26:13PM +0100, Claudio Jeker wrote:
> > rde_reload_done() iterates over all ribs and reloads them.
> > 
> > Now if the state is RECONF_DELETE the rib is freed but the code then
> > progresses and accesses rib after the switch statement.
> > 
> > The simplest fix is to use a continue (for the for loop) instead of a
> > break in the RECONF_DELETE case. An other option is to set rib to NULL
> > and then check after the switch statement if rib == NULL.
> > 
> > Fix for CID 492352 and CID 492343
> 
> Wrong diff (noticed by tb@)

I prefer the continue. You could consider setting rib to NULL anyway,
so it is guaranteed to be a valid pointer or NULL after the loop.

ok tb

> 
> -- 
> :wq Claudio
> 
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> diff -u -p -r1.659 rde.c
> --- rde.c	29 Oct 2025 15:27:07 -0000	1.659
> +++ rde.c	4 Nov 2025 12:21:36 -0000
> @@ -3841,7 +3841,7 @@ rde_reload_done(void)
>  		switch (rib->state) {
>  		case RECONF_DELETE:
>  			rib_free(rib);
> -			break;
> +			continue;
>  		case RECONF_RELOAD:
>  			if (rib_update(rib)) {
>  				RB_FOREACH(peer, peer_tree, &peertable) {
>