Remi Locherer(remi.loche...@relo.ch) on 2019.12.10 22:39:32 +0100:
> On Tue, Dec 10, 2019 at 07:05:27PM +0100, Hiltjo Posthuma wrote:
> > Hi,
> > 
> > While looking at the code of ripd:
> > 
> > I think there are (also) 2 small memleaks in a debug/error path
> > (IMSG_REQUEST_ADD and IMSG_RESPONSE_ADD). It breaks out before adding the
> > struct rip_route as an entry by the add_entry function (which adds it and 
> > adds
> > a reference count) in the log_debug block.
> > 
> > clang-analyzer also pointed at a double-free and use of free'd data: the
> > function kroute_insert frees kr and returns -1 when struct kroute is 
> > duplicate.
> > 
> > Patch below (untested):
> > 
> 
> OK remi@

go ahead and commit it, ok benno@

> 
> > 
> > diff --git a/usr.sbin/ripd/kroute.c b/usr.sbin/ripd/kroute.c
> > index 6e7449e0909..71758a75e44 100644
> > --- a/usr.sbin/ripd/kroute.c
> > +++ b/usr.sbin/ripd/kroute.c
> > @@ -183,8 +183,7 @@ kr_change_fib(struct kroute_node *kr, struct kroute 
> > *kroute, int action)
> >  
> >             if (kroute_insert(kr) == -1) {
> >                     log_debug("kr_update_fib: cannot insert %s",
> > -                       inet_ntoa(kr->r.nexthop));
> > -                   free(kr);
> > +                       inet_ntoa(kroute->nexthop));
> >             }
> >     } else
> >             kr->r.nexthop.s_addr = kroute->nexthop.s_addr;
> > diff --git a/usr.sbin/ripd/ripe.c b/usr.sbin/ripd/ripe.c
> > index d83901e245f..1f6f9b6583f 100644
> > --- a/usr.sbin/ripd/ripe.c
> > +++ b/usr.sbin/ripd/ripe.c
> > @@ -351,6 +351,7 @@ ripe_dispatch_rde(int fd, short event, void *bula)
> >                                 NULL) {
> >                                     log_debug("unknown neighbor id %u",
> >                                         imsg.hdr.peerid);
> > +                                   free(rr);
> >                                     break;
> >                             }
> >                             add_entry(&nbr->rq_list, rr);
> > @@ -396,6 +397,7 @@ ripe_dispatch_rde(int fd, short event, void *bula)
> >                     if ((nbr = nbr_find_peerid(imsg.hdr.peerid)) == NULL) {
> >                             log_debug("unknown neighbor id %u",
> >                                 imsg.hdr.peerid);
> > +                           free(rr);
> >                             break;
> >                     }
> >                     iface = nbr->iface;
> > 
> > -- 
> > Kind regards,
> > Hiltjo
> > 
> 

Reply via email to