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
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
