Claudio Jeker(cje...@diehard.n-r-g.com) on 2018.12.29 23:46:18 +0100:
> On Sat, Dec 29, 2018 at 08:24:50PM +0100, Sebastian Benoit wrote:
> > Hi,
> > 
> > we allocate the wrong size here i think.
> > 
> > ok?
> 
> No, this diff is wrong. Check out the parse.y code. It uses MRT2MC to
> access fields in struct mrt_config. I know this is not the best style but
                                                     ^^^^^^^^^^^^ +1

thx

> it is currently correct.
>  
> > (benno_bgpd_mrt.diff)
> > 
> > diff --git usr.sbin/bgpd/mrt.c usr.sbin/bgpd/mrt.c
> > index 7c7f2193db3..2502c792c55 100644
> > --- usr.sbin/bgpd/mrt.c
> > +++ usr.sbin/bgpd/mrt.c
> > @@ -976,43 +976,43 @@ mrt_get(struct mrt_head *c, struct mrt *m)
> >             if (t->type != m->type)
> >                     continue;
> >             if (strcmp(t->rib, m->rib))
> >                     continue;
> >             if (t->peer_id == m->peer_id &&
> >                 t->group_id == m->group_id)
> >                     return (t);
> >     }
> >     return (NULL);
> >  }
> >  
> >  int
> >  mrt_mergeconfig(struct mrt_head *xconf, struct mrt_head *nconf)
> >  {
> >     struct mrt      *m, *xm;
> >  
> >     /* both lists here are actually struct mrt_conifg nodes */
> >     LIST_FOREACH(m, nconf, entry) {
> >             if ((xm = mrt_get(xconf, m)) == NULL) {
> >                     /* NEW */
> > -                   if ((xm = malloc(sizeof(struct mrt_config))) == NULL)
> > +                   if ((xm = malloc(sizeof(struct mrt))) == NULL)
> >                             fatal("mrt_mergeconfig");
> > -                   memcpy(xm, m, sizeof(struct mrt_config));
> > +                   memcpy(xm, m, sizeof(*xm));
> >                     xm->state = MRT_STATE_OPEN;
> >                     LIST_INSERT_HEAD(xconf, xm, entry);
> >             } else {
> >                     /* MERGE */
> >                     if (strlcpy(MRT2MC(xm)->name, MRT2MC(m)->name,
> >                         sizeof(MRT2MC(xm)->name)) >=
> >                         sizeof(MRT2MC(xm)->name))
> >                             fatalx("mrt_mergeconfig: strlcpy");
> >                     MRT2MC(xm)->ReopenTimerInterval =
> >                         MRT2MC(m)->ReopenTimerInterval;
> >                     xm->state = MRT_STATE_REOPEN;
> >             }
> >     }
> >  
> >     LIST_FOREACH(xm, xconf, entry)
> >             if (mrt_get(nconf, xm) == NULL)
> >                     /* REMOVE */
> >                     xm->state = MRT_STATE_REMOVE;
> >  
> >     /* free config */
> > diff --git usr.sbin/bgpd/parse.y usr.sbin/bgpd/parse.y
> > index 984a1639300..272475fe6e4 100644
> > --- usr.sbin/bgpd/parse.y
> > +++ usr.sbin/bgpd/parse.y
> > @@ -3809,41 +3809,41 @@ add_mrtconfig(enum mrt_type type, char *name, int 
> > timeout, struct peer *p,
> >     struct mrt      *m, *n;
> >  
> >     LIST_FOREACH(m, conf->mrt, entry) {
> >             if ((rib && strcmp(rib, m->rib)) ||
> >                 (!rib && *m->rib))
> >                     continue;
> >             if (p == NULL) {
> >                     if (m->peer_id != 0 || m->group_id != 0)
> >                             continue;
> >             } else {
> >                     if (m->peer_id != p->conf.id ||
> >                         m->group_id != p->conf.groupid)
> >                             continue;
> >             }
> >             if (m->type == type) {
> >                     yyerror("only one mrtdump per type allowed.");
> >                     return (-1);
> >             }
> >     }
> >  
> > -   if ((n = calloc(1, sizeof(struct mrt_config))) == NULL)
> > +   if ((n = calloc(1, sizeof(struct mrt))) == NULL)
> >             fatal("add_mrtconfig");
> >  
> >     n->type = type;
> >     if (strlcpy(MRT2MC(n)->name, name, sizeof(MRT2MC(n)->name)) >=
> >         sizeof(MRT2MC(n)->name)) {
> >             yyerror("filename \"%s\" too long: max %zu",
> >                 name, sizeof(MRT2MC(n)->name) - 1);
> >             free(n);
> >             return (-1);
> >     }
> >     MRT2MC(n)->ReopenTimerInterval = timeout;
> >     if (p != NULL) {
> >             if (curgroup == p) {
> >                     n->peer_id = 0;
> >                     n->group_id = p->conf.id;
> >             } else {
> >                     n->peer_id = p->conf.id;
> >                     n->group_id = 0;
> >             }
> >     }
> > 
> 
> -- 
> :wq Claudio
> 

Reply via email to