From: Claudio Jeker Subject: bgpd: rework pt_fill and friends To: tech@openbsd.org Date: Wed, 13 May 2026 16:15:40 +0200 pt_fill() is in some cases used by semi-trusted content (e.g. from bgpctl). The fatalx calls in that function are therefor a problem. This alters pt_fill to instead return a pt_entry object that can not exist. I decided to return a pt_entry initalised with 0xff. Also if the prefixlen is too big for the address family just clip it down to the maximum (with a log message). In pt_add(), the only place a pt_fill() object would be added to the tree, check if the returned object is valid. There it is ok to fatal (at least for now) since the code previous to pt_add() should validate the prefix. On top of this I uniformed some of the error messages and switched the prefixlen argument to u_int (there is no negative prefix length). -- :wq Claudio Index: rde.h =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v diff -u -p -r1.347 rde.h --- rde.h 7 May 2026 11:21:24 -0000 1.347 +++ rde.h 13 May 2026 09:39:38 -0000 @@ -582,10 +582,10 @@ void pt_init(void); void pt_shutdown(void); void pt_getaddr(struct pt_entry *, struct bgpd_addr *); int pt_getflowspec(struct pt_entry *, uint8_t **); -struct pt_entry *pt_fill(struct bgpd_addr *, int); -struct pt_entry *pt_get(struct bgpd_addr *, int); -struct pt_entry *pt_get_next(struct bgpd_addr *, int); -struct pt_entry *pt_add(struct bgpd_addr *, int); +struct pt_entry *pt_fill(struct bgpd_addr *, u_int); +struct pt_entry *pt_get(struct bgpd_addr *, u_int); +struct pt_entry *pt_get_next(struct bgpd_addr *, u_int); +struct pt_entry *pt_add(struct bgpd_addr *, u_int); struct pt_entry *pt_get_flow(struct flowspec *); struct pt_entry *pt_add_flow(struct flowspec *); struct pt_entry *pt_first(uint8_t); Index: rde_prefix.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde_prefix.c,v diff -u -p -r1.60 rde_prefix.c --- rde_prefix.c 24 Dec 2025 07:59:55 -0000 1.60 +++ rde_prefix.c 13 May 2026 11:53:23 -0000 @@ -46,7 +46,7 @@ */ /* internal prototypes */ -static struct pt_entry *pt_alloc(struct pt_entry *, int len); +static struct pt_entry *pt_alloc(struct pt_entry *, size_t len); static void pt_free(struct pt_entry *); struct pt_entry4 { @@ -223,7 +223,7 @@ pt_getaddr(struct pt_entry *pte, struct addr, &pflow->prefixlen, NULL); break; default: - fatalx("pt_getaddr: unknown af"); + fatalx("%s: unknown aid %d", __func__, pte->aid); } } @@ -239,12 +239,16 @@ pt_getflowspec(struct pt_entry *pte, uin *flow = pflow->flow; return pflow->len - PT_FLOW_SIZE; default: - fatalx("pt_getflowspec: unknown af"); + fatalx("%s: unknown aid %d", __func__, pte->aid); } } +/* + * Fill out a pt_entry for lookup, on failure return an object initalized + * with 0xff. pt_add must reject such objects (by checking pte->aid). + */ struct pt_entry * -pt_fill(struct bgpd_addr *prefix, int prefixlen) +pt_fill(struct bgpd_addr *prefix, u_int prefixlen) { static struct pt_entry4 pte4; static struct pt_entry6 pte6; @@ -258,8 +262,10 @@ pt_fill(struct bgpd_addr *prefix, int pr pte4.len = sizeof(pte4); pte4.refcnt = UINT32_MAX; pte4.aid = prefix->aid; - if (prefixlen > 32) - fatalx("pt_fill: bad IPv4 prefixlen"); + if (prefixlen > 32) { + log_warnx("pt_fill: %d, bad IPv4 prefixlen", prefixlen); + prefixlen = 32; + } inet4applymask(&pte4.prefix4, &prefix->v4, prefixlen); pte4.prefixlen = prefixlen; return ((struct pt_entry *)&pte4); @@ -268,8 +274,10 @@ pt_fill(struct bgpd_addr *prefix, int pr pte6.len = sizeof(pte6); pte6.refcnt = UINT32_MAX; pte6.aid = prefix->aid; - if (prefixlen > 128) - fatalx("pt_fill: bad IPv6 prefixlen"); + if (prefixlen > 128) { + log_warnx("pt_fill: %d, bad IPv6 prefixlen", prefixlen); + prefixlen = 128; + } inet6applymask(&pte6.prefix6, &prefix->v6, prefixlen); pte6.prefixlen = prefixlen; return ((struct pt_entry *)&pte6); @@ -278,8 +286,10 @@ pt_fill(struct bgpd_addr *prefix, int pr pte_vpn4.len = sizeof(pte_vpn4); pte_vpn4.refcnt = UINT32_MAX; pte_vpn4.aid = prefix->aid; - if (prefixlen > 32) - fatalx("pt_fill: bad IPv4 prefixlen"); + if (prefixlen > 32) { + log_warnx("pt_fill: %d, bad IPv4 prefixlen", prefixlen); + prefixlen = 32; + } inet4applymask(&pte_vpn4.prefix4, &prefix->v4, prefixlen); pte_vpn4.prefixlen = prefixlen; pte_vpn4.rd = prefix->rd; @@ -292,8 +302,10 @@ pt_fill(struct bgpd_addr *prefix, int pr pte_vpn6.len = sizeof(pte_vpn6); pte_vpn6.refcnt = UINT32_MAX; pte_vpn6.aid = prefix->aid; - if (prefixlen > 128) - fatalx("pt_fill: bad IPv6 prefixlen"); + if (prefixlen > 128) { + log_warnx("pt_fill: %d, bad IPv6 prefixlen", prefixlen); + prefixlen = 128; + } inet6applymask(&pte_vpn6.prefix6, &prefix->v6, prefixlen); pte_vpn6.prefixlen = prefixlen; pte_vpn6.rd = prefix->rd; @@ -310,13 +322,24 @@ pt_fill(struct bgpd_addr *prefix, int pr /* See rfc7432 section 7.2 */ break; case AID_INET: + if (prefixlen > 32) { + log_warnx("pt_fill: %d, bad IPv4 prefixlen", + prefixlen); + prefixlen = 32; + } pte_evpn.prefix4 = prefix->evpn.v4; break; case AID_INET6: + if (prefixlen > 128) { + log_warnx("pt_fill: %d, bad IPv6 prefixlen", + prefixlen); + prefixlen = 128; + } pte_evpn.prefix6 = prefix->evpn.v6; break; default: - fatalx("pt_fill: bad EVPN prefixlen"); + log_warnx("pt_fill: unknown EVPN prefix"); + goto fail; } pte_evpn.aid = prefix->aid; pte_evpn.vpnaid = prefix->evpn.aid; @@ -333,12 +356,17 @@ pt_fill(struct bgpd_addr *prefix, int pr sizeof(prefix->evpn.mac)); return ((struct pt_entry *)&pte_evpn); default: - fatalx("pt_fill: unknown af"); + log_warnx("%s: unknown aid %d", __func__, prefix->aid); + goto fail; } + + fail: + memset(&pte4, 0xff, sizeof(pte4)); + return ((struct pt_entry *)&pte4); } struct pt_entry * -pt_get(struct bgpd_addr *prefix, int prefixlen) +pt_get(struct bgpd_addr *prefix, u_int prefixlen) { struct pt_entry *pte; @@ -347,7 +375,7 @@ pt_get(struct bgpd_addr *prefix, int pre } struct pt_entry * -pt_get_next(struct bgpd_addr *prefix, int prefixlen) +pt_get_next(struct bgpd_addr *prefix, u_int prefixlen) { struct pt_entry *pte; @@ -356,11 +384,13 @@ pt_get_next(struct bgpd_addr *prefix, in } struct pt_entry * -pt_add(struct bgpd_addr *prefix, int prefixlen) +pt_add(struct bgpd_addr *prefix, u_int prefixlen) { struct pt_entry *p = NULL; p = pt_fill(prefix, prefixlen); + if (p->aid == 0xff) + fatalx("pt_add: insert failed, pt_fill failed"); p = pt_alloc(p, p->len); if (RB_INSERT(pt_tree, &pttable, p) != NULL) @@ -457,7 +487,8 @@ pt_lookup(struct bgpd_addr *addr) i = 128; break; default: - fatalx("pt_lookup: unknown af"); + log_warnx("%s: unknown aid %d", __func__, addr->aid); + return (NULL); } for (; i >= 0; i--) { p = pt_get(addr, i); @@ -582,7 +613,7 @@ pt_prefix_cmp(const struct pt_entry *a, return (-1); break; default: - fatalx("pt_prefix_cmp: unknown evpn af %d", ea->vpnaid); + fatalx("%s: unknown evpn aid %d", __func__, ea->vpnaid); } return (0); case AID_FLOWSPECv4: @@ -593,7 +624,7 @@ pt_prefix_cmp(const struct pt_entry *a, bf->flow, bf->len - PT_FLOW_SIZE, a->aid == AID_FLOWSPECv6); default: - fatalx("pt_prefix_cmp: unknown af %d", a->aid); + fatalx("%s: unknown aid %d", __func__, a->aid); } return (-1); } @@ -603,7 +634,7 @@ pt_prefix_cmp(const struct pt_entry *a, * Function may not return on failure. */ static struct pt_entry * -pt_alloc(struct pt_entry *op, int len) +pt_alloc(struct pt_entry *op, size_t len) { struct pt_entry *p;