From be25e90dc9a3c3ca2ed97f564f5ffdb984c6a561 Mon Sep 17 00:00:00 2001 From: claudio Date: Tue, 9 Jan 2024 13:41:32 +0000 Subject: [PATCH] Convert the parent process imsg handling over to the new imsg API. This simplifies the code a fair bit and removes direct unchecked memory access to imsg.data. OK tb@ --- usr.sbin/bgpd/bgpd.c | 159 ++++++++++++++++++++--------------------- usr.sbin/bgpd/kroute.c | 51 ++++++------- 2 files changed, 105 insertions(+), 105 deletions(-) diff --git a/usr.sbin/bgpd/bgpd.c b/usr.sbin/bgpd/bgpd.c index c744783f21a..85b02707de6 100644 --- a/usr.sbin/bgpd/bgpd.c +++ b/usr.sbin/bgpd/bgpd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: bgpd.c,v 1.261 2024/01/04 10:26:14 claudio Exp $ */ +/* $OpenBSD: bgpd.c,v 1.262 2024/01/09 13:41:32 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -834,6 +834,11 @@ dispatch_imsg(struct imsgbuf *imsgbuf, int idx, struct bgpd_config *conf) struct imsg imsg; struct peer *p; struct rtr_config *r; + struct kroute_full kf; + struct bgpd_addr addr; + struct pftable_msg pfmsg; + struct demote_msg demote; + char reason[REASON_LEN], ifname[IFNAMSIZ]; ssize_t n; u_int rtableid; int rv, verbose; @@ -846,81 +851,73 @@ dispatch_imsg(struct imsgbuf *imsgbuf, int idx, struct bgpd_config *conf) if (n == 0) break; - switch (imsg.hdr.type) { + switch (imsg_get_type(&imsg)) { case IMSG_KROUTE_CHANGE: if (idx != PFD_PIPE_RDE) log_warnx("route request not from RDE"); - else if (imsg.hdr.len != IMSG_HEADER_SIZE + - sizeof(struct kroute_full)) - log_warnx("wrong imsg len"); - else if (kr_change(imsg.hdr.peerid, imsg.data)) + else if (imsg_get_data(&imsg, &kf, sizeof(kf)) == -1) + log_warn("wrong imsg len"); + else if (kr_change(imsg_get_id(&imsg), &kf)) rv = -1; break; case IMSG_KROUTE_DELETE: if (idx != PFD_PIPE_RDE) log_warnx("route request not from RDE"); - else if (imsg.hdr.len != IMSG_HEADER_SIZE + - sizeof(struct kroute_full)) - log_warnx("wrong imsg len"); - else if (kr_delete(imsg.hdr.peerid, imsg.data)) + else if (imsg_get_data(&imsg, &kf, sizeof(kf)) == -1) + log_warn("wrong imsg len"); + else if (kr_delete(imsg_get_id(&imsg), &kf)) rv = -1; break; case IMSG_KROUTE_FLUSH: if (idx != PFD_PIPE_RDE) log_warnx("route request not from RDE"); - else if (imsg.hdr.len != IMSG_HEADER_SIZE) - log_warnx("wrong imsg len"); - else if (kr_flush(imsg.hdr.peerid)) + else if (kr_flush(imsg_get_id(&imsg))) rv = -1; break; case IMSG_NEXTHOP_ADD: if (idx != PFD_PIPE_RDE) log_warnx("nexthop request not from RDE"); - else if (imsg.hdr.len != IMSG_HEADER_SIZE + - sizeof(struct bgpd_addr)) - log_warnx("wrong imsg len"); + else if (imsg_get_data(&imsg, &addr, sizeof(addr)) == + -1) + log_warn("wrong imsg len"); else { rtableid = conf->default_tableid; - if (kr_nexthop_add(rtableid, imsg.data) == -1) + if (kr_nexthop_add(rtableid, &addr) == -1) rv = -1; } break; case IMSG_NEXTHOP_REMOVE: if (idx != PFD_PIPE_RDE) log_warnx("nexthop request not from RDE"); - else if (imsg.hdr.len != IMSG_HEADER_SIZE + - sizeof(struct bgpd_addr)) - log_warnx("wrong imsg len"); + else if (imsg_get_data(&imsg, &addr, sizeof(addr)) == + -1) + log_warn("wrong imsg len"); else { rtableid = conf->default_tableid; - kr_nexthop_delete(rtableid, imsg.data); + kr_nexthop_delete(rtableid, &addr); } break; case IMSG_PFTABLE_ADD: if (idx != PFD_PIPE_RDE) log_warnx("pftable request not from RDE"); - else - if (imsg.hdr.len != IMSG_HEADER_SIZE + - sizeof(struct pftable_msg)) - log_warnx("wrong imsg len"); - else if (pftable_addr_add(imsg.data) != 0) - rv = -1; + else if (imsg_get_data(&imsg, &pfmsg, sizeof(pfmsg)) == + -1) + log_warn("wrong imsg len"); + else if (pftable_addr_add(&pfmsg) != 0) + rv = -1; break; case IMSG_PFTABLE_REMOVE: if (idx != PFD_PIPE_RDE) log_warnx("pftable request not from RDE"); - else - if (imsg.hdr.len != IMSG_HEADER_SIZE + - sizeof(struct pftable_msg)) - log_warnx("wrong imsg len"); - else if (pftable_addr_remove(imsg.data) != 0) - rv = -1; + else if (imsg_get_data(&imsg, &pfmsg, sizeof(pfmsg)) == + -1) + log_warn("wrong imsg len"); + else if (pftable_addr_remove(&pfmsg) != 0) + rv = -1; break; case IMSG_PFTABLE_COMMIT: if (idx != PFD_PIPE_RDE) log_warnx("pftable request not from RDE"); - else if (imsg.hdr.len != IMSG_HEADER_SIZE) - log_warnx("wrong imsg len"); else if (pftable_commit() != 0) rv = -1; break; @@ -929,7 +926,7 @@ dispatch_imsg(struct imsgbuf *imsgbuf, int idx, struct bgpd_config *conf) log_warnx("pfkey reload request not from SE"); break; } - p = getpeerbyid(conf, imsg.hdr.peerid); + p = getpeerbyid(conf, imsg_get_id(&imsg)); if (p != NULL) { if (pfkey_establish(p) == -1) log_peer_warnx(&p->conf, @@ -941,24 +938,24 @@ dispatch_imsg(struct imsgbuf *imsgbuf, int idx, struct bgpd_config *conf) log_warnx("reload request not from SE"); else { reconfig = 1; - reconfpid = imsg.hdr.pid; - if (imsg.hdr.len == IMSG_HEADER_SIZE + - REASON_LEN && ((char *)imsg.data)[0]) + reconfpid = imsg_get_pid(&imsg); + if (imsg_get_data(&imsg, reason, + sizeof(reason)) == 0 && reason[0] != '\0') log_info("reload due to: %s", - log_reason(imsg.data)); + log_reason(reason)); } break; case IMSG_CTL_FIB_COUPLE: if (idx != PFD_PIPE_SESSION) log_warnx("couple request not from SE"); else - kr_fib_couple(imsg.hdr.peerid); + kr_fib_couple(imsg_get_id(&imsg)); break; case IMSG_CTL_FIB_DECOUPLE: if (idx != PFD_PIPE_SESSION) log_warnx("decouple request not from SE"); else - kr_fib_decouple(imsg.hdr.peerid); + kr_fib_decouple(imsg_get_id(&imsg)); break; case IMSG_CTL_KROUTE: case IMSG_CTL_KROUTE_ADDR: @@ -973,28 +970,29 @@ dispatch_imsg(struct imsgbuf *imsgbuf, int idx, struct bgpd_config *conf) case IMSG_SESSION_DEPENDON: if (idx != PFD_PIPE_SESSION) log_warnx("DEPENDON request not from SE"); - else if (imsg.hdr.len != IMSG_HEADER_SIZE + IFNAMSIZ) - log_warnx("DEPENDON request with wrong len"); + else if (imsg_get_data(&imsg, ifname, sizeof(ifname)) == + -1) + log_warn("wrong imsg len"); else - kr_ifinfo(imsg.data); + kr_ifinfo(ifname); break; case IMSG_DEMOTE: if (idx != PFD_PIPE_SESSION) log_warnx("demote request not from SE"); - else if (imsg.hdr.len != IMSG_HEADER_SIZE + - sizeof(struct demote_msg)) - log_warnx("DEMOTE request with wrong len"); - else { - struct demote_msg *msg; - - msg = imsg.data; - carp_demote_set(msg->demote_group, msg->level); - } + else if (imsg_get_data(&imsg, &demote, sizeof(demote)) + == -1) + log_warn("wrong imsg len"); + else + carp_demote_set(demote.demote_group, + demote.level); break; case IMSG_CTL_LOG_VERBOSE: /* already checked by SE */ - memcpy(&verbose, imsg.data, sizeof(verbose)); - log_setverbose(verbose); + if (imsg_get_data(&imsg, &verbose, sizeof(verbose)) == + -1) + log_warn("wrong imsg len"); + else + log_setverbose(verbose); break; case IMSG_RECONF_DONE: if (reconfpending == 0) { @@ -1037,12 +1035,12 @@ dispatch_imsg(struct imsgbuf *imsgbuf, int idx, struct bgpd_config *conf) log_warnx("connect request not from RTR"); } else { SIMPLEQ_FOREACH(r, &conf->rtrs, entry) { - if (imsg.hdr.peerid == r->id) + if (imsg_get_id(&imsg) == r->id) break; } if (r == NULL) log_warnx("unknown rtr id %d", - imsg.hdr.peerid); + imsg_get_id(&imsg)); else bgpd_rtr_connect(r); } @@ -1050,32 +1048,35 @@ dispatch_imsg(struct imsgbuf *imsgbuf, int idx, struct bgpd_config *conf) case IMSG_CTL_SHOW_RTR: if (idx == PFD_PIPE_SESSION) { SIMPLEQ_FOREACH(r, &conf->rtrs, entry) { - imsg_compose(ibuf_rtr, imsg.hdr.type, - r->id, imsg.hdr.pid, -1, NULL, 0); + imsg_compose(ibuf_rtr, + IMSG_CTL_SHOW_RTR, r->id, + imsg_get_pid(&imsg), -1, NULL, 0); } imsg_compose(ibuf_rtr, IMSG_CTL_END, - 0, imsg.hdr.pid, -1, NULL, 0); - } else if (imsg.hdr.len != IMSG_HEADER_SIZE + - sizeof(struct ctl_show_rtr)) { - log_warnx("IMSG_CTL_SHOW_RTR with wrong len"); + 0, imsg_get_pid(&imsg), -1, NULL, 0); } else if (idx == PFD_PIPE_RTR) { + struct ctl_show_rtr rtr; + if (imsg_get_data(&imsg, &rtr, sizeof(rtr)) == + -1) { + log_warn("wrong imsg len"); + break; + } + SIMPLEQ_FOREACH(r, &conf->rtrs, entry) { - if (imsg.hdr.peerid == r->id) + if (imsg_get_id(&imsg) == r->id) break; } if (r != NULL) { - struct ctl_show_rtr *msg; - msg = imsg.data; - strlcpy(msg->descr, r->descr, - sizeof(msg->descr)); - msg->local_addr = r->local_addr; - msg->remote_addr = r->remote_addr; - msg->remote_port = r->remote_port; - - imsg_compose(ibuf_se, imsg.hdr.type, - imsg.hdr.peerid, imsg.hdr.pid, - -1, imsg.data, - imsg.hdr.len - IMSG_HEADER_SIZE); + strlcpy(rtr.descr, r->descr, + sizeof(rtr.descr)); + rtr.local_addr = r->local_addr; + rtr.remote_addr = r->remote_addr; + rtr.remote_port = r->remote_port; + + imsg_compose(ibuf_se, IMSG_CTL_SHOW_RTR, + imsg_get_id(&imsg), + imsg_get_pid(&imsg), -1, + &rtr, sizeof(rtr)); } } break; @@ -1085,9 +1086,7 @@ dispatch_imsg(struct imsgbuf *imsgbuf, int idx, struct bgpd_config *conf) log_warnx("connect request not from RTR"); break; } - imsg_compose(ibuf_se, imsg.hdr.type, imsg.hdr.peerid, - imsg.hdr.pid, -1, imsg.data, - imsg.hdr.len - IMSG_HEADER_SIZE); + imsg_forward(ibuf_se, &imsg); break; default: break; diff --git a/usr.sbin/bgpd/kroute.c b/usr.sbin/bgpd/kroute.c index 5915165595d..6da5986faec 100644 --- a/usr.sbin/bgpd/kroute.c +++ b/usr.sbin/bgpd/kroute.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kroute.c,v 1.308 2024/01/08 15:08:34 claudio Exp $ */ +/* $OpenBSD: kroute.c,v 1.309 2024/01/09 13:41:32 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -883,27 +883,30 @@ kr_show_route(struct imsg *imsg) struct kroute *kr, *kn; struct kroute6 *kr6, *kn6; struct kroute_full *kf; - struct bgpd_addr *addr; + struct bgpd_addr addr; struct ctl_kroute_req req; struct ctl_show_nexthop snh; struct knexthop *h; struct kif *kif; + uint32_t tableid; + pid_t pid; u_int i; u_short ifindex = 0; - switch (imsg->hdr.type) { + tableid = imsg_get_id(imsg); + pid = imsg_get_pid(imsg); + switch (imsg_get_type(imsg)) { case IMSG_CTL_KROUTE: - if (imsg->hdr.len != IMSG_HEADER_SIZE + sizeof(req)) { + if (imsg_get_data(imsg, &req, sizeof(req)) == -1) { log_warnx("%s: wrong imsg len", __func__); break; } - kt = ktable_get(imsg->hdr.peerid); + kt = ktable_get(tableid); if (kt == NULL) { log_warnx("%s: table %u does not exist", __func__, - imsg->hdr.peerid); + tableid); break; } - memcpy(&req, imsg->data, sizeof(req)); if (!req.af || req.af == AF_INET) RB_FOREACH(kr, kroute_tree, &kt->krt) { if (req.flags && (kr->flags & req.flags) == 0) @@ -913,7 +916,7 @@ kr_show_route(struct imsg *imsg) kf = kr_tofull(kn); kf->priority = kr_priority(kf); send_imsg_session(IMSG_CTL_KROUTE, - imsg->hdr.pid, kf, sizeof(*kf)); + pid, kf, sizeof(*kf)); } while ((kn = kn->next) != NULL); } if (!req.af || req.af == AF_INET6) @@ -925,50 +928,48 @@ kr_show_route(struct imsg *imsg) kf = kr6_tofull(kn6); kf->priority = kr_priority(kf); send_imsg_session(IMSG_CTL_KROUTE, - imsg->hdr.pid, kf, sizeof(*kf)); + pid, kf, sizeof(*kf)); } while ((kn6 = kn6->next) != NULL); } break; case IMSG_CTL_KROUTE_ADDR: - if (imsg->hdr.len != IMSG_HEADER_SIZE + - sizeof(struct bgpd_addr)) { + if (imsg_get_data(imsg, &addr, sizeof(addr)) == -1) { log_warnx("%s: wrong imsg len", __func__); break; } - kt = ktable_get(imsg->hdr.peerid); + kt = ktable_get(tableid); if (kt == NULL) { log_warnx("%s: table %u does not exist", __func__, - imsg->hdr.peerid); + tableid); break; } - addr = imsg->data; kr = NULL; - switch (addr->aid) { + switch (addr.aid) { case AID_INET: - kr = kroute_match(kt, addr, 1); + kr = kroute_match(kt, &addr, 1); if (kr != NULL) { kf = kr_tofull(kr); kf->priority = kr_priority(kf); send_imsg_session(IMSG_CTL_KROUTE, - imsg->hdr.pid, kf, sizeof(*kf)); + pid, kf, sizeof(*kf)); } break; case AID_INET6: - kr6 = kroute6_match(kt, addr, 1); + kr6 = kroute6_match(kt, &addr, 1); if (kr6 != NULL) { kf = kr6_tofull(kr6); kf->priority = kr_priority(kf); send_imsg_session(IMSG_CTL_KROUTE, - imsg->hdr.pid, kf, sizeof(*kf)); + pid, kf, sizeof(*kf)); } break; } break; case IMSG_CTL_SHOW_NEXTHOP: - kt = ktable_get(imsg->hdr.peerid); + kt = ktable_get(tableid); if (kt == NULL) { log_warnx("%s: table %u does not exist", __func__, - imsg->hdr.peerid); + tableid); break; } RB_FOREACH(h, knexthop_tree, KT2KNT(kt)) { @@ -997,14 +998,14 @@ kr_show_route(struct imsg *imsg) kr_show_interface(kif), sizeof(snh.iface)); } - send_imsg_session(IMSG_CTL_SHOW_NEXTHOP, imsg->hdr.pid, + send_imsg_session(IMSG_CTL_SHOW_NEXTHOP, pid, &snh, sizeof(snh)); } break; case IMSG_CTL_SHOW_INTERFACE: RB_FOREACH(kif, kif_tree, &kit) send_imsg_session(IMSG_CTL_SHOW_INTERFACE, - imsg->hdr.pid, kr_show_interface(kif), + pid, kr_show_interface(kif), sizeof(struct ctl_show_interface)); break; case IMSG_CTL_SHOW_FIB_TABLES: @@ -1022,14 +1023,14 @@ kr_show_route(struct imsg *imsg) TAILQ_INIT(&ktab.krn); send_imsg_session(IMSG_CTL_SHOW_FIB_TABLES, - imsg->hdr.pid, &ktab, sizeof(ktab)); + pid, &ktab, sizeof(ktab)); } break; default: /* nada */ break; } - send_imsg_session(IMSG_CTL_END, imsg->hdr.pid, NULL, 0); + send_imsg_session(IMSG_CTL_END, pid, NULL, 0); } static void -- 2.20.1